diff --git a/docs/plans/2026-09-10-handshake-reset-echo-design.md b/docs/plans/2026-09-10-handshake-reset-echo-design.md index ced2069d..d637db0d 100644 --- a/docs/plans/2026-09-10-handshake-reset-echo-design.md +++ b/docs/plans/2026-09-10-handshake-reset-echo-design.md @@ -212,3 +212,32 @@ fast-path, and the trigger was not written until the device echoed the reset 751 response wait is dominated by the Device script's own two data writes plus the subscription round-trip. The optional negative control (planting a stale `True` with the OPC UA tool) was skipped: `infra/tools/opcua_tool.py` needs the `asyncua` module, which is not installed here. + +## 11. Addendum 2026-11-10 — implicit reset echo on the six-argument overload + +The database-owned MES/Delmia scripts (13, 14, 36; receiver scripts 15, 16, 25, 26, 37, 38) were +never ported onto the nine-argument overload (§9 / register row 119), so every unported run still +had the §1 race. §9's "existing scripts behave identically" is therefore relaxed for **exactly +one** shape — the defect itself: + +- `ScriptRuntimeContext` records each successful `SetAttribute` of the run (scope-resolved name → + encoded value, last write wins). +- The six-argument `WriteBatchAndWait`, when THIS run wrote `responseKey` to a value other than + `responseValue` (the script's own `CompleteFlag = false` reset), first waits event-driven for + that value to echo and only then writes batch + trigger — the same ordering §2 requires and the + reset-aware overload enforces. The pending write is consumed by that handshake (and by the + reset-aware overload's reset leg), so a second handshake in the same run without a new reset is + unchanged. +- The echo wait **shares** the call's `timeout` with the response wait (unlike the additive + nine-argument form), so an unported script can never run past the deadline it was sized for; a + reset that never echoes returns `false` with nothing written and logs a warning. +- A call with no prior reset write in the run, and the edge-triggered question of §2, are + unchanged. `WaitAsync` used as a handshake is NOT gated: there the trigger is written before the + wait, so a device flipping inside one sampling interval would never report the reset (§2). + +Tests: `HandshakeResetEchoTests` — `LegacyOverload_AfterTheScriptsOwnReset_*` (held until the +echo / no echo ⇒ false and no batch), `LegacyOverload_ResetIsConsumedByTheHandshakeThatObservedIt`, +`ResetAwareOverload_ConsumesItsReset_SoALaterLegacyHandshakeIsNotGated`; the cached-true +legacy test still passes unchanged (no reset in that run). Porting the scripts to the nine-argument +overload is still recommended (phase-aware result, explicit reset timeout) but is no longer needed +to close the race; dropping their `Task.Delay(1000)` remains a script edit. diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 3f0c6366..d49ac577 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Diagnostics; using System.Text.Json; using System.Text.RegularExpressions; @@ -226,6 +227,16 @@ public class ScriptRuntimeContext /// private int _logSeq; + /// + /// Gitea #36: scope-resolved attribute → codec-encoded value THIS run last wrote through + /// and has not yet had a handshake consume. The six-argument + /// reads it to recognise the script's own reset of the response + /// flag (CompleteFlag = false right before the call) and waits for that reset to ECHO + /// before writing the trigger, so the previous run's cached completion value can never satisfy + /// the new handshake. Concurrent because a script may run writes on parallel tasks. + /// + private readonly ConcurrentDictionary _pendingResetWrites = new(StringComparer.Ordinal); + /// /// Step logs (design 2026-09-09 D4/D5): the Log global site scripts call — /// Log.Debug/Info/Warn/Error(message, exception?). @@ -951,6 +962,10 @@ public class ScriptRuntimeContext throw new InvalidOperationException( $"SetAttribute('{attributeName}') failed: {response.ErrorMessage}"); } + + // Remember the write so a following legacy handshake on this attribute can wait for its + // echo (Gitea #36). Last write wins; a handshake consumes the entry. + _pendingResetWrites[attributeName] = value; } /// @@ -970,11 +985,48 @@ public class ScriptRuntimeContext /// Codec-encoded target value (null ⇒ any change). /// How long to wait for the response before returning false. /// true on response match within the timeout; false on timeout. + /// + /// + /// Implicit reset echo (Gitea #36, 2026-11-10). When THIS run wrote + /// through to a value other than + /// — the MES/Delmia pattern + /// CompleteFlag = false; WriteBatchAndWaitAsync(…, "CompleteFlag", true, …) — the call + /// first waits event-driven for the device to echo that reset and only then writes the batch and + /// trigger. Without it the response wait fast-paths on the previous run's cached true + /// (the reset has not echoed yet) and the script reads results the device has not written. The + /// echo wait shares with the response wait, so the call never runs + /// longer than before; a reset that never echoes returns false (timeout) and writes + /// nothing. A call with no such prior write in this run behaves exactly as it always did — + /// the caller still owns the reset, and the nine-argument reset-aware overload remains the + /// explicit, phase-reporting form. + /// + /// public async Task WriteBatchAndWait( IReadOnlyDictionary encodedValues, string triggerAttr, string? triggerEncoded, string responseAttr, string? responseEncoded, TimeSpan timeout) - => (await WriteBatchAndWaitCore(encodedValues, triggerAttr, triggerEncoded, responseAttr, responseEncoded, timeout) + { + if (_pendingResetWrites.TryRemove(responseAttr, out var resetEncoded) + && !string.Equals(resetEncoded, responseEncoded, StringComparison.Ordinal)) + { + var budget = Stopwatch.StartNew(); + var echo = await WaitInternal(responseAttr, resetEncoded, null, timeout, requireGoodQuality: false) + .ConfigureAwait(false); + if (!echo.Matched) + { + _logger.LogWarning( + "Handshake on {Instance}.{Attribute}: the script's reset write ({Reset}) did not echo within {Timeout}s; " + + "trigger '{Trigger}' NOT written (a stale cached completion must not satisfy the handshake)", + _instanceName, responseAttr, resetEncoded, timeout.TotalSeconds, triggerAttr); + return false; + } + + var remaining = timeout - budget.Elapsed; + timeout = remaining > TimeSpan.Zero ? remaining : TimeSpan.FromMilliseconds(1); + } + + return (await WriteBatchAndWaitCore(encodedValues, triggerAttr, triggerEncoded, responseAttr, responseEncoded, timeout) .ConfigureAwait(false)).Matched; + } /// /// Reset-aware handshake (design 2026-09-10 §5) backing the nine-argument @@ -1038,6 +1090,10 @@ public class ScriptRuntimeContext : $"Reset wait for '{resetAttr}' did not match")); } + // The reset leg's own SetAttribute recorded a pending write; it has now been observed, + // so a later legacy handshake in this run must not wait for it again (Gitea #36). + _pendingResetWrites.TryRemove(resetAttr, out _); + // Legs 3 + 4 — unchanged batch/trigger write and response wait. var response = await WriteBatchAndWaitCore( encodedValues, triggerAttr, triggerEncoded, responseAttr, responseEncoded, timeout).ConfigureAwait(false); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/HandshakeResetEchoTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/HandshakeResetEchoTests.cs index e0489b6a..b3e013ef 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/HandshakeResetEchoTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/HandshakeResetEchoTests.cs @@ -87,10 +87,11 @@ public class HandshakeResetEchoTests : TestKit, IDisposable => new Dictionary { ["MoveOutBatchID"] = AttributeValueCodec.Encode(batchId) }; /// - /// Pins the PERMANENT legacy behaviour, not a bug awaiting a fix: the six-argument overload is - /// deliberately unchanged (deployed scripts keep their semantics) and answers from the stale - /// cache — callers migrate to the reset-aware overload instead. Never "fix" the fast-path in - /// place; delete this test only if the legacy overload itself is removed. + /// Pins the legacy behaviour when the run did NOT write a reset itself: the six-argument + /// overload answers from the cache (level-triggered, the caller owns the reset). The waiter's + /// fast-path is not "fixed" in place — the implicit reset echo (Gitea #36, 2026-11-10) engages + /// only when this run's own SetAttribute reset the response attribute; see + /// . /// /// A task that represents the asynchronous operation. [Fact] @@ -109,6 +110,131 @@ public class HandshakeResetEchoTests : TestKit, IDisposable Assert.True(await call); } + private async Task ResetThroughScript(TestProbe dcl, ScriptRuntimeContext ctx) + { + // The MES script's own reset: `rx.Attributes["MoveOutCompleteFlag"] = false;` — a device + // write the DCL acks, with NO echo yet (the cache still holds the previous run's True). + var set = ctx.SetAttribute("MoveOutCompleteFlag", AttributeValueCodec.Encode(false)!); + var reset = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(CompleteTag, reset.TagPath); + dcl.Reply(new WriteTagResponse(reset.CorrelationId, true, null, DateTimeOffset.UtcNow)); + await set; + } + + /// + /// Gitea #36 regression (MesMoveOut, QA 2026-09-10): the script resets CompleteFlag and then + /// calls the SIX-argument overload. Before the fix the response wait fast-pathed on the previous + /// run's cached True (0 ms) and the script read stale results. Now the batch is held until the + /// reset echoes, and only the device's fresh True completes the call. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task LegacyOverload_AfterTheScriptsOwnReset_WaitsForTheEcho_ThenMatchesTheFreshTrue() + { + var (actor, dcl, ctx) = Boot(); + await ResetThroughScript(dcl, ctx); + + var call = ctx.WriteBatchAndWait( + Values(28950), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromSeconds(10)); + + // Stale True in the cache, reset not echoed → no batch, no completion. + dcl.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + Assert.False(call.IsCompleted); + + actor.Tell(new TagValueUpdate(Conn, CompleteTag, false, QualityCode.Good, DateTimeOffset.UtcNow)); + + var batch = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(TriggerTag, batch.TriggerTagPath); + dcl.Reply(new WriteTagBatchResponse(batch.CorrelationId, true, null, DateTimeOffset.UtcNow)); + + // The cache holds the echoed False — only THIS request's completion may finish the call. + dcl.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + Assert.False(call.IsCompleted); + + actor.Tell(new TagValueUpdate(Conn, CompleteTag, true, QualityCode.Good, DateTimeOffset.UtcNow)); + Assert.True(await call); + } + + /// + /// Gitea #36: a reset that never echoes makes the legacy handshake report a timeout and write + /// NOTHING — it must not complete from the stale cached True, nor send the trigger. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task LegacyOverload_AfterTheScriptsOwnReset_NoEcho_ReturnsFalse_AndNeverWritesTheBatch() + { + var (_, dcl, ctx) = Boot(); + await ResetThroughScript(dcl, ctx); + + var matched = await ctx.WriteBatchAndWait( + Values(28950), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromMilliseconds(700)); + + Assert.False(matched); + dcl.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// + /// Gitea #36: the pending reset is consumed by the handshake that observed it — a second legacy + /// handshake in the same run (no new reset) keeps the plain level-triggered behaviour instead + /// of waiting for an echo of a reset that already happened. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task LegacyOverload_ResetIsConsumedByTheHandshakeThatObservedIt() + { + var (actor, dcl, ctx) = Boot(); + await ResetThroughScript(dcl, ctx); + + var first = ctx.WriteBatchAndWait( + Values(1), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromSeconds(10)); + actor.Tell(new TagValueUpdate(Conn, CompleteTag, false, QualityCode.Good, DateTimeOffset.UtcNow)); + var b1 = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + dcl.Reply(new WriteTagBatchResponse(b1.CorrelationId, true, null, DateTimeOffset.UtcNow)); + actor.Tell(new TagValueUpdate(Conn, CompleteTag, true, QualityCode.Good, DateTimeOffset.UtcNow)); + Assert.True(await first); + + var second = ctx.WriteBatchAndWait( + Values(2), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromSeconds(5)); + var b2 = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + dcl.Reply(new WriteTagBatchResponse(b2.CorrelationId, true, null, DateTimeOffset.UtcNow)); + Assert.True(await second); + } + + /// + /// Gitea #36: the reset-aware overload's own reset write is observed by its reset leg, so it + /// must not be left pending for a later six-argument handshake in the same run (which would + /// otherwise wait for an echo of a reset that already happened and time out). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ResetAwareOverload_ConsumesItsReset_SoALaterLegacyHandshakeIsNotGated() + { + var (actor, dcl, ctx) = Boot(); + + var first = ctx.WriteBatchResetAndWait( + Values(1), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromSeconds(10), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(false)!, TimeSpan.FromSeconds(10)); + var reset = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + dcl.Reply(new WriteTagResponse(reset.CorrelationId, true, null, DateTimeOffset.UtcNow)); + actor.Tell(new TagValueUpdate(Conn, CompleteTag, false, QualityCode.Good, DateTimeOffset.UtcNow)); + var b1 = dcl.ExpectMsg(TimeSpan.FromSeconds(5)); + dcl.Reply(new WriteTagBatchResponse(b1.CorrelationId, true, null, DateTimeOffset.UtcNow)); + actor.Tell(new TagValueUpdate(Conn, CompleteTag, true, QualityCode.Good, DateTimeOffset.UtcNow)); + Assert.True((await first).Matched); + + var second = ctx.WriteBatchAndWait( + Values(2), "MoveOutFlag", AttributeValueCodec.Encode(true), + "MoveOutCompleteFlag", AttributeValueCodec.Encode(true), TimeSpan.FromSeconds(3)); + var b2 = dcl.ExpectMsg(TimeSpan.FromSeconds(2)); + dcl.Reply(new WriteTagBatchResponse(b2.CorrelationId, true, null, DateTimeOffset.UtcNow)); + Assert.True(await second); + } + /// Verifies that the reset-aware overload holds the batch until the reset echoes and then matches the fresh true value. /// A task that represents the asynchronous operation. [Fact]