diff --git a/docs/plans/2026-06-18-script-editor-ctx-completions.md b/docs/plans/2026-06-18-script-editor-ctx-completions.md new file mode 100644 index 00000000..dc0dc8c2 --- /dev/null +++ b/docs/plans/2026-06-18-script-editor-ctx-completions.md @@ -0,0 +1,263 @@ +# Script-editor `ctx` tag-completion correctness + truthfulness — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task. + +**Goal:** Make the Monaco script-editor's tag-path completion/hover fire only on `ctx.` receivers (matching the runtime harvest) and stop advertising `ctx.SetVirtualTag(...)` cross-tag writes as if they work (they are a no-op in the live single-tag engine). + +**Architecture:** Both `ScriptAnalysisService.CompleteAsync` and `.Hover` route through one private syntactic gate, `TryGetTagPathLiteral`. Add a receiver guard (`ctx` identifier) there, and surface which method matched so `Hover` can append a truthful note for `SetVirtualTag`. AdminUI-only; no runtime/Commons/proto/EF change; no bUnit. + +**Tech Stack:** C# / .NET 10, Roslyn syntax API (`Microsoft.CodeAnalysis.CSharp`), xUnit + Shouldly. Design: `docs/plans/2026-06-18-script-editor-ctx-completions-design.md` (committed `e84f7a6e`). + +--- + +### Task 1: Receiver-type guard + truthful `SetVirtualTag` hover + +**Classification:** small +**Estimated implement time:** ~5 min +**Parallelizable with:** none + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs` + - `TryGetTagPathLiteral` (currently ~line 213–229): add the `ctx` receiver guard + a new `out bool isSetVirtualTag`. + - `CompleteAsync` call site (~line 164): pass `out _` for the new param. + - `Hover` call site (~line 255) + the `tagMd` build (~line 268–272): pass `out var isSetVirtualTag`; append the single-tag-drop note when it is true. +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs` (create) + +(The `Files:` block IS the contract — do not touch the runtime, Commons, or the catalog. `TryGetTagPathLiteral` is purely syntactic; keep it that way — do NOT introduce a semantic/type check on the receiver. The `ctx` identifier match is intentional: it mirrors `Commons/Types/EquipmentScriptPaths.GetTagRefRegex` = `ctx\s*\.\s*(?:GetTag|SetVirtualTag)`, so the editor accepts exactly what `Phase7Composer` harvests.) + +**Context — current gate (verbatim):** +```csharp +private static bool TryGetTagPathLiteral(SyntaxToken token, out string prefix) +{ + prefix = ""; + var literal = token.Parent as LiteralExpressionSyntax + ?? token.GetPreviousToken().Parent as LiteralExpressionSyntax; + if (literal is null || !literal.IsKind(SyntaxKind.StringLiteralExpression)) return false; + if (literal.Parent is not ArgumentSyntax arg) return false; + if (arg.Parent is not ArgumentListSyntax argList) return false; + if (argList.Parent is not InvocationExpressionSyntax inv) return false; + if (inv.Expression is not MemberAccessExpressionSyntax ma) return false; + var method = ma.Name.Identifier.ValueText; + if (method is not ("GetTag" or "SetVirtualTag")) return false; + // only the FIRST argument is the path + if (argList.Arguments.Count > 0 && argList.Arguments[0] != arg) return false; + prefix = literal.Token.ValueText ?? ""; + return true; +} +``` + +The two callers today: +- `CompleteAsync`: `if (_catalog != null && TryGetTagPathLiteral(token, out var pathPrefix))` +- `Hover`: `if (_catalog is not null && TryGetTagPathLiteral(token, out var tagPath) && !string.IsNullOrEmpty(tagPath))`, which then builds: +```csharp +var info = await _catalog.GetTagInfoAsync(tagPath, CancellationToken.None); +var tagMd = info is null + ? $"**Tag path** `{Code(tagPath)}`\n\n⚠ Not a known configured tag path." + : $"**Tag path** `{Code(info.Path)}`\n\n{info.Kind} · Type **{info.DataType}**" + + (info.DriverInstanceId is null ? "" : $" · Driver `{Code(info.DriverInstanceId)}`"); +return new HoverResponse(tagMd); +``` + +**Step 1: Write the failing tests** + +Create `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs`: + +```csharp +using System.Threading; +using System.Threading.Tasks; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +public sealed class CtxCompletionGuardTests +{ + // Catalog that always offers one distinctive path + resolves a vtag, so the tag-path branch is + // observable without a DB. The path "Line1.Speed" never appears in C# scoped/dot completions, + // so its presence/absence cleanly distinguishes the tag-path branch from the fallback. + private sealed class FakeCatalog : IScriptTagCatalog + { + public Task> GetPathsAsync(string? f, CancellationToken ct) + => Task.FromResult>(new[] { "Line1.Speed" }); + public Task GetTagInfoAsync(string path, CancellationToken ct) + => Task.FromResult(new ScriptTagInfo(path, "Virtual tag", "Double", null)); + public Task> GetEquipmentRelativeLeavesAsync(string? f, CancellationToken ct) + => Task.FromResult>(System.Array.Empty()); + } + + private static readonly ScriptAnalysisService Svc = new(new FakeCatalog()); + + // Caret column lands INSIDE the string literal (just after the opening quote). If a test does not + // hit the tag-path branch, nudge the column to sit on a literal char and re-run — the gate keys off + // the token under the caret. Columns are 1-based. + + [Fact] public async Task Completion_on_ctx_GetTag_literal_offers_catalog_paths() + { + // return ctx.GetTag("| — caret just after the opening quote + var res = await Svc.CompleteAsync(new CompletionsRequest("""return ctx.GetTag("""" + "\"", 1, 19)); + res.Items.ShouldContain(i => i.InsertText == "Line1.Speed"); + } + + [Fact] public async Task Completion_on_non_ctx_receiver_does_NOT_offer_catalog_paths() + { + // foo.GetTag("| — receiver is not ctx, so the tag-path branch must NOT fire. + var res = await Svc.CompleteAsync(new CompletionsRequest("""return foo.GetTag("""" + "\"", 1, 19)); + res.Items.ShouldNotContain(i => i.InsertText == "Line1.Speed"); + } + + [Fact] public async Task Hover_on_ctx_SetVirtualTag_literal_warns_write_is_dropped() + { + // hover the path literal inside ctx.SetVirtualTag("V") + var md = (await Svc.Hover(new HoverRequest("""return ctx.SetVirtualTag("V", 1);""", 1, 27))).Markdown; + md.ShouldNotBeNull(); + md!.ShouldContain("single-tag mode"); + } + + [Fact] public async Task Hover_on_ctx_GetTag_literal_has_no_dropped_write_note() + { + var md = (await Svc.Hover(new HoverRequest("""return ctx.GetTag("V").Value;""", 1, 20))).Markdown; + md.ShouldNotBeNull(); + md!.ShouldNotContain("single-tag mode"); + } + + [Fact] public async Task Hover_on_non_ctx_receiver_literal_is_not_treated_as_a_tag_path() + { + // bar.GetTag("V") — receiver not ctx; must not produce the "Tag path" hover. + var md = (await Svc.Hover(new HoverRequest("""return bar.GetTag("V");""", 1, 20))).Markdown; + (md is null || !md.Contains("Tag path")).ShouldBeTrue(); + } +} +``` + +**Step 2: Run the tests; verify they FAIL** + +Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests --filter "FullyQualifiedName~CtxCompletionGuardTests"` (use `dangerouslyDisableSandbox: true`). +Expected: the non-`ctx` tests FAIL (today the gate fires on any receiver) and the SetVirtualTag-note test FAILS (no note yet). The two positive `ctx.` tests may already pass. If a caret-column does not hit the literal branch, adjust the column and re-run until the positive cases hit the tag-path branch, THEN confirm the negatives still fail for the right reason. + +**Step 3: Implement Component 1 (receiver guard) + Component 2 (method-discriminated note)** + +In `TryGetTagPathLiteral`, change the signature and body: +```csharp +private static bool TryGetTagPathLiteral(SyntaxToken token, out string prefix, out bool isSetVirtualTag) +{ + prefix = ""; + isSetVirtualTag = false; + var literal = token.Parent as LiteralExpressionSyntax + ?? token.GetPreviousToken().Parent as LiteralExpressionSyntax; + if (literal is null || !literal.IsKind(SyntaxKind.StringLiteralExpression)) return false; + if (literal.Parent is not ArgumentSyntax arg) return false; + if (arg.Parent is not ArgumentListSyntax argList) return false; + if (argList.Parent is not InvocationExpressionSyntax inv) return false; + if (inv.Expression is not MemberAccessExpressionSyntax ma) return false; + // Receiver guard: only ctx.GetTag(...) / ctx.SetVirtualTag(...) are real tag-path calls. Mirrors the + // runtime harvest (EquipmentScriptPaths.GetTagRefRegex is syntactically `ctx`-anchored), so the editor + // offers tag completions/hover for exactly what Phase7Composer harvests — not an unrelated x.GetTag(...). + if (ma.Expression is not IdentifierNameSyntax { Identifier.ValueText: "ctx" }) return false; + var method = ma.Name.Identifier.ValueText; + if (method is not ("GetTag" or "SetVirtualTag")) return false; + // only the FIRST argument is the path + if (argList.Arguments.Count > 0 && argList.Arguments[0] != arg) return false; + prefix = literal.Token.ValueText ?? ""; + isSetVirtualTag = method == "SetVirtualTag"; + return true; +} +``` + +Update `CompleteAsync` (the tag-path branch condition) to discard the new param: +```csharp +if (_catalog != null && TryGetTagPathLiteral(token, out var pathPrefix, out _)) +``` + +Update `Hover`'s condition + `tagMd` build: +```csharp +if (_catalog is not null && TryGetTagPathLiteral(token, out var tagPath, out var isSetVirtualTag) && !string.IsNullOrEmpty(tagPath)) +{ + ... + var info = await _catalog.GetTagInfoAsync(tagPath, CancellationToken.None); + var tagMd = info is null + ? $"**Tag path** `{Code(tagPath)}`\n\n⚠ Not a known configured tag path." + : $"**Tag path** `{Code(info.Path)}`\n\n{info.Kind} · Type **{info.DataType}**" + + (info.DriverInstanceId is null ? "" : $" · Driver `{Code(info.DriverInstanceId)}`"); + // Truthfulness: ctx.SetVirtualTag(...) is a no-op in the live single-tag evaluator + // (RoslynVirtualTagEvaluator drops cross-tag writes; the cascade VirtualTagEngine is dormant). + // Surface that so an author is not misled into relying on it. + if (isSetVirtualTag) + tagMd += "\n\n⚠ Cross-tag `SetVirtualTag` writes are currently dropped in single-tag mode " + + "(the cascade engine is not wired into the host); this write will not take effect at runtime."; + return new HoverResponse(tagMd); +} +``` +(Leave the `{{equip}}` short-circuit branch above this untouched — `SetVirtualTag("{{equip}}.x")` is a rare combination and out of scope.) + +**Step 4: Run the tests; verify they PASS** + +Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests --filter "FullyQualifiedName~CtxCompletionGuardTests"` +Expected: all 5 PASS. + +**Step 5: Run the full AdminUI suite to confirm no regression** + +Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests` (use `dangerouslyDisableSandbox: true`). +Expected: green. Existing tag-path tests (e.g. `HoverSignatureTests`) use `ctx.` receivers, so the guard does not affect them. If any pre-existing test used a non-`ctx` receiver on a tag-path literal, that is a real behavior change — surface it, do not weaken the guard. + +**Step 6: Commit** +```bash +git add src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs \ + tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CtxCompletionGuardTests.cs +git commit -m "fix(adminui): ctx-receiver guard + truthful SetVirtualTag hover in script-editor completions" +``` + +--- + +### Task 2: Docs + full build/test + live `/run` + finish + +**Classification:** small +**Estimated implement time:** ~4 min (excluding the live-rig step, which the controller drives) +**Parallelizable with:** none (blocked by Task 1) + +**Files:** +- Modify: `docs/ScriptEditor.md` (the completions/hover section — note the `ctx`-receiver scoping and that `SetVirtualTag` cross-tag writes are dropped in single-tag mode, hover surfaces this). + +**Step 1: Update docs** + +In `docs/ScriptEditor.md`, find the section describing tag-path completions/hover and add a short, accurate note: +- Tag-path completions/hover fire only on `ctx.GetTag(...)` / `ctx.SetVirtualTag(...)` (a non-`ctx` receiver is not treated as a tag call), matching the deploy-time dependency harvest. +- `ctx.SetVirtualTag(...)` cross-tag writes are currently dropped by the live single-tag evaluator (the cascade engine is not wired into the host); the editor's hover surfaces this so it is not relied upon. Do NOT overclaim a fix to the cross-tag write path itself. + +**Step 2: Full build** + +Run: `dotnet build ZB.MOM.WW.OtOpcUa.slnx` (use `dangerouslyDisableSandbox: true`). +Expected: 0 errors. + +**Step 3: AdminUI tests** + +Run: `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests` (use `dangerouslyDisableSandbox: true`). +Expected: green. + +**Step 4: Commit docs** +```bash +git add docs/ScriptEditor.md +git commit -m "docs(scripteditor): ctx-receiver scoping + SetVirtualTag single-tag-drop note" +``` + +**Step 5: Live `/run` verification (controller-driven, docker-dev)** + +The docker-dev rig is LOCAL + login-disabled (no sign-in needed). Rebuild the AdminUI from the branch and drive the script editor via the Chrome tools against `http://localhost:9200`: +- Open a virtual-tag's ScriptEdit page (or the inline vtag-modal Monaco). +- Type `ctx.GetTag("` → tag-path completions appear. +- Type a non-`ctx` receiver `foo.GetTag("` → tag-path completions do NOT appear (C# completions only). +- Hover a `ctx.SetVirtualTag("…")` literal → the single-tag-drop note shows. + +If the rig cannot exercise a step (e.g. Monaco completion timing), record it honestly as unit-proven + smoke-checked rather than overclaiming. + +**Step 6: Finish — merge to master + push** + +Use superpowers-extended-cc:finishing-a-development-branch: verify tests green, then merge `feat/script-editor-ctx-completions` to master (ff) and push. Stage by explicit path only; never `git add .`; never stage `stillpending.md` / `pending.md` / `docker-dev/docker-compose.yml` / `sql_login.txt` / `pki/`. + +--- + +## Notes for the executor + +- **Two tasks, sequential** (Task 2 blocked by Task 1) — both touch a tightly-coupled surface; no parallelism. +- **Hard rules:** stage by path (never `git add .`); never stage the never-stage files; NO runtime/Commons/proto/EF change; NO bUnit; `dangerouslyDisableSandbox: true` for all build/test/rig commands. +- **Done =** build 0 errors + AdminUI tests green + live `/run` confirms the three behaviors → merged to master + pushed. diff --git a/docs/plans/2026-06-18-script-editor-ctx-completions.md.tasks.json b/docs/plans/2026-06-18-script-editor-ctx-completions.md.tasks.json new file mode 100644 index 00000000..04bf5e6e --- /dev/null +++ b/docs/plans/2026-06-18-script-editor-ctx-completions.md.tasks.json @@ -0,0 +1,14 @@ +{ + "planPath": "docs/plans/2026-06-18-script-editor-ctx-completions.md", + "designPath": "docs/plans/2026-06-18-script-editor-ctx-completions-design.md", + "designCommit": "e84f7a6e", + "baseMaster": "651018f9", + "branch": "feat/script-editor-ctx-completions", + "scope": "AdminUI-only ScriptAnalysis fix. Component 1: ctx-receiver guard in ScriptAnalysisService.TryGetTagPathLiteral (mirror the runtime EquipmentScriptPaths.GetTagRefRegex `ctx`-anchor) so tag-path completion/hover fires only on ctx.GetTag/ctx.SetVirtualTag, not an unrelated x.GetTag(...). Component 2: surface which method matched; Hover appends a truthful note that ctx.SetVirtualTag cross-tag writes are dropped in single-tag mode (RoslynVirtualTagEvaluator drops them; cascade VirtualTagEngine dormant). The 'authoritative write-key' goal is dropped as moot. NO runtime/Commons/proto/EF change; NO bUnit.", + "dependencyGraph": "T1 -> T2", + "tasks": [ + {"id": 1, "nativeId": 532, "subject": "ctx-receiver guard + truthful SetVirtualTag hover", "classification": "small", "status": "pending"}, + {"id": 2, "nativeId": 533, "subject": "Docs + build/test + live /run + finish", "classification": "small", "status": "pending", "blockedBy": [1]} + ], + "lastUpdated": "2026-06-18" +}