fix(central-ui): Flow Debugger prediction no longer deadlocks the Blazor circuit

FlowDebugService.PredictMethodAsync resolved route targets with a sync-over-async GetAwaiter().GetResult() inside the predictor's synchronous resolver — on the circuit's single-threaded renderer sync context that blocked the dispatcher forever the moment a method whose script routes to a site script was chosen (every later click and JS-interop reply queued behind it; the page looked inert with a healthy websocket and no error). Misdiagnosed on 2026-09-06 as a login-disabled localStorage stall. Now a resolve-and-rewalk loop: each pass records the targets it could not answer, they are awaited between passes, and the walk converges once a pass asks for nothing new (bounded by MaxPredictionPasses). Regression test drives the service under a single-thread pump sync context (fails in 10 s on the old code, passes now) plus a nested Scripts.Call resolve-across-passes test. Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 10:57:09 -04:00
parent 5394872040
commit 4769e192e3
3 changed files with 249 additions and 33 deletions
@@ -1,17 +1,94 @@
 {
  "planPath": "docs/plans/2026-09-06-flow-debugger-followups.md",
  "tasks": [
-    {"id": 0, "subject": "Task 0: Diagnose the login-disabled interop stall (investigation)", "status": "pending"},
-    {"id": 1, "subject": "Task 1: FlowStep.CorrelationId + CLI column", "status": "pending", "blockedBy": [0]},
-    {"id": 2, "subject": "Task 2: Site Calls query — filter by TrackedOperationId", "status": "pending", "blockedBy": [0]},
-    {"id": 3, "subject": "Task 3: Notification Outbox query — filter by NotificationId", "status": "pending", "blockedBy": [0]},
-    {"id": 4, "subject": "Task 4: Site Calls page accepts ?correlationId=", "status": "pending", "blockedBy": [2]},
-    {"id": 5, "subject": "Task 5: Notification report page accepts ?correlationId=", "status": "pending", "blockedBy": [3]},
-    {"id": 6, "subject": "Task 6: FlowStepDetail — report deep links + Retry/Discard controls", "status": "pending", "blockedBy": [1]},
-    {"id": 7, "subject": "Task 7: IFlowDebugService.RetryOutboundAsync / DiscardOutboundAsync", "status": "pending", "blockedBy": [1]},
-    {"id": 8, "subject": "Task 8: Flow Debugger page — wire actions, confirm, scope check, re-poll", "status": "pending", "blockedBy": [6, 7]},
-    {"id": 9, "subject": "Task 9: Playwright — browser Live run + localStorage restore (rig)", "status": "pending", "blockedBy": [8, 4, 5]},
-    {"id": 10, "subject": "Task 10: Docs, register, memory", "status": "pending", "blockedBy": [8]}
+    {
+      "id": 0,
+      "subject": "Task 0: Diagnose the login-disabled interop stall (investigation)",
+      "status": "completed"
+    },
+    {
+      "id": 1,
+      "subject": "Task 1: FlowStep.CorrelationId + CLI column",
+      "status": "pending",
+      "blockedBy": [
+        0
+      ]
+    },
+    {
+      "id": 2,
+      "subject": "Task 2: Site Calls query \u2014 filter by TrackedOperationId",
+      "status": "pending",
+      "blockedBy": [
+        0
+      ]
+    },
+    {
+      "id": 3,
+      "subject": "Task 3: Notification Outbox query \u2014 filter by NotificationId",
+      "status": "pending",
+      "blockedBy": [
+        0
+      ]
+    },
+    {
+      "id": 4,
+      "subject": "Task 4: Site Calls page accepts ?correlationId=",
+      "status": "pending",
+      "blockedBy": [
+        2
+      ]
+    },
+    {
+      "id": 5,
+      "subject": "Task 5: Notification report page accepts ?correlationId=",
+      "status": "pending",
+      "blockedBy": [
+        3
+      ]
+    },
+    {
+      "id": 6,
+      "subject": "Task 6: FlowStepDetail \u2014 report deep links + Retry/Discard controls",
+      "status": "pending",
+      "blockedBy": [
+        1
+      ]
+    },
+    {
+      "id": 7,
+      "subject": "Task 7: IFlowDebugService.RetryOutboundAsync / DiscardOutboundAsync",
+      "status": "pending",
+      "blockedBy": [
+        1
+      ]
+    },
+    {
+      "id": 8,
+      "subject": "Task 8: Flow Debugger page \u2014 wire actions, confirm, scope check, re-poll",
+      "status": "pending",
+      "blockedBy": [
+        6,
+        7
+      ]
+    },
+    {
+      "id": 9,
+      "subject": "Task 9: Playwright \u2014 browser Live run + localStorage restore (rig)",
+      "status": "pending",
+      "blockedBy": [
+        8,
+        4,
+        5
+      ]
+    },
+    {
+      "id": 10,
+      "subject": "Task 10: Docs, register, memory",
+      "status": "pending",
+      "blockedBy": [
+        8
+      ]
+    }
  ],
-  "lastUpdated": "2026-09-06T00:00:00Z"
-}
+  "lastUpdated": "2026-09-06T15:12:00Z"
+}
@@ -61,6 +61,13 @@ public sealed class FlowDebugService : IFlowDebugService
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
    }

+    /// <summary>
+    /// Upper bound on the resolve-and-rewalk passes of <see cref="PredictMethodAsync"/>. Each pass can
+    /// only discover targets one nesting level deeper than the last, so the predictor's own depth cap
+    /// bounds the useful pass count; the constant is a safety net, not a tuning knob.
+    /// </summary>
+    internal const int MaxPredictionPasses = 8;
+
    /// <inheritdoc/>
    public async Task<PredictedFlow> PredictMethodAsync(int apiMethodId, CancellationToken cancellationToken = default)
    {
@@ -68,32 +75,53 @@ public sealed class FlowDebugService : IFlowDebugService
        if (method == null)
            return PredictedFlow.Empty;

-        // The predictor is synchronous and takes a synchronous resolver, so the instance →
-        // template → script lookups are pre-fetched lazily through a small memo that blocks
-        // on the repository — acceptable for a handful of constant route targets, and the
-        // predictor never asks for anything else.
-        var memo = new Dictionary<(string, string), string?>();
-        string? Resolve(string instanceCode, string scriptName)
+        // The predictor is synchronous and takes a synchronous resolver, and this method runs on the
+        // Blazor circuit's single-threaded renderer sync context — so the instance → template → script
+        // lookups must NEVER block on the repository from inside the resolver. The original
+        // `.GetAwaiter().GetResult()` there deadlocked the whole circuit (every later click and JS
+        // interop reply queued behind the blocked dispatcher forever) the moment a method whose script
+        // routes to a site script was chosen; found 2026-09-06 while chasing a "login-disabled interop
+        // stall" that was really this. Instead the walk is repeated: each pass records the route targets
+        // it asked for and could not answer, those are awaited between passes, and the walk converges
+        // once a pass asks for nothing new. Cheap — a pass is a Roslyn syntax walk over a short script.
+        var memo = new Dictionary<(string Instance, string Script), string?>();
+        var script = method.Script ?? string.Empty;
+        for (var pass = 0; pass < MaxPredictionPasses; pass++)
        {
-            var key = (instanceCode, scriptName);
-            if (memo.TryGetValue(key, out var cached))
-                return cached;
-            string? body;
-            try
+            var requested = new List<(string Instance, string Script)>();
+            string? Resolve(string instanceCode, string scriptName)
            {
-                body = ResolveSiteScriptAsync(instanceCode, scriptName, cancellationToken).GetAwaiter().GetResult();
-            }
-            catch (Exception ex)
-            {
-                _logger.LogDebug(ex, "Flow prediction could not resolve {Instance}.{Script}", instanceCode, scriptName);
-                body = null;
+                var key = (instanceCode, scriptName);
+                if (memo.TryGetValue(key, out var cached))
+                    return cached;
+                if (!requested.Contains(key))
+                    requested.Add(key);
+                return null;
            }

-            memo[key] = body;
-            return body;
+            var flow = FlowShapePredictor.PredictInboundMethod(method.Name, script, Resolve);
+            if (requested.Count == 0)
+                return flow;
+
+            foreach (var key in requested)
+            {
+                string? body;
+                try
+                {
+                    body = await ResolveSiteScriptAsync(key.Instance, key.Script, cancellationToken);
+                }
+                catch (Exception ex) when (ex is not OperationCanceledException)
+                {
+                    _logger.LogDebug(ex, "Flow prediction could not resolve {Instance}.{Script}", key.Instance, key.Script);
+                    body = null;
+                }
+
+                memo[key] = body;
+            }
        }

-        return FlowShapePredictor.PredictInboundMethod(method.Name, method.Script ?? string.Empty, Resolve);
+        // Pathological: a walk still discovering new targets past the pass cap — predict with what is known.
+        return FlowShapePredictor.PredictInboundMethod(method.Name, script, (i, s) => memo.GetValueOrDefault((i, s)));
    }

    /// <inheritdoc/>
@@ -15,6 +15,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
 using ZB.MOM.WW.ScadaBridge.ManagementService;
+using ZB.MOM.WW.ScadaBridge.ScriptAnalysis;
 using ZB.MOM.WW.ScadaBridge.Security;

 namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Services;
@@ -199,6 +200,116 @@ public sealed class FlowDebugServiceTests : IDisposable
        Assert.Equal("Sim01.Cmd", flow.Nodes[3].Title);
    }

+    /// <summary>
+    /// Regression for the 2026-09-06 circuit deadlock: <see cref="FlowDebugService.PredictMethodAsync"/>
+    /// runs on Blazor Server's single-threaded renderer sync context, and the route-target resolver used
+    /// to block on the repository with <c>GetAwaiter().GetResult()</c> from inside the synchronous
+    /// predictor walk. A repository continuation posted back to that context could then never run —
+    /// the whole circuit hung (every later click / JS-interop reply queued behind the blocked dispatcher).
+    /// The pump below IS that context: one thread, posts queued, a blocked callback starves the queue.
+    /// </summary>
+    [Fact]
+    public void PredictMethod_OnASingleThreadedSyncContext_NeverBlocksTheDispatcher()
+    {
+        _inboundApi.GetApiMethodByIdAsync(4, Arg.Any<CancellationToken>())
+            .Returns(new ApiMethod("Delmia", """return await Route.To("Sim01").Call("Process", new { });""") { Id = 4 });
+        // The instance lookup completes only after a Yield — i.e. only when the sync context gets to
+        // run the continuation. Blocking on it from the context's own thread is the deadlock.
+        _templates.GetInstanceByUniqueNameAsync("Sim01", Arg.Any<CancellationToken>())
+            .Returns(_ => YieldThen(new Instance("Sim01") { Id = 9, TemplateId = 10, SiteId = 1 }));
+        _templates.GetScriptsByTemplateIdAsync(10, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<IReadOnlyList<TemplateScript>>(
+            [
+                new TemplateScript("Process", """await Attributes.SetAsync("Cmd", 1);""") { Id = 1, TemplateId = 10 },
+            ]));
+        _templates.GetTemplateByIdAsync(10, Arg.Any<CancellationToken>())
+            .Returns(new Template("Base") { Id = 10, ParentTemplateId = null });
+        var svc = CreateService(reply: null);
+
+        var pump = new SingleThreadPump();
+        PredictedFlow? flow = null;
+        Exception? failure = null;
+        var done = new ManualResetEventSlim();
+        var thread = new Thread(() =>
+        {
+            try { flow = pump.Run(() => svc.PredictMethodAsync(4)); }
+            catch (Exception ex) { failure = ex; }
+            finally { done.Set(); }
+        }) { IsBackground = true, Name = "flow-predict-pump" };
+        thread.Start();
+
+        Assert.True(done.Wait(TimeSpan.FromSeconds(10)),
+            "PredictMethodAsync deadlocked the single-threaded sync context (sync-over-async in the route resolver).");
+        Assert.Null(failure);
+        Assert.NotNull(flow);
+        Assert.Contains(flow!.Nodes, n => n.Kind == FlowStepKind.ScriptRun && n.Title == "Sim01.Process");
+        Assert.Contains(flow.Nodes, n => n.Kind == FlowStepKind.AttributeWrite && n.Title == "Sim01.Cmd");
+    }
+
+    [Fact]
+    public async Task PredictMethod_NestedRouteTargets_ResolveAcrossPasses()
+    {
+        // Process calls a second script on the same instance via Scripts.Call — discovered only once
+        // Process itself has been resolved, so the service needs a second resolve-and-rewalk pass.
+        _inboundApi.GetApiMethodByIdAsync(4, Arg.Any<CancellationToken>())
+            .Returns(new ApiMethod("Delmia", """return await Route.To("Sim01").Call("Process", new { });""") { Id = 4 });
+        _templates.GetInstanceByUniqueNameAsync("Sim01", Arg.Any<CancellationToken>())
+            .Returns(new Instance("Sim01") { Id = 9, TemplateId = 10, SiteId = 1 });
+        _templates.GetScriptsByTemplateIdAsync(10, Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<IReadOnlyList<TemplateScript>>(
+            [
+                new TemplateScript("Process", """await Scripts.Call("Inner", new { });""") { Id = 1, TemplateId = 10 },
+                new TemplateScript("Inner", """await Attributes.SetAsync("Cmd", 1);""") { Id = 2, TemplateId = 10 },
+            ]));
+        _templates.GetTemplateByIdAsync(10, Arg.Any<CancellationToken>())
+            .Returns(new Template("Base") { Id = 10, ParentTemplateId = null });
+        var svc = CreateService(reply: null);
+
+        var flow = await svc.PredictMethodAsync(4);
+
+        var kinds = flow.Nodes.Select(n => n.Kind).ToList();
+        Assert.Contains(FlowStepKind.AttributeWrite, kinds);
+        Assert.Contains(flow.Nodes, n => n.Title == "Sim01.Inner");
+        Assert.Contains(flow.Nodes, n => n.Title == "Sim01.Cmd");
+    }
+
+    private static async Task<Instance?> YieldThen(Instance instance)
+    {
+        await Task.Yield();
+        return instance;
+    }
+
+    /// <summary>A Blazor-Server-shaped sync context: a single thread draining a queue of posted callbacks.</summary>
+    private sealed class SingleThreadPump : SynchronizationContext
+    {
+        private readonly System.Collections.Concurrent.BlockingCollection<(SendOrPostCallback Callback, object? State)> _queue = new();
+
+        public override void Post(SendOrPostCallback d, object? state) => _queue.Add((d, state));
+
+        public override void Send(SendOrPostCallback d, object? state) => throw new NotSupportedException();
+
+        public T Run<T>(Func<Task<T>> body)
+        {
+            var previous = Current;
+            SetSynchronizationContext(this);
+            try
+            {
+                var task = body();
+                while (!task.IsCompleted)
+                {
+                    var (callback, state) = _queue.Take();
+                    callback(state);
+                }
+
+                return task.GetAwaiter().GetResult();
+            }
+            finally
+            {
+                SetSynchronizationContext(previous);
+            }
+        }
+    }
+
    [Fact]
    public async Task PredictMethod_UnknownMethod_IsEmpty()
    {