docs(plans): add WaitAsync compile-surface mirror-gaps plan + tasks
This commit is contained in:
Joseph Doherty
@@ -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<bool> 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
|
||||
/// <summary>Mirrors <c>AttributeAccessor.WaitAsync</c>.</summary>
|
||||
public Task<bool> WaitAsync(string key, object? targetValue, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly);
|
||||
public Task<bool> WaitAsync(string key, Func<object?, bool> predicate, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly);
|
||||
/// <summary>Mirrors <c>AttributeAccessor.WaitForAsync</c>.</summary>
|
||||
public Task<WaitResult> WaitForAsync(string key, object? targetValue, TimeSpan timeout, bool requireGoodQuality = false) => throw new NotSupportedException(CompileOnly);
|
||||
public Task<WaitResult> WaitForAsync(string key, Func<object?, bool> 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<bool> 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).
|
||||
@@ -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"
|
||||
}
|
||||
Reference in New Issue
Block a user