Merge branch 'worktree-agent-ae3652f34db9ac0b4' into fix/flow-debugger-arch-review

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
     /// <c>Trace</c>, 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, <see cref="DisarmTrace"/>);
+    /// a successful write's echo consumes it.
     /// </summary>
     private readonly Dictionary<string, (Guid SourceExecutionId, DateTimeOffset ExpiresAt)> _traceArms = new();
     // Maps each tag path to every attribute canonical name that references it.
@@ -339,6 +341,17 @@ public class InstanceActor : ReceiveActor
         Receive<TagValueUpdate>(HandleTagValueUpdate);
         Receive<SubscribeTagsResponse>(HandleSubscribeTagsResponse); // S4/UA6: real retry-aware handler
         Receive<RetryTagSubscribe>(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<DisarmTrace>(msg =>
+        {
+            if (_traceArms.TryGetValue(msg.AttributeName, out var arm)
+                && arm.SourceExecutionId == msg.SourceExecutionId)
+            {
+                _traceArms.Remove(msg.AttributeName);
+            }
+        });
         Receive<ConnectionQualityChanged>(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<WriteTagResponse>(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
     /// </summary>
     private sealed record RetryTagSubscribe(string ConnectionName);
 
+    /// <summary>
+    /// 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 <see cref="_traceArms"/> itself; matching on <paramref name="SourceExecutionId"/> keeps
+    /// a late failure from stealing an arm a newer traced write has since taken.
+    /// </summary>
+    /// <param name="AttributeName">Canonical name of the attribute whose arm is to be dropped.</param>
+    /// <param name="SourceExecutionId">Execution id of the writer that took the arm.</param>
+    private sealed record DisarmTrace(string AttributeName, Guid SourceExecutionId);
+
     /// <summary>Self-tell that fires the coalesced static-override persist flush (P4, arch-review WP1.5).</summary>
     private sealed class FlushStaticOverrides
     {

tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs

@@ -89,6 +89,17 @@ public class InstanceActorTraceArmTests : TestKit, IDisposable
         Assert.True(ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5)).Success);
     }
 
+    /// <summary>Same as <see cref="Write"/> but the DCL rejects the write, so the arm must be dropped.</summary>
+    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<SetStaticAttributeResponse>(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);
     }
 
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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);
+    }
+
+    /// <summary>
+    /// 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).
+    /// </summary>
+    [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<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5)).Success);
+
+        var published = Echo(actor, sub, "echo");
+        Assert.Equal(winner, published.SourceExecutionId);
+        Assert.True(published.Trace);
+    }
+
     [Fact]
     public void Arm_IsConsumedOnce()
     {