From 1d5ed26f21bdb20609dc79cd444110e849f171a6 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Wed, 29 Apr 2026 09:48:37 +0200 Subject: [PATCH 1/3] fix(add_library): SP-version-aware librarymanager + find_library dispatch The SP22 stub at Stubs/scriptengine/ScriptLibManObject.pyi documents: 'An instance implementing this interface is injected into the scriptengine scope under the name library_manager.' That is wrong on SP22 in practice. Verified live by probing watcher globals 2026-04-29: the actual attribute is 'librarymanager' (one word, no underscore), exposed both as a top-level global and as script_engine.librarymanager. The stub-documented 'library_manager' name is NOT defined. Result: every call to add_library against an SP22 IDE silently went through the 'global library_manager not in scope' fallback path, producing managed references with auto-derived namespaces ('Net Base Services' -> ns='Net_Base_Services') instead of the IDE-managed placeholder + ns ('NetBaseSrv' -> ns='NBS'). That made library types unreachable from IEC code -- e.g. NBS.IPv4Address compiled clean only when the user added the lib through the IDE Add Library dialog. Fix: * _detect_sp_version() parses sys.version once. * _get_lib_manager() picks 'librarymanager' first on SP22, falls back to 'library_manager' for older SPs, tries both in script_engine and bare module globals. * _resolve_in_repo() dispatches on SP version. SP22's find_library rejects bare strings (raises with 'stDisplayName' payload) and rejects the keyword form too, so the SP22 path falls back to a lm.repositories walk -- trying 'get_libraries' / 'libraries' / 'libs' / 'all_libraries' / iter() because LibRepository's iteration API is undocumented on SP22. * Pre-SP22 path keeps the documented signature. Verification path: with this fix, calling mcp__codesys__add_library(libraryName='Net Base Services') should land a placeholder named 'NetBaseSrv' with ns='NBS', matching what the IDE's Add Library dialog produces. Test in next session by re-launching CODESYS so the new add_library.py is loaded into the exec_globals of execute_script in watcher.py. This commits the fix to a feature branch so subsequent linter passes can't revert the working tree. --- src/scripts/add_library.py | 186 ++++++++++++++++++++++++++++++++----- 1 file changed, 162 insertions(+), 24 deletions(-) diff --git a/src/scripts/add_library.py b/src/scripts/add_library.py index 9c2cf3c..2d02e8f 100644 --- a/src/scripts/add_library.py +++ b/src/scripts/add_library.py @@ -46,46 +46,184 @@ FORCE_DUP = "{FORCE_DUP}" == "1" ALLOW_UNRESOLVED = "{ALLOW_UNRESOLVED}" == "1" +# ─── SP-version detection ───────────────────────────────────────────────── +# +# CODESYS reports its build through `sys.version` in the IronPython +# embedding (e.g. "CODESYS V3.5 SP22 Patch 1, ScriptEngine 4.2.0.0"). +# Several scriptengine APIs differ across SPs; the version-aware +# dispatchers below switch on this. + +def _detect_sp_version(): + """Return (sp_int, patch_int) parsed from sys.version, or (0, 0) if + not detectable.""" + try: + sv = sys.version + except Exception: + return (0, 0) + import re as _re + m = _re.search(r'SP(\d+)(?:\s+Patch\s+(\d+))?', sv) + if not m: + return (0, 0) + try: + sp = int(m.group(1)) + except Exception: + sp = 0 + try: + patch = int(m.group(2)) if m.group(2) else 0 + except Exception: + patch = 0 + return (sp, patch) + + +_SP_VERSION = _detect_sp_version() + + +def _get_lib_manager(): + """Return the IDE-level LibManager instance. + + Per-SP dispatch: + SP22+: actual attribute is `librarymanager` (one word). The + Stubs/scriptengine/ScriptLibManObject.pyi documents `library_manager` + but that name is NOT defined on SP22 -- verified live by probing + watcher globals 2026-04-29. + Older SPs: keep the documented `library_manager` name as primary. + + Tries each candidate in scriptengine module + bare globals, returns + the first that exposes `find_library`.""" + sp, _patch = _SP_VERSION + if sp >= 22: + primary_names = ('librarymanager', 'library_manager') + else: + primary_names = ('library_manager', 'librarymanager') + + candidates = [] + for nm in primary_names: + try: + candidates.append(getattr(script_engine, nm, None)) + except Exception: + pass + for nm in primary_names: + try: + candidates.append(eval(nm)) # bare global; eval avoids NameError + except Exception: + pass + + for cand in candidates: + if cand is None: + continue + if hasattr(cand, 'find_library'): + return cand + return None + + def _resolve_in_repo_accessible(): - """True iff the IDE-level library_manager global is accessible AND - exposes find_library. Used to distinguish 'verified missing' from - 'could not verify' so the refuse-on-miss guard doesn't fire when we - just couldn't access the repository at all.""" - try: - lm_global = library_manager # noqa: F821 -- injected by scriptengine - except NameError: - return False - return hasattr(lm_global, 'find_library') + """True iff the IDE-level library manager is accessible AND exposes + find_library.""" + return _get_lib_manager() is not None -def _resolve_in_repo(name): - """Try the IDE-level library_manager.find_library(name) and return the - ManagedLib if found, else None. Defensive against older SPs that may - not expose find_library or the global injection.""" +def _find_library_sp22(lm_global, name): + """SP22 dispatcher for find_library. The SP22 stub documents + `find_library(display_name: str)` but the live API rejects bare + strings with an exception whose payload contains 'stDisplayName' + (the C# parameter), AND the keyword form rejects 'stDisplayName='. + Workaround: walk `lm.repositories` looking for a library whose + displayname / title / name matches. Multiple accessor names tried + because LibRepository's iteration API is undocumented on SP22.""" + # Try the documented signature first; some installs accept it. try: - lm_global = library_manager # noqa: F821 -- injected by scriptengine - except NameError: - print("DEBUG: global 'library_manager' not in scope; skipping pre-resolve.") - return None - if not hasattr(lm_global, 'find_library'): - print("DEBUG: library_manager.find_library not available; skipping pre-resolve.") + result = lm_global.find_library(name) + if result is not None: + try: + return result[0] + except Exception: + return result + except Exception as e: + print("DEBUG: SP22 find_library(%r) raised: %s: %s -- trying repository walk" + % (name, type(e).__name__, e)) + + repos = [] + try: + repos = list(lm_global.repositories) + except Exception as e: + print("DEBUG: lm.repositories raised: %s" % e) return None + + candidates = [] + for repo in repos: + try: + repo_name = str(getattr(repo, 'name', '?')) + except Exception: + repo_name = '?' + libs = None + # Try several iteration accessors; SP22 doesn't document one. + for accessor in ('get_libraries', 'libraries', 'libs', 'all_libraries'): + try: + attr = getattr(repo, accessor, None) + if attr is None: + continue + libs = list(attr() if callable(attr) else attr) + if libs is not None: + break + except Exception: + libs = None + if libs is None: + try: + libs = list(repo) # iter() fallback + except Exception: + continue + if not libs: + continue + for lib in libs: + try: + disp = str(getattr(lib, 'displayname', '') or '') + title = str(getattr(lib, 'title', '') or '') + ln = str(getattr(lib, 'name', '') or '') + if (disp == name or title == name or ln == name + or disp.startswith(name + ',') + or ln.startswith(name + ',')): + candidates.append((repo_name, disp, ln, lib)) + except Exception: + pass + if candidates: + print("DEBUG: SP22 repo walk: %d match(es) for %r" % (len(candidates), name)) + # Highest version first (descending displayname sort). + candidates.sort(key=lambda t: t[1], reverse=True) + return candidates[0][3] + print("DEBUG: SP22 repo walk: no match for %r across %d repo(s)" % (name, len(repos))) + return None + + +def _find_library_default(lm_global, name): + """Pre-SP22 dispatcher: trust the documented stub signature.""" try: result = lm_global.find_library(name) except Exception as e: - print("DEBUG: library_manager.find_library('%s') raised: %s" % (name, e)) + print("DEBUG: find_library(%r) raised: %s" % (name, e)) return None if result is None: return None - # Stub says: returns tuple(ManagedLib, LibRepository) or None. try: - managed_lib = result[0] - return managed_lib + return result[0] except Exception: - # Some SPs may return the ManagedLib directly. return result +def _resolve_in_repo(name): + """Find a ManagedLib by display_name in the installed repository. + Returns the ManagedLib instance, or None on miss. Dispatches on + detected SP version because find_library's behaviour differs.""" + lm_global = _get_lib_manager() + if lm_global is None: + print("DEBUG: IDE library manager not accessible (tried librarymanager + " + "library_manager in script_engine and bare globals).") + return None + sp, _patch = _SP_VERSION + if sp >= 22: + return _find_library_sp22(lm_global, name) + return _find_library_default(lm_global, name) + + def _ref_name_matches(ref_name, target): """A managed ref shows up as 'Name, Version (Company)'; a placeholder shows up as '#Name'. Match on the bare target name in either form.""" From c565888ff5534689b06bd3182d208d5dc8779948 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Wed, 29 Apr 2026 09:50:54 +0200 Subject: [PATCH 2/3] feat(live-values): atomic writer for tui-live-values.json Task 5 of v0.3 plan. writeLiveValues(filePath, projectDir, payload) wraps the caller's {device, pou_name, values} in the v1 envelope (version, updated_at, project_dir) and writes atomically via ..tmp + rename. Creates parent dirs as needed. Mirrors src/tui/shared/state-write.ts. Kept separate because the TUI subpackage is ESM and the server is CJS; the duplication is ~15 lines of code that almost never changes. --- src/live-values-write.ts | 41 ++++++++++++++++++++++++ tests/unit/live-values-write.test.ts | 48 ++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 src/live-values-write.ts create mode 100644 tests/unit/live-values-write.test.ts diff --git a/src/live-values-write.ts b/src/live-values-write.ts new file mode 100644 index 0000000..2ce23d5 --- /dev/null +++ b/src/live-values-write.ts @@ -0,0 +1,41 @@ +import * as fs from 'fs/promises'; +import * as path from 'path'; + +export interface LiveValueSnapshotIn { + value: string; + type?: string; + ts: number; +} + +export interface LiveValuesPayloadIn { + device: string; + pou_name: string; + values: Record; +} + +/** + * Server-side counterpart to the TUI's readLiveValues. + * + * Wraps the caller's payload in the v1 envelope, writes atomically via + * `..tmp` + rename, and creates parent dirs as needed. Mirrors + * src/tui/shared/state-write.ts (the selection writer); kept separate + * because that one's ESM and the server is CJS. + */ +export async function writeLiveValues( + filePath: string, + projectDir: string, + payload: LiveValuesPayloadIn +): Promise { + await fs.mkdir(path.dirname(filePath), { recursive: true }); + const envelope = { + version: 1, + updated_at: new Date().toISOString(), + project_dir: projectDir, + device: payload.device, + pou_name: payload.pou_name, + values: payload.values, + }; + const tmp = `${filePath}.${process.pid}.tmp`; + await fs.writeFile(tmp, JSON.stringify(envelope, null, 2), 'utf8'); + await fs.rename(tmp, filePath); +} diff --git a/tests/unit/live-values-write.test.ts b/tests/unit/live-values-write.test.ts new file mode 100644 index 0000000..db630f1 --- /dev/null +++ b/tests/unit/live-values-write.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'fs/promises'; +import * as path from 'path'; +import * as os from 'os'; +import { writeLiveValues } from '../../src/live-values-write'; + +async function tmpDir(): Promise { + return fs.mkdtemp(path.join(os.tmpdir(), 'phobics-lvw-')); +} + +const sample = { + device: 'D1', + pou_name: 'PLC_PRG', + values: { + counter: { value: '47', type: 'INT', ts: Date.now() }, + }, +}; + +describe('writeLiveValues', () => { + it('writes the v1 envelope with required fields', async () => { + const dir = await tmpDir(); + const target = path.join(dir, 'tui-live-values.json'); + await writeLiveValues(target, '/abs/project', sample); + + const parsed = JSON.parse(await fs.readFile(target, 'utf8')); + expect(parsed.version).toBe(1); + expect(parsed.project_dir).toBe('/abs/project'); + expect(parsed.device).toBe('D1'); + expect(parsed.pou_name).toBe('PLC_PRG'); + expect(parsed.values.counter.value).toBe('47'); + expect(typeof parsed.updated_at).toBe('string'); + }); + + it('creates parent dirs as needed', async () => { + const dir = await tmpDir(); + const target = path.join(dir, 'a', 'b', 'tui-live-values.json'); + await writeLiveValues(target, '/abs/project', sample); + expect((await fs.stat(target)).isFile()).toBe(true); + }); + + it('does not leave .tmp residue on success', async () => { + const dir = await tmpDir(); + const target = path.join(dir, 'tui-live-values.json'); + await writeLiveValues(target, '/abs/project', sample); + const entries = await fs.readdir(dir); + expect(entries.filter((e) => e.endsWith('.tmp'))).toEqual([]); + }); +}); From 4c955886ffffe780e9cbc65e3e53c427f3e30430 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Wed, 29 Apr 2026 10:03:32 +0200 Subject: [PATCH 3/3] Revert "feat(live-values): atomic writer for tui-live-values.json" This reverts commit c565888ff5534689b06bd3182d208d5dc8779948. --- src/live-values-write.ts | 41 ------------------------ tests/unit/live-values-write.test.ts | 48 ---------------------------- 2 files changed, 89 deletions(-) delete mode 100644 src/live-values-write.ts delete mode 100644 tests/unit/live-values-write.test.ts diff --git a/src/live-values-write.ts b/src/live-values-write.ts deleted file mode 100644 index 2ce23d5..0000000 --- a/src/live-values-write.ts +++ /dev/null @@ -1,41 +0,0 @@ -import * as fs from 'fs/promises'; -import * as path from 'path'; - -export interface LiveValueSnapshotIn { - value: string; - type?: string; - ts: number; -} - -export interface LiveValuesPayloadIn { - device: string; - pou_name: string; - values: Record; -} - -/** - * Server-side counterpart to the TUI's readLiveValues. - * - * Wraps the caller's payload in the v1 envelope, writes atomically via - * `..tmp` + rename, and creates parent dirs as needed. Mirrors - * src/tui/shared/state-write.ts (the selection writer); kept separate - * because that one's ESM and the server is CJS. - */ -export async function writeLiveValues( - filePath: string, - projectDir: string, - payload: LiveValuesPayloadIn -): Promise { - await fs.mkdir(path.dirname(filePath), { recursive: true }); - const envelope = { - version: 1, - updated_at: new Date().toISOString(), - project_dir: projectDir, - device: payload.device, - pou_name: payload.pou_name, - values: payload.values, - }; - const tmp = `${filePath}.${process.pid}.tmp`; - await fs.writeFile(tmp, JSON.stringify(envelope, null, 2), 'utf8'); - await fs.rename(tmp, filePath); -} diff --git a/tests/unit/live-values-write.test.ts b/tests/unit/live-values-write.test.ts deleted file mode 100644 index db630f1..0000000 --- a/tests/unit/live-values-write.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import * as fs from 'fs/promises'; -import * as path from 'path'; -import * as os from 'os'; -import { writeLiveValues } from '../../src/live-values-write'; - -async function tmpDir(): Promise { - return fs.mkdtemp(path.join(os.tmpdir(), 'phobics-lvw-')); -} - -const sample = { - device: 'D1', - pou_name: 'PLC_PRG', - values: { - counter: { value: '47', type: 'INT', ts: Date.now() }, - }, -}; - -describe('writeLiveValues', () => { - it('writes the v1 envelope with required fields', async () => { - const dir = await tmpDir(); - const target = path.join(dir, 'tui-live-values.json'); - await writeLiveValues(target, '/abs/project', sample); - - const parsed = JSON.parse(await fs.readFile(target, 'utf8')); - expect(parsed.version).toBe(1); - expect(parsed.project_dir).toBe('/abs/project'); - expect(parsed.device).toBe('D1'); - expect(parsed.pou_name).toBe('PLC_PRG'); - expect(parsed.values.counter.value).toBe('47'); - expect(typeof parsed.updated_at).toBe('string'); - }); - - it('creates parent dirs as needed', async () => { - const dir = await tmpDir(); - const target = path.join(dir, 'a', 'b', 'tui-live-values.json'); - await writeLiveValues(target, '/abs/project', sample); - expect((await fs.stat(target)).isFile()).toBe(true); - }); - - it('does not leave .tmp residue on success', async () => { - const dir = await tmpDir(); - const target = path.join(dir, 'tui-live-values.json'); - await writeLiveValues(target, '/abs/project', sample); - const entries = await fs.readdir(dir); - expect(entries.filter((e) => e.endsWith('.tmp'))).toEqual([]); - }); -});