docs(scripts): design for #36 — reset-aware WriteBatchAndWaitAsync handshake
The MES/Delmia handshake fast-paths on the previous run's cached CompleteFlag because the reset write has not echoed back before the wait registers. An edge-triggered wait is rejected (a subscription can swallow the reset when the device flips inside one sampling interval); the fix encodes the reset leg inside a new WriteBatchAndWaitAsync overload that observes the reset echo before writing the trigger and returns a phase-aware HandshakeResult. Claude-Session: https://claude.ai/code/session_01XPsuy2ZZ5Gbyfpt8PrK5BV
This commit is contained in:
Joseph Doherty
@@ -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<bool> Attributes.WriteBatchAndWaitAsync(
|
||||
IReadOnlyDictionary<string, object?> 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<HandshakeResult> Attributes.WriteBatchAndWaitAsync(
|
||||
IReadOnlyDictionary<string, object?> 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.
|
||||
Reference in New Issue
Block a user