From 50bf9ac74ad7b77c9462a3157d39f7baf97c8aee Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 6 Sep 2026 14:12:09 -0400 Subject: [PATCH] fix(site-runtime): trace arm is cleared when the traced DCL write fails (arch-review F6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Flow Debugger trace arm was recorded when a traced write was ISSUED, not when the DCL acknowledged it. A write the device rejected (or that timed out) left the arm standing for the whole TraceArmWindowSeconds (30 s), so the next unrelated subscription update on that attribute was stamped with the debugger's execution id and spawned a traced, parented run — the debugger drew a "device echo" of a write that never landed. The write's continuation runs off the actor's dispatcher, so it cannot touch _traceArms directly; it Tells self a new DisarmTrace message instead, handled in a Receive<>. The disarm is keyed on the writer's execution id, so a late failure cannot steal an arm a newer traced write has since taken. Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29 --- .../Actors/InstanceActor.cs | 37 +++++++++++- .../Actors/InstanceActorTraceArmTests.cs | 59 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index 3989ca20..7c16b147 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -179,6 +179,8 @@ public class InstanceActor : ReceiveActor /// DCL value change for that attribute is published stamped with the writer's id and /// Trace, and the arm is consumed. Bounded by the attribute count (one entry per /// attribute, newest wins); no sweep — an unconsumed arm is overwritten or expires in place. + /// Armed at ISSUE, disarmed on a failed/timed-out write (arch-review F6, ); + /// a successful write's echo consumes it. /// private readonly Dictionary _traceArms = new(); // Maps each tag path to every attribute canonical name that references it. @@ -339,6 +341,17 @@ public class InstanceActor : ReceiveActor Receive(HandleTagValueUpdate); Receive(HandleSubscribeTagsResponse); // S4/UA6: real retry-aware handler Receive(msg => SendTagSubscribe(msg.ConnectionName)); + + // Arch-review F6: a traced write the DCL rejected (or that timed out) drops its own arm, + // so an unrelated later update is not stamped as that write's device echo. + Receive(msg => + { + if (_traceArms.TryGetValue(msg.AttributeName, out var arm) + && arm.SourceExecutionId == msg.SourceExecutionId) + { + _traceArms.Remove(msg.AttributeName); + } + }); Receive(HandleConnectionQualityChanged); // Handle alarm state changes from Alarm Actors (Tell pattern) @@ -756,17 +769,29 @@ public class InstanceActor : ReceiveActor // Flow Debugger trace arm (design §5.3): only a TRACED write with a known writer arms; // the device echo that follows is stamped in HandleTagValueUpdate. Untraced writes - // never touch the map, so production behaviour is unchanged. + // never touch the map, so production behaviour is unchanged. The arm is taken at ISSUE + // and dropped again on a failed/timed-out write (arch-review F6, via the DisarmTrace + // self-message below); a successful write's echo consumes it. + Guid? armed = null; if (command.Trace && command.SourceExecutionId is { } traceSource) { _traceArms[attributeName] = (traceSource, DateTimeOffset.UtcNow.AddSeconds(_options.TraceArmWindowSeconds)); + armed = traceSource; } + // Captured for the continuation: it runs OFF the actor's dispatcher, so it must never + // touch _traceArms directly — it Tells self instead. + var self = Self; + // Ask the DCL and pipe the result back to the original caller. The DCL // returns the failure synchronously so the script can handle it. _dclManager.Ask(writeRequest, TimeSpan.FromSeconds(30)) .ContinueWith(t => { + var succeeded = t.IsCompletedSuccessfully && t.Result.Success; + if (!succeeded && armed is { } armedSource) + self.Tell(new DisarmTrace(attributeName, armedSource)); + if (t.IsCompletedSuccessfully) return new SetStaticAttributeResponse( correlationId, instanceName, attributeName, @@ -2112,6 +2137,16 @@ public class InstanceActor : ReceiveActor /// private sealed record RetryTagSubscribe(string ConnectionName); + /// + /// Self-message (arch-review F6): a traced data write FAILED, so drop its trace arm — but only + /// if it is still the armed one. The write's continuation runs off the dispatcher and must not + /// touch itself; matching on keeps + /// a late failure from stealing an arm a newer traced write has since taken. + /// + /// Canonical name of the attribute whose arm is to be dropped. + /// Execution id of the writer that took the arm. + private sealed record DisarmTrace(string AttributeName, Guid SourceExecutionId); + /// Self-tell that fires the coalesced static-override persist flush (P4, arch-review WP1.5). private sealed class FlushStaticOverrides { diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs index 3e0d1f81..d8166bcf 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs @@ -89,6 +89,17 @@ public class InstanceActorTraceArmTests : TestKit, IDisposable Assert.True(ExpectMsg(TimeSpan.FromSeconds(5)).Success); } + /// Same as but the DCL rejects the write, so the arm must be dropped. + private void FailingWrite(IActorRef actor, TestProbe dcl, string instance, Guid? source, bool trace) + { + actor.Tell(new SetStaticAttributeCommand("c", instance, "Processed", "true", DateTimeOffset.UtcNow, + SourceExecutionId: source, Trace: trace)); + var write = ExpectWrite(dcl); + Assert.Equal(Tag, write.TagPath); + dcl.Reply(new WriteTagResponse(write.CorrelationId, false, "rejected", DateTimeOffset.UtcNow)); + Assert.False(ExpectMsg(TimeSpan.FromSeconds(5)).Success); + } + private static AttributeValueChanged Echo(IActorRef actor, TestProbe subscriber, object? value) { actor.Tell(new TagValueUpdate("PLC", Tag, value, QualityCode.Good, DateTimeOffset.UtcNow)); @@ -123,6 +134,54 @@ public class InstanceActorTraceArmTests : TestKit, IDisposable Assert.False(published.Trace); } + /// + /// Arch-review F6: the arm is recorded when the write is ISSUED, not when the DCL acknowledges + /// it. A write the device rejects must drop the arm, otherwise the next unrelated subscription + /// update on that attribute is stamped with the debugger's execution id for the rest of the + /// 30 s window — the Flow Debugger would draw a "device echo" of a write that never landed. + /// + [Fact] + public void TracedDataWrite_ThatFails_DisarmsTheAttribute_SoTheNextEchoIsUnstamped() + { + var (actor, dcl, sub) = Create("P6"); + + FailingWrite(actor, dcl, "P6", Guid.NewGuid(), trace: true); + var published = Echo(actor, sub, "unrelated"); + + Assert.Null(published.SourceExecutionId); + Assert.False(published.Trace); + } + + /// + /// A failed write must only drop ITS OWN arm — a newer traced write that succeeded in the + /// meantime owns the arm and keeps it (the disarm is keyed on the writer's execution id). + /// + [Fact] + public void FailedWrite_DoesNotDisarm_AnArmTakenByALaterWriter() + { + var (actor, dcl, sub) = Create("P7"); + var loser = Guid.NewGuid(); + var winner = Guid.NewGuid(); + + // Issue the traced write that will fail, but hold its DCL reply back. The reply goes to the + // Ask's temporary ref, so capture it now — a later Reply() would answer the second write. + actor.Tell(new SetStaticAttributeCommand("c1", "P7", "Processed", "true", DateTimeOffset.UtcNow, + SourceExecutionId: loser, Trace: true)); + var first = ExpectWrite(dcl); + var firstAsker = dcl.LastSender; + + // A second traced write lands and succeeds — it now owns the arm. + Write(actor, dcl, "P7", winner, trace: true); + + // Only now does the first write fail; its disarm must not steal the winner's arm. + firstAsker.Tell(new WriteTagResponse(first.CorrelationId, false, "rejected", DateTimeOffset.UtcNow), dcl.Ref); + Assert.False(ExpectMsg(TimeSpan.FromSeconds(5)).Success); + + var published = Echo(actor, sub, "echo"); + Assert.Equal(winner, published.SourceExecutionId); + Assert.True(published.Trace); + } + [Fact] public void Arm_IsConsumedOnce() {