diff --git a/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md b/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md new file mode 100644 index 00000000..61564559 --- /dev/null +++ b/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md @@ -0,0 +1,199 @@ +# WaitAsync / WaitForAttribute compile-surface & cross-process mirror gaps — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task. + +**Goal:** Make the already-shipped `Attributes.WaitAsync`/`WaitForAsync` (template/`call` scripts) and +`Route.To(...).WaitForAttribute(...)` (inbound scripts) actually *usable*, by closing the +compile-/analysis-surface mirror gaps that currently reject them at validation time, and to harden the +parity test so this class of bug can't recur. + +**Background (why this exists):** The WaitAsync feature (spec `docs/plans/2026-06-17-waitfor-attribute-change-helper-spec.md` ++ deferred-items `docs/plans/2026-06-17-waitfor-deferred-items.md`) is merged to `origin/main` and the +**runtime** works. But scripts that call it **fail deployment** because the *compile-validation* layer — +fake "compile surface" classes that scripts are Roslyn-compiled against during validation — was never +updated to mirror the new methods. This was hit live: deploying a template script calling +`Attributes.WaitAsync(...)` to wonder-app-vd03 failed with: + +> `'ScriptCompileSurface.CompileAttributeAccessor' does not contain a definition for 'WaitAsync'` + +…which forced a bounded-poll-loop workaround in the DELMIA/MES receiver scripts on prod. Once this plan +ships, those scripts can be switched back to the cheaper event-driven `WaitAsync`. + +**Tech Stack:** C#/.NET 10, Roslyn (`Microsoft.CodeAnalysis.CSharp.Scripting`), xUnit. No new dependencies. + +**Branch/worktree:** Implement off `origin/main` (currently `c2e89e9`, which carries the WaitAsync runtime + +the routed `WaitForAttribute` runtime). Create a **fresh git worktree** off `origin/main` — do NOT use the +`~/Desktop/ScadaBridge` main checkout (it sits on `feat/ipsen-mes-movein`) and do NOT touch the existing +`.claude/worktrees/*` (other sessions are active there). E.g.: +`git worktree add .claude/worktrees/waitasync-surface origin/main -b fix/waitasync-compile-surface`. + +**Verified current source state (origin/main @ c2e89e9):** +- Runtime methods EXIST: + - `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScopeAccessors.cs` — `AttributeAccessor.WaitAsync` (2 overloads, lines ~110/136) + `WaitForAsync` (2 overloads, lines ~158/175), each `(..., bool requireGoodQuality = false)`. `CompositionAccessor.Attributes` returns `AttributeAccessor` (so `Children["x"].Attributes.WaitAsync(...)` uses the same type). + - `src/ZB.MOM.WW.ScadaBridge.InboundAPI/RouteHelper.cs` — `public async Task WaitForAttribute(string attributeName, object? targetValue, TimeSpan timeout, CancellationToken cancellationToken = default)` (line ~220). `IInstanceRouter.RouteToWaitForAttributeAsync` + the site handler `DeploymentManagerActor.RouteInboundApiWaitForAttribute` (line ~1091) already exist and work. + - `WaitResult`: `src/ZB.MOM.WW.ScadaBridge.Commons/Types/WaitResult.cs` — `public readonly record struct WaitResult(bool Matched, object? Value, string? Quality, bool TimedOut)`. +- Mirror gaps (the bugs): + - **GAP 1 (PRIMARY):** `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptCompileSurface.cs` → `CompileAttributeAccessor` (class at line ~171) has `this[]`, `GetAsync`, `SetAsync`, `Resolve` — but **no `WaitAsync`/`WaitForAsync`**. This is the compile surface for `call`/template scripts (and, via `CompileCompositionAccessor.Attributes`, for `Children["x"].Attributes` too). + - **GAP 3 (secondary):** the inbound-script *analysis* mirrors — `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/InboundScriptHost.cs` (`RouteTarget`, line ~40) and `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxInboundScriptHost.cs` (`RouteTarget`, line ~38) — expose `Call`/`GetAttributes`/`SetAttributes` but **no `WaitForAttribute`**. (Inbound scripts compile/run at deploy time against the real `InboundScriptContext` via `globalsType: typeof(InboundScriptContext)` in `InboundScriptExecutor`, so deploy is fine — but the CentralUI editor's static analysis uses these mirrors, so `Route.To(...).WaitForAttribute(...)` shows as an error there.) + - **GAP 4 (defensive):** `DeploymentManagerActor.RouteInboundApiWaitForAttribute` returns `RouteToWaitForAttributeResponse` with `t.Result.Value` passed straight through (NOT normalized), unlike `RouteInboundApiCall` which wraps the return in `NormalizeRoutedReturnValue(...)` (added because anon/complex types don't survive the cross-process Newtonsoft serializer). The wait `Value` is a single attribute value (scalar/string/array) and normally round-trips, but this is the same risk surface — verify and harden if needed. +- Parity test that SHOULD have caught GAP 1: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs`. + +--- + +### Task 1: Mirror `WaitAsync`/`WaitForAsync` on the template compile surface (PRIMARY FIX) + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 3 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptCompileSurface.cs` (`CompileAttributeAccessor`, ~line 171) +- Test: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs` and/or `tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/RoslynScriptCompilerTests.cs` + +**Step 1: Write the failing test.** Add a test that a `call`-script body using each wait form **passes** +validation (compiles against the surface). Cover root scope and composed scope. Use the same harness the +existing `GetAsync`/`SetAsync` validation tests use (see `RoslynScriptCompilerTests` / `CompileSurfaceParityTests`). +Example bodies that must compile: +```csharp +await Attributes.WaitAsync("Flag", true, System.TimeSpan.FromSeconds(5)); +await Attributes.WaitAsync("Flag", v => v != null, System.TimeSpan.FromSeconds(5), true); +var r = await Attributes.WaitForAsync("Flag", true, System.TimeSpan.FromSeconds(5)); +await Children["LeftMESReceiver"].Attributes.WaitAsync("MoveInCompleteFlag", true, System.TimeSpan.FromSeconds(5)); +``` + +**Step 2: Run it — expect FAIL** with "does not contain a definition for 'WaitAsync'". +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/ -f` (or the SiteRuntime parity test project). + +**Step 3: Add the four mirror methods** to `CompileAttributeAccessor`, signatures **identical** to the +runtime `AttributeAccessor` (ScopeAccessors.cs), bodies throwing the existing `CompileOnly` sentinel like +the sibling members: +```csharp +/// Mirrors AttributeAccessor.WaitAsync. +public Task WaitAsync(string key, object? targetValue, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly); +public Task WaitAsync(string key, Func predicate, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly); +/// Mirrors AttributeAccessor.WaitForAsync. +public Task WaitForAsync(string key, object? targetValue, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly); +public Task WaitForAsync(string key, Func predicate, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly); +``` +`WaitResult` resolves via the existing `using ZB.MOM.WW.ScadaBridge.Commons.Types;` (already at the top of the +file). Ensure `using System;` (for `Func<>`/`TimeSpan`) and `System.Threading.Tasks` are present (they are). +Only `CompileAttributeAccessor` needs editing — `CompileCompositionAccessor.Attributes` already returns it, so +`Children["x"].Attributes.WaitAsync(...)` is covered automatically. + +**Step 4: Run the test — expect PASS.** Also run the full ScriptAnalysis + SiteRuntime script test projects to +confirm no regressions. + +**Step 5: Commit** (pathspec form): `git commit -m "fix(scriptanalysis): mirror WaitAsync/WaitForAsync on CompileAttributeAccessor" -- src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/ScriptCompileSurface.cs tests/...` + +--- + +### Task 2: Harden `CompileSurfaceParityTests` so missing mirrors fail the build + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** none (do after Task 1 so the new methods exist) + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs` + +**Why:** This test exists to keep the compile surface in lockstep with the runtime script API, yet it did +not catch the WaitAsync omission. Close that hole. + +**Steps:** +1. Read the current test to learn its parity strategy (how it maps runtime types ↔ compile-surface types). +2. Add/strengthen a reflection check: for each **public instance method** on the runtime `AttributeAccessor` + (`ScopeAccessors.cs`), assert a method of the **same name and matching parameter count/arity** exists on + `ScriptCompileSurface.CompileAttributeAccessor`. Do the same mapping for `CompositionAccessor` ↔ + `CompileCompositionAccessor` and `ChildrenAccessor` ↔ `CompileChildrenAccessor` if not already covered. + (Match by name + parameter count to tolerate the `object?`-vs-mirror type differences; the goal is to fail + when a runtime method has **no** counterpart, as WaitAsync did.) +3. Run it — it must PASS now (Task 1 added the methods). Temporarily delete one mirror method locally to + confirm the test goes RED, then restore. +4. Commit (pathspec form). + +--- + +### Task 3: Mirror `WaitForAttribute` on the inbound-script analysis surfaces + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/InboundScriptHost.cs` (`RouteTarget`, ~line 40) +- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/SandboxInboundScriptHost.cs` (`RouteTarget`, ~line 38) +- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs` + +**Note:** Inbound deploy/runtime already works (it compiles against the real `InboundScriptContext`), so this +is purely to fix the CentralUI **editor static analysis** flagging a valid call. If, after reading the test, +you find inbound scripts are *not* statically analysed against these mirror `RouteTarget`s anywhere a user +sees the result, this task may be dropped — confirm before deciding. + +**Steps:** +1. Add a `WaitForAttribute` method to each analysis `RouteTarget`, mirroring the runtime + `RouteHelper.WaitForAttribute` signature, in the same throwing/`CompileOnly` style as the sibling `Call`: + ```csharp + public System.Threading.Tasks.Task WaitForAttribute( + string attributeName, object? targetValue, System.TimeSpan timeout, + System.Threading.CancellationToken cancellationToken = default) => throw new NotSupportedException(/* CompileOnly */); + ``` + (Match each file's existing convention for the throw/sentinel and namespacing.) +2. Add a test asserting an inbound script using `await Route.To("X").WaitForAttribute("Flag", true, System.TimeSpan.FromSeconds(5))` passes analysis. +3. Run the CentralUI analysis test project; commit (pathspec form). + +--- + +### Task 4: Verify (and if needed normalize) the routed `WaitForAttribute` response value + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** none + +**Files:** +- Inspect/Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs` (`RouteInboundApiWaitForAttribute`, ~line 1091; `NormalizeRoutedReturnValue`, ~line 999) +- Test: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/` (the routed-wait / DeploymentManager tests) + +**Why:** `RouteInboundApiCall` normalizes its return value before sending it back across the Central↔Site +Akka boundary (anon/complex types are silently dropped by the cross-process serializer → caller `Ask` +hangs to timeout). `RouteInboundApiWaitForAttribute` passes `t.Result.Value` through un-normalized. The wait +value is a single attribute value and usually round-trips, so this may already be safe. + +**Steps:** +1. Determine whether a `WaitForAttribute` `Value` can be a type that does NOT round-trip the configured + cross-process serializer (check what `InstanceActor` puts in `WaitForAttributeResponse.Value` — raw boxed + attribute value? a `List<...>` for array attributes? already codec-encoded?). +2. If any non-trivially-serializable shape is possible, wrap it: `NormalizeRoutedReturnValue(t.Result.Value)` + in the success branch (reuse the existing helper). If it is provably always a primitive/string/`null`, + instead add a brief code comment documenting *why* it is safe and a test asserting an array-valued wait + round-trips end-to-end (a regression guard). +3. Add/extend a test for the routed-wait response carrying a representative value across the boundary. +4. Run the SiteRuntime actor tests; commit (pathspec form). + +--- + +## After all tasks + +1. Full solution build + run the affected test projects (ScriptAnalysis, SiteRuntime, CentralUI, InboundAPI). +2. Dispatch a final reviewer over the cumulative diff. +3. Use superpowers-extended-cc:finishing-a-development-branch. + +## Downstream follow-up (NOT part of this plan — do after it ships & deploys) +Once this is merged and deployed to **wonder-app-vd03**, the DELMIA/MES receiver scripts there +(`MesMoveIn`/`MesMoveOut` on T1 + `ProcessRecipeDownload` on T1, provisioned 2026-06-17) currently use a +bounded poll loop as the WaitAsync fallback. They can be switched to the event-driven form via the mgmt API +`UpdateTemplateScript`, replacing: +```csharp +bool done = false; +for (int i = 0; i < 100 && !done && !CancellationToken.IsCancellationRequested; i++) { + await System.Threading.Tasks.Task.Delay(250, CancellationToken); + done = IsTrue(await recv.Attributes.GetAsync("MoveInCompleteFlag")); +} +if (!done) return /* timeout */; +``` +with: +```csharp +if (!await recv.Attributes.WaitAsync("MoveInCompleteFlag", true, System.TimeSpan.FromSeconds(30))) + return /* timeout */; +``` +(and the analogous `MoveOutCompleteFlag` / `RecipeProcessedFlag` lines). Verify the deployed build includes +Task 1 first (a deploy of a WaitAsync script will otherwise fail validation again). diff --git a/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md.tasks.json b/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md.tasks.json new file mode 100644 index 00000000..5170cefc --- /dev/null +++ b/docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md.tasks.json @@ -0,0 +1,13 @@ +{ + "planPath": "docs/plans/2026-06-17-waitasync-compile-surface-mirror-gaps.md", + "branch": "fix/waitasync-compile-surface", + "worktree": ".claude/worktrees/waitasync-surface", + "baseSha": "c2e89e9d40fbfd0b11b1b5124efcb835dfb9eb90", + "tasks": [ + {"id": 158, "key": "WS-1", "subject": "Mirror WaitAsync/WaitForAsync on template compile surface", "classification": "small", "parallelizableWith": ["WS-3"], "status": "pending"}, + {"id": 159, "key": "WS-2", "subject": "Harden CompileSurfaceParityTests to catch missing mirrors", "classification": "standard", "parallelizableWith": [], "blockedBy": [158], "status": "pending"}, + {"id": 160, "key": "WS-3", "subject": "Mirror WaitForAttribute on inbound-script analysis surfaces", "classification": "small", "parallelizableWith": ["WS-1"], "status": "pending"}, + {"id": 161, "key": "WS-4", "subject": "Verify/normalize routed WaitForAttribute response value", "classification": "standard", "parallelizableWith": [], "status": "pending"} + ], + "lastUpdated": "2026-06-17" +}