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() {