Files
ScadaBridge/docs/plans/2026-09-06-flow-debugger-arch-review-remediation.md
T
Joseph Doherty e15aa0e430 docs: Flow Debugger design reconciled with the implementation (arch-review F12, F13)
The Flow Debugger design doc still described the pre-remediation shape: a
debug-stream subscription for the value-change entry, Deployer everywhere, an
unredacted inline trace copy and "~7 audit rows per call". Reconciled with what
shipped on this branch (Tasks 1-12):

- design D6 / §5.4 / §9 / §10: RequireFlowDebugger page policy with Deployer-only
  execution (F3), the PullTraceEvents polling path with AttributeValueUpdate
  fields 7/8 recorded as RESERVED and unconsumed (F12), the redacting + byte-
  capped shared FlowTraceCollector (F1), route-time site scope (F2), the derived
  client waits (F4), the 7-10 row estimate, the doubled central audit write per
  routed call, and the inbound-row-after-the-run ordering.
- sitestream.proto: fields 7/8 marked reserved-but-populated, numbers not to be
  reused (comment only; no regeneration needed).
- Component-InboundAPI: SkipBodyCapture on the RouteCall row (F11),
  SchemaRefResolver's public visibility, WithSiteScope / SITE_SCOPE_DENIED (F2),
  Replay's wider role set (F3).
- Component-CentralUI: page policy vs Deployer-only controls (F3), route-time
  site scope (F2), client wait derivation (F4).
- "7-10 audit rows per call (3 of them trace-only)" everywhere the estimate is
  quoted: the method form, both CLI --trace-all-calls help strings, the CLI
  README, the ApiMethod entity doc, the manual migration script and the two
  Flow Debugger plans.
- arch-review doc: new §7 Resolution listing F1-F13 with the fixing commit, plus
  the tracking-store test race fixed alongside them (86451382).

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 14:35:10 -04:00

42 KiB
Raw Blame History

Flow Debugger Arch-Review Remediation Implementation Plan

For Claude: REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.

Goal: Resolve every finding F1–F13 of docs/plans/2026-09-06-flow-debugger-arch-review.md so the Flow Debugger's stated security and operator-facing properties hold, and the design docs agree with the code.

Architecture: No redesign. The inline trace copy is redacted and byte-capped at the collector boundary (site and central), the method-entry Live run gets the same site-scope gate the value-change entry has, Replay opens to OperationalAudit roles while Live stays Deployer-only, and client waits derive from the server's bound. The Low findings are local fixes in the site runtime, the audit repository, the graph builder and the SVG renderer, plus a documentation pass that records the switches the implementation made.

Tech Stack: .NET 10 / C# 13, Akka.NET, Blazor Server (bUnit tests), System.CommandLine (CLI), xUnit. Build with dotnet build ZB.MOM.WW.ScadaBridge.slnx; test one project with dotnet test tests/<Project>/<Project>.csproj.

Branch: fix/flow-debugger-arch-review (from main @ 0ccbe140). Commit trailer on every commit: Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29

Execution notes for implementers

  • Work ONLY in the files listed under Files:. If you need another file, stop and report it as a plan defect.
  • TDD: write the failing test first, run it, implement, run again. Run the whole test project you touched before reporting.
  • Do not edit CLAUDE.md, the arch-review doc, or the design docs unless your task lists them (Task 13 owns docs).
  • Keep every contract change additive (trailing optional parameters, new proto field numbers never reused).

Wave plan (parallelism)

Wave Tasks (mutually non-overlapping files)
1 1 (F1 site), 3 (F2), 4 (F3), 5 (F4), 7 (F6), 10 (F9), 11 (F10)
2 2 (F1 central; after 1, 3), 8 (F7; after 1), 12 (F11; after 3)
3 6 (F5; after 2), 9 (F8; after 1, 3, 8, 12), 13 (docs; after everything)
4 14 (full build, full test run, rig rebuild + FLOW_ONLY=1 verify-delmia-notifier.sh)

Task 1: F1 (site) — FlowTraceCollector redacts and byte-caps, moves to Commons

Classification: standard Estimated implement time: ~5 min Parallelizable with: Task 3, 4, 5, 7, 10, 11

Files:

  • Create: src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowTraceCollector.cs (moved from SiteRuntime, namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging)
  • Delete: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/FlowTraceCollector.cs
  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs:250 (construct with redactor), :335 region (resolve IAuditRedactor from the scope beside IAuditWriter)
  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs (add using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;)
  • Move test: tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/FlowTraceCollectorTests.cs → tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowTraceCollectorTests.cs (namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Debugging), add the two tests below
  • Modify test: tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptRuntimeContextTraceEmissionTests.cs (using only; add one redaction test)

Why Commons: the central FlowDebugRunner (Task 2) needs the same collector; Commons already references the ZB.MOM.WW.Audit package (IAuditRedactor, AuditEvent) and hosts Types/Debugging/.

Step 1: Failing tests (in the moved FlowTraceCollectorTests)

private sealed class UpperRedactor : IAuditRedactor
{
    public AuditEvent Apply(AuditEvent evt) => new()
    {
        EventId = evt.EventId, OccurredAtUtc = evt.OccurredAtUtc, Action = evt.Action,
        DetailsJson = evt.DetailsJson?.ToUpperInvariant(),
        // copy every other property the AuditEvent record carries
    };
}

[Fact]
public void Add_AppliesTheRedactor_SoTheInlineCopyIsTheFilteredEvent()
{
    var c = new FlowTraceCollector(redactor: new UpperRedactor());
    var evt = Evt(1); evt.DetailsJson = "{\"secret\":\"abc\"}";
    c.Add(evt);
    Assert.Equal("{\"SECRET\":\"ABC\"}", c.Complete()[0].DetailsJson);
}

[Fact]
public void Add_OverByteBudget_Truncates_KeepsEarlierEvents()
{
    var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 100);
    var big = Evt(1); big.DetailsJson = new string('x', 80);
    var big2 = Evt(2); big2.DetailsJson = new string('y', 80);
    c.Add(big); c.Add(big2);
    Assert.True(c.Truncated);
    Assert.Single(c.Complete());
}

Step 2: dotnet test tests/ZB.MOM.WW.ScadaBridge.Commons.Tests --filter FlowTraceCollectorTests → FAIL (no ctor overload).

Step 3: Implementation — the collector gains:

/// <summary>Default inline byte budget per run (sum of redacted DetailsJson lengths).</summary>
public const int DefaultMaxBytes = 1_048_576;

private readonly IAuditRedactor? _redactor;
private readonly int _maxBytes;
private int _bytes;

public FlowTraceCollector(int maxEvents = DefaultMaxEvents, int maxBytes = DefaultMaxBytes, IAuditRedactor? redactor = null)
{ /* validate both > 0 */ }

public void Add(AuditEvent evt)
{
    ArgumentNullException.ThrowIfNull(evt);
    // Redaction OUTSIDE the lock (pure function, may be non-trivial) — arch-review F1:
    // the inline copy must be the same filtered record the writer persists.
    var filtered = _redactor is null ? evt : _redactor.Apply(evt);
    var size = filtered.DetailsJson?.Length ?? 0;
    lock (_gate)
    {
        if (Completed) return;
        if (_events.Count >= _max || _bytes + size > _maxBytes) { Truncated = true; return; }
        _events.Add(filtered);
        _bytes += size;
    }
}

Update the class doc: "the inline copy is REDACTED and capped by count AND bytes; the audit rows written through IAuditWriter are redacted again by the writer (idempotent)". Keep Snapshot, Count, Complete as they are.

ScriptRunLauncher.LaunchScript: resolve var redactor = serviceScope.ServiceProvider.GetService<IAuditRedactor>(); beside auditWriter (line ~335) and construct the collector AFTER the scope exists: collector = trace && !replyTo.IsNobody() ? new FlowTraceCollector(redactor: redactor) : null; — move the collector creation from line 250 to just after the scope block (it is only handed to ScriptRuntimeContext and EmitScriptRun, both later). IAuditRedactor is registered by AddAuditLog on every site host; null in tests = no redaction, still capped.

ScriptRuntimeContext: only the using. ScriptRuntimeContextTraceEmissionTests: add one test that builds the context with new FlowTraceCollector(redactor: new UpperRedactor()), triggers an AttributeWrite emission, and asserts the collected DetailsJson is the redacted form while the IAuditWriter fake still receives the raw event (the writer redacts itself).

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.Commons.Tests and dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests → PASS.

Step 5: Commit fix(site-runtime): Flow Debugger inline trace copy is redacted and byte-capped at the collector (arch-review F1, site half)


Task 2: F1 (central) — FlowDebugRunner collects through the same redacting collector

Classification: small Estimated implement time: ~4 min Parallelizable with: Task 8, 12 Blocked by: Task 1, Task 3

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs:103-146 (RunMethodAsync)
  • Test: tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs

Step 1: Failing tests

  • RunMethod_SinkEvents_AreRedacted_WhenARedactorIsRegistered: register an IAuditRedactor fake in the test IServiceProvider that upper-cases DetailsJson; the fake executor pushes one event through route.TraceSink; assert the result's event DetailsJson is upper-cased.
  • RunMethod_OverCap_ReportsTruncated: fake executor pushes 501 events; assert result.Truncated == true and result.Events.Count == 501 (inbound row + 500).

Step 2: run → FAIL.

Step 3: Implementation — replace the ConcurrentQueue<AuditEvent> with

var collector = new FlowTraceCollector(redactor: sp.GetService<IAuditRedactor>());
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(collector.Add);
...
var events = collector.Complete();
var ordered = new List<AuditEvent>(events.Count + 1) { inboundRow };
ordered.AddRange(events.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId)); // Task 6 changes this line
return new FlowDebugRunResult(root, ..., ordered, Truncated: collector.Truncated, startedAt, finishedAt);

ICentralAuditWriter.WriteAsync already redacts the stored inbound row; leave WriteCentralAsync alone. Note in the method doc that the redactor is AddAuditLog's singleton (Program.cs:333) and that a host without one collects raw but still capped.

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests --filter FlowDebugRunnerTests → PASS.

Step 5: Commit fix(management): Flow Debugger Live result is the redacted, capped trace copy (arch-review F1, central half)


Task 3: F2 — site scope on the method-entry Live run (RouteHelper.WithSiteScope)

Classification: high-risk (security gate) Estimated implement time: ~5 min Parallelizable with: Task 1, 4, 5, 7, 10, 11

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/RouteHelper.cs (new RouteSiteScopeException, WithSiteScope, check in RouteTarget.ResolveSiteAsync:540)
  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs:509-521 (map the new exception to error code SITE_SCOPE_DENIED)
  • Modify: src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs:79-146 (RunMethodAsync sets the scope; rethrows as SiteScopeViolationException after writing the inbound row)
  • Test: tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/RouteHelperTests.cs, tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs

Design. IInstanceLocator.GetSiteIdForInstanceAsync returns the site identifier (string, e.g. site-a), while AuthenticatedUser.PermittedSiteIds holds numeric ids as strings. The runner therefore resolves the user's permitted numeric ids to identifiers through ISiteRepository.GetSiteByIdAsync once per run and hands RouteHelper an identifier set. Administrators and unscoped users (PermittedSiteIds.Length == 0) get no scope (null), exactly like EnforceSiteScope. SiteScopeViolationException lives in ManagementService, which InboundAPI cannot reference, so InboundAPI throws its own RouteSiteScopeException : InvalidOperationException; the executor maps it to SITE_SCOPE_DENIED (the script never sees a routed reply — the check runs before any site traffic); the runner converts that code to SiteScopeViolationException → ManagementUnauthorized, AFTER writing the inbound audit row so the denied attempt is recorded with the operator's name. The HTTP endpoint passes no scope — unchanged.

Step 1: Failing tests

RouteHelperTests:

[Fact]
public async Task Call_OutsideSiteScope_ThrowsRouteSiteScopeException_BeforeRouting()
{
    // locator resolves "site-b"; scope = {"site-a"}; router must never be called
    var helper = new RouteHelper(locator, router).WithSiteScope(new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "site-a" });
    await Assert.ThrowsAsync<RouteSiteScopeException>(() => helper.To("Inst1").Call("S"));
    Assert.Equal(0, router.CallCount);
}
[Fact] public async Task Call_InsideSiteScope_Routes() { /* scope {"site-b"}, locator "site-b" → routes */ }
[Fact] public async Task Call_NullScope_IsUnscoped() { /* WithSiteScope(null) routes */ }

FlowDebugRunnerTests:

[Fact]
public async Task RunMethod_ScopedDeployer_RouteToOtherSite_IsUnauthorized_AndInboundRowIsWritten()
{
    // user PermittedSiteIds = ["1"]; ISiteRepository fake: id 1 → "site-a"; fake executor returns
    // new InboundScriptResult(false, null, "Access denied ...", "SITE_SCOPE_DENIED") when route scope excludes "site-b"
    await Assert.ThrowsAsync<SiteScopeViolationException>(() => runner.RunMethodAsync(sp, cmd, scopedUser, ct));
    Assert.Single(centralWriter.Written, e => AuditDetailsCodec.Deserialize(e.DetailsJson).Kind == nameof(AuditKind.InboundRequest));
}
[Fact] public async Task RunMethod_Administrator_IsNeverScoped() { /* Roles contains Administrator, PermittedSiteIds ["1"], route to site-b succeeds */ }

Step 2: run both projects' filters → FAIL.

Step 3: Implementation

RouteHelper.cs — beside SiteUnreachableException:

/// <summary>Thrown by a routed call whose target site is outside the caller's site scope (arch-review F2). Raised BEFORE any site traffic.</summary>
public sealed class RouteSiteScopeException : InvalidOperationException
{
    public RouteSiteScopeException(string message) : base(message) { }
}

Add IReadOnlySet<string>? permittedSiteIdentifiers as a trailing field/ctor parameter on both RouteHelper and RouteTarget, thread it through every With* copy and To(...), add:

/// <summary>Restricts routed calls to the given site identifiers (case-insensitive); null = unscoped. Flow Debugger Live runs set it from the operator's site-scoped Deployer role.</summary>
public RouteHelper WithSiteScope(IReadOnlySet<string>? permittedSiteIdentifiers) => new(..., permittedSiteIdentifiers);

In RouteTarget.ResolveSiteAsync, after the null check:

if (_permittedSiteIdentifiers is not null && !_permittedSiteIdentifiers.Contains(siteId))
    throw new RouteSiteScopeException(
        $"Access denied: instance '{_instanceCode}' is at site '{siteId}', outside your site scope.");

(ResolveSiteAsync is the single choke point — all four routed operations at lines 256/395/460/510 call it.)

InboundScriptExecutor.cs — add before the SiteUnreachableException catch (line ~509):

catch (RouteSiteScopeException ex)
{
    _logger.LogWarning("API method {Method} refused a routed call outside the caller's site scope: {Message}", method.Name, ex.Message);
    return new InboundScriptResult(false, null, ex.Message, "SITE_SCOPE_DENIED");
}

Check whether the script body wraps exceptions in TargetInvocationException/AggregateException on this path — mirror whatever SiteUnreachableException needs (ex.GetBaseException() unwrapping) so the mapping is symmetrical.

FlowDebugRunner.RunMethodAsync — after EnsureActiveNode:

var scope = await ResolvePermittedSiteIdentifiersAsync(sp, user, ct).ConfigureAwait(false);
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(...).WithSiteScope(scope);
...
await WriteCentralAsync(sp, inboundRow, ct).ConfigureAwait(false);
if (result.ErrorCode == "SITE_SCOPE_DENIED")
    throw new SiteScopeViolationException(result.ErrorMessage ?? "Access denied: routed call outside your site scope.");
/// <summary>Arch-review F2: the user's PermittedSiteIds (numeric) → site identifiers; null = unscoped (empty list or Administrator).</summary>
private static async Task<IReadOnlySet<string>?> ResolvePermittedSiteIdentifiersAsync(IServiceProvider sp, AuthenticatedUser user, CancellationToken ct)
{
    if (user.PermittedSiteIds.Length == 0) return null;
    if (user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase)) return null;
    var sites = sp.GetRequiredService<ISiteRepository>();
    var set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
    foreach (var raw in user.PermittedSiteIds)
    {
        if (!int.TryParse(raw, out var id)) continue;
        var site = await sites.GetSiteByIdAsync(id, ct).ConfigureAwait(false);
        if (site is not null) set.Add(site.SiteIdentifier);
    }
    return set; // possibly empty: a scoped user whose sites no longer exist routes nowhere
}

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests and tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests → PASS.

Step 5: Commit fix(inbound-api): Flow Debugger method-entry Live run enforces the operator's site scope at route time (arch-review F2)


Task 4: F3 — Replay opens to OperationalAudit; Live stays Deployer-only

Classification: standard (authorization) Estimated implement time: ~5 min Parallelizable with: Task 1, 3, 5, 7, 10, 11

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.Security/AuthorizationPolicies.cs (new policy RequireFlowDebugger + FlowDebuggerAccessRoles)
  • Modify: src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs:192-310 (FlowDebugReplayCommand → new FlowDebuggerReplayRoles)
  • Modify: src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor (page attribute, gate the Live controls) and FlowDebugger.razor.cs (CanRun also requires the Deployer role; _canLive from AuthenticationStateProvider)
  • Modify: src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor:238 (Retry/Discard delegates only wired when _canLive)
  • Test: tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs:144, tests/ZB.MOM.WW.ScadaBridge.Security.Tests/ (find the existing policy test class and add a row), tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs

Step 1: Failing tests

  • RequiredRoleMatrixTests: ["FlowDebugReplay"] = [Roles.Deployer, Roles.Administrator, Roles.Viewer] (the two run commands stay [Roles.Deployer]).
  • Security policy test: a Viewer principal satisfies RequireFlowDebugger; a Designer does not; a Deployer does.
  • FlowDebuggerPageTests: rendering the page with a Viewer-only principal shows the Replay controls (data-test="flow-entry-replay" or the execution-id input — use whatever the page's replay data-test markers are) and NOT data-test="flow-run"; with Deployer it shows both. Follow the existing test's auth setup (bUnit TestAuthorizationContext).

Step 2: run → FAIL.

Step 3: Implementation

AuthorizationPolicies.cs:

/// <summary>Flow Debugger page access (arch-review F3 / design D6): Replay is an audit read, so
/// OperationalAudit roles may open the page; Live runs are additionally gated by
/// <see cref="RequireDeployment"/> on the Run controls and by the ManagementActor.</summary>
public const string RequireFlowDebugger = "RequireFlowDebugger";
public static readonly string[] FlowDebuggerAccessRoles = { Roles.Deployer, Roles.Administrator, Roles.Viewer };
// in AddScadaBridgeAuthorization:
options.AddPolicy(RequireFlowDebugger, policy => policy.RequireClaim(JwtTokenService.RoleClaimType, FlowDebuggerAccessRoles));

ManagementActor.cs:

private static readonly string[] FlowDebuggerReplayRoles = AuthorizationPolicies.FlowDebuggerAccessRoles;
...
FlowDebugRunMethodCommand or FlowDebugRunValueChangeCommand => DeployerOnly,
// Replay reads the audit chain — OperationalAudit roles + Deployer (design D6, arch-review F3).
FlowDebugReplayCommand => FlowDebuggerReplayRoles,

Page: @attribute [Authorize(Policy = AuthorizationPolicies.RequireFlowDebugger)]; in OnInitializedAsync compute _canLive = (await Auth.GetAuthenticationStateAsync()).User.HasClaim(JwtTokenService.RoleClaimType, Roles.Deployer) (inject AuthenticationStateProvider if the page does not already); hide/disable the Live entry buttons, the Run button and the value input when !_canLive (default the mode to Replay for such a user; if the page opens with ?executionId= it already goes to Replay); pass OnRetry/OnDiscard to FlowStepDetail only when _canLive. Keep the NavMenu entry where it is (inside the Deployment section) — auditors arrive through the Audit Log / Execution Chain drill-ins, which is the point of the finding.

Step 4: dotnet test on ManagementService.Tests, Security.Tests, CentralUI.Tests → PASS.

Step 5: Commit fix(central-ui): Flow Debugger Replay opens to OperationalAudit roles; Live run and Retry/Discard stay Deployer-only (arch-review F3)


Task 5: F4 — client waits derive from the server's bound

Classification: small Estimated implement time: ~4 min Parallelizable with: Task 1, 3, 4, 7, 10, 11

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/FlowDebugService.cs:26-40, 178-185
  • Modify: src/ZB.MOM.WW.ScadaBridge.CLI/Commands/FlowDebugCommands.cs:17-18, 37-97
  • Modify: src/ZB.MOM.WW.ScadaBridge.CLI/README.md (the flow-debug run-method / run-value-change option tables)
  • Test: tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/FlowDebugServiceTests.cs, tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/FlowDebugCommandsTests.cs

Step 1: Failing tests

  • FlowDebugServiceTests: RunAskTimeoutFor(ApiMethod) (make it a public static TimeSpan RunAskTimeoutFor(int timeoutSeconds)) returns timeoutSeconds + RunGrace(10 s) + ClientSlack(15 s), never below MinRunAskTimeout (30 s); ValueChangeAskTimeoutFor(int waitSeconds) returns waitSeconds + ClientSlack. Assert RunAskTimeoutFor(180) == 205 s, RunAskTimeoutFor(1) == 30 s, ValueChangeAskTimeoutFor(45) == 60 s.
  • FlowDebugCommandsTests: run-method accepts --timeout-seconds (default 120); run-value-change has no such option and derives waitSeconds + 25; --timeout-seconds 0 is rejected with INVALID_ARGUMENT.

Step 2: run → FAIL.

Step 3: Implementation

FlowDebugService: CentralUI references ManagementService, so use FlowDebugRunner.RunGrace directly.

/// <summary>Slack the client adds over the server's own bound (Ask hop + serialization).</summary>
public static readonly TimeSpan ClientSlack = TimeSpan.FromSeconds(15);
public static readonly TimeSpan MinRunAskTimeout = TimeSpan.FromSeconds(30);
public static TimeSpan RunAskTimeoutFor(int timeoutSeconds) { var t = TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds)) + FlowDebugRunner.RunGrace + ClientSlack; return t < MinRunAskTimeout ? MinRunAskTimeout : t; }
public static TimeSpan ValueChangeAskTimeoutFor(int waitSeconds) => TimeSpan.FromSeconds(Math.Max(1, waitSeconds)) + ClientSlack;

RunMethodAsync reads the method (_inboundApi.GetApiMethodByIdAsync) and uses RunAskTimeoutFor(method?.TimeoutSeconds ?? 30); RunValueChangeAsync uses ValueChangeAskTimeoutFor(45) (the command record default — read it from new FlowDebugRunValueChangeCommand(...).WaitSeconds so the two never drift). Delete the fixed RunAskTimeout and fix the class doc ("both fit inside two minutes" is gone).

CLI: run-method gains var timeoutOption = new Option<int>("--timeout-seconds") { Description = "Client-side wait for the run; set to the method's TimeoutSeconds + 25 for long methods", DefaultValueFactory = _ => 120 }; validated > 0; run-value-change passes TimeSpan.FromSeconds(waitSeconds + 25). Remove RunTimeout. README: add the option row and the derivation sentence.

Step 4: both test projects → PASS.

Step 5: Commit fix(central-ui,cli): Flow Debugger client waits derive from the method timeout / wait window instead of a fixed 2 minutes (arch-review F4)


Task 6: F5 — emission order wins over cross-node timestamps

Classification: small Estimated implement time: ~4 min Parallelizable with: Task 9, 13 Blocked by: Task 2

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.Commons/Types/Debugging/FlowGraphBuilder.cs:38-50, 95-97
  • Modify: src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs (RunMethodAsync ordered list: keep collector order, no OrderBy)
  • Test: tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Debugging/FlowGraphBuilderTests.cs, tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/FlowDebugRunnerTests.cs:89 (..._Ordered_... test: assert emission order is preserved)

Step 1: Failing test (FlowGraphBuilderTests): SiteClockAhead_SiteStepsStillFollowTheirRouteCall_AndPrecedeResponse — inbound (central t0), RouteCall (central t0+50 ms, ChildExecutionId = child), site ScriptRun (child, t0 + 2 s — clock ahead), site AttributeWrite (child, t0 + 2.01 s), given in that emission order; assert Steps order is inbound, RouteCall, ScriptRun, AttributeWrite, Response and the Response's OccurredAtUtc is derived from the inbound row as today.

Step 2: run → FAIL (site steps sort after the Response).

Step 3: Implementation — sort per execution, order executions by first appearance:

var seq = new Dictionary<Guid, int>();               // EventId → input index
for (var i = 0; i < events.Count; i++) seq.TryAdd(events[i].EventId, i);
int Seq(FlowStep s) => seq.TryGetValue(s.EventId, out var i) ? i : int.MaxValue;
var firstSeqByExecution = folded.GroupBy(s => s.ExecutionId ?? Guid.Empty).ToDictionary(g => g.Key, g => g.Min(Seq));
var steps = folded
    .OrderBy(s => firstSeqByExecution[s.ExecutionId ?? Guid.Empty])   // executions in emission order
    .ThenBy(s => s.OccurredAtUtc)                                       // one clock inside an execution
    .ThenBy(s => KindRank(s.Kind))
    .ThenBy(Seq)
    .ToList();

Replay input arrives timestamp-sorted from the repository, so its order is unchanged. The spawner search (line 96): prefer the parent step whose Seq is the greatest below the child's first step's Seq (non-async, non-Response), falling back to the timestamp rule, then parentChain[0]. Runner: ordered.AddRange(collector.Complete()) — no re-sort (the value-change path keeps its OrderBy, all rows there share the site clock).

Doc comment on Build: "Order = emission order between executions, timestamp within one (arch-review F5: central and site clocks differ)".

Step 4: Commons.Tests + ManagementService.Tests → PASS (fix any existing ordering test that pinned the old global sort; the Delmia-shape test passes unchanged because its input is already in timestamp order).

Step 5: Commit fix(commons): Flow graph orders executions by emission sequence, timestamps only within one clock (arch-review F5)


Task 7: F6 — trace arm cleared when the DCL write fails

Classification: standard (actor) Estimated implement time: ~4 min Parallelizable with: Task 1, 3, 4, 5, 10, 11

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs:183, 339 (Receive registration), 757-779
  • Test: tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTraceArmTests.cs

Step 1: Failing test — TracedDataWrite_ThatFails_DisarmsTheAttribute_SoTheNextEchoIsUnstamped: DCL fake answers WriteTagResponse(Success:false, "rejected"); then a TagValueUpdate on the attribute; assert the published AttributeValueChanged has SourceExecutionId == null && Trace == false. Mirror the existing TracedDataWrite_ThenEcho_... fixture.

Step 2: run → FAIL.

Step 3: Implementation

/// <summary>Self-message: a traced data write failed, drop its trace arm if it is still the armed one (arch-review F6).</summary>
private sealed record DisarmTrace(string AttributeName, Guid SourceExecutionId);
// ctor: Receive<DisarmTrace>(m => { if (_traceArms.TryGetValue(m.AttributeName, out var a) && a.SourceExecutionId == m.SourceExecutionId) _traceArms.Remove(m.AttributeName); });

In HandleSetDataAttribute capture var self = Self; Guid? armed = command.Trace ? command.SourceExecutionId : null; and in the ContinueWith body, when !t.IsCompletedSuccessfully || !t.Result.Success, if (armed is { } a) self.Tell(new DisarmTrace(attributeName, a)); before returning the response. Update the arm comment: "armed at issue, disarmed on a failed/timed-out write; a successful write's echo consumes it".

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests --filter InstanceActorTraceArmTests → PASS.

Step 5: Commit fix(site-runtime): trace arm is cleared when the traced DCL write fails (arch-review F6)


Task 8: F7 — a shed or rate-limited traced run leaves a Failed ScriptRun row

Classification: standard (actor + launcher) Estimated implement time: ~5 min Parallelizable with: Task 2, 12 Blocked by: Task 1

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs (new internal static void EmitSuppressedRun(...), reusing EmitScriptRun)
  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/ScriptActor.cs:519-541 (TrySpawnExecution), 566-580 (SpawnExecution shed branch), 653 (ShedRun signature gains parentExecutionId/trace/triggerExtra)
  • Test: tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptRunShedTests.cs

Step 1: Failing tests

  • TracedRun_ShedAtTheCap_EmitsAFailedScriptRunRow_ParentedToTheWriter: fill the cap with untraced runs, then a traced AttributeValueChanged (Trace:true, SourceExecutionId: parent); assert the IAuditWriter fake received one ScriptTrace/ScriptRun Failed row with ParentExecutionId == parent and ErrorMessage starting shed: MaxConcurrentRunsPerScript.
  • TracedRun_SuppressedByMinTimeBetweenRuns_EmitsAFailedScriptRunRow: MinTimeBetweenRuns = 10 min, two traced changes back to back; assert the second yields a Failed row with suppressed: MinTimeBetweenRuns.

Step 2: run → FAIL.

Step 3: Implementation — in the launcher:

/// <summary>Arch-review F7: a traced run that never launched (shed / rate-limited) still leaves a
/// Failed ScriptRun row so the Flow Debugger explains the gap instead of NO_TRIGGER_WITHIN_WINDOW.
/// No collector (there is no reply to ride on); writer + identity from a fresh scope.</summary>
internal static void EmitSuppressedRun(IServiceProvider? serviceProvider, ILogger logger, string scriptName, string instanceName,
    Guid? parentExecutionId, string reason, ScriptTraceExtra? triggerExtra)
{
    if (serviceProvider is null) return;
    using var scope = serviceProvider.CreateScope();
    var writer = scope.ServiceProvider.GetService<IAuditWriter>();
    if (writer is null) return;
    var siteId = scope.ServiceProvider.GetService<ISiteIdentityProvider>()?.SiteId ?? string.Empty;
    var sourceNode = scope.ServiceProvider.GetService<INodeIdentityProvider>()?.NodeName;
    EmitScriptRun(trace: true, collector: null, writer, logger, scriptName, instanceName, siteId, sourceNode,
        executionId: Guid.NewGuid(), parentExecutionId, DateTime.UtcNow, elapsedMs: 0,
        parameters: null, returnValue: null, error: reason, triggerExtra);
}

ScriptActor: in the MinTimeBetweenRuns branch if (trace) ScriptRunLauncher.EmitSuppressedRun(_serviceProvider, _logger, _scriptName, _instanceName, parentExecutionId, "suppressed: MinTimeBetweenRuns", triggerExtra); and in the shed branch the same with "shed: MaxConcurrentRunsPerScript" (pass parentExecutionId, trace, triggerExtra into ShedRun).

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests → PASS.

Step 5: Commit fix(site-runtime): shed or rate-limited traced runs emit a Failed ScriptRun row (arch-review F7)


Task 9: F8 — fault continuations on TaskScheduler.Default, central emitters log

Classification: small Estimated implement time: ~3 min Parallelizable with: Task 6, 13 Blocked by: Task 1, 3, 8, 12

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs:446-448
  • Modify: src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRunLauncher.cs:544-548
  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/RouteHelper.cs:339 (+ optional ILogger? threaded through RouteHelper/RouteTarget ctors and With* copies; DI ctor takes ILogger<RouteHelper>? logger = null)
  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundDatabaseHelper.cs:271 (+ optional ILogger? logger = null ctor parameter; InboundScriptExecutor.cs:391-400 passes _logger)
  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs:391-400 (pass the logger)
  • Test: tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundDatabaseHelperAuditTests.cs (one test: a faulting writer produces a Warning log entry and the call still succeeds)

Step 3: every site is

_ = t.ContinueWith(
    x => logger?.LogWarning(x.Exception, "... write failed for {X}", ...),
    CancellationToken.None,
    TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
    TaskScheduler.Default);

(the two site emitters keep their existing messages, just add the token/options/scheduler arguments; the two central ones replace static _ => { } with a Warning log).

Step 4: SiteRuntime.Tests + InboundAPI.Tests → PASS.

Step 5: Commit fix(audit): trace/outbound fault continuations run on the default scheduler and central emitters log (arch-review F8)


Task 10: F9 — one execution-chain walker in AuditLogRepository

Classification: small Estimated implement time: ~5 min Parallelizable with: Task 1, 3, 4, 5, 7, 11

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/AuditLogRepository.cs:922-1241
  • Test: tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Repositories/AuditLogRepositoryTraceEventsTests.cs (+ the existing execution-tree tests in that folder — find them with grep -rl GetExecutionTreeAsync tests/)

Step 3: Implementation — pure refactor, behaviour pinned by the existing tests (they need the rig's MSSQL; SkippableFact skips without it — run with the container up):

private sealed record ChainAnchor(Guid RootExecutionId, DateTime? RootFirstOccurred);

/// <summary>Phase 1 + window of the execution-chain walk, shared by GetExecutionTreeAsync and GetTraceEventsAsync (arch-review F9).</summary>
private static async Task<ChainAnchor> ResolveChainAnchorAsync(DbConnection conn, Guid executionId, CancellationToken ct) { /* the up-walk loop + MIN(OccurredAtUtc) */ }

/// <summary>The Edges/Chain CTE prefix (identical in both walkers); callers append their projection.</summary>
private static string ChainCtePrefix(string edgesWindowPredicate) => $@"WITH Edges AS (...), Chain AS (...)";

private static void AddChainParameters(DbCommand cmd, ChainAnchor anchor) { /* @root, @winStart/@winEnd when windowed */ }

GetExecutionTreeAsync keeps its ChainIds + aggregate projection appended to the prefix; GetTraceEventsAsync appends the id projection. Both keep OPTION (MAXRECURSION …). Delete the duplicated blocks. No SQL text change other than the split.

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests --filter "AuditLogRepository" → PASS, none skipped (MSSQL up).

Step 5: Commit refactor(config-db): GetTraceEventsAsync and GetExecutionTreeAsync share one chain walker (arch-review F9)


Task 11: F10 — prediction overlay matches by (Kind, Title) before Kind

Classification: small Estimated implement time: ~3 min Parallelizable with: Task 1, 3, 4, 5, 7, 10

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/FlowGraphView.razor.cs:411-423
  • Test: tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/FlowGraphViewTests.cs

Step 1: Failing test — Overlay_TwoPredictedRouteCalls_RealSecondCallMatchesByTitle_FirstIsTheGhost: predicted nodes RouteCall "A.X", RouteCall "B.Y"; real graph has one RouteCall step titled B.Y; assert the ghost key set contains the predicted A.X node and the real step maps to B.Y.

Step 3:

var m = unmatched.FirstOrDefault(p => p.Kind == s.Kind && string.Equals(p.Title, s.Title, StringComparison.OrdinalIgnoreCase))
     ?? unmatched.FirstOrDefault(p => p.Kind == s.Kind);

(check the predicted node's title property name — PredictedNode.Title or Label — and the real step's FlowStep.Title).

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests --filter FlowGraphViewTests → PASS.

Step 5: Commit fix(central-ui): Flow Debugger prediction overlay matches by kind and title (arch-review F10)


Task 12: F11 — RouteCall row honours the method's SkipBodyCapture

Classification: small Estimated implement time: ~4 min Parallelizable with: Task 2, 8 Blocked by: Task 3

Files:

  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/RouteHelper.cs (WithBodyCapture(bool), field threaded through RouteHelper/RouteTarget; EmitRouteCall passes null summaries when off)
  • Modify: src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs:415-420 (resolve IOptionsMonitor<AuditLogOptions> from the scope; PerTargetOverrides.TryGetValue(method.Name, …)?.SkipBodyCapture exactly as AuditWriteMiddleware.cs:183-187; .WithBodyCapture(!skip))
  • Test: tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/RouteHelperTests.cs, tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs

Step 1: Failing tests — RouteCallRow_WithBodyCaptureOff_HasNoRequestOrResponseSummary (RouteHelperTests, using the trace sink to read the row) and Executor_MethodWithSkipBodyCapture_RoutesWithBodyCaptureOff (executor test with an options monitor fake carrying the override for the method name).

Step 3: EmitRouteCall(...): requestSummary: _captureBodies ? TraceJson(args) : null, responseSummary: _captureBodies ? TraceJson(returnValue) : null, extra: … and add "bodyCapture":"skipped"-style marker only if ScriptTraceExtra already has a free-form field; otherwise leave extra unchanged.

Step 4: dotnet test tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests → PASS.

Step 5: Commit fix(inbound-api): RouteCall audit row honours the method's SkipBodyCapture override (arch-review F11)


Task 13: F12 + F13 — documentation reconciled with the implementation

Classification: trivial (docs only) Estimated implement time: ~5 min Parallelizable with: Task 6, 9 Blocked by: Tasks 1–12 (so the text describes the final code)

Files:

  • Modify: docs/plans/2026-09-05-flow-debugger-design.md (D6 row; §5.4 value-change paragraph; §9 role + redaction bullets; §10 volume + inbound-row-after-run note)
  • Modify: src/ZB.MOM.WW.ScadaBridge.Communication/Protos/sitestream.proto:102-106 (comment only — the vendored C# needs no regeneration for a comment)
  • Modify: src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor:50, src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ApiMethodCommands.cs:65,101, src/ZB.MOM.WW.ScadaBridge.CLI/README.md:2122,2140, docs/requirements/Component-InboundAPI.md:73 ("~7" → "7–10 audit rows per call (3 of them trace-only)")
  • Modify: docs/requirements/Component-InboundAPI.md (new sentences: SchemaRefResolver is public for the Flow Debugger runner; RouteHelper.WithSiteScope + SITE_SCOPE_DENIED; RouteCall honours SkipBodyCapture)
  • Modify: docs/requirements/Component-CentralUI.md (Flow Debugger: page policy RequireFlowDebugger, Live controls Deployer-only, client wait derivation)
  • Modify: CLAUDE.md (Flow Debugger bullet: the per-call trace-row estimate → "7–10", one sentence on F1/F2/F3 outcomes with the review doc path)
  • Modify: docs/plans/2026-09-06-flow-debugger-arch-review.md (append ## 7. Resolution listing each finding → commit)

Text to write (verbatim where possible):

  • D6: "Live run is a Deployer act. RequireDeployment gates the Run controls and the two run commands; the page and Replay are open to OperationalAudit roles + Deployer (RequireFlowDebugger, arch-review F3)."
  • §5.4: replace the debug-stream subscription paragraph with: "For the value-change entry there is no routed reply to ride on. The runner polls the site's PullTraceEvents RPC by ParentExecutionId (1 s cadence, 2 s settle window, WaitSeconds bound) — simpler than the debug-stream subscription originally planned. AttributeValueUpdate.source_execution_id/trace (fields 7/8) are populated by the site and consumed by nothing; they are reserved for a future debug-stream path and must not be reused."
  • §9 redaction bullet: "Trace payloads pass through the existing redaction policy on BOTH copies: the stored row (writer) and the inline Live copy (FlowTraceCollector applies IAuditRedactor and a 1 MB byte budget beside the 500-event cap — arch-review F1)." Add: "The method entry enforces the operator's site scope at route time (RouteHelper.WithSiteScope, SITE_SCOPE_DENIED → ManagementUnauthorized, arch-review F2)."
  • §10: "≈ 7–10 rows (a cached DB write and a notification add 3 trace-only rows)"; "Every routed call adds one central RouteCall insert — at inbound-driven rates that doubles the per-call central audit write (accepted, D5)"; "The Live run's inbound row is written AFTER the run completes, so a Replay taken during a long Live run shows only the RouteCall."

Step 4: grep docs src CLAUDE.md for the pre-remediation estimate wordings (the tilde-7 forms and the spelled-out "seven audit rows" one) → no hits. The patterns are deliberately not written out in this file so the check can return zero hits.

Step 5: Commit docs: Flow Debugger design reconciled with the implementation (arch-review F12, F13)


Task 14: Full gate — build, full test run, rig rebuild, live verification

Classification: high-risk (rig) Estimated implement time: ~15 min wall (mostly waiting) Parallelizable with: none — last

Steps

  1. dotnet build ZB.MOM.WW.ScadaBridge.slnx → 0 errors.
  2. dotnet test ZB.MOM.WW.ScadaBridge.slnx → every project green except the 11 known Playwright role-negative tests (.claude/skills/scadabridge-cluster-ops/SKILL.md).
  3. bash docker/deploy.sh (site runtime, inbound API, management, central UI all changed).
  4. Mint a fresh key (see memory delmia-rig-standin-state), FLOW_ONLY=1 DELMIA_API_KEY=… bash docker/verify-delmia-notifier.sh → H live+replay YES, I outbound live+replay YES.
  5. Browser: open http://localhost:9000/deployment/flow-debugger, run the FlowOutboundDemo method Live, confirm the External lane still draws and the run completes.
  6. Report; do not merge or push without the user.