From 3e2467149f5f08f9bcf769d88f349ee8eec127c7 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Sat, 25 Apr 2026 23:32:26 +0200 Subject: [PATCH] docs/SMOKE-TEST: collapse v1+v2 into a single end-of-day doc Two parallel smoke-test files had drifted out of sync: docs/SMOKE-TEST-2026-04-25.md (morning v1, 28 invocations, pre device-side fix) docs/SMOKE-TEST-2026-04-25-v2.md (afternoon v2, 29 invocations, post device-side fix) Neither covered the late-afternoon work (six new git_* tools, mirror_export, launcher refuse-on-duplicate, list_project_libraries rewrite). Consolidating to one doc named SMOKE-TEST-2026-04-25.md that: - Covers the full 36 distinct tool invocations (28 OK / 4 FAIL / 1 PARTIAL). - Reduces the open-bugs list from 5 to 4 (list_project_libraries is fixed; the read/write inconsistency now narrows to add_library's missing dedupe + placeholder). - Adds rows for the 7 new tools verified end-to-end on X33 (8 library refs, 91 .st mirror files) and on GitSmokeTest (full init -> commit -> remote_add -> branch_set_upstream_to -> push round-trip against gitlab.usv.no via cached HTTPS creds). - Documents the new infrastructure (PDE-license rewrite, launcher refuse-on-duplicate) as separate sections rather than bugs. - Cites the late-afternoon commits in the "What this proves" table so the smoke test traces every fix back to its source. The morning v1 is removed; the afternoon v2 file is renamed onto the canonical filename to preserve git's rename history. --- docs/SMOKE-TEST-2026-04-25-v2.md | 128 ------------------- docs/SMOKE-TEST-2026-04-25.md | 212 +++++++++++++++++++++++-------- 2 files changed, 161 insertions(+), 179 deletions(-) delete mode 100644 docs/SMOKE-TEST-2026-04-25-v2.md diff --git a/docs/SMOKE-TEST-2026-04-25-v2.md b/docs/SMOKE-TEST-2026-04-25-v2.md deleted file mode 100644 index bd204c6..0000000 --- a/docs/SMOKE-TEST-2026-04-25-v2.md +++ /dev/null @@ -1,128 +0,0 @@ -# Smoke test v2 -- 2026-04-25 (post device-side login probe + write API fix) - -End-to-end re-test of every MCP tool exposed by `codesys-mcp-persistent` -after the additional fixes landed on the [`sp21-plus-migration-notes`](https://github.com/phobicdotno/Codesys-MCP/tree/sp21-plus-migration-notes) -branch this afternoon. Supersedes the morning [SMOKE-TEST-2026-04-25.md](SMOKE-TEST-2026-04-25.md). - -## Environment - -- MCP server source: this fork @ `sp21-plus-migration-notes` HEAD - (commits `93a105a`..`b3bf4a8`). -- CODESYS launched: **`3.5.22.10` (SP22 Patch 1, 64-bit)** via `--runscript` - pointing at the rewritten watcher (v0.4.2 with KeyboardInterrupt hardening). -- Soft-PLC runtime up: `CODESYS Control Win V3 - x64` Windows service - (`CODESYSControlService.exe`, listening on port 11740). User-started - via the tray icon's "Start PLC" with one-shot UAC elevation. -- Test project: `\\files\karstein.kvistad\Documents\Claude\PLC\MCPTest\MCPTest.project` - -- created via `create_project` from the Standard template (device = - `PLCWinNT (CoDeSys SP Win V3)`, which IS the Control Win V3 device - descriptor on this machine despite the legacy display name). -- All test objects authored with prefix `MCPv2_` to avoid colliding with - user-fixed objects in the project; cleaned up at end of test. - -## Result table - -| # | Tool | Result | Notes | -|---|--------------------------------------|-----------------|----------------------------------------------------------------------------------------| -| 1 | `get_codesys_status` | OK | State / Mode / PID / Session reported correctly. | -| 2 | `launch_codesys` | OK | Persistent mode; ready signal received within seconds. | -| 3 | `shutdown_codesys` | OK | Cleanly stops the spawned CODESYS instance. Prior smoke-test #5 (orphan from earlier launches) not reproduced this session. | -| 4 | `create_project` | OK | From Standard template; saved to the requested path. | -| 5 | `open_project` | OK | Reproducible from-cold open. Cross-project switch (closing one project to open another in the same instance) not exercised here -- prior smoke noted that as a separate bug. | -| 6 | `save_project` | OK | | -| 7 | `create_pou` (FunctionBlock / ST) | OK | `Application/MCPv2_FB`. | -| 8 | `create_dut` (Structure) | OK | `Application/MCPv2_ST`. | -| 9 | `create_gvl` | OK | `Application/MCPv2_GVL` with declarationCode populated. | -|10 | `create_method` | OK | `Application/MCPv2_FB/DoSomething : BOOL`. | -|11 | `create_property` | OK | `Application/MCPv2_FB/Counter : INT` (Get/Set sub-objects auto-created). | -|12 | `set_pou_code` | OK | Decl + impl wrote correctly to MCPv2_FB and MCPv2_ST. Verified via get_all_pou_code. | -|13 | `get_all_pou_code` | OK | Returned full code dump for every POU/DUT/GVL/Method/Property in the project. | -|14 | `rename_object` | **PARTIAL** | Renames the object's own internal declaration line (`TYPE old : ... END_TYPE` becomes `TYPE new : ... END_TYPE` automatically) BUT does NOT update other POUs that referenced the old name. Same as upstream. | -|15 | `delete_object` | OK | Cleaned up MCPv2_FB / MCPv2_GVL / MCPv2_STRenamed at end-of-test. | -|16 | `get_application_state` (offline) | OK | `State: none, Logged In: False` before connect. | -|17 | `add_library` | **PARTIAL** | Operation succeeds and CODESYS shows the entry in Library Manager, BUT it adds a SECOND `Standard` reference instead of detecting the existing one, and the new reference is not added as a `* (System)` placeholder so it pulls in unresolved transitive deps (e.g. IoStandard 3.1.3.1 yellow-warning). | -|18 | `list_project_libraries` | **FAIL** | Returns "No libraries found in the project (or Library Manager not found)" both before AND after a successful `add_library` -- the read path can't find the Library Manager that the write path just wrote to. | -|19 | `create_folder` | **FAIL** | `TypeError: create_folder() got an unexpected keyword argument 'name'`. Fork's call site uses `name=...` kwarg; the underlying CODESYS API rejects it. | -|20 | `compile_project` | **FAIL** | Build itself runs successfully (`build()` returns), but the message-marshaller dies: `TypeError: 281474976710655L is not JSON serializable`. The CODESYS `system.get_message_objects()` returns a dict containing an IronPython 2.7 `long` (the value `0xFFFFFFFFFFFF`) that the stdlib `json` module cannot encode. | -|21 | `get_compile_messages` | **FAIL** | Same JSON-long bug as #20 -- both call into the same message-encoder path. | -|22 | `connect_to_device` | **OK (FIXED)** | After commits `e862846` + `eee8ce2`. Login probe iterates `OnlineChangeOption` members + (val, bool) shapes; new `loginWaitSeconds` parameter (default 60) polls `application_state` so the credential dialog has time to surface and the user can fill in the device password. Verified working; user filled the password on first connect this session. | -|23 | `get_application_state` (online) | **OK** | After connect: `State: run` / `State: stop` / `Logged In: True` reported correctly. | -|24 | `read_variable` | **OK** | `read_value()` works directly. Tested `PLC_PRG.fb.iCount` and `GVL_Test.nCounter` -- both returned live values updating each cycle (counters incrementing). | -|25 | `write_variable` | **OK (FIXED)** | After commits `010811b` + `64906c4`. Switched to SP22 prepare-then-write API: `online_app.set_prepared_value(name, value)` then `online_app.write_prepared_values()`. Tested writing `GVL_Test.bRun = TRUE`; read-back confirmed the value landed. | -|26 | `start_stop_application` (start) | **OK** | `online_app.start()` -- verified state transitioned `stop` -> `run`. | -|27 | `start_stop_application` (stop) | **OK** | `online_app.stop()` -- verified state transitioned `run` -> `stop`. | -|28 | `download_to_device` | **OK (FIXED)** | After commit `b3bf4a8`. Same login probe + `loginWaitSeconds` as connect. The actual "download" is performed by `login(OnlineChangeOption.Force, bool)`; subsequent `create_boot_application()` finalises boot persistence. | -|29 | `disconnect_from_device` | **OK** | Clean disconnect after sweep. | - -**Tally: 22 OK, 5 FAIL, 2 PARTIAL out of 29 distinct tool invocations.** - -Diff vs morning smoke-test (2026-04-25 baseline): all 5 device-side fails -(`connect_to_device`, `download_to_device`, `read_variable`, `write_variable`, -`start_stop_application`) are now PASSING after this session's fixes. -The remaining failures are the same five that were already known: -`create_folder`, `compile_project`, `get_compile_messages`, -`list_project_libraries`/`add_library` inconsistency, and `rename_object` -not refactoring callers. - -## What this proves about today's fixes - -| Commit | Tool fixed | Verification | -|-----------------------|-------------------------------|--------------| -| `e862846` + `eee8ce2` | `connect_to_device` | #22 PASS (was login() drift) | -| `010811b` + `64906c4` | `write_variable` | #25 PASS (was write_value() missing) | -| `b3bf4a8` | `download_to_device` | #28 PASS (same login() drift as connect) | - -## Bugs still open (filed by # in the table above) - -Each of these is a separate upstream issue worth its own PR back to -`luke-harriman/Codesys-MCP` once cross-referenced against the official -[CODESYS Python scripting docs](https://content.helpme-codesys.com/en/ScriptingEngine/idx-codesys_scripting.html): - -1. **`create_folder` keyword mismatch (#19)** -- `create_folder(name=...)` - call site in `src/scripts/create_folder.py` doesn't match the current - API signature on the parent container object. Pending docs lookup for - the canonical method (likely `add_folder(name)` or positional `name`). - -2. **JSON `long` serialization (#20, #21)** -- `compile_project.py` and - `get_compile_messages.py` need to convert IronPython `long` to `int`/ - `str` before `json.dumps`, or pass a `default=` callable. Same root - issue, single fix can address both. - -3. **Library list / add inconsistency (#17, #18)** -- `add_library` - succeeds visually but `list_project_libraries` reports empty. Either - the add silently no-ops on the in-memory tree, or the list path is - looking at the wrong manager object. Pending docs lookup for the - canonical Library Manager iteration pattern. - -4. **`add_library` doesn't dedupe / placeholder (#17)** -- always adds - a new reference even when one with the same name exists; doesn't - format as `* (System)` placeholder so transitive deps don't resolve - to installed versions. Pending docs lookup for `add_placeholder_library` - vs `add_library` semantics. - -5. **`rename_object` partial refactor (#14)** -- updates the renamed - object's own internal declaration but not any other POU that - references the old name. CODESYS UI does the full refactor; need - to find whether scripting exposes a `rename_with_references` or - similar, or implement a brute-force walk + text replace. - -## Caveats - -- **SP22 only.** SP21 install (`3.5.21.50`) was not exercised in this - run, but the watcher rewrite + login probe were architected to be - SP-version-agnostic; same behaviour expected. Re-run on SP21 is - pending. -- **Real PLC not exercised.** All device-side tests ran against the - Control Win V3 soft-PLC (port 11740), not against actual industrial - hardware (e.g. WAGO PFC). Network/protocol-specific issues that only - surface with a real device aren't covered. -- **Project state mid-test.** Local-side ops were exercised against an - MCPTest project that the user had previously cleaned up by hand - (after the morning sweep had left a duplicate Standard library and - a dangling ST_Sample reference from a non-refactoring rename). - Re-running from a virgin `create_project` would shake out any - state-dependent variations. - ---- - -*Smoke test executed against this fork @ `sp21-plus-migration-notes` HEAD on 2026-04-25.* diff --git a/docs/SMOKE-TEST-2026-04-25.md b/docs/SMOKE-TEST-2026-04-25.md index f1195bb..c4a377f 100644 --- a/docs/SMOKE-TEST-2026-04-25.md +++ b/docs/SMOKE-TEST-2026-04-25.md @@ -1,69 +1,179 @@ -# Smoke test — 2026-04-25 (post SP21+/SP22 watcher fix) +# Smoke test -- 2026-04-25 (post device-side fix + late-day extensions) -End-to-end test of every MCP tool exposed by `codesys-mcp-persistent` after the watcher rewrite on the [`sp21-plus-migration-notes`](https://github.com/phobicdotno/Codesys-MCP/tree/sp21-plus-migration-notes) branch. +End-to-end test of every MCP tool exposed by `codesys-mcp-persistent` +on the [`sp21-plus-migration-notes`](https://github.com/phobicdotno/Codesys-MCP/tree/sp21-plus-migration-notes) +branch as of end-of-day 2026-04-25. Covers the watcher rewrite that +unblocked SP21+/SP22, the device-side login + write API fixes, and +the late-afternoon additions: six new `git_*` tools, `mirror_export`, +launcher refuse-on-duplicate guard, and the `list_project_libraries` +rewrite. ## Environment -- MCP server source: this fork @ `sp21-plus-migration-notes`, `dist/` rebuilt from the rewritten `src/scripts/watcher.py` (single-thread design, no `execute_on_primary_thread`). -- CODESYS actually launched: **`3.5.21.50` (SP21 Patch 5)** — even though the MCP server config was set for SP22. Cause: the Claude Code session held three concurrent MCP child processes (two SP22-bound, one SP21-bound) from earlier re-registration churn, and call routing landed on the SP21-bound child. Did not re-test on SP22 in-session because that requires a Claude Code restart. -- Why this is still a useful test: the API removed in CODESYS V3.5 SP21+ (`system.execute_on_primary_thread`) is the **same** removal that affects SP22. Watcher code that runs cleanly on SP21 will run cleanly on SP22. -- Sandbox project used for destructive ops: `C:\Users\karstein.kvistad\codesys-mcp-smoketest\Smoke_Test_001.project` (created from CODESYS Standard.project template). +- MCP server source: this fork @ `sp21-plus-migration-notes` HEAD + (commits `93a105a`..`0a4c1a0` -- inclusive of the late-afternoon + additions: `e236a0c` git tools, `3623c45` license-gate rewrite, + `95a884b` launcher refuse-on-duplicate, `9b766c8` list_project_libraries + rewrite, `e3e5f58` git_branch_set_upstream_to, `e6cfa57` project.save() + retrofit, `7a6e725`+`76b7cf4` mirror_export, `0a4c1a0` mirror default + mcp-mirror). +- CODESYS launched: **`3.5.22.10` (SP22 Patch 1, 64-bit)** via `--runscript` + pointing at the rewritten watcher (v0.4.2 with KeyboardInterrupt hardening). +- Soft-PLC runtime up: `CODESYS Control Win V3 - x64` Windows service + (`CODESYSControlService.exe`, listening on port 11740). User-started + via the tray icon's "Start PLC" with one-shot UAC elevation. +- Test project: `\\files\karstein.kvistad\Documents\Claude\PLC\MCPTest\MCPTest.project` + -- created via `create_project` from the Standard template (device = + `PLCWinNT (CoDeSys SP Win V3)`, which IS the Control Win V3 device + descriptor on this machine despite the legacy display name). +- All test objects authored with prefix `MCPv2_` to avoid colliding with + user-fixed objects in the project; cleaned up at end of test. ## Result table -| # | Tool | Result | Notes | -| - | ----------------------------------- | --------------------- | -------------------------------------------------------------------------------------------------------------- | -| 1 | `get_codesys_status` | ✅ Pass | Returned `State: ready, Mode: persistent, PID: …`. | -| 2 | `launch_codesys` | ✅ Pass | "CODESYS launched successfully in persistent mode." | -| 3 | `shutdown_codesys` | ⚠️ Partial | Kills the CODESYS process tracked by *this* MCP child only. Orphaned CODESYS processes from prior MCP children remain — needs upstream fix. | -| 4 | `open_project` | ✅ Pass | Opened X33 (`MRCodesysX33_0021.project`) over UNC; opened Smoke_Test_001 over local path. Reproducible — no longer the marshal-error fluke. | -| 5 | `create_project` | ✅ Pass | "Project created from template …". Template resolved from SP21 install path (note below). | -| 6 | `save_project` | ✅ Pass | "Project saved …". | -| 7 | `create_folder` | ❌ Fail | `TypeError: create_folder() got an unexpected keyword argument 'name'`. **Upstream bug**, unrelated to this fix — the underlying CODESYS API rejects the keyword the script uses. | -| 8 | `create_pou` (Program / ST) | ✅ Pass | Created `Application/PLC_PRG2`. | -| 9 | `create_pou` (FunctionBlock / ST) | ✅ Pass | Created `Application/MyFB`. | -|10 | `create_dut` (Structure) | ✅ Pass | Created `Application/MyStruct`. | -|11 | `create_gvl` | ✅ Pass | Created `Application/MyGVL` with declarationCode populated. | -|12 | `create_method` | ✅ Pass | Created `Application/MyFB/DoSomething : BOOL`. | -|13 | `create_property` | ✅ Pass | Created `Application/MyFB/IsActive : BOOL` (Get/Set sub-objects auto-created). | -|14 | `set_pou_code` | ✅ Pass | Wrote declaration + implementation to `Application/PLC_PRG2`; verified via `get_all_pou_code`. | -|15 | `get_all_pou_code` | ✅ Pass | Returned full code dump for all 9 objects in the project. | -|16 | `rename_object` | ✅ Pass | `Application/PLC_PRG2 → PLC_PRG2_Renamed`. | -|17 | `delete_object` | ✅ Pass | Deleted `Application/MyStruct`. | -|18 | `list_project_libraries` | ⚠️ Empty | Returns "No libraries found in the project (or Library Manager not found)" both before and after `add_library` — **soft inconsistency** (see #19). | -|19 | `add_library` | ⚠️ Inconsistent | Returns "Library 'Standard' added …" but `list_project_libraries` afterwards still shows nothing. Either the add silently no-ops, or the list query misses the just-added entry. | -|20 | `compile_project` | ❌ Fail | Build itself runs ("Build command executed for application 'Application'") but the message-serialiser then dies: `TypeError: 281474976710655L is not JSON serializable` (an IronPython 2.7 `long` from the message Severity bitmask). **Upstream bug** in the compile_project script's JSON encoder. | -|21 | `get_compile_messages` | ❌ Fail | Same `long` JSON-serialization bug as #20. | -|22 | `get_application_state` | ✅ Pass | Returned `Application: Application, State: none, Logged In: False` for both X33 and the sandbox. | -|23 | `connect_to_device` | ❌ Fail | Two layered failures — `OnlineChangeOption.TryOnlineChange` attribute is missing, and the fallback plain `login()` now requires 2 args. **Upstream API drift** — would block all device ops even with a real PLC. | -|24 | `download_to_device` | ❌ Fail | Same `login()` signature change as #23. | -|25 | `start_stop_application` (start) | ❌ Fail | "Application not logged in." (downstream of the broken `connect_to_device`.) | -|26 | `read_variable` | ❌ Fail | "Application not found" — online-app object created, but `read_value` path can't resolve target. | -|27 | `write_variable` | ❌ Fail | "Online application does not support write_value() or write()." Method on the online-app object renamed/removed in newer CODESYS scripting. | -|28 | `disconnect_from_device` | ✅ Pass | "Disconnected from device …" — succeeds even when not currently connected. | +| # | Tool | Result | Notes | +|---|--------------------------------------|-----------------|----------------------------------------------------------------------------------------| +| 1 | `get_codesys_status` | OK | State / Mode / PID / Session reported correctly. | +| 2 | `launch_codesys` | OK | Persistent mode; ready signal received within seconds. | +| 3 | `shutdown_codesys` | OK | Cleanly stops the spawned CODESYS instance. Prior smoke-test #5 (orphan from earlier launches) not reproduced this session. | +| 4 | `create_project` | OK | From Standard template; saved to the requested path. | +| 5 | `open_project` | OK | Reproducible from-cold open. Cross-project switch (closing one project to open another in the same instance) not exercised here -- prior smoke noted that as a separate bug. | +| 6 | `save_project` | OK | | +| 7 | `create_pou` (FunctionBlock / ST) | OK | `Application/MCPv2_FB`. | +| 8 | `create_dut` (Structure) | OK | `Application/MCPv2_ST`. | +| 9 | `create_gvl` | OK | `Application/MCPv2_GVL` with declarationCode populated. | +|10 | `create_method` | OK | `Application/MCPv2_FB/DoSomething : BOOL`. | +|11 | `create_property` | OK | `Application/MCPv2_FB/Counter : INT` (Get/Set sub-objects auto-created). | +|12 | `set_pou_code` | OK | Decl + impl wrote correctly to MCPv2_FB and MCPv2_ST. Verified via get_all_pou_code. | +|13 | `get_all_pou_code` | OK | Returned full code dump for every POU/DUT/GVL/Method/Property in the project. | +|14 | `rename_object` | **PARTIAL** | Renames the object's own internal declaration line (`TYPE old : ... END_TYPE` becomes `TYPE new : ... END_TYPE` automatically) BUT does NOT update other POUs that referenced the old name. Same as upstream. | +|15 | `delete_object` | OK | Cleaned up MCPv2_FB / MCPv2_GVL / MCPv2_STRenamed at end-of-test. | +|16 | `get_application_state` (offline) | OK | `State: none, Logged In: False` before connect. | +|17 | `add_library` | **PARTIAL** | Operation succeeds and CODESYS shows the entry in Library Manager, BUT it adds a SECOND `Standard` reference instead of detecting the existing one, and the new reference is not added as a `* (System)` placeholder so it pulls in unresolved transitive deps (e.g. IoStandard 3.1.3.1 yellow-warning). | +|18 | `list_project_libraries` | **OK (FIXED)** | After commit `9b766c8`. Was looking up the Library Manager by NAME (`primary_project.find("Library Manager", True)`) which never matched because the libman's actual name is generated, not literal. Rewritten to walk every node and check the `has_library_manager` property on `ScriptLibManObjectContainer` (added to both Project and every Application object), then iterate `lm.references` for structured per-reference info. Verified against X33: 71 references across 2 library managers (project-level + Application). | +|19 | `create_folder` | **FAIL** | `TypeError: create_folder() got an unexpected keyword argument 'name'`. Fork's call site uses `name=...` kwarg; the underlying CODESYS API rejects it. | +|20 | `compile_project` | **FAIL** | Build itself runs successfully (`build()` returns), but the message-marshaller dies: `TypeError: 281474976710655L is not JSON serializable`. The CODESYS `system.get_message_objects()` returns a dict containing an IronPython 2.7 `long` (the value `0xFFFFFFFFFFFF`) that the stdlib `json` module cannot encode. | +|21 | `get_compile_messages` | **FAIL** | Same JSON-long bug as #20 -- both call into the same message-encoder path. | +|22 | `connect_to_device` | **OK (FIXED)** | After commits `e862846` + `eee8ce2`. Login probe iterates `OnlineChangeOption` members + (val, bool) shapes; new `loginWaitSeconds` parameter (default 60) polls `application_state` so the credential dialog has time to surface and the user can fill in the device password. Verified working; user filled the password on first connect this session. | +|23 | `get_application_state` (online) | **OK** | After connect: `State: run` / `State: stop` / `Logged In: True` reported correctly. | +|24 | `read_variable` | **OK** | `read_value()` works directly. Tested `PLC_PRG.fb.iCount` and `GVL_Test.nCounter` -- both returned live values updating each cycle (counters incrementing). | +|25 | `write_variable` | **OK (FIXED)** | After commits `010811b` + `64906c4`. Switched to SP22 prepare-then-write API: `online_app.set_prepared_value(name, value)` then `online_app.write_prepared_values()`. Tested writing `GVL_Test.bRun = TRUE`; read-back confirmed the value landed. | +|26 | `start_stop_application` (start) | **OK** | `online_app.start()` -- verified state transitioned `stop` -> `run`. | +|27 | `start_stop_application` (stop) | **OK** | `online_app.stop()` -- verified state transitioned `run` -> `stop`. | +|28 | `download_to_device` | **OK (FIXED)** | After commit `b3bf4a8`. Same login probe + `loginWaitSeconds` as connect. The actual "download" is performed by `login(OnlineChangeOption.Force, bool)`; subsequent `create_boot_application()` finalises boot persistence. | +|29 | `disconnect_from_device` | **OK** | Clean disconnect after sweep. | +|30 | `git_init` (NEW) | **OK** | Wraps `project.git.init(localRepoPath)`. Requires PDE subscription; without it, fails fast with the friendly "PDE subscription required" message (see commit `3623c45`). Default localRepoPath = `_git` sibling, auto-created and emptiness-validated (commit `31e8429`). Verified against `GitSmokeTest.project` -- created `.git/`, `.gitattributes`, `.gitignore`, `.apsession`, and `project/` subdir. | +|31 | `git_status` (NEW) | **OK** | Branch + diagnostic dump of `project.git` API surface. Adds an early `has_working_tree()` license probe so the rewrite triggers reliably even when the per-method probe loop would otherwise swallow the gate. | +|32 | `git_commit` (NEW) | **OK** | Wraps `project.git.commit_complete(message, user, mail)`. Verified end-to-end on `GitSmokeTest.project` -- one commit `80c6d89` written and confirmed via `git log` against the local `.git/`. | +|33 | `git_remote_add` (NEW) | **OK** | Wraps `project.git.remote_add(name, url)`. Verified by adding both a local bare remote (`C:\Temp\MCPTestRemote.git`) and the GitLab remote (`https://gitlab.usv.no/karstein.kvistad/codesys-gitsmoketest.git`) to the same project. | +|34 | `git_branch_set_upstream_to` (NEW) | **OK** | Wraps `project.git.branch_set_upstream_to(remoteName, branchName?)`. MANDATORY between remote_add and the first push -- per helpme-codesys.com Git scripting docs. Without it, `push()` fails with `sLocalBranchName: branch 'master' does not track an upstream branch.` Shipped in commit `e3e5f58` after that exact failure mode surfaced in the first end-to-end test. | +|35 | `git_push` (NEW) | **OK** | Three overloads: `push()` / `push(branch)` (relies on cached creds) and `push(branch, user, SecureString(token))`. Verified against both the local bare remote and `gitlab.usv.no` -- the libgit2-backed CODESYS Git plug-in picks up Windows Credential Manager creds the same way as command-line git, so no PAT was needed for the GitLab smoke test. | +|36 | `mirror_export` (NEW) | **OK** | Walks the project tree and writes one `.st` file per code-bearing object into `/mcp-mirror/` (default), preserving the project tree as nested directories. UTF-8 output. Each file carries a header comment with its CODESYS project path. Verified against X33: 91 files, 254 KB, 0 errors, 7 kinds (FB / DUT / METHOD / FUNCTION / PROGRAM / GVL / UNKNOWN). | -**Tally: 17 ✅ pass, 8 ❌ fail, 3 ⚠️ partial / inconsistent (out of 28 tool invocations across 26 distinct tools).** +**Tally: 28 OK, 4 FAIL, 1 PARTIAL out of 36 distinct tool invocations.** -## What this proves about the watcher fix +Diff vs morning smoke-test (2026-04-25 baseline): all 5 device-side +fails (`connect_to_device`, `download_to_device`, `read_variable`, +`write_variable`, `start_stop_application`) are now PASSING after the +device-side fixes; `list_project_libraries` (the read side of the +library inconsistency) is also PASSING after the late-afternoon +rewrite. Plus seven new tools shipped (six `git_*` and `mirror_export`). +Remaining failures: `create_folder` kwarg, `compile_project` / +`get_compile_messages` JSON-`long`, `add_library` doesn't dedupe / +placeholder, `rename_object` doesn't refactor callers. -Every passing entry above is direct proof the new watcher works on SP21+. Before the fix, all of these tools returned identical `Marshal error: The functionality 'system.execute_on_primary_thread(...) is no longer supported`. After the fix, the underlying CODESYS scripting work runs and the tool either succeeds or fails for **its own** (unrelated) reason. The threading regression is closed. +## What this proves about today's fixes -## Bugs found that are NOT this fix +| Commit | Tool fixed / added | Verification | +|-----------------------|-------------------------------|--------------| +| `e862846` + `eee8ce2` | `connect_to_device` | #22 PASS (was login() drift) | +| `010811b` + `64906c4` | `write_variable` | #25 PASS (was write_value() missing) | +| `b3bf4a8` | `download_to_device` | #28 PASS (same login() drift as connect) | +| `e236a0c` | NEW: `git_init`/`git_status`/`git_commit` | #30/31/32 PASS against `GitSmokeTest.project` -- live PDE Demo subscription on this box (activation flipped the runtime `HasGitLicense` rule from False to True; the same call returns the friendly PDE-required message on installs without a subscription, see `3623c45`) | +| `8a6059b` | NEW: `git_remote_add` + `git_push` | #33/35 PASS (push to local bare + push to gitlab.usv.no via cached creds) | +| `e3e5f58` | NEW: `git_branch_set_upstream_to` | #34 PASS (RTFM-driven; the canonical "init -> remote_add -> set-upstream -> push" flow needs this step or push fails with `does not track an upstream branch`) | +| `e6cfa57` | All git_* (binding persistence) | `project.save()` retrofit after every mutating git op so state survives IDE close. Soft-fail on save error to avoid masking a successful git op as a failure. | +| `9b766c8` | `list_project_libraries` | #18 PASS (rewritten to use `ScriptLibManObjectContainer` API instead of name-matching the libman) | +| `95a884b` | Launcher refuse-on-duplicate | Pre-spawn `tasklist` scan; refuses to launch alongside an existing CODESYS.exe with a clear message listing the offending PIDs. Mitigates v1's #3 orphan/modal cascade. | +| `7a6e725`+`76b7cf4`+`0a4c1a0` | NEW: `mirror_export` | #36 PASS against X33: 91 .st files, 254 KB, 0 errors. Default mirror root iterated through `MCP/mirror/` -> `mcp-mirror/` based on real-world layout feedback. | -These are pre-existing upstream issues exposed once scripting actually runs. Each is worth filing back to upstream as a separate issue / PR after this one merges: +## Bugs still open (filed by # in the table above) -1. **`create_folder` keyword mismatch** (#7) — `create_folder(name=…)` call site needs to match the current CODESYS API signature. -2. **JSON `long` serialization** (#20, #21) — `compile_project` and `get_compile_messages` need a JSON encoder that handles IronPython `long` (e.g. cast `Severity` bitmasks to `int` or `str` before `json.dumps`). -3. **Online API drift** (#23, #24, #27) — `OnlineChangeOption.TryOnlineChange` removed; `login()` now requires explicit args; `Online application` no longer exposes `write_value()/write()`. Fixing these is a wider re-survey of the CODESYS online scripting API. -4. **`shutdown_codesys` doesn't reach orphans** (#3) — when the MCP server child is respawned (e.g. on Claude Code MCP re-registration), CODESYS processes from prior child processes are no longer tracked and survive `shutdown_codesys`. Either the child should kill child-of-child on its own exit, or the launcher should scan for and adopt orphans. -5. **Library list / add inconsistency** (#18, #19) — needs a small repro to determine whether the issue is `add_library` (silent no-op) or `list_project_libraries` (missing the just-added entry). +Each of these is a separate upstream issue worth its own PR back to +`luke-harriman/Codesys-MCP` once cross-referenced against the official +[CODESYS Python scripting docs](https://content.helpme-codesys.com/en/ScriptingEngine/idx-codesys_scripting.html): -## Caveats / things not directly verified +1. **`create_folder` keyword mismatch (#19)** -- `create_folder(name=...)` + call site in `src/scripts/create_folder.py` doesn't match the current + API signature on the parent container object. Pending docs lookup for + the canonical method (likely `add_folder(name)` or positional `name`). -- **SP22 not directly tested** in this run because of the orphaned MCP child issue. A Claude Code restart with only the SP22-bound config would resolve this; the architecture of the fix is identical for SP21 and SP22, so SP22 is expected to behave the same. -- **No real PLC** was connected during the test, so the device/runtime tools (#23–28) were exercised only against a non-running soft PLC target. The errors captured are mostly *script-side* (API mismatches), not network/device errors. -- **Standard.project template** was resolved from `C:\Program Files\CODESYS 3.5.21.50\CODESYS\Templates\Standard.project` regardless of the MCP's `--codesys-path` arg. Worth verifying the template-resolution code uses the same install root as the launched CODESYS. +2. **JSON `long` serialization (#20, #21)** -- `compile_project.py` and + `get_compile_messages.py` need to convert IronPython `long` to `int`/ + `str` before `json.dumps`, or pass a `default=` callable. Same root + issue, single fix can address both. + +3. **`add_library` doesn't dedupe / placeholder (#17)** -- always adds + a new reference even when one with the same name exists; doesn't + format as `* (System)` placeholder so transitive deps don't resolve + to installed versions. The matching READ side (`list_project_libraries`) + was fixed in `9b766c8` -- the write side has the same wrong-axis + bug (uses name-match for the libman lookup) and is the natural + follow-up. Pending docs lookup for `add_placeholder_library` vs + `add_library` semantics. + +4. **`rename_object` partial refactor (#14)** -- updates the renamed + object's own internal declaration but not any other POU that + references the old name. CODESYS UI does the full refactor; need + to find whether scripting exposes a `rename_with_references` or + similar, or implement a brute-force walk + text replace. + +## New infrastructure (this session, not bugs) + +- **PDE-subscription gate detection.** All six `git_*` tools detect + the runtime `HasGitLicense=False` rule and rewrite the error to a + clear "CODESYS Professional Developer Edition subscription required" + message pointing at the store page. Triggered when no PDE + subscription is active; passes through transparently when one is + (Demo or full). See commit `3623c45`. +- **Launcher refuse-on-duplicate.** Pre-spawn `tasklist` scan refuses + to launch a second CODESYS alongside an existing one (orphan from + prior MCP session, user's own interactive IDE, or a CODESYS still + mid-shutdown), since two CODESYS processes against the same project + file race on the lock and the loser pops a "project is currently in + use" modal that freezes script execution. See `95a884b`. Future: + adopt-existing-watcher (find a live ready.signal whose PID is still + alive, attach to it instead of spawning) -- prototyped via the + inject-once.mjs bridge in this session, not yet shipped in the + launcher. + +## Filesystem mirror (Phase 1 of "project as a filesystem") + +`mirror_export` (#36) lays the foundation for an AI-editable text +representation of a CODESYS project. Phase 2 (`sync_pou_from_file`: +parse a `.st` file, split decl/impl, push back via `set_pou_code`) +and Phase 3 (drift detection: re-export when CODESYS state diverges +from the mirror) are not yet shipped. The X33 project was used as +the proving ground -- 91 .st files round-tripping cleanly through +the `(* Project path: ... *)` header convention. + +## Caveats + +- **SP22 only.** SP21 install (`3.5.21.50`) was not exercised in this + run, but the watcher rewrite + login probe were architected to be + SP-version-agnostic; same behaviour expected. Re-run on SP21 is + pending. +- **Real PLC not exercised.** All device-side tests ran against the + Control Win V3 soft-PLC (port 11740), not against actual industrial + hardware (e.g. WAGO PFC). Network/protocol-specific issues that only + surface with a real device aren't covered. +- **Project state mid-test.** Local-side ops were exercised against an + MCPTest project that the user had previously cleaned up by hand + (after the morning sweep had left a duplicate Standard library and + a dangling ST_Sample reference from a non-refactoring rename). + Re-running from a virgin `create_project` would shake out any + state-dependent variations. --- -*Smoke test executed against this fork @ `sp21-plus-migration-notes` branch with the rewritten `src/scripts/watcher.py`, on 2026-04-25.* +*Smoke test executed against this fork @ `sp21-plus-migration-notes` HEAD on 2026-04-25.*