diff --git a/docs/plans/2026-09-10-handshake-reset-echo-design.md b/docs/plans/2026-09-10-handshake-reset-echo-design.md new file mode 100644 index 00000000..4d008f04 --- /dev/null +++ b/docs/plans/2026-09-10-handshake-reset-echo-design.md @@ -0,0 +1,179 @@ +# Design — reset-aware `WriteBatchAndWaitAsync` handshake (Gitea #36) + +**Date:** 2026-09-10 +**Issue:** Gitea #36 — *Scripts: WriteBatchAndWaitAsync handshake fast-paths on the previous run's +cached CompleteFlag (MesMoveOut returns stale BatchID/result)* +**Status:** approved 2026-09-10; scope for this change is **repo only** (runtime, mirrors, tests, +docs, rig). The QA/prod MES template-script rewrite is a deployment record, not part of this change. + +## 1. The defect + +The MES/Delmia receiver handshake is a level-triggered flag protocol on data-sourced attributes: + +``` +CompleteFlag = false // reset (device write; the site cache is NOT updated optimistically) +WriteBatchAndWaitAsync(values, "MoveOutFlag", true, "MoveOutCompleteFlag", true, 25 s) +read Successful / BatchID / ErrorText +``` + +`WriteBatchAndWaitAsync` writes the batch and trigger and then registers an event-driven wait on +`CompleteFlag == true`. The waiter's **current-value fast-path** (spec §5, by design) answers +immediately if the cache already holds the target. After the first run, the cache holds the +previous run's `true` until the device echoes the reset back through the subscription — and that +echo has not arrived by the time the wait registers (16 ms after the reset write on QA). The wait +therefore matches in 0 ms, and the script reads result attributes the device has not yet written. +The fixed `Task.Delay(1000)` in the scripts masks the race only when the device completes inside a +second; QA measured 0.4–1.2 s, so the outcome is a coin flip (issue evidence table). + +## 2. Why an edge-triggered wait is not the fix + +The obvious alternative — a waiter that ignores the cached value and matches only on a change +event arriving after registration — trades one race for a worse one. A subscription reports a +**change from the last reported value**. If the reset (`false`) and the trigger are written back to +back and the device flips `false → true` inside one sampling interval, the `false` is never +reported: the last reported value stays `true`, the device's new `true` is not a change, and the +waiter never resolves. Nothing site-side can recover that; the protocol itself requires the reset +to be **observed** before the trigger is written, so the device cannot start until the flag is +provably low. That is exactly what the interim QA patch does (`SetAsync(false)` then +`WaitAsync(false, 10 s)` before the batch call), and it observed a real edge on every run since. + +Once the reset echo has been observed, the existing level-triggered fast-path is correct again: +the only `true` the cache can hold afterwards is one the device wrote after seeing the trigger. + +## 3. Decision + +**Encode the reset leg inside `WriteBatchAndWaitAsync`** as a new overload, so a handshake cannot +be written with the legs in the wrong order, and return a **phase-aware result** so a script can +report *which* leg timed out. The runtime primitive lives in `ScriptRuntimeContext`; the +`InstanceActor` waiter is unchanged. + +Alternatives considered: + +| Option | Verdict | +|---|---| +| Edge-triggered flag on `WaitAsync` / the batch call | Rejected — §2, non-deterministic under sampling. | +| Standalone `Attributes.SetAndWaitAsync(key, value, timeout)` → `bool` | Smaller and reusable, but nothing stops an author from omitting it; two calls per handshake. Not chosen. | +| Keep `bool`, throw on reset timeout | Simplest surface, but an inbound method would surface `SCRIPT_ERROR` instead of a structured `WasSuccessful=false`. Not chosen. | +| **Reset leg inside the batch call + `HandshakeResult`** | **Chosen.** | + +## 4. Script-facing API + +```csharp +// Existing (unchanged): level-triggered, caller owns the reset. +Task Attributes.WriteBatchAndWaitAsync( + IReadOnlyDictionary values, string flagKey, object? flagValue, + string responseKey, object? responseValue, TimeSpan timeout); + +// New: reset-aware. Reset arguments LAST so the six leading positions keep their meaning. +Task Attributes.WriteBatchAndWaitAsync( + IReadOnlyDictionary values, string flagKey, object? flagValue, + string responseKey, object? responseValue, TimeSpan timeout, + string resetKey, object? resetValue, TimeSpan? resetTimeout = null); +``` + +```csharp +// Commons/Types, beside WaitResult. +public enum HandshakePhase { Reset, Response } + +public readonly record struct HandshakeResult( + bool Matched, object? Value, string? Quality, bool TimedOut, + HandshakePhase Phase, string? ErrorMessage); +``` + +Semantics of the new overload, in execution order: + +1. **Reset write** — `resetKey = resetValue` through the existing `SetAttribute` path (static or + data-sourced; a device-write failure throws `InvalidOperationException`, exactly as the batch + leg does today). +2. **Reset wait** — the existing event-driven waiter on `resetKey == resetValue`, bounded by + `resetTimeout ?? timeout`, quality-agnostic. Not matched ⇒ return + `HandshakeResult { Matched=false, TimedOut, Phase=Reset, ErrorMessage }` and **never write the + batch or the trigger**. On a static attribute the cache updates synchronously, so this wait is + an immediate fast-path — the overload is safe to use uniformly. +3. **Batch + trigger** — unchanged existing leg (all values data-sourced, one connection, trigger + written after the batch; failure throws). +4. **Response wait** — unchanged existing waiter on `responseKey == responseValue`, bounded by + `timeout`; returns `Phase=Response` with the matched value/quality, or `TimedOut`. + +Typical use (the MES scripts after the port): + +```csharp +var hs = await rx.Attributes.WriteBatchAndWaitAsync( + values, "MoveOutFlag", true, "MoveOutCompleteFlag", true, TimeSpan.FromSeconds(25), + resetKey: "MoveOutCompleteFlag", resetValue: false, resetTimeout: TimeSpan.FromSeconds(10)); +if (!hs.Matched) + return new { WasSuccessful = false, BatchID = 0, + ErrorText = hs.Phase == HandshakePhase.Reset + ? "Timeout waiting for MoveOutCompleteFlag to reset" + : "Timeout waiting for MoveOut to complete" }; +// result attributes are now the device's answer to THIS request — no Task.Delay needed +``` + +## 5. Runtime shape + +- `ScriptRuntimeContext.WriteBatchResetAndWait(...)` composes the existing building blocks: + `SetAttribute` → `WaitInternal` → the existing batch Ask → `WaitInternal`. No new actor messages, + no proto change, no InstanceActor change. Each leg is already a Flow Debugger step + (`AttributeWrite` + `AttributeWait` for the reset, the batch `AttributeWrite`, the response + `AttributeWait`), so a traced run shows all four legs with no `FlowGraphBuilder` work. +- The script execution-timeout token bounds every Ask as it does today; a deadline hit mid-wait + propagates as cancellation (spec §4.3), unchanged. +- **Residual, documented not fixed:** a *previous* run that timed out and whose late `true` echo + arrives after this run's reset echo can still satisfy the response wait. A level-triggered flag + protocol cannot distinguish that case; it needs a sequence/handle attribute on the device side. + +## 6. Mirrors and analysis surfaces + +- `ScriptCompileSurface` (ScriptAnalysis) — compile-only mirror of the new overload + (`NotSupportedException(CompileOnly)`), plus `HandshakeResult`/`HandshakePhase` visible to scripts. +- `SandboxScriptHost` (CentralUI Test Run) — mirror that throws the labelled `NotInSandbox` + exception, the same as the existing batch overload (no device batch transport in the sandbox). +- `FlowShapePredictor` — the nine-argument form predicts **four** nodes: reset `AttributeWrite` + (Tags), reset `AttributeWait` (Site), batch `AttributeWrite` (Tags), response `AttributeWait` + (Site). Argument indices 1 and 3 keep their existing meaning; the reset key is index 6. +- `CompileSurfaceParityTests` enforces the mirrors by reflection; no manual list to update. +- The routed `Route.To(code).WaitForAttribute` and the sandbox `WaitAsync` are untouched — the + overload is a site-local composition, not a new wire verb. + +## 7. Tests + +1. **`HandshakeResetEchoTests`** (SiteRuntime.Tests/Scripts) over the **real `InstanceActor`** with + a DCL test probe and data-sourced Boolean attributes, cache seeded to `CompleteFlag=true`: + - *Reset gates the trigger:* the probe receives `WriteTagRequest(reset=false)` and answers + success but does **not** echo. Assert **no** `WriteTagBatchRequest` arrives and the task is + still pending. Echo `false` via `TagValueUpdate`; assert the batch request arrives. Echo + `true`; assert `Matched`, `Phase=Response`, `Value=true`. + - *Reset timeout:* no echo ever; assert `Matched=false, TimedOut=true, Phase=Reset` and that no + batch request was ever sent. + - *Static reset attribute:* reset on a static attribute fast-paths; the batch is written at once. +2. **Trace emission** (`ScriptRuntimeContextTraceEmissionTests`): a traced reset-aware call emits + the reset `AttributeWrite` + `AttributeWait` rows before the batch row. +3. **`FlowShapePredictorTests`**: the nine-argument form yields the four nodes in order. +4. Parity: `CompileSurfaceParityTests` (existing) covers the mirrors. + +## 8. Docs and scripts (repo) + +- `docs/plans/2026-06-17-waitfor-attribute-change-helper-spec.md` §5 — add a *handshake reset + ordering* correctness note pointing here. +- `docs/requirements/Component-ScriptAnalysis.md` — list the overload among the mirrored members. +- `docs/api-specs/Delmia-Recipe-Download-API.md` §4 / `docs/plans/2026-06-16-ipsen-mes-movein-design.md` + — note the race and the intended rewrite (the deployed prod/QA bodies are unchanged by this repo + change). +- `docker/seed-delmia-sim.sh` — unchanged behaviour (static attributes; cache updates + synchronously so the race cannot occur); add a comment saying so. +- **Rig live gate** — `docker/seed-handshake-demo.sh`: a template with two data-sourced Boolean + attributes on writable OPC UA sim nodes (`Motor.Running` class, `CurrentReadOrWrite`, not + simulated so they hold what is written), a `Call` script using the new overload, and a + ValueChange "device" script that sets the complete flag when the trigger rises. Proves the reset + echo round-trips through a real subscription and that `Phase` reports correctly. +- Deferred-work register — one row: *port the QA/prod MES receiver scripts (13/14/15/16/25/26/36/ + 37/38) onto the overload and drop the `Task.Delay(1000)`*, with the §4 shape; requires the build + on the target central first. +- CLAUDE.md — key-decision bullet under *Architecture & Runtime*. + +## 9. Compatibility + +Additive only: a new overload, two new Commons types, no message/proto/schema change. Existing +scripts compile and behave identically. An older site receiving a script that uses the overload +fails the deploy gate's compile (the member does not exist there), which is the intended +mixed-version outcome — upgrade sites before porting scripts.