fix(management): FlowDebugRunner — transport faults still write the root row; write-kind completion is script-specific; standby/unreadable-trigger tests

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29

src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs

@@ -53,6 +53,15 @@ public sealed class FlowDebugRunner
     /// </summary>
     public const int DefaultScriptTimeoutSeconds = 30;
 
+    /// <summary>
+    /// Ceiling on a write-kind run's spawned-run wait, whatever the command asks for. It must stay
+    /// UNDER <c>ManagementServiceOptions.LongRunningCommandTimeout</c> (5 minutes by default), which
+    /// is the Ask the management endpoint wraps this command in: a longer wait would leave the poll
+    /// loop running on the node after the caller's Ask has already expired, holding its scoped
+    /// provider and pulling from the site for a result nobody can receive.
+    /// </summary>
+    public const int MaxWaitSeconds = 300;
+
     /// <summary>
     /// Arch-review F2: <see cref="InboundScriptExecutor"/>'s code for a routed call refused
     /// because the target site is outside the operator's site scope. InboundAPI cannot
@@ -304,7 +313,9 @@ public sealed class FlowDebugRunner
             DateTimeOffset.UtcNow, ParentExecutionId: root, Trace: true);
 
         RouteToCallResponse response;
-        var timedOut = false;
+        // Null while the site ANSWERED (a success or a Success=false reply); otherwise the code to
+        // report for a routed Ask that threw instead of answering.
+        string? faultCode = null;
         using (var runCts = CancellationTokenSource.CreateLinkedTokenSource(ct))
         {
             runCts.CancelAfter(timeout + RunGrace);
@@ -314,10 +325,27 @@ public sealed class FlowDebugRunner
             }
             catch (OperationCanceledException) when (!ct.IsCancellationRequested)
             {
-                timedOut = true;
+                faultCode = TimeoutCode;
                 response = new RouteToCallResponse(
                     request.CorrelationId, false, null, TimeoutMessage(timeout), DateTimeOffset.UtcNow);
             }
+            catch (Exception ex) when (ex is not OperationCanceledException)
+            {
+                // RouteToCallAsync is an Akka Ask bounded by CommunicationOptions.IntegrationTimeout,
+                // so it throws AskTimeoutException (NOT an OperationCanceledException) when the site
+                // never answers — and any transport fault throws too. "No answer" is NOT "did not
+                // happen": the site may already have run the script. Letting the exception escape
+                // would lose the root row entirely and show the operator MapFault's generic internal
+                // error for a call that ran, so it is converted into a failed response the same way
+                // RouteHelper.CallAsync types its Ask faults and WriteAndPollAsync reports a failed
+                // write. A cancellation of the CALLER's token still propagates (filtered out above).
+                faultCode = ex is Akka.Actor.AskTimeoutException ? TimeoutCode : CallFailedCode;
+                _logger.LogWarning(ex,
+                    "Flow Debugger routed call to site {Site} for {Instance}.{Script} faulted",
+                    site.SiteIdentifier, instance.UniqueName, script.CanonicalName);
+                response = new RouteToCallResponse(
+                    request.CorrelationId, false, null, ex.Message, DateTimeOffset.UtcNow);
+            }
         }
 
         // Everything from here to WriteCentralAsync runs AFTER the site already executed the
@@ -366,7 +394,7 @@ public sealed class FlowDebugRunner
         ordered.AddRange(events);
         return new FlowDebugRunResult(
             root, response.Success, response.ErrorMessage,
-            response.Success ? null : timedOut ? TimeoutCode : CallFailedCode,
+            response.Success ? null : faultCode ?? CallFailedCode,
             ordered, Truncated: collector.Truncated, startedAt, DateTimeOffset.UtcNow);
     }
 
@@ -424,11 +452,26 @@ public sealed class FlowDebugRunner
                 [redactedRoot], false, startedAt, DateTimeOffset.UtcNow);
         }
 
+        // The run that COUNTS as completion. For the script entry (design 2026-09-08 §4.3) that is
+        // the script the operator chose, matched on Target — the site stamps ScriptRunLauncher's
+        // `target: scriptName` on the row. Other scripts the same write happens to fire are still
+        // collected below (the graph draws the siblings) but must never report the CHOSEN script as
+        // having run. triggerScript == null is the attribute-level command, which expects no
+        // particular script, so any spawned run satisfies it.
+        bool IsAwaitedRun(AuditEvent e)
+        {
+            var d = AuditDetailsCodec.Deserialize(e.DetailsJson);
+            if (d.Kind != nameof(AuditKind.ScriptRun) || d.ParentExecutionId != root) return false;
+            return triggerScript is null
+                || string.Equals(e.Target, triggerScript, StringComparison.OrdinalIgnoreCase);
+        }
+
         // Poll the site for the traced run(s) the write spawned — matched by ParentExecutionId
-        // == root, then their children — until a ScriptRun parented to root exists AND the
+        // == root, then their children — until the awaited ScriptRun exists AND the set has been
-        // set has been quiet for SettleWindow, or WaitSeconds elapses.
+        // quiet for SettleWindow, or the (clamped) wait elapses.
         var puller = sp.GetRequiredService<IPullTraceEventsClient>();
-        var deadline = _now() + TimeSpan.FromSeconds(Math.Max(1, waitSeconds));
+        var wait = Math.Clamp(waitSeconds, 1, MaxWaitSeconds);
+        var deadline = _now() + TimeSpan.FromSeconds(wait);
         var known = new Dictionary<Guid, AuditEvent>();
         DateTimeOffset? lastNew = null;
         var since = startedAt.UtcDateTime.AddSeconds(-5);
@@ -448,11 +491,7 @@ public sealed class FlowDebugRunner
             }
             if (added) lastNew = _now();
 
-            var spawned = known.Values.Any(e =>
+            var spawned = known.Values.Any(IsAwaitedRun);
-            {
-                var d = AuditDetailsCodec.Deserialize(e.DetailsJson);
-                return d.Kind == nameof(AuditKind.ScriptRun) && d.ParentExecutionId == root;
-            });
             var now = _now();
             if (spawned && lastNew is { } ln && now - ln >= SettleWindow) break;
             if (now >= deadline) break;
@@ -461,16 +500,12 @@ public sealed class FlowDebugRunner
 
         var ordered = new List<AuditEvent> { redactedRoot };
         ordered.AddRange(known.Values.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId));
-        var completed = known.Values.Any(e =>
+        var completed = known.Values.Any(IsAwaitedRun);
-        {
-            var d = AuditDetailsCodec.Deserialize(e.DetailsJson);
-            return d.Kind == nameof(AuditKind.ScriptRun) && d.ParentExecutionId == root;
-        });
         return new FlowDebugRunResult(
             root, completed,
             completed ? null : triggerScript is null
-                ? $"No traced run was spawned within {waitSeconds}s (no value-change trigger on '{attributeName}', or no device echo)."
+                ? $"No traced run was spawned within {wait}s (no value-change trigger on '{attributeName}', or no device echo)."
-                : $"No traced run of '{triggerScript}' was spawned within {waitSeconds}s after writing '{attributeName}' (trigger condition not met, or no device echo).",
+                : $"No traced run of '{triggerScript}' was spawned within {wait}s after writing '{attributeName}' (trigger condition not met, or no device echo).",
             completed ? null : NoTriggerCode,
             ordered, false, startedAt, DateTimeOffset.UtcNow);
     }

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs

@@ -519,6 +519,39 @@ public class FlowDebugRunnerTests
         await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default);
     }
 
+    [Fact]
+    public async Task RunScript_OnStandby_RefusesWithStandbyCode_BeforeTouchingTheInstance()
+    {
+        var (templates, _, _, pipeline) = WireScript(CallScript("Manual"));
+        _gate.IsActiveNode.Returns(false);
+
+        var ex = await Assert.ThrowsAsync<ManagementCommandException>(() => Runner().RunScriptAsync(
+            _services.BuildServiceProvider(), new FlowDebugRunScriptCommand(5, "Manual", null, null), Deployer, CancellationToken.None));
+
+        Assert.Equal(FlowDebugRunner.StandbyNodeCode, ex.Code);
+        Assert.Contains("ACTIVE central node", ex.Message);
+        await templates.DidNotReceiveWithAnyArgs().GetInstanceByIdAsync(default, default);
+        await pipeline.DidNotReceiveWithAnyArgs().FlattenAndValidateAsync(default, default, default, default);
+    }
+
+    [Fact]
+    public async Task RunScript_WriteKind_UnreadableTriggerConfig_FailsValidation()
+    {
+        // A write-kind trigger whose configuration names no attribute: there is nothing to write,
+        // so the run is refused rather than guessing an attribute.
+        WireScript(new ResolvedScript
+        {
+            CanonicalName = "OnLevel", Code = "return 1;", TriggerType = "ValueChange", TriggerConfiguration = "{}",
+        });
+
+        var ex = await Assert.ThrowsAsync<ManagementCommandException>(() => Runner().RunScriptAsync(
+            _services.BuildServiceProvider(), new FlowDebugRunScriptCommand(5, "OnLevel", null, "7"), Deployer, CancellationToken.None));
+
+        Assert.Equal(FlowDebugRunner.ValidationFailedCode, ex.Code);
+        Assert.Contains("trigger configuration", ex.Message);
+        await _router.DidNotReceiveWithAnyArgs().RouteToSetAttributesAsync(default!, default!, default);
+    }
+
     [Fact]
     public async Task RunScript_WriteKind_WithoutValue_FailsValidation_BeforeAnyWrite()
     {
@@ -529,6 +562,28 @@ public class FlowDebugRunnerTests
         await _router.DidNotReceiveWithAnyArgs().RouteToSetAttributesAsync(default!, default!, default);
     }
 
+    [Fact]
+    public async Task RunScript_WriteKind_SiblingScriptRun_DoesNotCountAsCompletion()
+    {
+        // The write may fire several scripts. A sibling's run is still COLLECTED (the graph draws
+        // it) but must not report the script the operator chose as having run.
+        var (_, _, puller, _) = WireScript(WriteScript("OnLevel", "ValueChange", "Level"));
+        puller.PullAsync("site-a", Arg.Any<IReadOnlyCollection<Guid>>(), Arg.Any<DateTime>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
+            .Returns(ci => (IReadOnlyList<AuditEvent>)[ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
+                eventId: Guid.Parse("cccccccc-0000-0000-0000-000000000001"),
+                executionId: Guid.NewGuid(), parentExecutionId: ((IReadOnlyCollection<Guid>)ci[1]).First(), target: "OtherScript")]);
+
+        var result = await Runner().RunScriptAsync(_services.BuildServiceProvider(),
+            new FlowDebugRunScriptCommand(5, "OnLevel", null, "7", WaitSeconds: 3), Deployer, CancellationToken.None);
+
+        Assert.False(result.Completed);
+        Assert.Equal(FlowDebugRunner.NoTriggerCode, result.ErrorCode);
+        Assert.Contains("OnLevel", result.Error);
+        // ...but the sibling is in the events, so the operator can see what DID run.
+        Assert.Equal(2, result.Events.Count);
+        Assert.Equal("OtherScript", result.Events[1].Target);
+    }
+
     [Fact]
     public async Task RunScript_CallKind_RoutesATracedCall_AndReturnsRootRouteCallRow_ThenSiteRows()
     {
@@ -636,6 +691,49 @@ public class FlowDebugRunnerTests
         Assert.Equal(AuditKind.RouteCall, written.AsRow().Kind);
     }
 
+    [Fact]
+    public async Task RunScript_CallKind_TransportFault_StillWritesTheRootRow_AsCallFailed()
+    {
+        // A throwing Ask is NOT proof the script did not run — the site may have executed it and
+        // the reply been lost. The root row must still be written against the operator's name.
+        WireScript(CallScript("Manual"));
+        _router.RouteToCallAsync("site-a", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(_ => Task.FromException<RouteToCallResponse>(new InvalidOperationException("channel is broken")));
+
+        var result = await Runner().RunScriptAsync(_services.BuildServiceProvider(),
+            new FlowDebugRunScriptCommand(5, "Manual", null, null), Deployer, CancellationToken.None);
+
+        Assert.False(result.Completed);
+        Assert.Equal(FlowDebugRunner.CallFailedCode, result.ErrorCode);
+        Assert.Contains("channel is broken", result.Error);
+        var root = Assert.Single(result.Events).AsRow();
+        Assert.Equal(AuditKind.RouteCall, root.Kind);
+        Assert.Equal(AuditStatus.Failed, root.Status);
+        Assert.Equal("ops", result.Events[0].Actor);
+        var written = Assert.Single(_central);
+        Assert.Equal(AuditKind.RouteCall, written.AsRow().Kind);
+    }
+
+    [Fact]
+    public async Task RunScript_CallKind_AskTimeout_ReportsTimeoutCode()
+    {
+        // The communication layer's own signal that the site never answered — an AskTimeoutException,
+        // NOT an OperationCanceledException, so it needs its own classification.
+        WireScript(CallScript("Manual"));
+        _router.RouteToCallAsync("site-a", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
+            .Returns(_ => Task.FromException<RouteToCallResponse>(
+                new Akka.Actor.AskTimeoutException("no response from site-a")));
+
+        var result = await Runner().RunScriptAsync(_services.BuildServiceProvider(),
+            new FlowDebugRunScriptCommand(5, "Manual", null, null), Deployer, CancellationToken.None);
+
+        Assert.False(result.Completed);
+        Assert.Equal(FlowDebugRunner.TimeoutCode, result.ErrorCode);
+        var root = Assert.Single(result.Events).AsRow();
+        Assert.Equal(AuditStatus.Failed, root.Status);
+        Assert.Single(_central);
+    }
+
     /// <summary>A routed call that never answers until its token is cancelled — the timeout path.</summary>
     private static async Task<RouteToCallResponse> NeverAnswersAsync(CancellationToken ct)
     {