Merge branch 'fix/flow-debugger-arch-review'

This commit is contained in:
Joseph Doherty
2026-09-06 17:05:57 -04:00
52 changed files with 3248 additions and 388 deletions
@@ -6,7 +6,7 @@ description: How to drive the local ScadaBridge docker cluster — CLI managemen
# ScadaBridge Cluster Ops (Docker / OrbStack)
- **Management URL**: `http://localhost:9000` — the CLI connects via the Traefik load balancer, which routes to the active central node. Direct access: central-a on port 9001, central-b on port 9002.
- **Central UI login is DISABLED on the rig (both central nodes, since 2026-09-06)** — `ScadaBridge:Security:Auth:DisableLogin=true` in `docker/central-node-{a,b}/appsettings.Central.json`, so `http://localhost:9000` (and 9001/9002) opens the UI as `multi-role` with every role, no LDAP login. The CLI/management API still takes `--username/--password` (Basic auth is a separate path), and `POST /auth/login` still answers 302, so the Playwright fixture login keeps working; only `LoginTests` (expects a `/login` redirect + 401 on bad credentials) fails against this rig. Flip the two keys back to `false` and recreate the node for real-login checks.
- **Central UI login is DISABLED on the rig (both central nodes, since 2026-09-06)** — `ScadaBridge:Security:Auth:DisableLogin=true` in `docker/central-node-{a,b}/appsettings.Central.json`, so `http://localhost:9000` (and 9001/9002) opens the UI as `multi-role` with every role, no LDAP login. The CLI/management API still takes `--username/--password` (Basic auth is a separate path), and `POST /auth/login` still answers 302, so the Playwright fixture login keeps working. Expected Playwright failures on this rig are exactly the 11 role-NEGATIVE tests (full run 2026-09-06: 164/175) — `RoleNavigationTests.*_DoesNotSee*`, `AuditLogPageTests.PermissionGating_DesignerWithoutOperationalAudit_IsDeniedAccess`, `LoginTests.UnauthenticatedUser_RedirectsToLogin` — because every browser session carries every role; any other Playwright failure is real. Flip the two keys back to `false` and recreate the node for real-login checks.
- **Test user**: `--username multi-role --password password` — has Admin, Design, and Deployment roles. The `admin` user only has the Admin role and cannot create templates, data connections, or deploy.
- **Config file**: `~/.scadabridge/config.json` — stores `managementUrl` and default format. See `docker/README.md` for a ready-to-use test config.
- **Rebuild cluster**: `bash docker/deploy.sh` — builds the `scadabridge:latest` image and recreates all containers. Run this after code changes to ManagementActor, Host, or any server-side component.
+1 -1
View File
File diff suppressed because one or more lines are too long
@@ -104,6 +104,8 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
| 109 | **`ZB.MOM.WW.Secrets.Replicator.SqlServer`'s store migrator retries its DDL with a flat `100 * attempt` backoff and no jitter — a latent concurrent-boot flake, open in Secrets** *(Low)*. Porting that migrator's retry loop verbatim for the Auth 0.3.0 SQL Server API-key store (register row 108) reproduced an intermittent exhaustion — about once in sixty 8-way concurrent-boot trials the five flat-backoff retries ran out before a DDL/lock conflict cleared; adding jitter to the Auth port's backoff took it to 0 failures in 180 trials. Secrets' own migrator has no concurrent-migrate test, so the same defect is presumed still open there — it just hasn't been exercised at 8-way concurrency, and a ScadaBridge central pair is only ever 2-way, so this is unlikely to bite in practice. | Found 2026-09-02 while implementing `docs/plans/2026-09-02-inbound-api-key-shared-store.md` (Auth 0.3.0 `SqlServerApiKeyStoreMigrator`) | Add jitter to `SqlServerSecretsStoreMigrator`'s backoff in `~/Desktop/scadaproj/ZB.MOM.WW.Secrets` (mirror the Auth fix) and add a `Migrate_Concurrently_FromEightTasks_AllSucceed`-shaped live test to catch a regression. Lives entirely in scadaproj, not this repo. | Next time Secrets' SQL Server migrator is touched, or if a wider-than-2-way concurrent boot of a Secrets-backed cluster is ever planned. |
| 110 | **Flow Debugger follow-ups: the *Vertical trace* list toggle (mockup Direction B) and console capture on `ScriptRun`** *(Low)*. v1 ships the Direction A swimlane only; the mockup's Direction B — the same steps as a vertical, filterable list with the lane as a column — was kept as an alternate for long flows and screen readers, and the detail panel's *Console* tab is absent because script `Console`/log output is not captured per run (`ScriptRun` rows carry parameters, return value, error and timing, not stdout). Both are additive: a list view over the existing `FlowGraph`, and an extra `Extra.console` field on the `ScriptRun` row's `ScriptTraceExtra` (payload cap applies). | Design `docs/plans/2026-09-05-flow-debugger-design.md` §11; plan `2026-09-05-flow-debugger.md` Task 23 | Neither was asked for by an operator; the swimlane + Timing/Writes tabs cover the acceptance flow (Delmia recipe download, rig step H), and console capture needs a per-run writer seam in `ScriptRuntimeContext` plus a redaction decision. | First operator request for a list view or for script log lines in the step detail. |
| 111 | ~~**Flow Debugger outbound nodes: no Retry/Discard on a parked node, and no `?correlationId=` deep link from the node to the Site Calls / Notification report pages** *(Low)*~~ **RESOLVED 2026-09-06 (plan `docs/plans/2026-09-06-flow-debugger-followups.md`).** `FlowStep` now carries the row's `CorrelationId`; the detail panel deep-links to `/site-calls/report?correlationId=` / `/notifications/report?correlationId=` (both pages accept it through additive per-id query filters — `SiteCallQueryRequest.TrackedOperationIdFilter`, `NotificationOutboxQueryRequest.NotificationIdFilter` — pre-filter to the row and auto-open its detail), and a `Parked` node offers Retry / Discard in place through `IFlowDebugService.RetryOutboundAsync` / `DiscardOutboundAsync`, which reuse the report pages' relay, site-scope re-check and confirm wording, after which the page re-polls Replay until the node leaves Parked. Per-attempt request/response diff is still not shown (the lifecycle rows carry the error summary only). Found on the way: the prediction circuit deadlock (`4769e192`). | Plan 2026-09-06 "Out of scope"; `FlowStepDetail.razor` parked hint | The Site Calls page owns the retry semantics and its audit rows; duplicating the action on the debugger node would need the same Deployer gate + confirm flow. | First operator who reaches a parked node from the debugger and asks for the button. |
| 112 | **Flow Debugger: a SITE-side inline trace truncation is invisible to the operator** *(Low)*. `ScriptCallResult` / `RouteToCallResponse` carry `TraceEvents` but no `Truncated` flag, so the page's *event list truncated* badge reflects central's `FlowTraceCollector` only — a site run whose own collector dropped rows returns a silently short list. The new 1 MB byte budget (arch-review F1) makes site truncation reachable sooner than the old 500-event count cap did. The fix is additive: a trailing `bool Truncated = false` on both records, a new field on `RouteToCallResponseDto` (regen via `docker/regen-proto.sh sitecommand`), and `FlowDebugRunner` ORing it into `FlowDebugRunResult.Truncated`. | Arch-review remediation code review 2026-09-06 | Needs a proto regeneration plus a rig rebuild to prove end-to-end, and nothing is lost meanwhile: the site's audit rows are written in full, so a Replay of the same execution is never truncated. | The first Live run that shows fewer site steps than its Replay of the same execution. |
| 113 | **Flow Debugger: the method-entry site scope (F2) gates ROUTED site traffic only** *(Low)*. A site-scoped Deployer who Live-runs a method belonging to another plant is still refused at `RouteTarget.ResolveSiteAsync`, but the method's central-side side effects — its `Database.*` calls, `ExternalSystem` calls, notifications — execute before or regardless of that refusal. Only routing is scoped. | Arch-review remediation code review 2026-09-06 | F2's stated property ("no site traffic outside the operator's scope") holds as written. Going further needs a per-method site affinity, which does not exist in the model — ApiMethods are central objects with no site column — so there is nothing to scope a method against without inventing that concept. | A decision to give `ApiMethod` a site affinity, or an operator report of a scoped user reaching another plant's central-side effects. |
| 68 | ~~**`deploy.sh` run from a git worktree silently rehomes every node's bind-mount data dir**~~ **RESOLVED 2026-08-15 (guard added in the same close-out).** The compose files' bind mounts are relative (`./site-a-node-a/data`), so a deploy from a worktree checkout mounts the worktree's copies — the rig keeps running and looks healthy, but its SQLite stores split from the main checkout's and everything written there is orphaned by the next deploy from main. Bit the rows-46–53 close-out live: the seeding agent's `rig-probe-1` deploy persisted into worktree-homed dirs and "vanished" after the orchestrator's redeploy (no tombstone, no product bug — LWW/tombstone forensics ruled out deletion; the row sat in `.claude/worktrees/…/docker/site-a-node-a/data/site-localdb.db` the whole time). Both `docker/deploy.sh` and `docker-env2/deploy.sh` now refuse to run from a `.claude/worktrees/` path (override: `SCADABRIDGE_ALLOW_WORKTREE_DEPLOY=1`). | Found during the rows-46–53 close-out | Closed 2026-08-15. | — |
## Resolved (verified against the code 2026-07-10)
+46 -20
View File
@@ -27,7 +27,7 @@ on the `DelmiaReceiver` child → seven attribute writes → wait for Galaxy to
| D3 | **Value-change on a data-sourced attribute does a REAL tag write** through the instance actor's normal write path; the device echo fires the trigger. Correlation is by a short-lived *trace arm* on the instance actor (§5.3). | A debug-only "inject value" command that synthesises the change without touching the device — leaves the instance's cached value diverged from the device until the next real update. |
| D4 | **Live gets its trace inline; Replay reads audit rows.** Site audit telemetry forwards on a 5 s busy / 30 s idle cadence (`AuditLog:SiteTelemetry`), too slow to drive an interactive run, so the routed reply carries the site steps back and the same steps are *also* written as audit rows for Replay. | Live waiting on the audit pipeline. |
| D5 | **Tracing is gated, not always-on.** Sites run scripts at high rates from timers and value changes; a trace flag rides beside `ParentExecutionId` and only flagged runs emit trace rows. One exception is always-on: a central `RouteCall` row per `Route.To().Call` (inbound-driven volume, joins the two halves of a Replay). | Tracing every run (volume), or a purely time-window join. |
| D6 | **Live run is a Deployer act.** `RequireDeployment` gates the page and the Run button; Replay needs `OperationalAudit`. | A separate `RequireOperator` gate for Live. |
| 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). | A separate `RequireOperator` gate for Live. |
## 2. What exists today (reused, not rebuilt)
@@ -159,20 +159,24 @@ the echo also fires inherit the parent id exactly as static cascades do today.
`RouteToCallResponse` gains additive `ChildExecutionId` (`Guid?`) and `TraceEvents`
(`IReadOnlyList<AuditEvent>?`, the very events that became the site's audit rows, in emission
order); on the wire it is `repeated AuditEventDto trace_events` (the DTO `sitestream.proto`
already defines). The site collects events in a per-run `FlowTraceCollector` held by
`ScriptRuntimeContext` and attaches the list to the `ScriptCallResult` → `RouteToCallResponse`
when `Trace` is set. Capped at `MaxTraceEventsPerRun` (500); beyond it the collector keeps the
first 500, sets `Truncated`, and the audit rows remain complete. A site on an older build ignores
already defines). The site collects events in a per-run `FlowTraceCollector` (Commons
`Types/Debugging/`, shared with the central `FlowDebugRunner`) held by `ScriptRuntimeContext` and
attaches the list to the `ScriptCallResult` → `RouteToCallResponse` when `Trace` is set. The
collector applies `IAuditRedactor` inside `Add`, so the inline record is the same filtered event
the writer persists, and it caps at `MaxTraceEventsPerRun` (500) **or** 1 MB of summed
`DetailsJson` (`DefaultMaxBytes`), whichever comes first; beyond either bound it keeps the earlier
events, sets `Truncated`, and the audit rows remain complete (arch-review F1). A site on an older build ignores
`Trace`, returns no `TraceEvents`, and the UI labels its lane `site not traced (older build)`.
`FlowStep` is a **central-side projection** (`Commons/Types/Debugging/FlowStepProjector`)
computed from `AuditEvent` + `AuditDetails`; nothing serializes it.
For the value-change entry there is no routed reply to ride on. The debugger instead subscribes
to the instance's debug stream (`SubscribeDebugView`, the Debug View's existing path) for the run's
duration and takes the spawned run's `ExecutionId` from the stamped `AttributeValueChanged`, then
pulls the site's trace rows for that id via a new `PullTraceEvents` RPC on `SiteStreamService`
(same PSK interceptor as `PullAuditEvents`; reads the site audit SQLite by `ExecutionId` /
`ParentExecutionId`). One additive RPC, no streaming change.
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. The RPC sits behind the same PSK interceptor as
`PullAuditEvents` and reads the site audit SQLite by `ExecutionId` / `ParentExecutionId` — one
additive RPC, no streaming change.
## 6. Message and wire changes (all additive)
@@ -257,9 +261,14 @@ surface); the walker is syntax-only and never executes anything.
## 9. Security, authorisation, audit
- **Page visibility and Live run:** `RequireDeployment` (same as Debug View) — decided
2026-09-05: a Live run writes real tags, but the Deployer already deploys and redeploys the
scripts that do exactly that, so no separate Operator gate. **Replay:** `OperationalAudit` (it
- **Page visibility:** `RequireFlowDebugger` — the `OperationalAudit` roles plus Deployer
(`AuthorizationPolicies.FlowDebuggerAccessRoles` = Deployer, Administrator, Viewer), so an
auditor can open the page and Replay a past execution (arch-review F3). **Live run:**
`RequireDeployment` (same as Debug View) — decided 2026-09-05: a Live run writes real tags, but
the Deployer already deploys and redeploys the scripts that do exactly that, so no separate
Operator gate. The two run commands, the Live controls, the parked-node Retry/Discard actions and
the `?methodId=`/`?instanceId=` drill-ins stay Deployer-only, and the page pins Replay mode for a
non-Deployer. **Replay:** `FlowDebugReplayCommand` accepts the same `OperationalAudit` roles (it
reads audit rows). No Verifier step — the debugger is a diagnostic, not a production write
control, and the audit row is the safeguard.
- **Inbound audit row for a Live run:** the shared `AuditEvent.Actor` carries the operator's
@@ -267,21 +276,38 @@ surface); the walker is syntax-only and never executes anything.
`Extra.runNumber`; `Target` is the method as usual. API-key authentication is bypassed (there is
no key), **parameter validation is not** — `ParameterValidator` runs exactly as for HTTP so a
bad input fails the same way. The method's `TimeoutSeconds` applies. Site-scope (`SiteScopeService`)
restricts which instances a value-change entry may target.
- Trace payloads pass through the existing redaction policy (auth headers redacted, 8 KB cap,
64 KB on error rows). Console output is capped the same way.
restricts which instances a value-change entry may target. The method entry enforces the
operator's site scope at route time (`RouteHelper.WithSiteScope`, `SITE_SCOPE_DENIED` →
`ManagementUnauthorized`, arch-review F2): the runner resolves the operator's permitted numeric
site ids to site identifiers and hands them to `RouteHelper`, which refuses an out-of-scope target
**before any site traffic**; the denial is rethrown as `SiteScopeViolationException` *after* the
inbound audit row is written, so the attempt is recorded under the operator's name.
Administrators and unscoped users are exempt, exactly as `EnforceSiteScope` treats them.
- 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). The row-level caps are unchanged (auth headers
redacted, 8 KB cap, 64 KB on error rows). Console output is capped the same way.
- Nothing here touches the PSK boundary; `PullTraceEvents` sits behind `ControlPlaneAuthInterceptor`.
## 10. Volume and failure behaviour
- Untraced production is unchanged except one `RouteCall` row per routed call (inbound-driven).
Every routed call adds one central `RouteCall` insert — at inbound-driven rates that doubles the
per-call central audit write (accepted, D5).
- A traced Delmia call emits ≈ 1 `InboundRequest` + 1 `RouteCall` + 1 `ScriptRun` + 2
`AttributeWrite` + 1 `AttributeWait` + 1 value-change `ScriptRun` ≈ 7 rows. `TraceAllCalls` on a
100-call/day method is ~700 rows/day.
`AttributeWrite` + 1 `AttributeWait` + 1 value-change `ScriptRun` ≈ **7–10 rows** (a cached DB
write and a notification add 3 trace-only rows). `TraceAllCalls` on a 100-call/day method is
~700–1 000 rows/day.
- Trace emission is best-effort like all audit (a failed trace write never fails the run).
- Live run timeout = method timeout + `FlowDebugRunGrace` (10 s); a timeout renders whatever steps
arrived and marks the run `Timed out`. The site's trace rows still land later, so Replay of the
same ExecutionId is complete.
same ExecutionId is complete. The client waits longer than the server on purpose: the UI derives
its Ask wait from the same bound (`RunAskTimeoutFor` = method timeout + grace + 15 s slack,
floored at 30 s; `ValueChangeAskTimeoutFor` = the wait window + 15 s) and the CLI's
`run-method --timeout-seconds` defaults to 120 while `run-value-change` waits
`--wait-seconds + 25` (arch-review F4).
- **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`.
- Older site build: no `TraceEvents`, lane labelled; older central build: ignores new fields; proto3
defaults negotiate as for stream batching.
- **Mixed-version caveat:** an OLD central ingesting `ScriptTrace` rows from a NEW site parses
+2 -2
View File
@@ -226,7 +226,7 @@ Add `using ZB.MOM.WW.Audit;` where `AuditEvent` is used (Commons already referen
**Step 1: Failing test** (follow the existing ManagementService test style for invoking `HandleUpdateApiMethod` — find a sibling test that exercises `UpdateApiMethodCommand` and copy its fixture; assert `method.TraceAllCalls` round-trips and defaults to `false` for the 5-arg constructor).
**Step 2: Implement**
- `ApiMethod`: `public bool TraceAllCalls { get; set; }` with summary "When true every production call to this method is fully traced (Flow Debugger design §5.2); intended for low-volume integration methods — ~7 audit rows per call."
- `ApiMethod`: `public bool TraceAllCalls { get; set; }` with summary "When true every production call to this method is fully traced (Flow Debugger design §5.2); intended for low-volume integration methods — 7-10 audit rows per call."
- EF: `builder.Property(m => m.TraceAllCalls).IsRequired().HasDefaultValue(false);`
- Migration: build first (CLAUDE.md gotcha), then `dotnet ef migrations add AddApiMethodTraceAllCalls --project src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase --startup-project src/ZB.MOM.WW.ScadaBridge.Host` (use the same `--startup-project` the previous migration used — check `git log -1 --stat -- src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Migrations/20260819201210_AddNotificationsListSortIndex.cs` for its commit message). Verify the generated `Up` contains exactly one `AddColumn<bool>("TraceAllCalls", "ApiMethods", nullable: false, defaultValue: false)`; delete and redo if empty.
- Commands: `CreateApiMethodCommand(..., string? ReturnDefinition, bool TraceAllCalls = false)`, `UpdateApiMethodCommand(..., string? ReturnDefinition, bool? TraceAllCalls = null)` (null = leave unchanged, so old CLIs don't reset it). Handlers copy the value.
@@ -754,7 +754,7 @@ public sealed record FlowDebugOutcome(FlowGraph? Graph, FlowDebugRunResult? Raw,
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Audit/AuditEventDetail.razor` (+ `.razor.cs`): button `Open in Flow Debugger` (`data-test="open-flow-debugger"`) beside *View execution chain* → `/deployment/flow-debugger?executionId={exec}`; rendered when `ExecutionId` is non-null.
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/ExecutionTreePage.razor`: same button next to *View this execution in the Audit Log*.
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor`: under the Timeout field a `form-check` "Trace all calls (Flow Debugger)" bound to `_traceAllCalls`, help text "Adds ~7 audit rows per call; for low-volume integration methods", saved onto `_existing.TraceAllCalls` / the create path.
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor`: under the Timeout field a `form-check` "Trace all calls (Flow Debugger)" bound to `_traceAllCalls`, help text "Adds 7–10 audit rows per call; for low-volume integration methods", saved onto `_existing.TraceAllCalls` / the create path.
- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Audit/FlowDebuggerDrillinTests.cs`, extend the existing `ApiMethodForm` test file if one exists (grep `ApiMethodForm` in tests).
Commit — `git commit -am "feat(central-ui): Flow Debugger drill-ins from Audit Log + Execution Chain; TraceAllCalls checkbox on the method form"`
@@ -0,0 +1,623 @@
# 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`)
```csharp
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:
```csharp
/// <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
```csharp
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:
```csharp
[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:
```csharp
[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`:
```csharp
/// <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:
```csharp
/// <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:
```csharp
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):
```csharp
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`:
```csharp
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.");
```
```csharp
/// <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:
```csharp
/// <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:
```csharp
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.
```csharp
/// <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:
```csharp
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**
```csharp
/// <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:
```csharp
/// <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
```csharp
_ = 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):
```csharp
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:**
```csharp
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.
@@ -0,0 +1,112 @@
{
"planPath": "docs/plans/2026-09-06-flow-debugger-arch-review-remediation.md",
"tasks": [
{
"id": 1,
"subject": "Task 1: F1 site \u2014 FlowTraceCollector redacts + byte-caps, moves to Commons",
"status": "completed"
},
{
"id": 2,
"subject": "Task 2: F1 central \u2014 FlowDebugRunner uses the redacting collector",
"status": "completed",
"blockedBy": [
1,
3
]
},
{
"id": 3,
"subject": "Task 3: F2 \u2014 site scope on method-entry Live run",
"status": "completed"
},
{
"id": 4,
"subject": "Task 4: F3 \u2014 Replay opens to OperationalAudit",
"status": "completed"
},
{
"id": 5,
"subject": "Task 5: F4 \u2014 client waits derive from server bound",
"status": "completed"
},
{
"id": 6,
"subject": "Task 6: F5 \u2014 emission order over cross-node timestamps",
"status": "completed",
"blockedBy": [
2
]
},
{
"id": 7,
"subject": "Task 7: F6 \u2014 trace arm cleared on failed write",
"status": "completed"
},
{
"id": 8,
"subject": "Task 8: F7 \u2014 shed/suppressed traced run row",
"status": "completed",
"blockedBy": [
1
]
},
{
"id": 9,
"subject": "Task 9: F8 \u2014 fault continuations scheduler + logging",
"status": "completed",
"blockedBy": [
1,
3,
8,
12
]
},
{
"id": 10,
"subject": "Task 10: F9 \u2014 shared chain walker",
"status": "completed"
},
{
"id": 11,
"subject": "Task 11: F10 \u2014 overlay matches by kind+title",
"status": "completed"
},
{
"id": 12,
"subject": "Task 12: F11 \u2014 RouteCall honours SkipBodyCapture",
"status": "completed",
"blockedBy": [
3
]
},
{
"id": 13,
"subject": "Task 13: F12+F13 \u2014 docs reconciled",
"status": "completed",
"blockedBy": [
1,
2,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12
]
},
{
"id": 14,
"subject": "Task 14: full gate \u2014 build, tests, rig, verify",
"status": "completed",
"blockedBy": [
13
]
}
],
"lastUpdated": "2026-09-06T18:46:58.108183+00:00"
}
@@ -0,0 +1,370 @@
# Flow Debugger — architectural / code review
**Date:** 2026-09-06 · **Status:** REVIEW REPORT (no code changed by this review)
**Scope:** every commit from `7fa916e2` (Flow Debugger design) to `0ccbe140` (cached-call forwarder
enqueue-and-observe) on `main` — Flow Debugger v1.0, v1.1 outbound calls, v1.2 follow-ups, the
outcome-merge fix and the two same-day performance commits. 165 files, +20 228 / −778 lines.
**Method:** read of every non-test source file in the range against the three design/plan documents
(`2026-09-05-flow-debugger-design.md`, `2026-09-06-flow-debugger-outbound-calls.md`,
`2026-09-06-flow-debugger-followups.md`) plus `2026-09-06-tracking-store-async-writes-design.md`;
targeted verification of the claims the code comments make (where redaction runs, how the
`ManagementActor` dispatches, proto field numbering, timestamp storage format, page gates); full
solution test run (§6).
## 1. Verdict
The feature is architecturally sound and lands where the design said it would: trace steps are
audit events, every contract change is additive, the site never depends on central for tracing,
the run path is one management command shared by UI and CLI, and the per-node actor is not blocked
by a long run. Test coverage is broad (≈ 6 000 lines of new tests across every layer).
Four findings warrant a fix before the feature is relied on outside the rig, all Medium, none
requiring a redesign:
| # | Severity | Finding | Owner component |
|---|---|---|---|
| F1 | **Medium** | The inline trace copy (routed reply + Live result) is the **pre-redaction, un-capped** event — per-target SQL parameter redaction and the 8 KB / 64 KB payload caps only apply to the stored row; the wire copy is bounded by count, not bytes | Site Runtime, Inbound API, Management |
| F2 | **Medium** | The **method-entry Live run has no site-scope check**; only the value-change entry enforces `PermittedSiteIds` | Management, Inbound API |
| F3 | **Medium** | **Replay is Deployer-gated**; the design (D6, §9) says `OperationalAudit`, and the Audit Log / Execution Chain drill-ins invite auditors onto a page they cannot open | Central UI, Management |
| F4 | **Medium** | **Three unrelated run timeouts**: server = method timeout + 10 s; UI Ask = fixed 2 min; CLI = fixed 2 min. A method with `TimeoutSeconds > ~110` makes both clients give up while the run continues | Central UI, CLI |
Then nine Low findings (F5–F13) — ordering under clock skew, a trace-arm false positive, silent
shedding of traced runs, continuation-scheduler consistency, a duplicated chain walker, a
kind-only prediction overlay, always-on `RouteCall` vs `SkipBodyCapture`, dead stream fields, and
doc/implementation drift — and a list of what was checked and found correct (§4).
## 2. Findings
Severity: **High** = data loss / security boundary / outage; **Medium** = wrong behaviour an operator
will hit, or a documented property that does not hold; **Low** = quality, consistency, or a
narrow edge. Each finding names the file, states what the code does, why it matters, and the fix.
### F1 — Inline trace copy bypasses redaction and payload caps `[Medium]`
**Where.** `SiteRuntime/Scripts/FlowTraceCollector.cs`; `ScriptRuntimeContext.Collect` /
`EmitTrace`; `ScriptRunLauncher.EmitScriptRun`; `InboundAPI/RouteHelper.cs` `EmitRouteCall`;
`InboundAPI/InboundDatabaseHelper.cs` `EmitAudit`; `ManagementService/FlowDebug/FlowDebugRunner.cs`.
**What.** Every emit site deliberately hands the collector / trace sink the *same* `AuditEvent`
instance it then hands the writer, "collector first". But redaction and capping live inside the
writers — `FallbackAuditWriter.WriteAsync` (`_redactor.Apply(evt)`, line 88) on the site and
`CentralAuditWriter.WriteAsync` (line 113) on central — and both return a *new* filtered record.
The collector therefore holds the raw event: no `PerTargetOverrides` SQL-parameter redaction, no
header redaction, no `DefaultCapBytes` (8 KB) / `ErrorCapBytes` (64 KB) truncation. That raw list
is what `ScriptCallResult.TraceEvents` → `RouteToCallResponse.TraceEvents` carries over gRPC, what
`FlowDebugRunResult.Events` returns to the page and the CLI (`--format json` prints it verbatim),
and what the page renders in a Live run. Replay reads the stored, redacted rows — so Live and
Replay of the same execution can show different payloads.
**Why it matters.**
- Design §9 states "Trace payloads pass through the existing redaction policy (auth headers
redacted, 8 KB cap, 64 KB on error rows)". It does not hold for the Live path.
- A per-target redaction override exists precisely to keep specific SQL parameter values out of
audit; a Deployer running the Flow Debugger sees them anyway.
- Size: `FlowTraceCollector` caps at **500 events, not bytes**. A traced run whose DB/API rows
carry large request/response summaries (a 64 KB error detail × a few hundred rows) exceeds the
gRPC default 4 MB receive limit on central's `GrpcSiteTransport` channel (no
`MaxReceiveMessageSize` is set anywhere in `src/`). The routed call then fails with
`ResourceExhausted` — and because the same reply carries the script's *return value*, a method
flagged `TraceAllCalls` would fail a **production** call on payload size alone.
**Fix.** Apply the redactor at the collector boundary, not the writer: inject `IAuditRedactor`
into `ScriptRuntimeContext`'s `Collect` (site) and into `FlowDebugRunner`'s sink (central) so the
collector stores `redactor.Apply(evt)`; the writers keep their own call (idempotent). Add a byte
budget to `FlowTraceCollector` (e.g. 1 MB, counted on the redacted `DetailsJson` length) alongside
the 500-event cap, setting `Truncated` when either trips. Tests: a per-target SQL redaction
override is honoured on `RouteToCallResponse.TraceEvents`; a collector over budget truncates.
Effort: small (≈ 60 lines + tests).
### F2 — Method-entry Live run has no site-scope check `[Medium]`
**Where.** `FlowDebugRunner.RunMethodAsync` vs `RunValueChangeAsync` (`EnforceSiteScope`).
**What.** The value-change entry refuses with `SiteScopeViolationException` when the instance's
site is outside `user.PermittedSiteIds`. The method entry has no equivalent: a Deployer whose role
is scoped to site A can pick any method whose script routes to site B and run it Live — real
`Route.To().Call`, `SetAttributes`, `Database.*`, `Notify.Send` against site B, with the operator's
username on the inbound row. Design §9 only mentions the value-change entry, so this is an
omission in the design carried into the code, not a deviation.
**Why it matters.** Site-scoped Deployer is the project's operator-of-one-plant model. The HTTP
inbound API does not weaken it (API keys are a separate credential); the Flow Debugger does.
**Fix.** Cheapest and complete: give `RouteHelper` a site-scope predicate (`WithSiteScope(
IReadOnlySet<int> permittedSiteIds)`), checked in `RouteTarget` right after `_instanceLocator`
resolves the target's site id, throwing `SiteScopeViolationException` (already mapped to
`ManagementUnauthorized`). `FlowDebugRunner.RunMethodAsync` sets it from `user`; the HTTP endpoint
passes nothing (unchanged). The static prediction already resolves constant route targets, so the
page can also warn before the run. Effort: small.
### F3 — Replay requires Deployer; the design says OperationalAudit `[Medium]`
**Where.** `ManagementActor.GetRequiredRoles` (`FlowDebugReplayCommand => DeployerOnly`);
`FlowDebugger.razor` (`[Authorize(Policy = RequireDeployment)]`); drill-in buttons in
`AuditEventDetail.razor` and `ExecutionTreePage.razor`.
**What.** Design D6 / §9: "Live run is a Deployer act … Replay needs `OperationalAudit`". The
implementation gates the page and all three commands on Deployer. The Audit Log and Execution
Chain pages — whose own gate is `OperationalAudit` — now show an *Open in Flow Debugger* button
that an auditor without Deployer cannot use.
**Fix.** Either amend D6 (and hide the drill-in for non-Deployers), or implement the split: page
policy "OperationalAudit or Deployment", `FlowDebugReplayCommand` accepts either role, Run button
and the two run commands stay Deployer-only. The split is ≈ 20 lines plus a
`RequiredRoleMatrixTests` row; it is the better answer because the audit drill-in is the feature's
main entry for a *past* incident.
### F4 — Three unrelated Live-run timeouts `[Medium]`
**Where.** `FlowDebugRunner.RunMethodAsync` (`timeout + RunGrace`, `CancellationToken.None` from
the actor); `CentralUI/Services/FlowDebugService.cs` (`RunAskTimeout = 2 min`);
`CLI/Commands/FlowDebugCommands.cs` (`RunTimeout = 2 min`); `ManagementEndpoints` long-running
list (server side is correctly exempted from the ordinary Ask timeout).
**What.** The server bounds a run by the method's `TimeoutSeconds` (unbounded positive int) plus
10 s. Both clients wait a fixed 2 minutes. `FlowDebugService`'s comment asserts "both fit inside
two minutes" — nothing enforces it. A method with `TimeoutSeconds = 180` (legal; long integration
handshakes exist) makes the page show `TRANSPORT_ERROR` and the CLI a 504 at 120 s while the run
continues to completion on central, writes its audit rows, and cannot be cancelled (the handler
runs on `CancellationToken.None`). The value-change entry has the same shape (`WaitSeconds` is
user-supplied and unbounded; UI sends the default 45).
**Fix.** Derive the client wait from the request: UI = `method.TimeoutSeconds + RunGrace + 15 s`
(the page already has the method list) and `WaitSeconds + 15 s`; CLI = same after a `GetApiMethod`
read, or a `--timeout` option. Optionally clamp `TimeoutSeconds` for Live runs and say so in the
error. Effort: small.
### F5 — Step ordering trusts cross-node timestamps `[Low]`
**Where.** `FlowDebugRunner.RunMethodAsync` (`events.OrderBy(e => e.OccurredAtUtc)`),
`FlowGraphBuilder.Build` (global sort by `OccurredAtUtc`, spawner search
`p.OccurredAtUtc <= first.OccurredAtUtc`).
**What.** Central rows carry central's clock, site rows the site's. The collector preserved
emission order, and the runner discards it by re-sorting. With a site clock 1–2 s ahead, site steps
sort after the Response node in `Steps` (the CLI table and the detail list) and a trigger cascade's
spawner search falls back to `parentChain[0]`. Edges are id-based so the graph stays connected;
this is display order only. Route calls are unaffected (linked by `ChildExecutionId`).
**Fix.** Keep a sequence number per event on the inline path (the collector's index) and sort by
`(OccurredAtUtc within an execution, sequence)`; or simply preserve the collector's order and
merge Replay rows behind it. Low.
### F6 — Trace arm can stamp an unrelated device update `[Low]`
**Where.** `InstanceActor.HandleSetDataAttribute` (arms before the DCL write completes),
`HandleTagValueUpdate` (consumes the arm "whatever the value").
**What.** The arm is recorded when the traced write is *issued*, not when the DCL acknowledges it.
If the write fails (bad quality, device rejects) the arm stays for 30 s; the next unrelated
subscription update on that attribute is stamped with the debugger's execution id and spawns a
traced, parented run — the debugger then shows a "device echo" of a write that never landed.
Design §5.3 accepts "whatever the value", which covers clamping, not failure.
**Fix.** Clear the arm in the `WriteTagResponse` failure continuation (the actor already pipes the
response back; a `Tell` to self with the attribute name is enough). Low.
### F7 — A shed or rate-limited traced run is invisible `[Low]`
**Where.** `ScriptActor.TrySpawnExecution` (`MinTimeBetweenRuns` suppression) and
`SpawnExecution` (`MaxConcurrentRunsPerScript` → `ShedRun`).
**What.** A traced value-change run dropped by either guard emits no `ScriptTrace` row; the
debugger reports `NO_TRIGGER_WITHIN_WINDOW` with the message "no value-change trigger on this
attribute, or no device echo" — both wrong in that case, and the operator is sent to check the
wrong things.
**Fix.** When `trace` is set, emit a `ScriptRun` row with `Failed` and an error of
`shed: MaxConcurrentRunsPerScript` / `suppressed: MinTimeBetweenRuns` (the launcher's
`EmitScriptRun` can be reused with `elapsedMs = 0`). Low.
### F8 — Fault continuations: scheduler and logging inconsistent `[Low]`
**Where.** `ScriptRuntimeContext.EmitTrace`, `ScriptRunLauncher.EmitScriptRun`,
`RouteTarget.EmitRouteCall`, `InboundDatabaseHelper.EmitAudit`.
**What.** All four use `ContinueWith(..., TaskContinuationOptions.OnlyOnFaulted)` with no
scheduler argument, so the continuation inherits `TaskScheduler.Current` — on a script thread that
is the blocking-I/O `ScriptExecutionScheduler`. `CachedCallTelemetryForwarder` (same commit range)
passes `TaskScheduler.Default` explicitly. The two central emitters also discard the fault
(`static _ => { }`) with no log line, so a central audit outage is silent on those rows.
**Fix.** Pass `CancellationToken.None, OnlyOnFaulted | ExecuteSynchronously, TaskScheduler.Default`
everywhere, and log at Warning in the two central emitters (a logger is one constructor parameter
away in both). Low.
### F9 — `GetTraceEventsAsync` duplicates the execution-tree walk `[Low]`
**Where.** `ConfigurationDatabase/Repositories/AuditLogRepository.cs`.
**What.** The new method re-implements `GetExecutionTreeAsync`'s three-statement walk (up to
root, root window, recursive `Edges`/`Chain` CTE with `MAXRECURSION` and the 7-day window) as a
second copy, then filters channels with EF. Two walkers that must stay in step (window slack,
depth, the DISTINCT-edges trick).
**Fix.** Extract `ResolveChainExecutionIdsAsync(executionId)` and have both call it. Low.
### F10 — Prediction overlay matches real steps by kind only `[Low]`
**Where.** `FlowGraphView.ComputeLayout` (`unmatched.FirstOrDefault(p => p.Kind == s.Kind)`).
**What.** A method with two route calls (`A.X` then `B.Y`) that only executes the second one
matches the real `B.Y` step to the predicted `A.X` node; the ghost shown is `B.Y`. Wrong greying
for multi-route or branching methods.
**Fix.** Match by `(Kind, Title)` first, then fall back to kind. Low, UI only.
### F11 — Always-on `RouteCall` row ignores the method's `SkipBodyCapture` `[Low]`
**Where.** `RouteTarget.EmitRouteCall` (`requestSummary: TraceJson(args)`), vs
`AuditWriteMiddleware` (honours `PerTargetOverrides[methodName].SkipBodyCapture` on the inbound
row).
**What.** An operator who configured `SkipBodyCapture` for a method to keep its body out of the
audit log now gets the routed *arguments* — derived from that body — on every call's `RouteCall`
row, keyed by `{instance}.{script}`, which the per-method override does not match. Also: every
routed call is now one extra central MS SQL insert (fire-and-forget); at inbound-driven rates that
doubles the per-call central audit write, which the design accepted (D5) but should be stated in
the volume section.
**Fix.** Thread the method's `SkipBodyCapture` into `RouteHelper` (the executor has the method)
and emit the row without summaries when set; add the sentence to design §10. Low.
### F12 — Shipped but unused: `AttributeValueUpdate.source_execution_id` / `trace` `[Low]`
**Where.** `sitestream.proto` fields 7/8, `StreamRelayActor`, `SiteStreamGrpcClient`.
**What.** Design §5.4 planned the value-change entry to subscribe to the debug stream and take
the spawned id from the stamped `AttributeValueChanged`. The implementation polls
`PullTraceEvents` by `ParentExecutionId` instead (simpler, and correct). The stream fields are
therefore populated on every change and consumed by nothing. Not harmful — additive, one string
per update — but the design should record the switch and the fields should be marked "reserved
for the debug-stream path" or dropped from the client mapping.
### F13 — Documentation drift to reconcile `[Low]`
- Design §5.4 (debug-stream subscription) vs the polling implementation — see F12.
- Design D6 / §9 (Replay = OperationalAudit) vs Deployer everywhere — see F3.
- Design §9 (redaction applies to trace payloads) vs F1.
- `ApiMethodForm` / CLI help understate the row count (they were written before the outbound-calls
addendum, which makes a call with one cached DB write and one notification **10 rows**, 3 of them
trace-only). Say "7–10 audit rows per call".
- `FlowDebugRunner.RunMethodAsync` writes the inbound row *after* the run, so a Replay taken during
a long Live run shows only the `RouteCall`. Fine, but not stated anywhere.
- `SchemaRefResolver` became `public` for the runner; note it in Component-InboundAPI or move the
validation into `InboundScriptExecutor`.
## 3. Performance commits (tracking store + forwarder)
`OperationTrackingStore` single-writer queue (`d91135f9`) — reviewed line by line, **no findings**:
`_pending` is balanced on every path (enqueue failure, batch complete, per-row fallback, pump
teardown); the read barrier is skipped only when nothing is queued and otherwise rides the same
FIFO, so read-your-writes holds; the per-row fallback keeps good rows; `DisposeAsync` drains and
sync `Dispose()` fails queued items as documented; timestamps are stamped at record time. The one
trade — a queued-but-unapplied row lost on a hard crash — is stated in the file and the design memo.
`CachedCallTelemetryForwarder` enqueue-and-observe (`0ccbe140`) — correct, and the design memo's
"correction" section is the right record: the first attribution (SQLite commits) was wrong, the
measurement said so, and the doc says so. The measured result (inbound 194 → 25 ms, script run
176 → 17 ms) stands.
## 4. Verified correct (not findings)
- **Additive contracts throughout.** Every record gains trailing optional parameters; proto fields
are new numbers (`RouteToCallRequestDto.trace = 7`, `RouteToCallResponseDto` 6/7,
`RouteToSetAttributesRequestDto.trace = 6`, `AttributeValueUpdate` 7/8, new `PullTraceEvents`
RPC); `AuditChannel.ScriptTrace` and the four kinds are appended last; `UpdateApiMethodCommand.
TraceAllCalls` is `bool?` so an older CLI never resets it; proto3 defaults are the negotiation,
as for stream batching. The `SiteCommandDtoMapper` maps an empty `trace_events` to `null`.
- **The `ManagementActor` is not blocked by a Live run** — `HandleEnvelope` runs `ProcessCommand`
and `PipeTo`s the result, so a 5-minute run does not stall other management commands. The
standby refusal (`IActiveNodeGate`, `STANDBY_NODE`) and the new `ManagementCommandException(
message, code)` path are correct and backwards compatible.
- **`FlowTraceCollector.Complete()`** closes the sink at the reply snapshot; later lifecycle rows
go to the writer only; `GuardTraceSink` means a throwing observer can neither fail a call nor skip
the durable write (pinned by tests).
- **`PullTraceEvents`** sits behind the existing `/sitestream.SiteStreamService/` PSK interceptor
prefix; caps ids at 64 and rows at 2000 on both ends; moves no forward-state cursor; the new
`IX_audit_event_occurred` index bounds the `json_extract` scan and the timestamp text format
matches the stored `"o"` form.
- **Migration** `AddApiMethodTraceAllCalls` + manual SQL twin with the `QUOTED_IDENTIFIER` header.
- **Central `Database.*` audit** (`InboundDatabaseHelper`): one `DbOutbound/DbWrite` row per call,
vocabulary-locked with the site's `AuditingDbCommand`; failure inside the audited span; result or
exception always reaches the script.
- **Graph builder**: fold by `(kind, CorrelationId)`, `LifecycleRank` tie-break, `IsAsync` rule,
leading-async linking from the spawner, stable synthesized Response id — all pinned by
`FlowGraphBuilderTests`.
- **Page**: outcome refresh merges by `EventId`; the pending-refresh loop is bounded (5 s / 2 min),
cancelled on mode switch, new run and dispose; Retry/Discard re-check site scope and reuse the
report pages' relay and wording; the report pages are `RequireDeployment` too, so the debugger's
actions add no privilege. The prediction circuit deadlock is fixed with a resolve-and-rewalk loop
and pinned by a single-thread-pump test.
- **Site-side gating**: untraced runs allocate no collector and emit nothing; only traced
`SetStaticAttributeCommand` writes arm; `TraceArmWindowSeconds` is validated `> 0`.
- **Rig proof**: `verify-delmia-notifier.sh` steps H and I (live + replay, outbound nodes) passed
on the rig per the commit ledger.
## 5. Recommended order
1. F1 (redact + byte-cap the inline copy) and F2 (route-time site scope) — same session, both
small, both close a stated security property.
2. F3 (Replay role split) and F4 (derive client waits) — small, operator-visible.
3. F6, F7, F8 — site-runtime polish, one commit.
4. F5, F9, F10, F11 — quality; can ride the next Flow Debugger change.
5. F12, F13 — docs; do with (1) so the design stops disagreeing with the code.
Register rows: F5, F10, F11, F12 are candidates for the deferred-work register if not fixed in the
next pass; F1–F4 should not be deferred.
## 6. Test evidence
Full `dotnet test ZB.MOM.WW.ScadaBridge.slnx` at review time (2026-09-06, `main` @ `0ccbe140`):
**29 of 30 projects green, 9 297 tests passed.** The one red project is
`CentralUI.PlaywrightTests` (164 / 175): its 11 failures are the role-negative browser checks
(`RoleNavigationTests.*_DoesNotSee*`, `AuditLogPageTests.PermissionGating_…_IsDeniedAccess`,
`LoginTests.UnauthenticatedUser_RedirectsToLogin`) — every one asserts that a *lesser* role is
denied something, which cannot hold on the rig since 2026-09-06 (`2b80e1d8`) because both central
nodes run `DisableLogin=true` and every browser session is the all-roles `multi-role` user. None
touches the Flow Debugger; `Deployment/FlowDebuggerTests` (Live run + localStorage restore) passed.
Consequence worth recording alongside the login-disabled decision: the rig can no longer prove
role gating in the browser, so the Playwright suite's role-negative tests need either a second,
login-enabled central to target or a fixture that overrides the identity per test.
## 7. Resolution
All thirteen findings were remediated on branch `fix/flow-debugger-arch-review` (plan
`docs/plans/2026-09-06-flow-debugger-arch-review-remediation.md`, 2026-09-06). Nothing was
deferred to the register.
| # | Outcome | Commit |
|---|---------|--------|
| F1 | `FlowTraceCollector` moved to Commons `Types/Debugging/`, applies `IAuditRedactor` inside `Add` and caps at 500 events **and** 1 MB of summed `DetailsJson`; the site launcher resolves the redactor from the run's scope, and the central `FlowDebugRunner` collects through the same instance and reports its `Truncated`. | `e79452d2` fix(site-runtime): Flow Debugger inline trace copy is redacted and byte-capped at the collector (arch-review F1, site half) · `def6bab3` fix(management): Flow Debugger Live result is the redacted, capped trace copy (arch-review F1, central half) |
| F2 | `RouteHelper.WithSiteScope(IReadOnlySet<string>?)` refuses an out-of-scope target site before any site traffic (`RouteSiteScopeException` → executor code `SITE_SCOPE_DENIED`); the runner rethrows it as `SiteScopeViolationException` (`ManagementUnauthorized`) **after** writing the inbound audit row. Administrators and unscoped users are exempt; the HTTP endpoint is unchanged. | `20a05941` fix(inbound-api): Flow Debugger method-entry Live run enforces the operator's site scope at route time (arch-review F2) |
| F3 | New `AuthorizationPolicies.RequireFlowDebugger` (`FlowDebuggerAccessRoles` = Deployer, Administrator, Viewer) gates the page, and `FlowDebugReplayCommand` accepts the same set; the two run commands, the Live controls, Retry/Discard and the `?methodId=`/`?instanceId=` drill-ins stay Deployer-only, with the page pinning Replay mode for a non-Deployer. | `f86eb61d` fix(central-ui): Flow Debugger Replay opens to OperationalAudit roles; Live run and Retry/Discard stay Deployer-only (arch-review F3) |
| F4 | Client waits derive from the server's bound: `RunAskTimeoutFor(timeoutSeconds)` = method timeout + `RunGrace` + 15 s slack (floor 30 s), `ValueChangeAskTimeoutFor(wait)` = wait + 15 s; the CLI gains `run-method --timeout-seconds` (default 120) and waits `--wait-seconds + 25` on `run-value-change`. | `97b0a4ba` fix(central-ui,cli): Flow Debugger client waits derive from the method timeout / wait window instead of a fixed 2 minutes (arch-review F4) |
| F5 | The graph orders executions by emission sequence and uses timestamps only **within** one execution, so a site clock offset can no longer reorder the swimlane; the Live runner keeps the collector's emission order. | `1dfb9cee` fix(commons): Flow graph orders executions by emission sequence, timestamps only within one clock (arch-review F5) |
| F6 | A failed traced DCL write disarms the trace arm (`DisarmTrace` self-message), so an unrelated device update can no longer consume it. | `50bf9ac7` fix(site-runtime): trace arm is cleared when the traced DCL write fails (arch-review F6) |
| F7 | A traced run shed by `MaxConcurrentRunsPerScript` or suppressed by `MinTimeBetweenRuns` emits a Failed `ScriptRun` row (`shed: MaxConcurrentRunsPerScript …` / `suppressed: MinTimeBetweenRuns …`) instead of vanishing from the graph. | `105541b6` fix(site-runtime): shed or rate-limited traced runs emit a Failed ScriptRun row (arch-review F7) |
| F8 | All four trace/outbound fault continuations run on `TaskScheduler.Default`, and the central emitters log a Warning on a failed audit write. | `262de38c` fix(audit): trace/outbound fault continuations run on the default scheduler and central emitters log (arch-review F8) |
| F9 | `GetTraceEventsAsync` and `GetExecutionTreeAsync` share one chain walker in `AuditLogRepository`. | `297ae56b` refactor(config-db): GetTraceEventsAsync and GetExecutionTreeAsync share one chain walker (arch-review F9) |
| F10 | The prediction overlay matches a real step by `(Kind, Title)` first, falling back to kind only, so two predicted nodes of the same kind no longer collapse onto one another. | `688136de` fix(central-ui): Flow Debugger prediction overlay matches by kind and title (arch-review F10) |
| F11 | The always-on `RouteCall` row honours the method's `SkipBodyCapture` — the executor builds the helper with `RouteHelper.WithBodyCapture(false)`; the row is still written, metadata only. | `453c3dec` fix(inbound-api): RouteCall audit row honours the method's SkipBodyCapture override (arch-review F11) |
| F12 | `AttributeValueUpdate.source_execution_id` / `trace` (fields 7/8) are **kept, populated and unconsumed** — documented as reserved for a future debug-stream path in `sitestream.proto` and design §5.4, with the field numbers pinned against reuse. Not removed: dropping them would be a wire change for no gain. | this commit |
| F13 | Design D6 / §5.4 / §9 / §10, `Component-CentralUI.md`, `Component-InboundAPI.md`, `CLAUDE.md`, the proto comment and the CLI/UI help strings reconciled with the shipped code; the trace-row estimate is "7–10 audit rows per call" everywhere; `SchemaRefResolver`'s `public` visibility and the inbound-row-after-the-run ordering are now stated. | this commit — docs: Flow Debugger design reconciled with the implementation (arch-review F12, F13) |
One test defect surfaced by §3's performance commits was fixed on the same branch:
`OperationTrackingStoreTests.ReadChangedSince_BoundaryTimestamp…` raced the new single-writer
queue (a missing `FlushAsync` before the test's direct `UPDATE`) — `86451382` test(site-runtime):
flush the tracking-store queue before the boundary-timestamp test's direct UPDATE.
### 7.1 Gate evidence (2026-09-06, branch `fix/flow-debugger-arch-review`)
- Full `dotnet test ZB.MOM.WW.ScadaBridge.slnx` on the merged branch: 30 projects; the only failures are the
11 known role-negative Playwright tests (login-disabled rig, §6) and one Central UI drill-in test that pinned
the old "~7 audit rows" help text, fixed in `6c9ea39f`. A code review of the merged diff (opus, read-only)
found one Medium residual — the runner's own synthesized inbound / root rows were returned un-redacted —
fixed with the spawner rule, byte-budget doc claim and CLI ceiling note in `f17bae09`; the two larger
follow-ups it raised are register rows 112 (site-side `Truncated` flag) and 113 (F2 gates routed traffic only).
- Rig: `bash docker/deploy.sh` rebuilt all 8 nodes; `FLOW_ONLY=1 docker/verify-delmia-notifier.sh` →
`H live+replay YES` (live 11 events, replay 17) and `I outbound live+replay YES` (api / db / notify
Delivered on replay).
+1 -1
View File
@@ -6,7 +6,7 @@ SET ANSI_NULLS ON;
GO
-- Flow Debugger (design 2026-09-05 §5.2): ApiMethod.TraceAllCalls — when 1, every
-- production call to the method is fully traced (~7 audit rows per call). Production
-- production call to the method is fully traced (7-10 audit rows per call). Production
-- twin of EF migration 20260906034113_AddApiMethodTraceAllCalls.
BEGIN TRANSACTION;
IF NOT EXISTS (
+4 -3
View File
@@ -215,13 +215,14 @@ Per-leaf alarm rendering (leaf nodes are individual conditions for native alarms
- **Row tooltip** — surfaces native metadata not warranting its own column: `AlarmTypeName`, category, operator user and comment, original raise time, current/limit value.
- **Computed alarms render unchanged** from the prior flat-table style; the enrichment is purely additive for native rows.
### Flow Debugger (Deployment Role)
### Flow Debugger (Flow Debugger policy; Live run = Deployment Role)
- **`/deployment/flow-debugger`** (nav: Deployment → Flow Debugger; design `docs/plans/2026-09-05-flow-debugger-design.md`, plan `docs/plans/2026-09-05-flow-debugger.md`). Runs one entry point with operator-supplied inputs and renders every step the run passes through as a **four-lane swimlane** (Central / Site / Tags / **External**) — request, route call, site script run, attribute writes, attribute waits, **outbound API / database / notification calls**, response — each node carrying its input, output, duration and outcome; a tabbed detail panel (Input · Output · Writes *or* Lifecycle · Timing, plus a *Where this ran* column) opens on the selected node with `Open in Debug View` (value-change entries, where the instance is known) and `Audit rows (n)` (→ `/audit/log?executionId=`) drill-ins.
- **Two modes.** *Live* is a real run — the method script executes on the active central node with tracing on, or a real attribute write goes through the routed-write path and the site's triggered run is polled back — audited like any other call. *Replay* executes nothing: it rebuilds the flow from an existing execution's `ScriptTrace` audit rows plus its `ApiOutbound` / `DbOutbound` / `Notification` rows (`?executionId=` drill-in from the Audit Log drawer's and the Execution Chain page's **Open in Flow Debugger** actions).
- **External lane (v1.1, plan `docs/plans/2026-09-06-flow-debugger-outbound-calls.md`).** Every `ExternalSystem.Call/CachedCall`, `Database.*` and `Notify.Send` a traced run makes is a node on the fourth lane — kinds `ApiCall` / `DbCall` / `Notify` (eyebrows `API CALL` / `DATABASE` / `NOTIFY`), title = the system.method / connection / list, a status pill (`Ok` / `Pending` / `Parked` / `Failed`) plus `sync` | `cached` | `notify`. **One node per operation:** a cached call's or notification's later lifecycle rows (Forwarded → Attempted → Delivered / Parked, written minutes later by the S&amp;F forwarder or the Notification Outbox) fold into the node (`FlowStep.Lifecycle`, a Lifecycle tab in the detail panel with *Delivered by*) instead of drawing four boxes. A sync call sits in the execution chain; a cached call or notification hangs off it on a dashed **async** connector, and a trigger cascade's spawn edge carries a bolt marker. A Live run returns only the rows the run itself emitted, so a graph left with `Pending` nodes **refreshes itself** through the Replay command every 5 s for up to 2 min (page constants), and a *Refresh outcomes* button re-reads on demand in either mode; the state chip stays *Completed* — pending outbound work is not a failed run. A refresh **merges** the re-read rows into the run's known rows by `EventId` (fresh copy wins) and rebuilds the graph from the union, so it only ever adds nodes: the site's outbound rows reach central's audit store by telemetry seconds after the run returned them inline, and a Replay read taken in between would otherwise erase an already-drawn node (observed on the rig 2026-09-06 with a resolved cached DB write). The union is exact because the inline rows are the very `AuditEvent` objects the site writer persists. **v1.2 (follow-ups plan `docs/plans/2026-09-06-flow-debugger-followups.md`, register row 111 closed):** a folded node carries its `CorrelationId` (`FlowStep.CorrelationId` — the `TrackedOperationId` / `NotificationId`), so the detail panel deep-links to **`/site-calls/report?correlationId=<id>`** / **`/notifications/report?correlationId=<id>`** (*Open in Site Calls* / *Open in Notification report*; both pages accept that query, pre-filter to the one row through an additive per-id query filter — `SiteCallQueryRequest.TrackedOperationIdFilter` / `NotificationOutboxQueryRequest.NotificationIdFilter` — show a removable *Operation* / *Notification* pill and auto-open the row's detail modal; note `NotificationId` is minted as a Guid in **`N` form** — no dashes — so the Notify link passes `N` and the report page normalises any Guid-shaped value to `N`, while `TrackedOperationId` is a `varchar(36)` `D` string and the Site Calls filter binds a `varchar` parameter for the PK seek), and a **`Parked` node offers Retry / Discard in place**: the same site-scope re-check, confirm dialog (Discard = danger) and central→site / Notification Outbox relay the report pages use (`IFlowDebugService.RetryOutboundAsync` / `DiscardOutboundAsync` → `CommunicationService`), then the page re-polls Replay (5 s / 2 min) until the node leaves `Parked`. **Prediction is async-safe:** `FlowDebugService.PredictMethodAsync` resolves route targets by a resolve-and-rewalk loop — its original blocking resolver deadlocked the whole Blazor circuit on the renderer sync context the moment a method routing to a site script was chosen (found 2026-09-06; pinned by a single-thread-pump unit test and the Playwright `FlowDebuggerTests`, which log in through the fixture and run a Live run of an ephemeral method in a real browser).
- **Two entry points.** An inbound API method (inputs form generated from the method's `ParameterDefinitions`, values coerced by declared type) or a value-change trigger on a named instance attribute (Site → Instance → Attribute pickers, site-scope filtered).
- **Authorization (arch-review F3).** The page is `RequireFlowDebugger` — `AuthorizationPolicies.FlowDebuggerAccessRoles` = Deployer, Administrator, Viewer — so an auditor can open it and Replay a past execution without the ability to run anything, and `FlowDebugReplayCommand` accepts the same set. **Everything that executes stays Deployer-only:** the two run commands (`FlowDebugRunMethodCommand` / `FlowDebugRunValueChangeCommand`), the Live entry-point controls, the parked-node Retry/Discard actions and the `?methodId=` / `?instanceId=` drill-ins; for a non-Deployer the page pins itself to Replay mode.
- **Two entry points.** An inbound API method (inputs form generated from the method's `ParameterDefinitions`, values coerced by declared type) or a value-change trigger on a named instance attribute (Site → Instance → Attribute pickers, site-scope filtered). A method-entry Live run is additionally site-scoped **at route time** on the server (`RouteHelper.WithSiteScope` → `SITE_SCOPE_DENIED`, arch-review F2), so an operator cannot reach a site outside their scope by choosing a method that routes there.
- **Predicted graph first.** Choosing an entry point renders the statically predicted shape (`FlowShapePredictor`, Script Analysis REQ-SA-7) before anything runs — nodes whose target depends on inputs render dashed; after a run, predicted nodes the run never produced render greyed.
- **Execution path.** `IFlowDebugService` dispatches `FlowDebugRunMethodCommand` / `FlowDebugRunValueChangeCommand` / `FlowDebugReplayCommand` to the **per-node `ManagementActor`** through `ManagementActorHolder` (the same seam the HTTP `/management` endpoint uses), so the server enforces the Deployer gate and the active-node refusal. A standby node answers the two run commands with `STANDBY_NODE`, which the page renders as an inline alert with the server message verbatim; Replay is served from any node. The same three commands back the CLI's `flow-debug run-method | run-value-change | replay`.
- **Execution path.** `IFlowDebugService` dispatches `FlowDebugRunMethodCommand` / `FlowDebugRunValueChangeCommand` / `FlowDebugReplayCommand` to the **per-node `ManagementActor`** through `ManagementActorHolder` (the same seam the HTTP `/management` endpoint uses), so the server enforces the Deployer gate and the active-node refusal. A standby node answers the two run commands with `STANDBY_NODE`, which the page renders as an inline alert with the server message verbatim; Replay is served from any node. The same three commands back the CLI's `flow-debug run-method | run-value-change | replay`. **The client wait is derived from the server's own bound, not a fixed duration (arch-review F4):** `FlowDebugService.RunAskTimeoutFor(timeoutSeconds)` = the method's `TimeoutSeconds` + the server's `FlowDebugRunner.RunGrace` (10 s) + 15 s client slack, floored at 30 s, and `ValueChangeAskTimeoutFor(waitSeconds)` = the trigger-wait window + 15 s (Replay stays a flat 30 s); the CLI mirrors it with `run-method --timeout-seconds` (default 120, set it to the method's timeout + 25 for long methods) and a `run-value-change` wait of `--wait-seconds + 25`. A long-running method used to be cut off client-side before the server's own grace expired.
- State machine `Idle → Predicted → Running → Completed | Failed | TimedOut`; the status strip shows run number, start, total duration, step/boundary/write counts and the root `ExecutionId` (linking to the Execution Chain page). A run whose route call came back with no child execution and no Site-lane steps shows a *site not traced* note (older site build, or an untraced call). Last entry point and per-method inputs persist in `localStorage`.
- The renderer is the custom-SVG `FlowGraphView` (no third-party charting) — pure, unit-tested layout; culture-invariant coordinates; each node a keyboard-activatable `role="button"`. Node text is HTML inside an SVG `foreignObject` so the browser measures it: the title wraps to two lines and ellipsizes by width (never by character count), the subtitle by width on one line, and the hover tooltip carries the full text. A lane whose real titles share a dotted head (Tags: `DelmiaReceiver.RecipeDownloadFlag`, `DelmiaReceiver.RecipeProcessedFlag`, …) shows that head once in the lane gutter (`DelmiaReceiver.*`) and each node shows only the tail that tells it apart (`FlowGraphView.CommonDottedPrefix`).
+6 -3
View File
@@ -70,7 +70,7 @@ Each API method definition includes:
- Field names and (extended-system) data types. Supports returning **lists of objects** and arbitrarily nested structures.
- **Implementation Script**: C# script that executes when the method is called. Stored **inline** in the method definition. Follows standard C# authoring patterns but has no template inheritance — it is a standalone script tied to this method.
- **Timeout**: Configurable per method. Defines the maximum time the method is allowed to execute (including any routed calls to sites) before returning a timeout error to the caller.
- **TraceAllCalls** (Flow Debugger, 2026-09-05): when set, every call to this method runs its routed site scripts with the trace flag on, so their `ScriptRun` / `AttributeWrite` / `AttributeWait` audit rows (`AuditChannel.ScriptTrace`) are recorded for Replay. Default off — adds roughly seven audit rows per call, so it is meant for low-volume integration methods. Set from the method form's *Trace all calls* checkbox or `api-method create|update --trace-all-calls`.
- **TraceAllCalls** (Flow Debugger, 2026-09-05): when set, every call to this method runs its routed site scripts with the trace flag on, so their `ScriptRun` / `AttributeWrite` / `AttributeWait` audit rows (`AuditChannel.ScriptTrace`) are recorded for Replay. Default off — adds 7–10 audit rows per call (3 of them trace-only; a cached DB write and a notification account for the upper end), so it is meant for low-volume integration methods. Set from the method form's *Trace all calls* checkbox or `api-method create|update --trace-all-calls`.
> **Authoring note — idempotent methods for at-least-once callers**: some external callers retry
> on their own connect-failure/timeout failover loop and cannot tell "the first attempt actually
@@ -299,11 +299,14 @@ For everything else, scripts interact with the system through the curated `Route
- The `Route.To()` helper resolves the instance's site assignment from the configuration database and routes the request to the correct site cluster via the Communication Layer.
- The call is **synchronous from the API caller's perspective** — the API method blocks until the site responds or the **method-level timeout** is reached.
- If the target site is unreachable or the call times out, the call fails and the API returns an error to the caller. No store-and-forward buffering is used for inbound API calls.
- **Every `Route.To(...).Call(...)` writes one always-on `ScriptTrace.RouteCall` audit row** (central direct write, actor `inbound-api`, target `{instance}.{script}`, `ExecutionId` = the inbound request's, `Extra.childExecutionId` = the site run's id) — this row is what joins the central and site halves of a Flow Debugger Replay, so it is not gated. The routed request carries a `Trace` flag beside `ParentExecutionId` (`RouteToCallRequest.Trace`, `RouteToSetAttributesRequest.Trace`); it is set when the method has `TraceAllCalls` or the call is a Flow Debugger Live run, and only then does the site emit its own trace rows and return them inline on `RouteToCallResponse.TraceEvents` (with `ChildExecutionId`). Untraced calls are byte-identical to before.
- **Every `Route.To(...).Call(...)` writes one always-on `ScriptTrace.RouteCall` audit row** (central direct write, actor `inbound-api`, target `{instance}.{script}`, `ExecutionId` = the inbound request's, `Extra.childExecutionId` = the site run's id) — this row is what joins the central and site halves of a Flow Debugger Replay, so it is not gated. The routed request carries a `Trace` flag beside `ParentExecutionId` (`RouteToCallRequest.Trace`, `RouteToSetAttributesRequest.Trace`); it is set when the method has `TraceAllCalls` or the call is a Flow Debugger Live run, and only then does the site emit its own trace rows and return them inline on `RouteToCallResponse.TraceEvents` (with `ChildExecutionId`). Untraced calls are byte-identical to before. **The `RouteCall` row honours the method's `SkipBodyCapture`** — the executor builds the helper with `RouteHelper.WithBodyCapture(!skipBodyCapture)`, so a method that opts out of body capture no longer leaks its arguments and reply through the always-on trace row; the row itself is still written, with metadata only (arch-review F11).
## Flow Debugger runs
The Flow Debugger (Central UI, design `docs/plans/2026-09-05-flow-debugger-design.md`) drives this component through three management commands handled by the per-node `ManagementActor` (Deployer role): `FlowDebugRunMethodCommand` executes a method exactly as an external caller would — `ParameterValidator` + `SchemaRefResolver`, then `InboundScriptExecutor.ExecuteAsync(..., trace: true)` with a `RouteHelper` trace sink collecting the `RouteCall` row and the site's inline events — and **synthesizes the `ApiInbound.InboundRequest` row itself** with the operator as actor (`flow-debugger` source tag) because no HTTP request passed through `AuditWriteMiddleware`; it refuses on a standby node with `STANDBY_NODE` (`IActiveNodeGate`), so the script runs only where a real call would. `FlowDebugRunValueChangeCommand` writes an attribute through the traced routed-write path and polls the site (`PullTraceEvents`) for the run the change spawned; `FlowDebugReplayCommand` reads the chain back from the central Audit Log. The result (`FlowDebugRunResult`) carries the audit events inline — the graph is a central-side projection, never a wire type.
The Flow Debugger (Central UI, design `docs/plans/2026-09-05-flow-debugger-design.md`) drives this component through three management commands handled by the per-node `ManagementActor` (Deployer role): `FlowDebugRunMethodCommand` executes a method exactly as an external caller would — `ParameterValidator` + `SchemaRefResolver`, then `InboundScriptExecutor.ExecuteAsync(..., trace: true)` with a `RouteHelper` trace sink collecting the `RouteCall` row and the site's inline events — and **synthesizes the `ApiInbound.InboundRequest` row itself** with the operator as actor (`flow-debugger` source tag) because no HTTP request passed through `AuditWriteMiddleware`; it refuses on a standby node with `STANDBY_NODE` (`IActiveNodeGate`), so the script runs only where a real call would. `FlowDebugRunValueChangeCommand` writes an attribute through the traced routed-write path and polls the site (`PullTraceEvents`) for the run the change spawned; `FlowDebugReplayCommand` reads the chain back from the central Audit Log and therefore accepts the wider `AuthorizationPolicies.FlowDebuggerAccessRoles` set (Deployer, Administrator, Viewer) while the two run commands stay Deployer-only (arch-review F3). The result (`FlowDebugRunResult`) carries the audit events inline — the graph is a central-side projection, never a wire type.
- **`SchemaRefResolver` is `public` for the runner's benefit.** It was internal to the HTTP path; the Flow Debugger's `FlowDebugRunMethodCommand` validates its operator-supplied parameters through exactly the same `ParameterValidator` + `SchemaRefResolver` pair a real HTTP call uses, so a Live run cannot accept an input the endpoint would reject. Keep the two paths on the one resolver — a second, debugger-local validator would silently fork the contract.
- **Site scope is enforced at route time on a Live run** (arch-review F2). `RouteHelper.WithSiteScope(IReadOnlySet<string>? permittedSiteIdentifiers)` carries the operator's permitted **site identifiers**; `RouteTarget.ResolveSiteAsync` throws `RouteSiteScopeException` before any site traffic when the resolved target site is outside that set, and `InboundScriptExecutor` maps it to error code **`SITE_SCOPE_DENIED`**. `FlowDebugRunner` translates that code into a `SiteScopeViolationException` (`ManagementUnauthorized`) *after* writing the inbound audit row, so the denied attempt is recorded under the operator's name. Administrators and unscoped users get a null scope (unrestricted), and the HTTP endpoint passes no scope at all — production routing is unchanged.
## Authentication Details
@@ -62,7 +62,7 @@ public static class ApiMethodCommands
var returnDefOption = new Option<string?>("--return-def") { Description = "Return type definition" };
var traceAllCallsOption = new Option<bool>("--trace-all-calls")
{
Description = "Trace every production call to this method for the Flow Debugger (adds ~7 audit rows per call; low-volume integration methods only)"
Description = "Trace every production call to this method for the Flow Debugger (adds 7-10 audit rows per call; low-volume integration methods only)"
};
var cmd = new Command("create") { Description = "Create an API method" };
@@ -98,7 +98,7 @@ public static class ApiMethodCommands
// bool? so an update that omits the flag leaves the stored value unchanged.
var traceAllCallsOption = new Option<bool?>("--trace-all-calls")
{
Description = "Trace every production call to this method for the Flow Debugger (true/false; omitted = unchanged; adds ~7 audit rows per call)"
Description = "Trace every production call to this method for the Flow Debugger (true/false; omitted = unchanged; adds 7-10 audit rows per call)"
};
var cmd = new Command("update") { Description = "Update an API method" };
@@ -14,8 +14,12 @@ namespace ZB.MOM.WW.ScadaBridge.CLI.Commands;
/// </summary>
public static class FlowDebugCommands
{
/// <summary>Client-side wait for the run commands — must exceed the server's run grace + trigger window.</summary>
internal static readonly TimeSpan RunTimeout = TimeSpan.FromMinutes(2);
/// <summary>
/// Slack the CLI's HTTP wait adds over the server's own bound (arch-review F4): the CLI
/// process is one more hop out from the management Ask than the Central UI's in-process
/// call, so it carries a larger fixed margin than <c>FlowDebugService.ClientSlack</c> (15s).
/// </summary>
private const int ClientSlackSeconds = 25;
/// <summary>Builds the <c>flow-debug</c> command with its subcommands using the given shared CLI options.</summary>
/// <param name="urlOption">Shared management URL option.</param>
@@ -38,11 +42,26 @@ public static class FlowDebugCommands
{
var methodIdOption = new Option<int>("--method-id") { Description = "Inbound API method ID", Required = true };
var paramsOption = new Option<string?>("--params") { Description = "Flat JSON object of method parameters (validated against the method's schema)" };
var timeoutOption = new Option<int>("--timeout-seconds")
{
Description = "Client-side wait for the run; set to the method's TimeoutSeconds + 25 for long methods. "
+ "The server caps long-running commands at its own ManagementService:LongRunningCommandTimeout "
+ "(default 5 min), so a larger value still 504s at that ceiling",
DefaultValueFactory = _ => 120,
};
var cmd = new Command("run-method") { Description = "Run an inbound API method on the active central node with full tracing and print the flow's steps" };
cmd.Add(methodIdOption);
cmd.Add(paramsOption);
cmd.Add(timeoutOption);
cmd.SetAction(async (ParseResult result) =>
{
var timeoutSeconds = result.GetValue(timeoutOption);
if (timeoutSeconds <= 0)
{
OutputFormatter.WriteError("--timeout-seconds must be greater than zero.", "INVALID_ARGUMENT");
return 1;
}
JsonElement? parameters;
try
{
@@ -57,7 +76,7 @@ public static class FlowDebugCommands
return await CommandHelpers.ExecuteCommandAsync(
result, urlOption, formatOption, usernameOption, passwordOption,
new FlowDebugRunMethodCommand(result.GetValue(methodIdOption), parameters),
timeout: RunTimeout,
timeout: TimeSpan.FromSeconds(timeoutSeconds),
tableProjector: FlowDebugTableProjection.ProjectEvents);
});
return cmd;
@@ -90,7 +109,7 @@ public static class FlowDebugCommands
result.GetValue(attributeOption)!,
result.GetValue(valueOption)!,
waitSeconds),
timeout: RunTimeout,
timeout: TimeSpan.FromSeconds(waitSeconds + ClientSlackSeconds),
tableProjector: FlowDebugTableProjection.ProjectEvents);
});
return cmd;
+8 -6
View File
@@ -2119,7 +2119,7 @@ scadabridge --url <url> api-method create --name <string> --script <string> [--t
| `--timeout` | no | `30` | Timeout in seconds |
| `--parameters` | no | — | Parameter definitions JSON |
| `--return-def` | no | — | Return type definition JSON |
| `--trace-all-calls` | no | off | Trace every production call for the Flow Debugger. **Adds ~7 audit rows per call** — low-volume integration methods only |
| `--trace-all-calls` | no | off | Trace every production call for the Flow Debugger. **Adds 7–10 audit rows per call** (3 of them trace-only) — low-volume integration methods only |
#### `api-method update`
@@ -2137,7 +2137,7 @@ scadabridge --url <url> api-method update --id <int> --script <string> [--timeou
| `--timeout` | no | `30` | Timeout in seconds |
| `--parameters` | no | — | Parameter definitions JSON |
| `--return-def` | no | — | Return type definition JSON |
| `--trace-all-calls` | no | unchanged | `true`/`false` — trace every production call for the Flow Debugger. **Adds ~7 audit rows per call.** Omitted = the stored value is left as-is (the one field the replace-all rule does not apply to) |
| `--trace-all-calls` | no | unchanged | `true`/`false` — trace every production call for the Flow Debugger. **Adds 7–10 audit rows per call** (3 of them trace-only). Omitted = the stored value is left as-is (the one field the replace-all rule does not apply to) |
Script changes take effect immediately — the updated code is recompiled in-memory on the active central node. No restart is required.
@@ -2305,16 +2305,17 @@ Run an inbound API method with the given parameters, exactly as an external call
and print the traced flow.
```sh
scadabridge --url <url> flow-debug run-method --method-id <int> [--params <json>]
scadabridge --url <url> flow-debug run-method --method-id <int> [--params <json>] [--timeout-seconds 120]
```
| Option | Required | Description |
|--------|----------|-------------|
| `--method-id` | yes | Inbound API method ID (`api-method list`) |
| `--params` | no | Flat JSON object of parameters, validated against the method's parameter schema. Omit for a parameterless method. |
| `--timeout-seconds` | no | Client-side wait for the run (default 120). Raise this above the method's own `TimeoutSeconds + 25` for a long-running method, or the client gives up before the server's own run grace does. **Server ceiling:** the management endpoint applies `ScadaBridge:ManagementService:LongRunningCommandTimeout` (default 5 min) to `flow-debug run-method`, so a value above that still returns `504` at the server's ceiling — raise the server setting too if a method legitimately runs longer. |
The client waits up to 2 minutes. Parameters that fail schema validation are reported as
`VALIDATION_FAILED` without running the script.
Parameters that fail schema validation are reported as `VALIDATION_FAILED` without running
the script.
#### `flow-debug run-value-change`
@@ -2332,7 +2333,8 @@ scadabridge --url <url> flow-debug run-value-change --instance-id <int> --attrib
| `--value` | yes | Value to write, in canonical string form |
| `--wait-seconds` | no | How long to wait for a triggered run before reporting `NO_TRIGGER_WITHIN_WINDOW` (default 45) |
The write itself is real — the attribute changes — and is the root step of the flow.
The client waits `--wait-seconds` + 25 (no separate option — the client's HTTP wait always
derives from `--wait-seconds`). The write itself is real — the attribute changes — and is the root step of the flow.
A static attribute triggers directly; a data-sourced attribute triggers when the device
echoes the write back, which the site's trace arm (`SiteRuntime:TraceArmWindowSeconds`,
default 30) correlates back to this run.
@@ -3,7 +3,10 @@
@using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Deployment
@using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared
@using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging
@attribute [Authorize(Policy = AuthorizationPolicies.RequireDeployment)]
@* arch-review F3: opening the page is a Replay-capable read (Deployer + the OperationalAudit
roles) so the Audit Log / Execution Chain drill-ins work for an auditor. Every Live-run
affordance below is additionally gated on _canLive (the Deployer role). *@
@attribute [Authorize(Policy = AuthorizationPolicies.RequireFlowDebugger)]
<PageTitle>Flow Debugger</PageTitle>
@@ -13,15 +16,19 @@
@* ── Toolbar ─────────────────────────────────────────────────────────── *@
<div class="row g-2 align-items-end mb-2" data-test="flow-toolbar">
<div class="col-auto">
<label class="form-label form-label-sm">Entry point</label>
<div class="btn-group btn-group-sm d-block" role="group" aria-label="Entry point">
<button type="button" class="btn @(_entry == EntryKind.Method ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-entry-method" disabled="@_running" @onclick="() => SetEntryAsync(EntryKind.Method)">Inbound API method</button>
<button type="button" class="btn @(_entry == EntryKind.ValueChange ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-entry-value-change" disabled="@_running" @onclick="() => SetEntryAsync(EntryKind.ValueChange)">Value-change trigger</button>
@* The entry point only selects what a Live run starts from, so it is Deployer-only. *@
@if (_canLive)
{
<div class="col-auto">
<label class="form-label form-label-sm">Entry point</label>
<div class="btn-group btn-group-sm d-block" role="group" aria-label="Entry point">
<button type="button" class="btn @(_entry == EntryKind.Method ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-entry-method" disabled="@_running" @onclick="() => SetEntryAsync(EntryKind.Method)">Inbound API method</button>
<button type="button" class="btn @(_entry == EntryKind.ValueChange ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-entry-value-change" disabled="@_running" @onclick="() => SetEntryAsync(EntryKind.ValueChange)">Value-change trigger</button>
</div>
</div>
</div>
}
@if (_mode == FlowMode.Live)
{
@@ -85,15 +92,19 @@
</div>
}
<div class="col-auto">
<label class="form-label form-label-sm">Mode</label>
<div class="btn-group btn-group-sm d-block" role="group" aria-label="Mode">
<button type="button" class="btn @(_mode == FlowMode.Live ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-mode-live" disabled="@_running" @onclick="() => SetMode(FlowMode.Live)">Live</button>
<button type="button" class="btn @(_mode == FlowMode.Replay ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-mode-replay" disabled="@_running" @onclick="() => SetMode(FlowMode.Replay)">Replay</button>
@* Without Deployer there is only Replay, so the toggle would be a single dead button. *@
@if (_canLive)
{
<div class="col-auto">
<label class="form-label form-label-sm">Mode</label>
<div class="btn-group btn-group-sm d-block" role="group" aria-label="Mode">
<button type="button" class="btn @(_mode == FlowMode.Live ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-mode-live" disabled="@_running" @onclick="() => SetMode(FlowMode.Live)">Live</button>
<button type="button" class="btn @(_mode == FlowMode.Replay ? "btn-primary" : "btn-outline-secondary")"
data-test="flow-mode-replay" disabled="@_running" @onclick="() => SetMode(FlowMode.Replay)">Replay</button>
</div>
</div>
</div>
}
<div class="col-auto ms-auto">
@if (_mode == FlowMode.Live)
@@ -115,6 +126,14 @@
</div>
</div>
@if (!_canLive)
{
<div class="text-muted small mb-2" data-test="flow-replay-only">
Replay only — rebuilding a past execution from the audit log. Starting a live run
requires the Deployer role.
</div>
}
@* ── Inline alerts ───────────────────────────────────────────────────── *@
@if (_error != null)
{
@@ -232,10 +251,13 @@
@* ── Detail panel ──────────────────────────────────────────────────── *@
<div class="col-12">
@* Retry / Discard relay a command to the site — only wired for a Deployer, so the
buttons never render for an auditor (arch-review F3). *@
<FlowStepDetail Step="_selected" Graph="_graph"
SiteDbId="@(_entry == EntryKind.ValueChange && _selectedSiteId > 0 ? _selectedSiteId : null)"
InstanceDbId="@(_entry == EntryKind.ValueChange && _selectedInstanceId > 0 ? _selectedInstanceId : null)"
OnRetry="OnRetryStepAsync" OnDiscard="OnDiscardStepAsync" ActionInProgress="_actionInProgress" />
OnRetry="RetryCallback" OnDiscard="DiscardCallback"
ActionInProgress="_actionInProgress" />
</div>
</div>
</div>
@@ -2,6 +2,7 @@ using System.Globalization;
using System.Text.Json;
using System.Text.Json.Nodes;
using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.Components.Authorization;
using Microsoft.JSInterop;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.CentralUI.Auth;
@@ -15,6 +16,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi;
using ZB.MOM.WW.ScadaBridge.ScriptAnalysis;
using ZB.MOM.WW.ScadaBridge.Security;
namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Deployment;
@@ -76,6 +78,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable
[Inject] private SiteScopeService SiteScope { get; set; } = default!;
[Inject] private IJSRuntime JS { get; set; } = default!;
[Inject] private IDialogService Dialog { get; set; } = default!;
[Inject] private AuthenticationStateProvider Auth { get; set; } = default!;
/// <summary>Drill-in: pre-select an inbound method.</summary>
[SupplyParameterFromQuery] public int? MethodId { get; set; }
@@ -93,6 +96,16 @@ public partial class FlowDebugger : ComponentBase, IDisposable
private FlowMode _mode = FlowMode.Live;
private FlowState _state = FlowState.Idle;
/// <summary>
/// True when the principal holds <see cref="Roles.Deployer"/> and may therefore start a
/// Live run (and relay Retry/Discard on a parked node). The page itself opens to the
/// wider <see cref="AuthorizationPolicies.RequireFlowDebugger"/> set so an auditor can
/// follow the Audit Log / Execution Chain drill-in into Replay (arch-review F3); every
/// Live affordance is hidden when this is false, and the server enforces the same split
/// (the two run commands stay Deployer-only in the ManagementActor role matrix).
/// </summary>
private bool _canLive;
private List<ApiMethod> _methods = [];
private List<Site> _sites = [];
private List<Instance> _instances = [];
@@ -146,7 +159,7 @@ public partial class FlowDebugger : ComponentBase, IDisposable
private bool CanRefresh => !_running && _raw is not null;
private bool CanRun => !_running && _mode == FlowMode.Live && (
private bool CanRun => _canLive && !_running && _mode == FlowMode.Live && (
_entry == EntryKind.Method ? _selectedMethodId > 0
: _selectedInstanceId > 0 && !string.IsNullOrWhiteSpace(_selectedAttribute));
@@ -184,9 +197,29 @@ public partial class FlowDebugger : ComponentBase, IDisposable
/// <inheritdoc/>
protected override async Task OnInitializedAsync()
{
// arch-review F3: page access is the wider RequireFlowDebugger set; only a Deployer
// gets the Live half. Resolved before anything else so the Live-only drill-ins below
// are skipped and the mode starts on Replay for an auditor.
_canLive = (await Auth.GetAuthenticationStateAsync()).User
.HasClaim(JwtTokenService.RoleClaimType, Roles.Deployer);
if (!_canLive)
_mode = FlowMode.Replay;
_methods = (await InboundApi.GetAllApiMethodsAsync()).OrderBy(m => m.Name, StringComparer.OrdinalIgnoreCase).ToList();
_sites = await SiteScope.FilterSitesAsync(await SiteRepository.GetAllSitesAsync());
if (!_canLive)
{
// ?methodId= / ?instanceId= are Live-run drill-ins; ?executionId= is the audit
// drill-in and is the only one that means anything without Deployer.
if (ExecutionId is Guid auditExec && auditExec != Guid.Empty)
{
_replayExecutionIdText = auditExec.ToString();
await LoadReplayAsync();
}
return;
}
if (InstanceId is int iid && iid > 0)
{
_entry = EntryKind.ValueChange;
@@ -383,6 +416,8 @@ public partial class FlowDebugger : ComponentBase, IDisposable
private void SetMode(FlowMode mode)
{
if (_mode == mode) return;
// Without Deployer there is no Live half to switch to (arch-review F3).
if (mode == FlowMode.Live && !_canLive) return;
CancelPendingRefresh();
_mode = mode;
}
@@ -558,6 +593,17 @@ public partial class FlowDebugger : ComponentBase, IDisposable
private Task OnDiscardStepAsync(FlowStep step) => ActOnStepAsync(step, retry: false);
/// <summary>
/// The parked-node Retry callback, left unset (<c>default</c>) without the Deployer role so
/// <c>FlowStepDetail</c>'s <c>OnRetry.HasDelegate</c> check hides the button (arch-review F3).
/// </summary>
private EventCallback<FlowStep> RetryCallback
=> _canLive ? EventCallback.Factory.Create<FlowStep>(this, OnRetryStepAsync) : default;
/// <summary>The parked-node Discard callback; unset without Deployer, as <see cref="RetryCallback"/>.</summary>
private EventCallback<FlowStep> DiscardCallback
=> _canLive ? EventCallback.Factory.Create<FlowStep>(this, OnDiscardStepAsync) : default;
/// <summary>
/// Retry / Discard a Parked outbound node: site-scope re-check → confirm → relay through the
/// service → toast → re-poll Replay until the node leaves Parked. Mirrors the Site Calls /
@@ -565,7 +611,10 @@ public partial class FlowDebugger : ComponentBase, IDisposable
/// </summary>
private async Task ActOnStepAsync(FlowStep step, bool retry)
{
if (_actionInProgress || step.Outcome != FlowStepOutcome.Parked || step.CorrelationId is not { } corr)
// _canLive: relaying Retry/Discard to the site is a Deployer act. The buttons are not
// rendered without it (the callbacks are left unset), but the gate is repeated here so
// the split does not rest on markup alone (arch-review F3).
if (!_canLive || _actionInProgress || step.Outcome != FlowStepOutcome.Parked || step.CorrelationId is not { } corr)
return;
if (!await IsStepSiteAllowedAsync(step))
@@ -47,7 +47,7 @@
</label>
<div class="form-text">
Every call to this method records its routed site steps as audit rows the
Flow Debugger can replay. Adds ~7 audit rows per call; for low-volume
Flow Debugger can replay. Adds 7–10 audit rows per call; for low-volume
integration methods.
</div>
</div>
@@ -410,11 +410,15 @@ public partial class FlowGraphView : ComponentBase
if (hasPredicted)
{
// Overlay: match predicted nodes to real steps by kind, in order; the leftovers are ghosts.
// Overlay: match predicted nodes to real steps by (kind, title) first, falling back to kind
// alone; the leftovers are ghosts. Kind-only matching pairs steps to predicted nodes in
// declaration order, which mismatches a method with multiple same-kind calls when only some
// of them actually ran (arch-review F10).
var unmatched = predicted!.Nodes.ToList();
foreach (var s in graph.Steps)
{
var m = unmatched.FirstOrDefault(p => p.Kind == s.Kind);
var m = unmatched.FirstOrDefault(p => p.Kind == s.Kind && string.Equals(p.Title, s.Title, StringComparison.OrdinalIgnoreCase))
?? unmatched.FirstOrDefault(p => p.Kind == s.Kind);
if (m != null)
{
unmatched.Remove(m);
@@ -12,6 +12,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Messages.Notification;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
using ZB.MOM.WW.ScadaBridge.Communication;
using ZB.MOM.WW.ScadaBridge.ManagementService;
using ZB.MOM.WW.ScadaBridge.ManagementService.FlowDebug;
using ZB.MOM.WW.ScadaBridge.ScriptAnalysis;
using ZB.MOM.WW.ScadaBridge.Security;
@@ -23,21 +24,46 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Services;
/// the run/replay commands go to the per-node <c>ManagementActor</c> through
/// <see cref="ManagementActorHolder"/>, exactly as <see cref="DataConnectionMoveService"/>
/// does, with the current Blazor principal projected to an <see cref="AuthenticatedUser"/>
/// so the server's Deployer gate runs against the real identity. The Live-run Ask waits up
/// to <see cref="RunAskTimeout"/> (the server bounds a run by the method timeout + grace and
/// a value-change by its wait window; both fit inside two minutes).
/// so the server's Deployer gate runs against the real identity. The Live-run Ask wait is
/// derived from the server's own bound (<see cref="RunAskTimeoutFor"/> /
/// <see cref="ValueChangeAskTimeoutFor"/>) rather than a fixed duration (arch-review F4) —
/// a run against a method with a long <c>TimeoutSeconds</c> used to be cut off client-side
/// well before the server's own grace period expired.
/// Retry / Discard of a parked outbound node (follow-ups plan 2026-09-06 D4) goes through
/// <see cref="CommunicationService"/> to the SAME central-local proxies the Site Calls and
/// Notification report pages use, so the page and the report never disagree on what a relay does.
/// </summary>
public sealed class FlowDebugService : IFlowDebugService
{
/// <summary>Ask timeout for the two Live-run commands.</summary>
public static readonly TimeSpan RunAskTimeout = TimeSpan.FromMinutes(2);
/// <summary>Ask timeout for Replay (a central DB read).</summary>
public static readonly TimeSpan ReplayAskTimeout = TimeSpan.FromSeconds(30);
/// <summary>Slack the client adds over the server's own bound (Ask hop + serialization).</summary>
public static readonly TimeSpan ClientSlack = TimeSpan.FromSeconds(15);
/// <summary>Floor for a method-run Ask wait, regardless of how short the method's own timeout is.</summary>
public static readonly TimeSpan MinRunAskTimeout = TimeSpan.FromSeconds(30);
/// <summary>
/// Client-side Ask wait for <see cref="RunMethodAsync"/>: the method's own
/// <c>TimeoutSeconds</c> plus the server's <see cref="FlowDebugRunner.RunGrace"/> plus
/// <see cref="ClientSlack"/>, floored at <see cref="MinRunAskTimeout"/> — the server bounds
/// the run itself at <c>timeoutSeconds + RunGrace</c> (see <c>FlowDebugRunner.RunMethodAsync</c>),
/// so the client must wait at least that long or it cuts the run off before the server does.
/// </summary>
public static TimeSpan RunAskTimeoutFor(int timeoutSeconds)
{
var t = TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds)) + FlowDebugRunner.RunGrace + ClientSlack;
return t < MinRunAskTimeout ? MinRunAskTimeout : t;
}
/// <summary>
/// Client-side Ask wait for <see cref="RunValueChangeAsync"/>: the trigger-wait window
/// plus <see cref="ClientSlack"/>.
/// </summary>
public static TimeSpan ValueChangeAskTimeoutFor(int waitSeconds)
=> TimeSpan.FromSeconds(Math.Max(1, waitSeconds)) + ClientSlack;
/// <summary>Upper bound on the template parent chain the script resolver walks (cycle guard).</summary>
private const int MaxTemplateChain = 16;
@@ -176,12 +202,19 @@ public sealed class FlowDebugService : IFlowDebugService
}
/// <inheritdoc/>
public Task<FlowDebugOutcome> RunMethodAsync(int apiMethodId, JsonElement? parameters, CancellationToken cancellationToken = default)
=> SendAndBuildAsync(new FlowDebugRunMethodCommand(apiMethodId, parameters), RunAskTimeout, cancellationToken);
public async Task<FlowDebugOutcome> RunMethodAsync(int apiMethodId, JsonElement? parameters, CancellationToken cancellationToken = default)
{
var method = await _inboundApi.GetApiMethodByIdAsync(apiMethodId, cancellationToken);
var timeout = RunAskTimeoutFor(method?.TimeoutSeconds ?? 30);
return await SendAndBuildAsync(new FlowDebugRunMethodCommand(apiMethodId, parameters), timeout, cancellationToken);
}
/// <inheritdoc/>
public Task<FlowDebugOutcome> RunValueChangeAsync(int instanceId, string attributeName, string value, CancellationToken cancellationToken = default)
=> SendAndBuildAsync(new FlowDebugRunValueChangeCommand(instanceId, attributeName, value), RunAskTimeout, cancellationToken);
{
var command = new FlowDebugRunValueChangeCommand(instanceId, attributeName, value);
return SendAndBuildAsync(command, ValueChangeAskTimeoutFor(command.WaitSeconds), cancellationToken);
}
/// <inheritdoc/>
public Task<FlowDebugOutcome> ReplayAsync(Guid executionId, CancellationToken cancellationToken = default)
@@ -17,7 +17,7 @@ public class ApiMethod
/// <summary>
/// When true every production call to this method is fully traced (Flow Debugger design
/// 2026-09-05 §5.2); intended for low-volume integration methods — ~7 audit rows per call.
/// 2026-09-05 §5.2); intended for low-volume integration methods — 7-10 audit rows per call.
/// </summary>
public bool TraceAllCalls { get; set; }
@@ -16,15 +16,25 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
/// takes the first row's identity, time, input and title, the latest row's outcome, subtitle, output and error;
/// <c>DurationMs</c> is the sync row's own, or submit→terminal wall time for a cached operation (null while pending);
/// <c>Lifecycle</c> lists every row in time order.</item>
/// <item>Steps are ordered by <c>OccurredAtUtc</c>, then a kind rank (inbound before route call
/// before script run before writes/waits before outbound), then <c>EventId</c>.</item>
/// <item><b>Order</b> = emission order between executions, timestamp within one (arch-review F5: central and
/// site clocks differ). Executions are ordered by the input index of their earliest-given row; inside one
/// execution — one clock — steps are ordered by <c>OccurredAtUtc</c>, then a kind rank (inbound before route
/// call before script run before writes/waits before outbound), then the input index. Replay input is
/// timestamp-sorted; executions are still grouped into contiguous blocks ordered by their first row, so a
/// root-execution step that ran after a routed call (e.g. a central <c>Database.*</c> call after
/// <c>Route.To().Call</c>) displays before the routed site steps — display order only, edges are id-based.</item>
/// <item>Within one <c>ExecutionId</c> consecutive steps are chained in that order, <b>skipping async steps</b> — a
/// <c>Notify</c>, or a cached operation whose first row is <c>CachedSubmit</c>. Each skipped step instead gets an
/// <c>Async</c> (dashed) edge from the chain step immediately before it in time.</item>
/// <item>The first step of an execution is linked from its spawner: a <c>RouteCall</c> whose
/// <c>childExecutionId</c> matches, else the latest step of the parent execution that occurred
/// no later than it — that second case is a <c>Spawn</c> edge (trigger cascade). The edge
/// <c>CrossesBoundary</c> when exactly one end is in the Central lane.</item>
/// <c>childExecutionId</c> matches, else — a <c>Spawn</c> edge (trigger cascade) — the last non-async,
/// non-<c>Response</c> parent step that <b>occurred at or before</b> the child's first step. Parent and
/// child of a cascade share the site clock, so that timestamp rule picks the write that actually fired
/// the trigger; only when it finds nothing (a child clock BEHIND its parent's — a mixed-clock Replay)
/// does the search fall back to display position, and then to the parent's first step. Position alone
/// is not enough: executions are contiguous blocks, so every parent step precedes the child block and
/// the search would always land on the parent's LAST step. The edge <c>CrossesBoundary</c> when
/// exactly one end is in the Central lane.</item>
/// <item>An <c>AttributeWait</c> that was satisfied gets an <c>External</c> (dashed) edge from the
/// preceding <c>AttributeWrite</c> in the same execution — the device echo the system inferred.</item>
/// <item>When an <c>InboundRequest</c> row carries a response summary, a <c>Response</c> step is
@@ -41,12 +51,47 @@ public static class FlowGraphBuilder
if (events.Count == 0)
return new FlowGraph(Array.Empty<FlowStep>(), Array.Empty<FlowEdge>(), Predicted: false, Truncated: truncated);
var steps = Fold(events)
.OrderBy(s => s.OccurredAtUtc)
// Arch-review F5: a step's position in the input is the only cross-node ordering we can trust —
// central rows carry central's clock, site rows the site's, and a 1-2 s skew used to sort a whole
// site execution ahead of the inbound row that started it.
var seq = new Dictionary<Guid, int>(events.Count);
for (var i = 0; i < events.Count; i++)
seq.TryAdd(events[i].EventId, i);
// A folded step keeps the identity of its earliest-by-TIME row, which need not be the row given
// first — so a folded node's index is the minimum over the rows folded into it.
int InputIndex(FlowStep step)
{
var index = seq.TryGetValue(step.EventId, out var own) ? own : int.MaxValue;
if (step.Lifecycle is { Count: > 1 } rows)
{
foreach (var row in rows)
{
if (seq.TryGetValue(row.EventId, out var i) && i < index)
index = i;
}
}
return index;
}
var folded = Fold(events).ToList();
var firstSeqByExecution = folded
.GroupBy(s => s.ExecutionId ?? Guid.Empty)
.ToDictionary(g => g.Key, g => g.Min(InputIndex));
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(s => s.EventId)
.ThenBy(InputIndex)
.ToList();
// Position in the ordered list — the "before" relation the spawner search uses, so a skewed
// clock can no longer push every candidate parent step "after" the child.
var position = new Dictionary<Guid, int>(steps.Count);
for (var i = 0; i < steps.Count; i++)
position[steps[i].EventId] = i;
var edges = new List<FlowEdge>();
var byExecution = steps
.Where(s => s.ExecutionId is not null)
@@ -93,8 +138,20 @@ public static class FlowGraphBuilder
if (routeCallsByChild.TryGetValue(executionId, out var routeCall))
spawner = routeCall;
else if (byExecution.TryGetValue(parentId, out var parentChain))
spawner = parentChain.LastOrDefault(p => p.OccurredAtUtc <= first.OccurredAtUtc && p.Kind != FlowStepKind.Response && !IsAsync(p))
{
// A trigger cascade's parent and child are both stamped by the SITE clock, so the step
// that fired it is the last eligible parent step at or before the child's first step.
// Display position cannot answer this: executions are contiguous blocks, so every parent
// step precedes the child and the position rule degenerates to "the parent's last step"
// (a run shaped ScriptRun → Write(A) → Wait → Write(B) would spawn from Write(B)).
// Position stays as the fallback for the mixed-clock case F5 was written for — a child
// clock behind its parent's, where no parent step satisfies the time rule.
var childTime = first.OccurredAtUtc;
var childPosition = position[first.EventId];
spawner = parentChain.LastOrDefault(p => Spawnable(p) && p.OccurredAtUtc <= childTime)
?? parentChain.LastOrDefault(p => Spawnable(p) && position[p.EventId] < childPosition)
?? parentChain[0];
}
if (spawner is null) continue;
var spawn = spawner.Kind != FlowStepKind.RouteCall;
@@ -163,6 +220,14 @@ public static class FlowGraphBuilder
return new FlowGraph(steps, edges, Predicted: false, Truncated: truncated);
}
/// <summary>
/// Can a step be the spawner of a cascaded child execution? The synthesized <c>Response</c> node is not
/// a real step, and an async step (a fire-and-forget notification / cached call) never blocks long
/// enough to be the thing that fired a trigger.
/// </summary>
private static bool Spawnable(FlowStep step)
=> step.Kind != FlowStepKind.Response && !IsAsync(step);
/// <summary>
/// A step the script did not wait for: a notification, or a cached operation — any API / DB step whose
/// first lifecycle row is not the sync one-shot kind (<c>ApiCall</c> / <c>DbWrite</c>), the same rule the
@@ -0,0 +1,126 @@
using ZB.MOM.WW.Audit;
namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
/// <summary>
/// Per-run sink for <c>ScriptTrace</c> audit events so a traced routed call can return them
/// inline on <c>ScriptCallResult.TraceEvents</c> (Flow Debugger design 2026-09-05 §5.4).
/// Lock-based: a run emits tens of events, not thousands. Beyond <see cref="DefaultMaxEvents"/>
/// the collector keeps the first N, sets <see cref="Truncated"/>, and the audit rows written
/// through <c>IAuditWriter</c> remain complete — Replay is never truncated, only the inline copy.
/// An untraced run holds <c>null</c> rather than an inert collector; callers null-check.
/// Outbound-calls addendum (plan 2026-09-06 D5): the <c>ApiOutbound</c> / <c>DbOutbound</c> /
/// <c>Notification</c> rows a run emits ride the same sink, so <see cref="Complete"/> closes it
/// when the launcher takes the reply snapshot — a lifecycle row the S&amp;F forwarder writes
/// minutes later, or an un-awaited call finishing after the run returned, must not mutate a
/// list the reply has already been built from. <see cref="Add"/> is a no-op once completed.
/// Arch-review F1 (2026-09-06): the inline copy is <b>REDACTED</b> — the collector applies the
/// host's <see cref="IAuditRedactor"/> itself rather than storing the raw event the emitter also
/// hands the writer, so Live shows exactly what Replay reads back (per-target SQL-parameter
/// overrides, header scrubbing and the 8 KB / 64 KB payload caps all apply). The writers keep
/// their own <c>Apply</c> call — redaction is idempotent, so the stored row is unaffected.
/// The copy is also capped by BYTES as well as by count, because it rides a gRPC reply whose
/// default 4 MB receive limit a few hundred large error rows would otherwise breach — and that
/// same reply carries the script's return value, so an oversized trace would fail a real call.
/// The budget is an approximation, not a wire measurement: it sums the UTF-16 <c>char</c> length
/// of <see cref="AuditEvent.DetailsJson"/> only, so it ignores the event's other fields and the
/// protobuf framing, and 1 048 576 chars is 1 MB of UTF-8 for ASCII payloads but up to ~3 MB for
/// non-ASCII ones. That worst case still fits the 4 MB limit with the rest of the reply, which is
/// what the budget exists to guarantee — raise it only with a real serialized-size measurement.
/// </summary>
public sealed class FlowTraceCollector
{
/// <summary>Default inline cap per run (design §5.4 <c>MaxTraceEventsPerRun</c>).</summary>
public const int DefaultMaxEvents = 500;
/// <summary>
/// Default inline byte budget per run — the summed redacted <see cref="AuditEvent.DetailsJson"/>
/// length in UTF-16 chars (see the type remarks: ~1 MB of UTF-8 for ASCII payloads, ~3 MB worst
/// case for non-ASCII ones, both under the 4 MB gRPC default receive limit before framing).
/// </summary>
public const int DefaultMaxBytes = 1_048_576;
private readonly List<AuditEvent> _events = new();
private readonly object _gate = new();
private readonly int _max;
private readonly int _maxBytes;
private readonly IAuditRedactor? _redactor;
private int _bytes;
/// <summary>
/// True once an <see cref="Add"/> was dropped because the event cap or the byte budget
/// was reached.
/// </summary>
public bool Truncated { get; private set; }
/// <summary>
/// True once <see cref="Complete"/> ran — the run's reply snapshot has been taken and
/// later <see cref="Add"/> calls are ignored (they never count as <see cref="Truncated"/>).
/// </summary>
public bool Completed { get; private set; }
/// <summary>
/// Creates a collector with the given inline caps (both must be positive) and the redactor
/// the inline copy is filtered through. A null <paramref name="redactor"/> — a host that
/// never called <c>AddAuditLog</c>, or a test — collects raw but is still capped.
/// </summary>
public FlowTraceCollector(
int maxEvents = DefaultMaxEvents,
int maxBytes = DefaultMaxBytes,
IAuditRedactor? redactor = null)
{
if (maxEvents <= 0) throw new ArgumentOutOfRangeException(nameof(maxEvents), "must be positive");
if (maxBytes <= 0) throw new ArgumentOutOfRangeException(nameof(maxBytes), "must be positive");
_max = maxEvents;
_maxBytes = maxBytes;
_redactor = redactor;
}
/// <summary>
/// Appends the REDACTED copy of an event, or marks the collector truncated once the event
/// cap or the byte budget is reached. A no-op after <see cref="Complete"/>.
/// </summary>
public void Add(AuditEvent evt)
{
ArgumentNullException.ThrowIfNull(evt);
// Redaction runs OUTSIDE the lock (arch-review F1): IAuditRedactor is a pure function
// but not necessarily a cheap one (regex scrubbing + payload truncation), and the
// collector is shared by every emit site of a run.
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;
}
}
/// <summary>
/// Closes the collector and returns the final snapshot — the launcher calls this exactly
/// where it builds <c>ScriptCallResult.TraceEvents</c>. Idempotent: a second call returns
/// the same contents.
/// </summary>
public IReadOnlyList<AuditEvent> Complete()
{
lock (_gate)
{
Completed = true;
return _events.ToArray();
}
}
/// <summary>Current event count.</summary>
public int Count { get { lock (_gate) return _events.Count; } }
/// <summary>
/// Copies the collected events in emission order WITHOUT closing the collector — a diagnostic /
/// test-only peek. A reply path must use <see cref="Complete"/>, otherwise an un-awaited call
/// finishing after the reply could mutate the list the reply was built from.
/// </summary>
public IReadOnlyList<AuditEvent> Snapshot()
{
lock (_gate) return _events.ToArray();
}
}
@@ -100,8 +100,10 @@ message AttributeValueUpdate {
Quality quality = 5;
google.protobuf.Timestamp timestamp = 6;
// Flow Debugger (design 2026-09-05 §5.3/§5.4): the writing execution's id ("D" GUID
// form; empty represents null) and the trace flag, so the debug stream can hand the
// debugger the spawned run's parent. Additive; an older site never sets them.
// form; empty represents null) and the trace flag. Additive; an older site never sets them.
// RESERVED — populated by the site, consumed by nothing (arch-review F12): the value-change
// entry polls the site's PullTraceEvents RPC instead of subscribing to the debug stream, so
// these two fields are kept for a future debug-stream path. Do not reuse the field numbers.
string source_execution_id = 7;
bool trace = 8;
}
@@ -1,4 +1,5 @@
using System.Data;
using System.Data.Common;
using System.Text;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
@@ -918,105 +919,114 @@ VALUES
private static readonly TimeSpan ExecutionTreeWindowSlack = TimeSpan.FromHours(1);
private static readonly TimeSpan ExecutionTreeMaxSpan = TimeSpan.FromDays(7);
/// <inheritdoc />
public async Task<IReadOnlyList<ExecutionTreeNode>> GetExecutionTreeAsync(
Guid executionId,
CancellationToken ct = default)
/// <summary>
/// The anchor of one execution chain: the root execution id reached by the upward
/// <c>ParentExecutionId</c> walk, plus that root's first event timestamp (null when the
/// root is a row-less stub). Produced by <see cref="ResolveChainAnchorAsync"/> and
/// consumed by <see cref="ChainCtePrefix"/> / <see cref="AddChainParameters"/>.
/// </summary>
private sealed record ChainAnchor(Guid RootExecutionId, DateTime? RootFirstOccurred)
{
var conn = _context.Database.GetDbConnection();
var openedHere = false;
if (conn.State != System.Data.ConnectionState.Open)
/// <summary>
/// The optional window predicate for the Edges CTE. When the root has rows the scan is
/// confined to the window (partition elimination); otherwise it stays unbounded.
/// </summary>
public string EdgesWindowPredicate => RootFirstOccurred is null
? string.Empty
: "AND OccurredAtUtc >= @winStart AND OccurredAtUtc < @winEnd";
}
/// <summary>
/// Phase 1 of the execution-chain walk, shared by <see cref="GetExecutionTreeAsync"/> and
/// <see cref="GetTraceEventsAsync"/> (arch-review F9): climb to the chain root, then read the
/// root's first event timestamp so the down-walk can be bounded to a root-anchored window.
/// The two walkers must stay in step on window slack, depth cap and the DISTINCT-edges trick,
/// so there is exactly one copy of each.
/// </summary>
private static async Task<ChainAnchor> ResolveChainAnchorAsync(
DbConnection conn,
Guid executionId,
CancellationToken ct)
{
// --- Phase 1: walk up to the root ---------------------------------
// Climb ParentExecutionId until a node has no parent (root) or the
// parent execution has no rows of its own (purged/stub — the climb
// cannot continue past a row-less node). The depth cap guards
// against a cycle in corrupt data; a tree never reaches it.
var rootExecutionId = executionId;
for (var depth = 0; depth < ExecutionChainMaxDepth; depth++)
{
await conn.OpenAsync(ct).ConfigureAwait(false);
openedHere = true;
Guid? parent;
await using (var upCmd = conn.CreateCommand())
{
upCmd.CommandText =
"SELECT TOP 1 ParentExecutionId FROM dbo.AuditLog " +
"WHERE ExecutionId = @cur AND ParentExecutionId IS NOT NULL;";
var pCur = upCmd.CreateParameter();
pCur.ParameterName = "@cur";
pCur.Value = rootExecutionId;
upCmd.Parameters.Add(pCur);
var result = await upCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
parent = result is null or DBNull ? null : (Guid)result;
}
if (parent is null)
{
// No parent row for the current node — it is the root (or a
// row-less stub at the top of what survives). Stop climbing.
break;
}
rootExecutionId = parent.Value;
}
try
// --- Between phases: bound the down-walk to a root-anchored window ---
// The root's first event timestamp anchors a [rootFirst - slack, rootFirst + maxSpan)
// window used to prune the Edges CTE scan to a handful of partitions. A row-less stub
// root (purged/no-action parent) has no MIN — leave the scan unbounded in that
// degenerate case (correctness over speed).
DateTime? rootFirstOccurred;
await using (var winCmd = conn.CreateCommand())
{
// --- Phase 1: walk up to the root ---------------------------------
// Climb ParentExecutionId until a node has no parent (root) or the
// parent execution has no rows of its own (purged/stub — the climb
// cannot continue past a row-less node). The depth cap guards
// against a cycle in corrupt data; a tree never reaches it.
var rootExecutionId = executionId;
for (var depth = 0; depth < ExecutionChainMaxDepth; depth++)
{
Guid? parent;
await using (var upCmd = conn.CreateCommand())
{
upCmd.CommandText =
"SELECT TOP 1 ParentExecutionId FROM dbo.AuditLog " +
"WHERE ExecutionId = @cur AND ParentExecutionId IS NOT NULL;";
var pCur = upCmd.CreateParameter();
pCur.ParameterName = "@cur";
pCur.Value = rootExecutionId;
upCmd.Parameters.Add(pCur);
winCmd.CommandText =
"SELECT MIN(OccurredAtUtc) FROM dbo.AuditLog WHERE ExecutionId = @root;";
var pWinRoot = winCmd.CreateParameter();
pWinRoot.ParameterName = "@root";
pWinRoot.Value = rootExecutionId;
winCmd.Parameters.Add(pWinRoot);
var result = await upCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
parent = result is null or DBNull ? null : (Guid)result;
}
var winResult = await winCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
rootFirstOccurred = winResult is null or DBNull ? null : (DateTime)winResult;
}
if (parent is null)
{
// No parent row for the current node — it is the root (or a
// row-less stub at the top of what survives). Stop climbing.
break;
}
return new ChainAnchor(rootExecutionId, rootFirstOccurred);
}
rootExecutionId = parent.Value;
}
// --- Between phases: bound the down-walk to a root-anchored window ---
// The root's first event timestamp anchors a [rootFirst - slack, rootFirst + maxSpan)
// window used to prune the Edges CTE scan to a handful of partitions. A row-less stub
// root (purged/no-action parent) has no MIN — leave the scan unbounded in that
// degenerate case (correctness over speed).
DateTime? rootFirstOccurred;
await using (var winCmd = conn.CreateCommand())
{
winCmd.CommandText =
"SELECT MIN(OccurredAtUtc) FROM dbo.AuditLog WHERE ExecutionId = @root;";
var pWinRoot = winCmd.CreateParameter();
pWinRoot.ParameterName = "@root";
pWinRoot.Value = rootExecutionId;
winCmd.Parameters.Add(pWinRoot);
var winResult = await winCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
rootFirstOccurred = winResult is null or DBNull ? null : (DateTime)winResult;
}
// Build the optional window predicate for the Edges CTE. When the root has rows the
// scan is confined to the window (partition elimination); otherwise it stays unbounded.
var edgesWindowPredicate = rootFirstOccurred is null
? string.Empty
: "AND OccurredAtUtc >= @winStart AND OccurredAtUtc < @winEnd";
// --- Phase 2: walk down from the root via a recursive CTE ---------
// Edges : a non-recursive, DISTINCT (ExecutionId, ParentExecutionId)
// edge set distilled from AuditLog. Recursing over edges
// instead of raw rows means an execution with N audit rows
// contributes ONE recursion path, not N — MAXRECURSION
// bounds depth, not per-level width, so the raw-row form
// could fan out badly. One edge per execution because all
// rows of an execution share a single ParentExecutionId
// (see the MIN(...) note on the final projection).
// Chain : seeded at the root edge, recursively joins each edge whose
// ParentExecutionId is an ExecutionId already in the chain.
// Each edge carries its own ParentExecutionId, so the chain
// of edges already surfaces every execution id in the tree
// — including a row-less stub parent, which appears as the
// ParentExecutionId of its child's edge. No separate
// union-back CTE is needed.
// Final : collect every distinct execution id reachable from the
// chain (each edge's ExecutionId plus its non-null
// ParentExecutionId), LEFT JOIN back to AuditLog and
// GROUP BY so a stub parent — which owns no edge of its own
// because it emitted no rows — still surfaces as a node with
// RowCount 0 and NULL aggregates.
var nodes = new List<ExecutionTreeNode>();
await using (var downCmd = conn.CreateCommand())
{
downCmd.CommandText = $@"
/// <summary>
/// Phase 2's Edges/Chain CTE prefix — identical in both walkers (arch-review F9). Callers
/// append their own projection: <see cref="GetExecutionTreeAsync"/> the <c>ChainIds</c> CTE
/// plus the per-execution aggregate, <see cref="GetTraceEventsAsync"/> the bare id union.
/// Both must terminate with <c>OPTION (MAXRECURSION …)</c>.
/// </summary>
/// <remarks>
/// Edges : a non-recursive, DISTINCT (ExecutionId, ParentExecutionId)
/// edge set distilled from AuditLog. Recursing over edges
/// instead of raw rows means an execution with N audit rows
/// contributes ONE recursion path, not N — MAXRECURSION
/// bounds depth, not per-level width, so the raw-row form
/// could fan out badly. One edge per execution because all
/// rows of an execution share a single ParentExecutionId
/// (see the MIN(...) note on the tree walker's projection).
/// Chain : seeded at the root edge, recursively joins each edge whose
/// ParentExecutionId is an ExecutionId already in the chain.
/// Each edge carries its own ParentExecutionId, so the chain
/// of edges already surfaces every execution id in the tree
/// — including a row-less stub parent, which appears as the
/// ParentExecutionId of its child's edge. No separate
/// union-back CTE is needed.
/// </remarks>
private static string ChainCtePrefix(string edgesWindowPredicate) => $@"
WITH Edges AS (
SELECT DISTINCT ExecutionId, ParentExecutionId
FROM dbo.AuditLog
@@ -1035,7 +1045,65 @@ VALUES
SELECT e.ExecutionId, e.ParentExecutionId
FROM Edges e
INNER JOIN Chain c ON e.ParentExecutionId = c.ExecutionId
),
)";
/// <summary>
/// Binds the parameters the <see cref="ChainCtePrefix"/> statement needs: <c>@root</c> always,
/// and the window bounds only when the anchor carries a root timestamp (matching
/// <see cref="ChainAnchor.EdgesWindowPredicate"/>).
/// </summary>
private static void AddChainParameters(DbCommand cmd, ChainAnchor anchor)
{
var pRoot = cmd.CreateParameter();
pRoot.ParameterName = "@root";
pRoot.Value = anchor.RootExecutionId;
cmd.Parameters.Add(pRoot);
if (anchor.RootFirstOccurred is { } rootFirst)
{
var pWinStart = cmd.CreateParameter();
pWinStart.ParameterName = "@winStart";
pWinStart.Value = rootFirst - ExecutionTreeWindowSlack;
cmd.Parameters.Add(pWinStart);
var pWinEnd = cmd.CreateParameter();
pWinEnd.ParameterName = "@winEnd";
pWinEnd.Value = rootFirst + ExecutionTreeMaxSpan;
cmd.Parameters.Add(pWinEnd);
}
}
/// <inheritdoc />
public async Task<IReadOnlyList<ExecutionTreeNode>> GetExecutionTreeAsync(
Guid executionId,
CancellationToken ct = default)
{
var conn = _context.Database.GetDbConnection();
var openedHere = false;
if (conn.State != System.Data.ConnectionState.Open)
{
await conn.OpenAsync(ct).ConfigureAwait(false);
openedHere = true;
}
try
{
// --- Phase 1 + window: shared with GetTraceEventsAsync (arch-review F9) ---
var anchor = await ResolveChainAnchorAsync(conn, executionId, ct).ConfigureAwait(false);
// --- Phase 2: walk down from the root via a recursive CTE ---------
// The Edges/Chain prefix is ChainCtePrefix (see its remarks for the
// DISTINCT-edges trick); this walker appends:
// ChainIds: collect every distinct execution id reachable from the
// chain (each edge's ExecutionId plus its non-null
// ParentExecutionId), LEFT JOIN back to AuditLog and
// GROUP BY so a stub parent — which owns no edge of its own
// because it emitted no rows — still surfaces as a node with
// RowCount 0 and NULL aggregates.
var nodes = new List<ExecutionTreeNode>();
await using (var downCmd = conn.CreateCommand())
{
downCmd.CommandText = ChainCtePrefix(anchor.EdgesWindowPredicate) + $@",
ChainIds AS (
SELECT ExecutionId FROM Chain
UNION
@@ -1071,23 +1139,7 @@ VALUES
GROUP BY ids.ExecutionId
OPTION (MAXRECURSION {ExecutionChainMaxDepth});";
var pRoot = downCmd.CreateParameter();
pRoot.ParameterName = "@root";
pRoot.Value = rootExecutionId;
downCmd.Parameters.Add(pRoot);
if (rootFirstOccurred is { } rootFirst)
{
var pWinStart = downCmd.CreateParameter();
pWinStart.ParameterName = "@winStart";
pWinStart.Value = rootFirst - ExecutionTreeWindowSlack;
downCmd.Parameters.Add(pWinStart);
var pWinEnd = downCmd.CreateParameter();
pWinEnd.ParameterName = "@winEnd";
pWinEnd.Value = rootFirst + ExecutionTreeMaxSpan;
downCmd.Parameters.Add(pWinEnd);
}
AddChainParameters(downCmd, anchor);
await using var reader = await downCmd.ExecuteReaderAsync(ct).ConfigureAwait(false);
while (await reader.ReadAsync(ct).ConfigureAwait(false))
@@ -1147,77 +1199,18 @@ VALUES
var chainIds = new List<Guid>();
try
{
var rootExecutionId = executionId;
for (var depth = 0; depth < ExecutionChainMaxDepth; depth++)
{
Guid? parent;
await using (var upCmd = conn.CreateCommand())
{
upCmd.CommandText =
"SELECT TOP 1 ParentExecutionId FROM dbo.AuditLog " +
"WHERE ExecutionId = @cur AND ParentExecutionId IS NOT NULL;";
var pCur = upCmd.CreateParameter();
pCur.ParameterName = "@cur";
pCur.Value = rootExecutionId;
upCmd.Parameters.Add(pCur);
var result = await upCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
parent = result is null or DBNull ? null : (Guid)result;
}
if (parent is null) break;
rootExecutionId = parent.Value;
}
DateTime? rootFirstOccurred;
await using (var winCmd = conn.CreateCommand())
{
winCmd.CommandText = "SELECT MIN(OccurredAtUtc) FROM dbo.AuditLog WHERE ExecutionId = @root;";
var pWinRoot = winCmd.CreateParameter();
pWinRoot.ParameterName = "@root";
pWinRoot.Value = rootExecutionId;
winCmd.Parameters.Add(pWinRoot);
var winResult = await winCmd.ExecuteScalarAsync(ct).ConfigureAwait(false);
rootFirstOccurred = winResult is null or DBNull ? null : (DateTime)winResult;
}
var edgesWindowPredicate = rootFirstOccurred is null
? string.Empty
: "AND OccurredAtUtc >= @winStart AND OccurredAtUtc < @winEnd";
var anchor = await ResolveChainAnchorAsync(conn, executionId, ct).ConfigureAwait(false);
await using (var idsCmd = conn.CreateCommand())
{
idsCmd.CommandText = $@"
WITH Edges AS (
SELECT DISTINCT ExecutionId, ParentExecutionId
FROM dbo.AuditLog
WHERE ExecutionId IS NOT NULL
{edgesWindowPredicate}
),
Chain AS (
SELECT CAST(@root AS uniqueidentifier) AS ExecutionId,
CAST(NULL AS uniqueidentifier) AS ParentExecutionId
UNION ALL
SELECT e.ExecutionId, e.ParentExecutionId
FROM Edges e
INNER JOIN Chain c ON e.ParentExecutionId = c.ExecutionId
)
idsCmd.CommandText = ChainCtePrefix(anchor.EdgesWindowPredicate) + $@"
SELECT ExecutionId FROM Chain
UNION
SELECT ParentExecutionId FROM Chain WHERE ParentExecutionId IS NOT NULL
OPTION (MAXRECURSION {ExecutionChainMaxDepth});";
var pRoot = idsCmd.CreateParameter();
pRoot.ParameterName = "@root";
pRoot.Value = rootExecutionId;
idsCmd.Parameters.Add(pRoot);
if (rootFirstOccurred is { } rootFirst)
{
var pWinStart = idsCmd.CreateParameter();
pWinStart.ParameterName = "@winStart";
pWinStart.Value = rootFirst - ExecutionTreeWindowSlack;
idsCmd.Parameters.Add(pWinStart);
var pWinEnd = idsCmd.CreateParameter();
pWinEnd.ParameterName = "@winEnd";
pWinEnd.Value = rootFirst + ExecutionTreeMaxSpan;
idsCmd.Parameters.Add(pWinEnd);
}
AddChainParameters(idsCmd, anchor);
await using var reader = await idsCmd.ExecuteReaderAsync(ct).ConfigureAwait(false);
while (await reader.ReadAsync(ct).ConfigureAwait(false))
{
@@ -2,6 +2,7 @@ using System.Data.Common;
using System.Diagnostics;
using System.Globalization;
using System.Text;
using Microsoft.Extensions.Logging;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
@@ -70,6 +71,7 @@ public sealed class InboundDatabaseHelper
private readonly Guid? _parentExecutionId;
private readonly string? _actor;
private readonly Action<AuditEvent>? _traceSink;
private readonly ILogger? _logger;
/// <summary>
/// Initializes the helper.
@@ -83,6 +85,7 @@ public sealed class InboundDatabaseHelper
/// <param name="parentExecutionId">The spawning execution's id, if any (an inbound request is a root, so callers normally pass null).</param>
/// <param name="methodName">The executing API method's name; the row's <c>Actor</c> becomes <c>api:{methodName}</c>. Null = NULL actor.</param>
/// <param name="traceSink">Flow Debugger trace sink; receives the same event instance before the writer does. Null = none.</param>
/// <param name="logger">Arch-review F8: logs a Warning when the best-effort audit write faults; null = silent.</param>
// Trailing optional parameters: the original (gateway, ct, commandTimeout) shape stays
// source-compatible for every existing caller and test.
public InboundDatabaseHelper(
@@ -94,7 +97,8 @@ public sealed class InboundDatabaseHelper
Guid? executionId = null,
Guid? parentExecutionId = null,
string? methodName = null,
Action<AuditEvent>? traceSink = null)
Action<AuditEvent>? traceSink = null,
ILogger? logger = null)
{
_gateway = gateway;
_ct = ct;
@@ -107,6 +111,7 @@ public sealed class InboundDatabaseHelper
_parentExecutionId = parentExecutionId;
_actor = string.IsNullOrEmpty(methodName) ? null : "api:" + methodName;
_traceSink = traceSink;
_logger = logger;
}
/// <summary>First column of the first row converted to <typeparamref name="T"/> (default if no rows).</summary>
@@ -268,7 +273,11 @@ public sealed class InboundDatabaseHelper
var t = _auditWriter.WriteAsync(evt, CancellationToken.None);
if (!t.IsCompletedSuccessfully)
{
_ = t.ContinueWith(static _ => { }, TaskContinuationOptions.OnlyOnFaulted);
_ = t.ContinueWith(
x => _logger?.LogWarning(x.Exception, "Flow trace DbOutbound write failed for {Connection}", connectionName),
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
}
catch (Exception)
@@ -4,6 +4,8 @@ using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Scripting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.ScadaBridge.AuditLog.Configuration;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
@@ -397,7 +399,8 @@ public class InboundScriptExecutor
executionId: parentExecutionId,
parentExecutionId: null,
methodName: method.Name,
traceSink: route.TraceSink);
traceSink: route.TraceSink,
logger: _logger);
// Bind the route helper to the method deadline so a
// routed Route.To(...).Call(...) inherits the method-level timeout
@@ -411,12 +414,23 @@ public class InboundScriptExecutor
// Also bind the inbound request's ExecutionId (ParentExecutionId) so a
// routed call carries it as ParentExecutionId — the
// spawned site script execution points back at this inbound request.
// Arch-review F11: the always-on RouteCall row must honour the method's
// SkipBodyCapture override — the same per-target lookup AuditWriteMiddleware
// performs for the inbound row (AuditWriteMiddleware.cs:183-187) — so an
// operator who suppressed a method's body from the audit log doesn't get it
// back via the routed call's arguments (derived from that same body).
var auditOptions = scope?.ServiceProvider.GetService<IOptionsMonitor<AuditLogOptions>>()?.CurrentValue;
var skipBodyCapture = auditOptions != null
&& auditOptions.PerTargetOverrides.TryGetValue(method.Name, out var perTargetOverride)
&& perTargetOverride.SkipBodyCapture;
var context = new InboundScriptContext(
parameters,
route.WithDeadline(cts.Token)
.WithRequestAborted(cancellationToken)
.WithParentExecutionId(parentExecutionId)
.WithTrace(trace),
.WithTrace(trace)
.WithBodyCapture(!skipBodyCapture),
dbHelper,
cts.Token);
@@ -506,6 +520,23 @@ public class InboundScriptExecutor
_logger.LogDebug("Inbound API request for method {Method} cancelled by client", method.Name);
return new InboundScriptResult(false, null, "Request cancelled by client");
}
catch (RouteSiteScopeException ex)
{
// Arch-review F2: the script routed to a site outside the caller's site
// scope. RouteTarget.ResolveSiteAsync refused BEFORE any site traffic, so
// the script never saw a routed reply — there is nothing to roll back.
// The message is the operator-facing denial, safe to surface verbatim
// (it names only the instance and its site), unlike the generic
// SCRIPT_ERROR path which deliberately hides internals.
//
// Ordered before the SiteUnreachableException catch purely for
// readability: the two types are siblings under InvalidOperationException,
// so neither shadows the other.
_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");
}
catch (SiteUnreachableException)
{
// C3: a routed Route.To(...).Call(...) reached a site that could not be
@@ -1,6 +1,7 @@
using System.Diagnostics;
using System.Text.Json;
using Akka.Actor;
using Microsoft.Extensions.Logging;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
@@ -36,6 +37,34 @@ public sealed class SiteUnreachableException : InvalidOperationException
public SiteUnreachableException(string message, Exception innerException) : base(message, innerException) { }
}
/// <summary>
/// Arch-review F2: a routed call whose target site is outside the caller's site
/// scope. Raised by <c>RouteTarget.ResolveSiteAsync</c> — the single choke point every routed
/// operation passes through — so it is thrown <b>before any site traffic</b>: nothing is sent,
/// nothing is read, nothing is written.
///
/// <para>
/// The scope exists for the Flow Debugger's method-entry Live run, where an operator picks a
/// method rather than a site: a site-scoped Deployer must not reach outside their sites just
/// because the method's script routes there. The ordinary HTTP inbound path attaches no scope
/// (an API key is not a site-scoped principal) and is unaffected.
/// </para>
///
/// <para>
/// Derives from <see cref="InvalidOperationException"/> so existing callers catching the base
/// type are unaffected, while <see cref="InboundScriptExecutor"/> catches this more specific
/// type to surface the <c>SITE_SCOPE_DENIED</c> error code instead of a generic
/// <c>SCRIPT_ERROR</c>. Deliberately NOT the ManagementService's <c>SiteScopeViolationException</c>
/// — InboundAPI cannot reference ManagementService; the runner converts the code back.
/// </para>
/// </summary>
public sealed class RouteSiteScopeException : InvalidOperationException
{
/// <summary>Initializes a new <see cref="RouteSiteScopeException"/> with the given message.</summary>
/// <param name="message">The scope-denial message naming the instance and its site.</param>
public RouteSiteScopeException(string message) : base(message) { }
}
/// <summary>
/// Route.To() helper for cross-site calls from inbound API scripts.
/// Resolves instance to site, routes via <see cref="IInstanceRouter"/>, blocks until
@@ -57,6 +86,9 @@ public class RouteHelper
private readonly INodeIdentityProvider? _nodeIdentity;
private readonly bool _trace;
private readonly Action<AuditEvent>? _traceSink;
private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
private readonly bool _captureBodies;
private readonly ILogger? _logger;
/// <summary>
/// Initializes a new <see cref="RouteHelper"/> with no deadline token and no parent execution id.
@@ -67,13 +99,15 @@ public class RouteHelper
/// <param name="instanceRouter">Service to route cross-site calls to the resolved site.</param>
/// <param name="auditWriter">Flow Debugger (design 2026-09-05 §5.1): central-direct-write audit writer for the always-on <c>RouteCall</c> row; null = no row.</param>
/// <param name="nodeIdentity">Stamps <c>SourceNode</c> (central-a/central-b) on the <c>RouteCall</c> row; null = NULL.</param>
/// <param name="logger">Arch-review F8: logs a Warning when the best-effort <c>RouteCall</c> audit write faults; null = silent.</param>
public RouteHelper(
IInstanceLocator instanceLocator,
IInstanceRouter instanceRouter,
ICentralAuditWriter? auditWriter = null,
INodeIdentityProvider? nodeIdentity = null)
INodeIdentityProvider? nodeIdentity = null,
ILogger<RouteHelper>? logger = null)
: this(instanceLocator, instanceRouter, CancellationToken.None, CancellationToken.None, parentExecutionId: null,
auditWriter, nodeIdentity, trace: false, traceSink: null)
auditWriter, nodeIdentity, trace: false, traceSink: null, permittedSiteIdentifiers: null, captureBodies: true, logger)
{
}
@@ -86,8 +120,12 @@ public class RouteHelper
ICentralAuditWriter? auditWriter,
INodeIdentityProvider? nodeIdentity,
bool trace,
Action<AuditEvent>? traceSink)
Action<AuditEvent>? traceSink,
IReadOnlySet<string>? permittedSiteIdentifiers,
bool captureBodies,
ILogger? logger)
{
_permittedSiteIdentifiers = permittedSiteIdentifiers;
_instanceLocator = instanceLocator;
_instanceRouter = instanceRouter;
_deadlineToken = deadlineToken;
@@ -97,6 +135,8 @@ public class RouteHelper
_nodeIdentity = nodeIdentity;
_trace = trace;
_traceSink = traceSink;
_captureBodies = captureBodies;
_logger = logger;
}
/// <summary>
@@ -110,7 +150,7 @@ public class RouteHelper
/// <returns>A new <see cref="RouteHelper"/> inheriting the given deadline token.</returns>
public RouteHelper WithDeadline(CancellationToken deadlineToken) =>
new(_instanceLocator, _instanceRouter, deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink);
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Returns a <see cref="RouteHelper"/> carrying the raw request-abort
@@ -124,7 +164,7 @@ public class RouteHelper
/// <returns>A new <see cref="RouteHelper"/> carrying the given request-abort token.</returns>
public RouteHelper WithRequestAborted(CancellationToken requestAbortedToken) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink);
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Audit Log (ParentExecutionId): returns a <see cref="RouteHelper"/> whose
@@ -139,7 +179,7 @@ public class RouteHelper
/// <returns>A new <see cref="RouteHelper"/> carrying the given parent execution id.</returns>
public RouteHelper WithParentExecutionId(Guid? parentExecutionId) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink);
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Flow Debugger (design 2026-09-05 §5.2): returns a <see cref="RouteHelper"/> whose routed
@@ -150,7 +190,7 @@ public class RouteHelper
/// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
public RouteHelper WithTrace(bool trace) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, trace, _traceSink);
_auditWriter, _nodeIdentity, trace, _traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Flow Debugger (design 2026-09-05 §7.1): returns a <see cref="RouteHelper"/> that also
@@ -162,7 +202,7 @@ public class RouteHelper
/// <returns>A new <see cref="RouteHelper"/> carrying the sink.</returns>
public RouteHelper WithTraceSink(Action<AuditEvent>? traceSink) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, traceSink);
_auditWriter, _nodeIdentity, _trace, traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Flow Debugger outbound calls (plan 2026-09-06, D5/D7): the trace sink this helper carries,
@@ -172,6 +212,47 @@ public class RouteHelper
/// </summary>
public Action<AuditEvent>? TraceSink => _traceSink;
/// <summary>
/// Arch-review F2: returns a <see cref="RouteHelper"/> that refuses a routed call
/// whose target site is not in <paramref name="permittedSiteIdentifiers"/> — site
/// <em>identifiers</em> (e.g. <c>site-a</c>), the values <see cref="IInstanceLocator"/> returns,
/// NOT the numeric ids an <c>AuthenticatedUser.PermittedSiteIds</c> carries. Matching is
/// whatever comparer the supplied set uses, so pass an ordinal-ignore-case set.
///
/// <para>
/// <c>null</c> = unscoped, the default and the state of every ordinary inbound HTTP request.
/// An EMPTY set is not the same thing: it permits nothing, which is the correct outcome for
/// a site-scoped user whose sites no longer exist.
/// </para>
///
/// <para>
/// The Flow Debugger's method-entry Live run sets this from the operator's site-scoped
/// Deployer role, because the operator picks a <em>method</em> — the site is only known once
/// the script routes, so the check cannot happen up front the way it does for a value-change
/// run's named instance.
/// </para>
/// </summary>
/// <param name="permittedSiteIdentifiers">The site identifiers routed calls may reach; null = unscoped.</param>
/// <returns>A new <see cref="RouteHelper"/> carrying the scope.</returns>
public RouteHelper WithSiteScope(IReadOnlySet<string>? permittedSiteIdentifiers) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink, permittedSiteIdentifiers, _captureBodies, _logger);
/// <summary>
/// Arch-review F11: returns a <see cref="RouteHelper"/> whose always-on <c>RouteCall</c> row
/// carries no request/response summary when <paramref name="captureBodies"/> is <c>false</c>.
/// <see cref="InboundScriptExecutor"/> sets this from the executing method's
/// <c>AuditLogOptions.PerTargetOverrides[method.Name].SkipBodyCapture</c> — the same override
/// <c>AuditWriteMiddleware</c> honours for the inbound row — so an operator who suppressed a
/// method's body from the audit log doesn't get it back via the routed call's arguments
/// (which are derived from that same body). Default <c>true</c> (unchanged behaviour).
/// </summary>
/// <param name="captureBodies">False to omit the request/response summary on the <c>RouteCall</c> row.</param>
/// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
public RouteHelper WithBodyCapture(bool captureBodies) =>
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, captureBodies, _logger);
/// <summary>
/// Creates a route target for the specified instance.
/// </summary>
@@ -181,7 +262,7 @@ public class RouteHelper
{
return new RouteTarget(
instanceCode, _instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
_auditWriter, _nodeIdentity, _trace, _traceSink);
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers, _captureBodies, _logger);
}
}
@@ -200,6 +281,9 @@ public class RouteTarget
private readonly INodeIdentityProvider? _nodeIdentity;
private readonly bool _trace;
private readonly Action<AuditEvent>? _traceSink;
private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
private readonly bool _captureBodies;
private readonly ILogger? _logger;
/// <summary>
/// Initializes a new <see cref="RouteTarget"/> for the specified instance.
@@ -210,6 +294,13 @@ public class RouteTarget
/// <param name="deadlineToken">Cancellation token representing the method-level deadline.</param>
/// <param name="requestAbortedToken">Raw client-disconnect token, independent of the method timeout.</param>
/// <param name="parentExecutionId">Optional parent execution id for audit correlation on routed calls.</param>
/// <param name="auditWriter">Central-direct-write audit writer for the always-on <c>RouteCall</c> row; null = no row.</param>
/// <param name="nodeIdentity">Stamps <c>SourceNode</c> on the <c>RouteCall</c> row; null = NULL.</param>
/// <param name="trace">True to trace the routed site run.</param>
/// <param name="traceSink">Receives the emitted audit events in order; null = none.</param>
/// <param name="permittedSiteIdentifiers">Arch-review F2: site identifiers this call may reach; null = unscoped.</param>
/// <param name="captureBodies">Arch-review F11: false omits the request/response summary on the <c>RouteCall</c> row.</param>
/// <param name="logger">Arch-review F8: logs a Warning when the best-effort <c>RouteCall</c> audit write faults; null = silent.</param>
internal RouteTarget(
string instanceCode,
IInstanceLocator instanceLocator,
@@ -220,8 +311,12 @@ public class RouteTarget
ICentralAuditWriter? auditWriter = null,
INodeIdentityProvider? nodeIdentity = null,
bool trace = false,
Action<AuditEvent>? traceSink = null)
Action<AuditEvent>? traceSink = null,
IReadOnlySet<string>? permittedSiteIdentifiers = null,
bool captureBodies = true,
ILogger? logger = null)
{
_permittedSiteIdentifiers = permittedSiteIdentifiers;
_instanceCode = instanceCode;
_instanceLocator = instanceLocator;
_instanceRouter = instanceRouter;
@@ -232,6 +327,8 @@ public class RouteTarget
_nodeIdentity = nodeIdentity;
_trace = trace;
_traceSink = traceSink;
_captureBodies = captureBodies;
_logger = logger;
}
/// <summary>
@@ -320,8 +417,10 @@ public class RouteTarget
parentExecutionId: null,
durationMs: (int)Math.Min(elapsedMs, int.MaxValue),
errorMessage: error,
requestSummary: TraceJson(args),
responseSummary: TraceJson(returnValue),
// Arch-review F11: a method with SkipBodyCapture set must not leak the routed
// call's arguments (derived from the same request body) or its return value.
requestSummary: _captureBodies ? TraceJson(args) : null,
responseSummary: _captureBodies ? TraceJson(returnValue) : null,
extra: childExecutionId is null ? null : new ScriptTraceExtra { ChildExecutionId = childExecutionId }.ToJson());
}
catch (Exception)
@@ -336,7 +435,11 @@ public class RouteTarget
var t = _auditWriter.WriteAsync(evt, CancellationToken.None);
if (!t.IsCompletedSuccessfully)
{
_ = t.ContinueWith(static _ => { }, TaskContinuationOptions.OnlyOnFaulted);
_ = t.ContinueWith(
x => _logger?.LogWarning(x.Exception, "Flow trace RouteCall write failed for {Target}", $"{_instanceCode}.{scriptName}"),
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
}
catch (Exception)
@@ -549,6 +652,16 @@ public class RouteTarget
$"Instance '{_instanceCode}' not found or has no assigned site");
}
// Arch-review F2: the site scope gate. This method is the single choke point —
// Call, GetAttributes, WaitForAttribute and SetAttributes all resolve the site
// here first — so refusing at this point guarantees no site traffic of any kind
// is emitted for an out-of-scope instance.
if (_permittedSiteIdentifiers is not null && !_permittedSiteIdentifiers.Contains(siteId))
{
throw new RouteSiteScopeException(
$"Access denied: instance '{_instanceCode}' is at site '{siteId}', outside your site scope.");
}
return siteId;
}
@@ -1,4 +1,3 @@
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Text.Json;
using Microsoft.Extensions.DependencyInjection;
@@ -38,6 +37,14 @@ public sealed class FlowDebugRunner
/// <summary>Error code when the method / instance / site does not exist.</summary>
public const string NotFoundCode = "NOT_FOUND";
/// <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
/// reference this assembly, so it raises its own <c>RouteSiteScopeException</c> and the
/// runner converts the code back to <see cref="SiteScopeViolationException"/>.
/// </summary>
public const string SiteScopeDeniedCode = "SITE_SCOPE_DENIED";
/// <summary>The <c>Extra.source</c> value stamped on rows this runner synthesizes.</summary>
public const string SourceTag = "flow-debugger";
@@ -75,6 +82,14 @@ public sealed class FlowDebugRunner
/// Live run of an inbound API method (design §7.1): active-node gate, schema validation,
/// synthesized inbound row (actor = operator), traced execution through the real
/// <see cref="InboundScriptExecutor"/> with a trace-sink <see cref="RouteHelper"/>.
///
/// <para>
/// Arch-review F1: the sink is a <see cref="FlowTraceCollector"/> — the same collector the
/// site launcher fills — so the returned copy is REDACTED and capped by count and bytes,
/// and a Live result shows exactly the filtered rows a later Replay reads back. The
/// redactor is <c>AddAuditLog</c>'s <see cref="IAuditRedactor"/> singleton; a host that
/// registered none (a test composition root) collects raw but is still capped.
/// </para>
/// </summary>
public async Task<FlowDebugRunResult> RunMethodAsync(
IServiceProvider sp, FlowDebugRunMethodCommand cmd, AuthenticatedUser user, CancellationToken ct)
@@ -98,12 +113,21 @@ public sealed class FlowDebugRunner
}
var root = Guid.NewGuid();
var events = new ConcurrentQueue<AuditEvent>();
// One redactor for the whole run: the collector filters the emitted rows, and the runner
// filters the rows it synthesizes ITSELF (review follow-up) — the central writer redacts a
// private copy, so without this the caller's copy of the inbound row would be the raw one.
var redactor = sp.GetService<IAuditRedactor>();
var collector = new FlowTraceCollector(redactor: redactor);
var startedAt = DateTimeOffset.UtcNow;
var sw = Stopwatch.StartNew();
var timeout = TimeSpan.FromSeconds(Math.Max(1, method.TimeoutSeconds));
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(events.Enqueue);
// Arch-review F2: a method Live run has no single target site to check up front the
// way RunValueChangeAsync does — the operator picks a METHOD, and where it routes is
// only known once the script runs. So the operator's scope is pushed into the
// RouteHelper and enforced at route time, before any site traffic.
var scope = await ResolvePermittedSiteIdentifiersAsync(sp, user, ct).ConfigureAwait(false);
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(collector.Add).WithSiteScope(scope);
var executor = sp.GetRequiredService<InboundScriptExecutor>();
InboundScriptResult result;
@@ -124,6 +148,9 @@ public sealed class FlowDebugRunner
}
var finishedAt = DateTimeOffset.UtcNow;
// Close the sink here, not at the point the list is built: an un-awaited call finishing
// after the run returned must not mutate the snapshot the reply is assembled from.
var events = collector.Complete();
var requestJson = cmd.Parameters is { } p ? p.GetRawText() : null;
var inboundRow = ScadaBridgeAuditEventFactory.Create(
AuditChannel.ApiInbound, AuditKind.InboundRequest,
@@ -140,12 +167,26 @@ public sealed class FlowDebugRunner
extra: new ScriptTraceExtra { Source = SourceTag }.ToJson());
await WriteCentralAsync(sp, inboundRow, ct).ConfigureAwait(false);
var ordered = new List<AuditEvent>(events.Count + 1) { inboundRow };
ordered.AddRange(events.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId));
// Arch-review F2: convert the executor's code into the same authorization failure
// RunValueChangeAsync raises, so both Live entry points surface one exception type
// (→ ManagementUnauthorized). Deliberately AFTER the inbound row is written: a
// denied attempt must still be audited against the operator's name.
if (string.Equals(result.ErrorCode, SiteScopeDeniedCode, StringComparison.Ordinal))
{
throw new SiteScopeViolationException(
result.ErrorMessage ?? "Access denied: routed call outside your site scope.");
}
// Arch-review F5: the collector's emission order is authoritative — central rows carry central's
// clock and site rows the site's, so re-sorting by OccurredAtUtc lets a skewed site clock float
// its steps ahead of the RouteCall that started them. (The value-change path below keeps its
// OrderBy: every row it merges was stamped by the same site.)
var ordered = new List<AuditEvent>(events.Count + 1) { redactor?.Apply(inboundRow) ?? inboundRow };
ordered.AddRange(events);
return new FlowDebugRunResult(
root, result.Success, result.ErrorMessage, result.Success ? null : result.ErrorCode,
ordered, Truncated: false, startedAt, finishedAt);
ordered, Truncated: collector.Truncated, startedAt, finishedAt);
}
/// <summary>
@@ -173,6 +214,9 @@ public sealed class FlowDebugRunner
var root = Guid.NewGuid();
var startedAt = DateTimeOffset.UtcNow;
var sw = Stopwatch.StartNew();
// Review follow-up: the root row is synthesized here, so the copy handed back must be
// redacted by this method — WriteCentralAsync's writer only redacts its own private copy.
var redactor = sp.GetService<IAuditRedactor>();
// IInstanceRouter is the same seam RouteHelper uses (central binds it to the comms layer).
var router = sp.GetRequiredService<IInstanceRouter>();
var write = await router.RouteToSetAttributesAsync(
@@ -202,11 +246,12 @@ public sealed class FlowDebugRunner
Writes = [new AttributeWriteDetail(cmd.AttributeName, cmd.Value, write.Success, write.ErrorMessage)],
}.ToJson());
await WriteCentralAsync(sp, rootRow, ct).ConfigureAwait(false);
var redactedRoot = redactor?.Apply(rootRow) ?? rootRow;
if (!write.Success)
{
return new FlowDebugRunResult(root, false, write.ErrorMessage ?? "Attribute write failed", "WRITE_FAILED",
[rootRow], false, startedAt, DateTimeOffset.UtcNow);
[redactedRoot], false, startedAt, DateTimeOffset.UtcNow);
}
// Poll the site for the traced run(s) the write spawned — matched by ParentExecutionId
@@ -244,7 +289,7 @@ public sealed class FlowDebugRunner
await _delay(PollInterval, ct).ConfigureAwait(false);
}
var ordered = new List<AuditEvent> { rootRow };
var ordered = new List<AuditEvent> { redactedRoot };
ordered.AddRange(known.Values.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId));
var completed = known.Values.Any(e =>
{
@@ -301,6 +346,35 @@ public sealed class FlowDebugRunner
}
}
/// <summary>
/// Arch-review F2: the operator's <c>PermittedSiteIds</c> — NUMERIC site ids as strings —
/// resolved to the site IDENTIFIERS <see cref="IInstanceLocator"/> speaks, for
/// <c>RouteHelper.WithSiteScope</c>. Returns null (unscoped) for an unscoped user or an
/// Administrator, exactly as <see cref="EnforceSiteScope"/> treats them.
///
/// <para>
/// A returned set may be EMPTY — a scoped user whose sites no longer exist routes nowhere.
/// That is the intended fail-closed outcome, and is why null and empty are not the same.
/// </para>
/// </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;
}
private async Task WriteCentralAsync(IServiceProvider sp, AuditEvent evt, CancellationToken ct)
{
var writer = sp.GetService<ICentralAuditWriter>();
@@ -195,6 +195,11 @@ public class ManagementActor : ReceiveActor
private static readonly string[] OperatorOnly = [Roles.Operator];
private static readonly string[] VerifierOnly = [Roles.Verifier];
// arch-review F3: Flow Debugger Replay is an audit read, so it carries the same role
// set as the RequireFlowDebugger page policy ({Deployer, Administrator, Viewer}). The
// two Live run commands stay DeployerOnly.
private static readonly string[] FlowDebuggerReplayRoles = AuthorizationPolicies.FlowDebuggerAccessRoles;
// arch-review C6: area management is reconciled to any-of [Designer, Deployer].
// Areas are authored on both real surfaces — the Designer template/topology
// tooling AND the Central UI Deployment Topology workflow (Topology.razor is
@@ -305,9 +310,13 @@ public class ManagementActor : ReceiveActor
or RetryParkedMessageCommand or DiscardParkedMessageCommand
or DebugSnapshotCommand => DeployerOnly,
// Flow Debugger (design 2026-09-05 §9): Live runs execute scripts and write attributes
// on operator input, Replay reads the audit chain — Deployer, like the Debug View.
FlowDebugRunMethodCommand or FlowDebugRunValueChangeCommand or FlowDebugReplayCommand => DeployerOnly,
// Flow Debugger (design 2026-09-05 §9 / D6, arch-review F3). A Live run executes
// scripts and writes attributes on operator input, so it stays Deployer-only, like
// the Debug View. Replay only reads the audit chain, so it additionally accepts the
// OperationalAudit roles — the Audit Log / Execution Chain drill-ins are the
// feature's main entry for a past incident and an auditor must be able to use them.
FlowDebugRunMethodCommand or FlowDebugRunValueChangeCommand => DeployerOnly,
FlowDebugReplayCommand => FlowDebuggerReplayRoles,
// Two-person secured write. Submit is an Operator action;
// approve/reject are Verifier actions. Separation of duties (a write may
@@ -149,6 +149,33 @@ public static class AuthorizationPolicies
/// </remarks>
public static readonly string[] AuditExportRoles = { Roles.Administrator };
/// <summary>
/// Permission to OPEN the Flow Debugger page (arch-review F3 / design D6).
/// Replay only reads the audit chain, and the Audit Log / Execution Chain
/// pages — themselves gated on <see cref="OperationalAudit"/> — carry an
/// "Open in Flow Debugger" drill-in that is the feature's main entry for a
/// past incident, so an auditor must be able to follow it.
/// </summary>
/// <remarks>
/// This is page ACCESS only, and is deliberately NOT sufficient for a Live
/// run: the Run controls, the Live entry-point pickers and the parked-node
/// Retry/Discard actions are gated on <see cref="RequireDeployment"/> in the
/// page, and <c>FlowDebugRunMethodCommand</c> /
/// <c>FlowDebugRunValueChangeCommand</c> stay Deployer-only in the
/// ManagementActor role matrix. Only <c>FlowDebugReplayCommand</c> accepts
/// <see cref="FlowDebuggerAccessRoles"/>.
/// </remarks>
public const string RequireFlowDebugger = "RequireFlowDebugger";
/// <summary>
/// Roles that satisfy <see cref="RequireFlowDebugger"/> — the
/// <see cref="OperationalAuditRoles"/> plus <c>Deployer</c>. Held in one
/// place so the policy, the page and the ManagementActor's
/// <c>FlowDebugReplayCommand</c> gate stay in lockstep.
/// </summary>
public static readonly string[] FlowDebuggerAccessRoles =
{ Roles.Deployer, Roles.Administrator, Roles.Viewer };
/// <summary>
/// Registers the ScadaBridge authorization policies (Admin, Design, Deployment, OperationalAudit, AuditExport).
/// </summary>
@@ -191,6 +218,12 @@ public static class AuthorizationPolicies
options.AddPolicy(AuditExport, policy =>
policy.RequireClaim(JwtTokenService.RoleClaimType, AuditExportRoles));
// Flow Debugger page access (arch-review F3): opening the page is a
// Replay-capable read; the Live run controls are separately gated on
// RequireDeployment inside the page.
options.AddPolicy(RequireFlowDebugger, policy =>
policy.RequireClaim(JwtTokenService.RoleClaimType, FlowDebuggerAccessRoles));
});
return services;
@@ -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
{
@@ -532,6 +532,17 @@ public class ScriptActor : ReceiveActor, IWithTimers
_logger.LogDebug(
"Script {Script} on {Instance}: skipping execution, min time between runs not elapsed ({Elapsed} < {Min})",
_scriptName, _instanceName, elapsed, _minTimeBetweenRuns.Value);
// Arch-review F7: a TRACED run refused here is otherwise invisible to the Flow
// Debugger, which then blames a missing trigger. Leave a Failed ScriptRun row
// naming the guard instead.
if (trace)
{
ScriptRunLauncher.EmitSuppressedRun(
_serviceProvider, _logger, _scriptName, _instanceName, parentExecutionId,
$"suppressed: MinTimeBetweenRuns ({_minTimeBetweenRuns.Value}) not elapsed " +
$"since the previous run ({elapsed} ago).",
triggerExtra);
}
return;
}
}
@@ -574,7 +585,7 @@ public class ScriptActor : ReceiveActor, IWithTimers
{
if (callDepth == 0 && _runsInFlight >= _options.MaxConcurrentRunsPerScript)
{
ShedRun(replyTo, correlationId);
ShedRun(replyTo, correlationId, parentExecutionId, trace, triggerExtra);
return;
}
@@ -649,8 +660,18 @@ public class ScriptActor : ReceiveActor, IWithTimers
/// an unrelated script's run) gets an explicit error so the caller fails fast rather than
/// hanging to its Ask timeout. Nested (<c>callDepth &gt; 0</c>) launches never reach here —
/// see <see cref="SpawnExecution"/>.
///
/// <para>Arch-review F7: a traced run shed here also leaves a Failed <c>ScriptRun</c> audit
/// row, so the Flow Debugger names the cap rather than reporting a missing trigger. The
/// trigger detail (<paramref name="triggerExtra"/>) rides along so the row still says which
/// attribute change would have fired the run.</para>
/// </summary>
private void ShedRun(IActorRef replyTo, string correlationId)
private void ShedRun(
IActorRef replyTo,
string correlationId,
Guid? parentExecutionId = null,
bool trace = false,
ScriptTraceExtra? triggerExtra = null)
{
_healthCollector?.IncrementScriptRunShed();
@@ -675,6 +696,15 @@ public class ScriptActor : ReceiveActor, IWithTimers
correlationId, false, null,
$"shed: {_runsInFlight} runs already in flight"));
}
if (trace)
{
ScriptRunLauncher.EmitSuppressedRun(
_serviceProvider, _logger, _scriptName, _instanceName, parentExecutionId,
$"shed: MaxConcurrentRunsPerScript reached — {_runsInFlight} runs already in " +
$"flight (cap {_options.MaxConcurrentRunsPerScript}).",
triggerExtra);
}
}
private void HandleExecutionCompleted(ScriptExecutionCompleted msg)
@@ -1,84 +0,0 @@
using ZB.MOM.WW.Audit;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// <summary>
/// Per-run sink for <c>ScriptTrace</c> audit events so a traced routed call can return them
/// inline on <c>ScriptCallResult.TraceEvents</c> (Flow Debugger design 2026-09-05 §5.4).
/// Lock-based: a run emits tens of events, not thousands. Beyond <see cref="DefaultMaxEvents"/>
/// the collector keeps the first N, sets <see cref="Truncated"/>, and the audit rows written
/// through <c>IAuditWriter</c> remain complete — Replay is never truncated, only the inline copy.
/// An untraced run holds <c>null</c> rather than an inert collector; callers null-check.
/// Outbound-calls addendum (plan 2026-09-06 D5): the <c>ApiOutbound</c> / <c>DbOutbound</c> /
/// <c>Notification</c> rows a run emits ride the same sink, so <see cref="Complete"/> closes it
/// when the launcher takes the reply snapshot — a lifecycle row the S&amp;F forwarder writes
/// minutes later, or an un-awaited call finishing after the run returned, must not mutate a
/// list the reply has already been built from. <see cref="Add"/> is a no-op once completed.
/// </summary>
public sealed class FlowTraceCollector
{
/// <summary>Default inline cap per run (design §5.4 <c>MaxTraceEventsPerRun</c>).</summary>
public const int DefaultMaxEvents = 500;
private readonly List<AuditEvent> _events = new();
private readonly object _gate = new();
private readonly int _max;
/// <summary>True once an <see cref="Add"/> was dropped because the cap was reached.</summary>
public bool Truncated { get; private set; }
/// <summary>
/// True once <see cref="Complete"/> ran — the run's reply snapshot has been taken and
/// later <see cref="Add"/> calls are ignored (they never count as <see cref="Truncated"/>).
/// </summary>
public bool Completed { get; private set; }
/// <summary>Creates a collector with the given inline cap (must be positive).</summary>
public FlowTraceCollector(int maxEvents = DefaultMaxEvents)
{
if (maxEvents <= 0) throw new ArgumentOutOfRangeException(nameof(maxEvents), "must be positive");
_max = maxEvents;
}
/// <summary>
/// Appends an event, or marks the collector truncated once the cap is reached. A no-op
/// after <see cref="Complete"/>.
/// </summary>
public void Add(AuditEvent evt)
{
ArgumentNullException.ThrowIfNull(evt);
lock (_gate)
{
if (Completed) return;
if (_events.Count >= _max) { Truncated = true; return; }
_events.Add(evt);
}
}
/// <summary>
/// Closes the collector and returns the final snapshot — the launcher calls this exactly
/// where it builds <c>ScriptCallResult.TraceEvents</c>. Idempotent: a second call returns
/// the same contents.
/// </summary>
public IReadOnlyList<AuditEvent> Complete()
{
lock (_gate)
{
Completed = true;
return _events.ToArray();
}
}
/// <summary>Current event count.</summary>
public int Count { get { lock (_gate) return _events.Count; } }
/// <summary>
/// Copies the collected events in emission order WITHOUT closing the collector — a diagnostic /
/// test-only peek. A reply path must use <see cref="Complete"/>, otherwise an un-awaited call
/// finishing after the reply could mutate the list the reply was built from.
/// </summary>
public IReadOnlyList<AuditEvent> Snapshot()
{
lock (_gate) return _events.ToArray();
}
}
@@ -247,7 +247,15 @@ internal static class ScriptRunLauncher
// caller to reply to — a trace-armed trigger run (replyTo == Nobody) writes every row to the
// audit writer for PullTraceEvents and would only keep a never-read list alive. The
// ScriptRun row itself is emitted below on every exit path of a traced run.
var collector = trace && !replyTo.IsNobody() ? new FlowTraceCollector() : null;
// Arch-review F1: the inline copy is redacted at the collector, so the collector carries
// the host's IAuditRedactor. That is a SINGLETON (AddAuditLog), resolved here from the
// ROOT provider for the same reason siteEventLogger below is — the traced ScriptRun row
// is emitted from the catch blocks too, including the shed-at-dequeue path that throws
// before the DI scope exists. Null in tests / hosts without AddAuditLog: raw, still capped.
var collector = trace && !replyTo.IsNobody()
? new FlowTraceCollector(
redactor: serviceProvider?.GetService<ZB.MOM.WW.Audit.IAuditRedactor>())
: null;
var startedUtc = DateTime.UtcNow;
ScriptRuntimeContext? context = null;
IAuditWriter? auditWriter = null;
@@ -507,6 +515,50 @@ internal static class ScriptRunLauncher
}
}
/// <summary>
/// Arch-review F7: a traced run that never launched (shed at the in-flight cap, or
/// suppressed by <c>MinTimeBetweenRuns</c>) still leaves a <c>Failed</c>
/// <c>ScriptTrace.ScriptRun</c> row, so the Flow Debugger explains the gap with the guard
/// that refused the run instead of reporting <c>NO_TRIGGER_WITHIN_WINDOW</c> — which reads
/// as "no value-change trigger on this attribute, or no device echo" and sends the operator
/// to check the wrong things.
///
/// <para>There is no collector: a shed run has no reply to ride on (an Ask caller gets the
/// shed <c>ScriptCallResult</c> from <c>ScriptActor.ShedRun</c>, not a trace list), so the
/// row goes to the audit writer only and reaches the debugger via <c>PullTraceEvents</c> /
/// Replay. The writer and identity providers come from a fresh scope because this runs on
/// the ACTOR thread, outside any launch scope. <c>elapsedMs</c> is 0 — the run never ran.
/// Best-effort and never throws: a no-op when there is no provider or no writer.</para>
/// </summary>
internal static void EmitSuppressedRun(
IServiceProvider? serviceProvider,
ILogger logger,
string scriptName,
string instanceName,
Guid? parentExecutionId,
string reason,
ScriptTraceExtra? triggerExtra)
{
if (serviceProvider is null) return;
try
{
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);
}
catch (Exception ex)
{
logger.LogWarning(ex, "Flow trace suppressed-run emission failed for {Script}", scriptName);
}
}
/// <summary>
/// Flow Debugger (design 2026-09-05 §5.1): emits the <c>ScriptTrace.ScriptRun</c> row for a
/// traced run — collector first (so the inline reply carries it), then the best-effort
@@ -544,7 +596,9 @@ internal static class ScriptRunLauncher
{
_ = t.ContinueWith(
x => logger.LogWarning(x.Exception, "Flow trace ScriptRun write failed for {Script}", scriptName),
TaskContinuationOptions.OnlyOnFaulted);
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
}
catch (Exception ex)
@@ -445,7 +445,9 @@ public class ScriptRuntimeContext
{
_ = t.ContinueWith(
x => _logger.LogWarning(x.Exception, "Flow trace write failed for {Kind}", kind),
TaskContinuationOptions.OnlyOnFaulted);
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
}
catch (Exception ex)
@@ -35,17 +35,41 @@ public class FlowDebugCommandsTests
var runMethod = group.Subcommands.Single(c => c.Name == "run-method");
Assert.Contains(runMethod.Options, o => o.Name == "--method-id" && o.Required);
Assert.Contains(runMethod.Options, o => o.Name == "--params" && !o.Required);
Assert.Contains(runMethod.Options, o => o.Name == "--timeout-seconds" && !o.Required);
Assert.DoesNotContain(runMethod.Options, o => o.Name == "--wait-seconds");
var runValueChange = group.Subcommands.Single(c => c.Name == "run-value-change");
Assert.Contains(runValueChange.Options, o => o.Name == "--instance-id" && o.Required);
Assert.Contains(runValueChange.Options, o => o.Name == "--attribute" && o.Required);
Assert.Contains(runValueChange.Options, o => o.Name == "--value" && o.Required);
Assert.Contains(runValueChange.Options, o => o.Name == "--wait-seconds" && !o.Required);
Assert.DoesNotContain(runValueChange.Options, o => o.Name == "--timeout-seconds");
var replay = group.Subcommands.Single(c => c.Name == "replay");
Assert.Contains(replay.Options, o => o.Name == "--execution-id" && o.Required);
}
[Fact]
public void RunMethod_TimeoutSecondsDefaultsTo120()
{
var group = FlowDebugCommands.Build(Url, Format, Username, Password);
var parse = group.Parse("run-method --method-id 4");
Assert.Empty(parse.Errors);
var timeoutOption = (Option<int>)parse.CommandResult.Command.Options.Single(o => o.Name == "--timeout-seconds");
Assert.Equal(120, parse.GetValue(timeoutOption));
}
[Fact]
public async Task RunMethod_TimeoutSecondsZero_IsRejectedWithInvalidArgument()
{
var group = FlowDebugCommands.Build(Url, Format, Username, Password);
var exitCode = await group.Parse("run-method --method-id 4 --timeout-seconds 0").InvokeAsync();
Assert.Equal(1, exitCode);
}
[Fact]
public void RunValueChange_WaitSecondsDefaultsTo45_MatchingTheCommandRecord()
{
@@ -88,7 +88,7 @@ public class FlowDebuggerDrillinTests : BunitContext
Assert.Contains("data-test=\"api-method-trace-all-calls\"", markup);
Assert.Contains("Trace all calls (Flow Debugger)", markup);
Assert.Contains("~7 audit rows per call", markup);
Assert.Contains("7–10 audit rows per call", markup);
// Load, update and create paths all carry the flag.
Assert.Contains("_traceAllCalls = _existing.TraceAllCalls;", markup);
Assert.Contains("_existing.TraceAllCalls = _traceAllCalls;", markup);
@@ -119,6 +119,73 @@ public class FlowDebuggerPageTests : BunitContext
Assert.NotNull(cut.Find("[data-test='flow-graph-empty']"));
Assert.Equal("Not run", cut.Find("[data-test='flow-state']").TextContent.Trim());
Assert.True(cut.Find("button[data-test='flow-run']").HasAttribute("disabled"));
// arch-review F3: a Deployer gets the whole surface — entry point, both modes, Run.
Assert.NotNull(cut.Find("button[data-test='flow-entry-method']"));
Assert.NotNull(cut.Find("button[data-test='flow-mode-live']"));
Assert.NotNull(cut.Find("button[data-test='flow-mode-replay']"));
}
// ---- F3: Replay opens to the OperationalAudit roles; Live stays Deployer-only ----------------
[Fact]
public void Renders_ForViewer_ReplayOnly_WithNoLiveControls()
{
// A Viewer holds OperationalAudit but not Deployer: the page opens (it is the
// Audit Log / Execution Chain drill-in target) pinned to Replay, with no way to
// start a Live run.
RegisterServices(role: "Viewer");
var cut = Render<FlowDebuggerPage>();
Assert.NotNull(cut.Find("[data-test='flow-toolbar']"));
Assert.NotNull(cut.Find("input[data-test='flow-execution-id']"));
Assert.NotNull(cut.Find("button[data-test='flow-load']"));
Assert.NotNull(cut.Find("[data-test='flow-replay-only']"));
Assert.Empty(cut.FindAll("button[data-test='flow-run']"));
Assert.Empty(cut.FindAll("button[data-test='flow-mode-live']"));
Assert.Empty(cut.FindAll("button[data-test='flow-entry-method']"));
Assert.Empty(cut.FindAll("button[data-test='flow-entry-value-change']"));
Assert.Empty(cut.FindAll("select[data-test='flow-method-select']"));
}
[Fact]
public void Viewer_LoadsAReplay_ButGetsNoRetryOrDiscardOnAParkedNode()
{
// Retry / Discard relay a command to the site — a Deployer act. The auditor sees
// the Parked node and its detail, but not the two action buttons.
RegisterServices(role: "Viewer");
var root = Guid.NewGuid();
var (parked, _, _) = ParkedThenResolved(root);
_flow.ReplayAsync(root, Arg.Any<CancellationToken>()).Returns(parked);
var nav = Services.GetRequiredService<NavigationManager>();
nav.NavigateTo($"/deployment/flow-debugger?executionId={root}");
var cut = Render<FlowDebuggerPage>();
cut.Instance.Delay = (_, _) => Task.CompletedTask;
cut.WaitForAssertion(() => Assert.Equal("Completed", cut.Find("[data-test='flow-state']").TextContent.Trim()));
cut.Find("g[data-test='flow-node'][data-kind='ApiCall']").Click();
Assert.Empty(cut.FindAll("button[data-test='flow-step-retry']"));
Assert.Empty(cut.FindAll("button[data-test='flow-step-discard']"));
_flow.Received(1).ReplayAsync(root, Arg.Any<CancellationToken>());
}
[Fact]
public void Viewer_DrillInWithAMethodId_DoesNotOpenTheLiveRunSurface()
{
// ?methodId= is a Live-run drill-in. For a non-Deployer it must not put the page
// into Live mode or surface the Run button.
RegisterServices(role: "Viewer");
var nav = Services.GetRequiredService<NavigationManager>();
nav.NavigateTo("/deployment/flow-debugger?methodId=4");
var cut = Render<FlowDebuggerPage>();
Assert.Empty(cut.FindAll("button[data-test='flow-run']"));
Assert.Empty(cut.FindAll("select[data-test='flow-method-select']"));
Assert.NotNull(cut.Find("input[data-test='flow-execution-id']"));
}
[Fact]
@@ -318,6 +318,24 @@ public sealed class FlowDebugServiceTests : IDisposable
Assert.Contains("Deployer", envelope.User.Roles);
}
[Theory]
[InlineData(180, 205)]
[InlineData(1, 30)]
public void RunAskTimeoutFor_IsMethodTimeoutPlusRunGracePlusClientSlack_FlooredAtMinRunAskTimeout(int timeoutSeconds, int expectedSeconds)
{
var timeout = FlowDebugService.RunAskTimeoutFor(timeoutSeconds);
Assert.Equal(TimeSpan.FromSeconds(expectedSeconds), timeout);
}
[Fact]
public void ValueChangeAskTimeoutFor_IsWaitSecondsPlusClientSlack()
{
var timeout = FlowDebugService.ValueChangeAskTimeoutFor(45);
Assert.Equal(TimeSpan.FromSeconds(60), timeout);
}
[Fact]
public async Task RunValueChange_AndReplay_SendTheirCommands()
{
@@ -193,6 +193,30 @@ public class FlowGraphViewTests : BunitContext
Assert.True(int.Parse(ghost.GetAttribute("data-column")!) > realMax);
}
[Fact]
public void Overlay_TwoPredictedRouteCalls_RealSecondCallMatchesByTitle_FirstIsTheGhost()
{
// The run only ever made the second of two predicted route calls (a branch skipped the first).
// Matching by kind alone would pair the real "B.Y" step with the first predicted node ("A.X")
// and leave "B.Y" as the ghost — wrong: "A.X" is the one that never ran.
var step = new FlowStep(Guid.NewGuid(), null, null, FlowStepKind.RouteCall, FlowLane.Central, "B.Y", null,
T0, 5, FlowStepOutcome.Ok, null, null, null, null, null, null, "B.Y", null);
var graph = new FlowGraph([step], [], false, false);
var predicted = new PredictedFlow(
[
new("n1", FlowStepKind.RouteCall, FlowLane.Central, "A.X", false, null),
new("n2", FlowStepKind.RouteCall, FlowLane.Central, "B.Y", false, null),
], []);
var layout = FlowGraphView.ComputeLayout(graph, predicted);
var real = Assert.Single(layout.Nodes, n => n.Step != null);
Assert.Equal("B.Y", real.Predicted?.Title);
var ghost = Assert.Single(layout.Nodes, n => n.Ghost);
Assert.Equal("p:n1", ghost.Key);
Assert.Equal("A.X", ghost.Predicted?.Title);
}
[Fact]
public void Render_UnderGermanCulture_CoordinatesStayInvariant()
{
@@ -46,8 +46,10 @@ public class FlowGraphBuilderTests
executionId: Cascade, parentExecutionId: SiteRun, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 10,
extra: new ScriptTraceExtra { TriggerKind = "valuechange", TriggerAttribute = "DelmiaReceiver.Trigger", TriggerValue = "false" }.ToJson());
// Deliberately shuffled: the builder must order by time.
return [wait, cascade, inbound, write2, run, route, write1];
// Deliberately shuffled WITHIN each execution: the builder must order by time inside one execution.
// The executions themselves are given in emission order (root, site run, cascade) — since arch-review
// F5 that is what fixes their relative order, because they do not share a clock.
return [inbound, route, wait, run, write2, write1, cascade];
}
[Fact]
@@ -365,6 +367,130 @@ public class FlowGraphBuilderTests
Assert.Equal(graph.Steps[1].EventId, edge.ToEventId);
}
// ---- Arch-review F5: emission order between executions, timestamps within one ------------
/// <summary>
/// Central and site clocks differ. The four rows are given in EMISSION order (the Live path's
/// collector order); the graph must keep the executions in that order whichever way the skew runs.
/// </summary>
private static List<AuditEvent> SkewedRouteShape(int siteSkewMs)
{
var inbound = ScadaBridgeAuditEventFactory.Create(AuditChannel.ApiInbound, AuditKind.InboundRequest, AuditStatus.Delivered,
occurredAtUtc: T0, actor: "k", target: "M", sourceNode: "central-a", executionId: Root, durationMs: 900,
requestSummary: "{}", responseSummary: "{\"ok\":true}");
var route = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.RouteCall, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(50), target: "M1.S", sourceNode: "central-a", executionId: Root,
extra: new ScriptTraceExtra { ChildExecutionId = SiteRun }.ToJson());
var run = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(siteSkewMs), actor: "S", target: "S", sourceNode: "node-a",
executionId: SiteRun, parentExecutionId: Root, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 20,
extra: new ScriptTraceExtra { TriggerKind = "call" }.ToJson());
var write = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.AttributeWrite, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(siteSkewMs + 10), actor: "S", target: "A", sourceNode: "node-a",
executionId: SiteRun, parentExecutionId: Root, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 2,
extra: new ScriptTraceExtra { Writes = [new AttributeWriteDetail("A", "1", true, null)] }.ToJson());
return [inbound, route, run, write];
}
[Fact]
public void SiteClockAhead_SiteStepsStillFollowTheirRouteCall_AndPrecedeResponse()
{
var events = SkewedRouteShape(siteSkewMs: 2000); // site 2 s ahead of central
var graph = FlowGraphBuilder.Build(events);
Assert.Equal(
new[]
{
FlowStepKind.InboundRequest, FlowStepKind.RouteCall, FlowStepKind.ScriptRun,
FlowStepKind.AttributeWrite, FlowStepKind.Response,
},
graph.Steps.Select(s => s.Kind));
Assert.Equal(events.Select(e => e.EventId), graph.Steps.Take(4).Select(s => s.EventId));
// The Response time still comes off the inbound row's own clock (inbound + DurationMs).
Assert.Equal(T0.AddMilliseconds(900), graph.Steps[4].OccurredAtUtc.UtcDateTime);
}
[Fact]
public void SiteClockBehind_SiteStepsDoNotSortAheadOfTheInboundRequest()
{
var events = SkewedRouteShape(siteSkewMs: -2000); // site 2 s behind central
var graph = FlowGraphBuilder.Build(events);
Assert.Equal(
new[]
{
FlowStepKind.InboundRequest, FlowStepKind.RouteCall, FlowStepKind.ScriptRun,
FlowStepKind.AttributeWrite, FlowStepKind.Response,
},
graph.Steps.Select(s => s.Kind));
Assert.Equal(events.Select(e => e.EventId), graph.Steps.Take(4).Select(s => s.EventId));
}
[Fact]
public void SiteClockBehind_TriggerCascade_SpawnsFromTheLastParentStep_NotTheFirst()
{
// Mixed-version shape: no RouteCall row, so the spawner is found by position, and a site
// clock behind central used to push every parent step "after" the child → parentChain[0].
var inbound = ScadaBridgeAuditEventFactory.Create(AuditChannel.ApiInbound, AuditKind.InboundRequest, AuditStatus.Delivered,
occurredAtUtc: T0, actor: "k", target: "M", sourceNode: "central-a", executionId: Root, durationMs: 900);
var centralDb = ScadaBridgeAuditEventFactory.Create(AuditChannel.DbOutbound, AuditKind.DbWrite, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(10), actor: "api:M", target: "CfgDb", sourceNode: "central-a", executionId: Root);
var run = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(-2000), actor: "S", target: "S", sourceNode: "node-a",
executionId: SiteRun, parentExecutionId: Root, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 20,
extra: new ScriptTraceExtra { TriggerKind = "call" }.ToJson());
var graph = FlowGraphBuilder.Build([inbound, centralDb, run]);
var spawn = Assert.Single(graph.Edges, e => e.Spawn);
Assert.Equal(centralDb.EventId, spawn.FromEventId); // the LAST parent step, not the inbound row
Assert.Equal(run.EventId, spawn.ToEventId);
}
[Fact]
public void SameClock_TriggerCascade_SpawnsFromTheWriteThatFiredIt_NotTheParentsLastStep()
{
// Review follow-up: executions are contiguous blocks, so "the last parent step BEFORE the child
// in display order" is always the parent's last step. Parent and child of a cascade share the
// SITE clock, so the write that fired the trigger is the last parent step at or before the child.
var run = SiteRunRow(0);
var writeA = WriteRow(10, "DelmiaReceiver.Trigger");
var wait = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.AttributeWait, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(20), actor: "S", target: "DelmiaReceiver.Processed", sourceNode: "node-a",
executionId: SiteRun, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 5,
extra: new ScriptTraceExtra { WaitAttribute = "DelmiaReceiver.Processed", WaitOutcome = "satisfied" }.ToJson());
var writeB = WriteRow(30, "DelmiaReceiver.Ack");
var cascade = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
occurredAtUtc: T0.AddMilliseconds(15), actor: "OnTrigger", target: "OnTrigger", sourceNode: "node-a",
executionId: Cascade, parentExecutionId: SiteRun, sourceSiteId: "site-a", sourceInstanceId: "M1", durationMs: 4,
extra: new ScriptTraceExtra { TriggerKind = "valuechange", TriggerAttribute = "DelmiaReceiver.Trigger" }.ToJson());
var graph = FlowGraphBuilder.Build([run, writeA, wait, writeB, cascade]);
var spawn = Assert.Single(graph.Edges, e => e.Spawn);
Assert.Equal(writeA.EventId, spawn.FromEventId);
Assert.Equal(cascade.EventId, spawn.ToEventId);
}
[Fact]
public void FoldedNode_TakesTheEmissionIndexOfItsEarliestEmittedRow()
{
// The folded step keeps the earliest-by-TIME row's id (the resolve here is emitted first),
// so its sequence must be the minimum input index over the rows folded into it.
var corr = Guid.NewGuid();
var resolve = Outbound(AuditChannel.ApiOutbound, AuditKind.CachedResolve, AuditStatus.Delivered, 4000, corr, "Delmia.Ack", response: "res");
var submit = Outbound(AuditChannel.ApiOutbound, AuditKind.CachedSubmit, AuditStatus.Submitted, 35, corr, "Delmia.Ack", request: "req");
var run = SiteRunRow(20, Cascade);
// Emission order interleaves the executions: the folded node's earliest row is index 0, so
// its execution leads — even though the id the folded step keeps (the submit) is index 2.
var graph = FlowGraphBuilder.Build([resolve, run, submit]);
var api = Assert.Single(graph.Steps, s => s.Kind == FlowStepKind.ApiCall);
Assert.Equal(submit.EventId, api.EventId);
Assert.Equal(new[] { FlowStepKind.ApiCall, FlowStepKind.ScriptRun }, graph.Steps.Select(s => s.Kind));
}
// ---- Review findings on Task 2 (2026-09-06) --------------------------------------------
[Fact]
@@ -1,7 +1,7 @@
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts;
namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Debugging;
/// <summary>Flow Debugger design 2026-09-05 §5.4: the per-run inline trace sink.</summary>
public class FlowTraceCollectorTests
@@ -85,6 +85,56 @@ public class FlowTraceCollectorTests
Assert.Equal(snapshot, c.Complete()); // idempotent
}
/// <summary>
/// Arch-review F1: the inline copy must be the SAME filtered record the writer persists —
/// the collector applies the redactor itself rather than handing on the raw event.
/// </summary>
[Fact]
public void Add_AppliesTheRedactor_SoTheInlineCopyIsTheFilteredEvent()
{
var c = new FlowTraceCollector(redactor: new UpperRedactor());
var evt = Evt(1) with { DetailsJson = "{\"secret\":\"abc\"}" };
c.Add(evt);
var stored = Assert.Single(c.Complete());
Assert.Equal("{\"SECRET\":\"ABC\"}", stored.DetailsJson);
Assert.NotSame(evt, stored);
}
/// <summary>
/// Arch-review F1: the 500-event cap alone does not bound the gRPC reply the inline copy
/// rides on — a byte budget over the redacted <c>DetailsJson</c> truncates first.
/// </summary>
[Fact]
public void Add_OverByteBudget_Truncates_KeepsEarlierEvents()
{
var c = new FlowTraceCollector(maxEvents: 500, maxBytes: 100);
var big = Evt(1) with { DetailsJson = new string('x', 80) };
var big2 = Evt(2) with { DetailsJson = new string('y', 80) };
c.Add(big);
c.Add(big2);
Assert.True(c.Truncated);
Assert.Single(c.Complete());
}
/// <summary>The byte budget has a documented default and is validated like the event cap.</summary>
[Fact]
public void DefaultByteBudget_Is1Mb_AndNonPositiveBudgetIsRejected()
{
Assert.Equal(1_048_576, FlowTraceCollector.DefaultMaxBytes);
Assert.Throws<ArgumentOutOfRangeException>(() => new FlowTraceCollector(maxBytes: 0));
}
/// <summary>Upper-cases <c>DetailsJson</c> so a test can see the filtered copy land.</summary>
private sealed class UpperRedactor : IAuditRedactor
{
public AuditEvent Apply(AuditEvent rawEvent)
=> rawEvent with { DetailsJson = rawEvent.DetailsJson?.ToUpperInvariant() };
}
private static AuditEvent NewEvent() => new()
{
EventId = Guid.NewGuid(),
@@ -1,6 +1,7 @@
using System.Data.Common;
using Microsoft.Data.Sqlite;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using NSubstitute;
using ZB.MOM.WW.Audit;
@@ -58,6 +59,26 @@ public class InboundDatabaseHelperAuditTests
=> throw new NotImplementedException();
}
/// <summary>
/// Arch-review F8: a minimal <see cref="ILogger"/> double that records every
/// Warning-level message, mirroring the <c>CountingLogger&lt;T&gt;</c> pattern in
/// <c>InboundScriptExecutorTests</c>.
/// </summary>
private sealed class RecordingLogger : ILogger
{
public List<(LogLevel Level, string Message, Exception? Exception)> Entries { get; } = new();
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
public bool IsEnabled(LogLevel logLevel) => true;
public void Log<TState>(
LogLevel logLevel, EventId eventId, TState state, Exception? exception,
Func<TState, Exception?, string> formatter)
{
Entries.Add((logLevel, formatter(state, exception), exception));
}
}
private static SqliteGateway SeededGateway() => new();
private readonly List<AuditEvent> _written = new();
@@ -166,6 +187,32 @@ public class InboundDatabaseHelperAuditTests
Assert.Equal(1, n2);
}
[Fact]
public async Task AuditWriterFaulting_LogsWarning_AndCallStillSucceeds()
{
// Arch-review F8: the best-effort continuation used to be `static _ => { }` — a
// faulting write vanished with no trace anywhere. It now logs a Warning (and runs
// on TaskScheduler.Default rather than riding whatever context completed the
// audit-writer task), while the call itself still succeeds either way.
var faultedTask = Substitute.For<ICentralAuditWriter>();
faultedTask.WriteAsync(Arg.Any<AuditEvent>(), Arg.Any<CancellationToken>())
.Returns(Task.FromException(new InvalidOperationException("audit down")));
var logger = new RecordingLogger();
var helper = new InboundDatabaseHelper(
SeededGateway(), CancellationToken.None, TimeSpan.Zero,
auditWriter: faultedTask, nodeIdentity: _node,
executionId: _exec, parentExecutionId: _parent, methodName: "movein",
traceSink: null, logger: logger);
var n = await helper.ExecuteAsync("MesDb", "UPDATE Machine SET Code=Code", null);
Assert.Equal(1, n);
var entry = Assert.Single(logger.Entries);
Assert.Equal(LogLevel.Warning, entry.Level);
Assert.NotNull(entry.Exception);
Assert.Contains("MesDb", entry.Message);
}
[Fact]
public async Task TraceSink_ReceivesTheSameInstance_BeforeTheWriter()
{
@@ -4,10 +4,13 @@ using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using NSubstitute;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.AuditLog.Configuration;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Types;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests;
@@ -57,6 +60,77 @@ public class InboundScriptExecutorTests
Assert.False(captured[1].Trace);
}
[Fact]
public async Task Executor_MethodWithSkipBodyCapture_RoutesWithBodyCaptureOff()
{
// Arch-review F11: a method configured with PerTargetOverrides[method].SkipBodyCapture
// must build its Route helper WithBodyCapture(false), so the always-on RouteCall row
// carries no request/response summary — mirroring AuditWriteMiddleware's own
// SkipBodyCapture lookup for the inbound row (AuditWriteMiddleware.cs:183-187).
var locator = Substitute.For<IInstanceLocator>();
var router = Substitute.For<IInstanceRouter>();
locator.GetSiteIdForInstanceAsync("inst", Arg.Any<CancellationToken>()).Returns("SiteA");
router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(((RouteToCallRequest)ci[1]).CorrelationId, true, 1, null, DateTimeOffset.UtcNow));
var writer = Substitute.For<ICentralAuditWriter>();
AuditEvent? written = null;
writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
var route = new RouteHelper(locator, router, writer, null);
var services = new ServiceCollection();
services.Configure<AuditLogOptions>(o =>
o.PerTargetOverrides["routed"] = new PerTargetRedactionOverride { SkipBodyCapture = true });
using var provider = services.BuildServiceProvider();
var executor = new InboundScriptExecutor(NullLogger<InboundScriptExecutor>.Instance, provider);
var method = new ApiMethod("routed", "x") { Id = 9, TimeoutSeconds = 10 };
executor.RegisterHandler("routed", ctx => ctx.Route.To("inst").Call("work", new { x = 1 }));
var result = await executor.ExecuteAsync(method, new Dictionary<string, object?>(), route, TimeSpan.FromSeconds(10));
Assert.True(result.Success, result.ErrorMessage);
Assert.NotNull(written);
var row = written!.AsRow();
Assert.Null(row.RequestSummary);
Assert.Null(row.ResponseSummary);
}
[Fact]
public async Task Executor_MethodWithoutSkipBodyCapture_RoutesWithBodyCaptureOn()
{
// Control: no PerTargetOverrides entry for this method → default behaviour unchanged.
var locator = Substitute.For<IInstanceLocator>();
var router = Substitute.For<IInstanceRouter>();
locator.GetSiteIdForInstanceAsync("inst", Arg.Any<CancellationToken>()).Returns("SiteA");
router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(((RouteToCallRequest)ci[1]).CorrelationId, true, 1, null, DateTimeOffset.UtcNow));
var writer = Substitute.For<ICentralAuditWriter>();
AuditEvent? written = null;
writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
var route = new RouteHelper(locator, router, writer, null);
var services = new ServiceCollection();
services.Configure<AuditLogOptions>(o =>
o.PerTargetOverrides["other-method"] = new PerTargetRedactionOverride { SkipBodyCapture = true });
using var provider = services.BuildServiceProvider();
var executor = new InboundScriptExecutor(NullLogger<InboundScriptExecutor>.Instance, provider);
var method = new ApiMethod("routed", "x") { Id = 9, TimeoutSeconds = 10 };
executor.RegisterHandler("routed", ctx => ctx.Route.To("inst").Call("work", new { x = 1 }));
var result = await executor.ExecuteAsync(method, new Dictionary<string, object?>(), route, TimeSpan.FromSeconds(10));
Assert.True(result.Success, result.ErrorMessage);
Assert.NotNull(written);
var row = written!.AsRow();
Assert.Contains("\"x\":1", row.RequestSummary);
Assert.Equal("1", row.ResponseSummary);
}
[Fact]
public async Task RegisteredHandler_ExecutesSuccessfully()
{
@@ -1,7 +1,9 @@
using NSubstitute;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Types;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests;
@@ -621,4 +623,163 @@ public class RouteHelperTests
await Assert.ThrowsAsync<SiteUnreachableException>(
() => CreateHelper().To("inst").GetAttributes(new[] { "a" }));
}
// --- Arch-review F2: site scope on routed calls ---
//
// The gate lives in RouteTarget.ResolveSiteAsync — the single choke point every
// routed operation passes through — so it fires BEFORE any site traffic.
private static IReadOnlySet<string> Scope(params string[] siteIdentifiers) =>
new HashSet<string>(siteIdentifiers, StringComparer.OrdinalIgnoreCase);
[Fact]
public async Task Call_OutsideSiteScope_ThrowsRouteSiteScopeException_BeforeRouting()
{
SiteResolves("inst-1", "site-b");
var helper = CreateHelper().WithSiteScope(Scope("site-a"));
var ex = await Assert.ThrowsAsync<RouteSiteScopeException>(
() => helper.To("inst-1").Call("doWork"));
Assert.Contains("site-b", ex.Message);
Assert.Contains("inst-1", ex.Message);
// Nothing was routed: the check runs before the site is contacted.
await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default);
}
[Fact]
public async Task Call_InsideSiteScope_Routes()
{
SiteResolves("inst-1", "site-b");
_router.RouteToCallAsync("site-b", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 42, null, DateTimeOffset.UtcNow));
var helper = CreateHelper().WithSiteScope(Scope("SITE-B"));
Assert.Equal(42, await helper.To("inst-1").Call("doWork"));
}
[Fact]
public async Task Call_NullScope_IsUnscoped()
{
SiteResolves("inst-1", "site-b");
_router.RouteToCallAsync("site-b", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 42, null, DateTimeOffset.UtcNow));
var helper = CreateHelper().WithSiteScope(null);
Assert.Equal(42, await helper.To("inst-1").Call("doWork"));
}
[Fact]
public async Task SetAttributes_OutsideSiteScope_ThrowsRouteSiteScopeException_BeforeRouting()
{
// ResolveSiteAsync is shared by all four routed operations, so the gate is
// not Call-specific.
SiteResolves("inst-1", "site-b");
var helper = CreateHelper().WithSiteScope(Scope("site-a"));
await Assert.ThrowsAsync<RouteSiteScopeException>(
() => helper.To("inst-1").SetAttributes(new Dictionary<string, string> { ["x"] = "1" }));
await _router.DidNotReceiveWithAnyArgs().RouteToSetAttributesAsync(default!, default!, default);
}
[Fact]
public async Task WithSiteScope_SurvivesTheOtherWithCopies()
{
// The scope must not be dropped by the copies InboundScriptExecutor makes
// (WithDeadline / WithRequestAborted / WithParentExecutionId / WithTrace).
SiteResolves("inst-1", "site-b");
var helper = CreateHelper()
.WithSiteScope(Scope("site-a"))
.WithDeadline(CancellationToken.None)
.WithRequestAborted(CancellationToken.None)
.WithParentExecutionId(Guid.NewGuid())
.WithTrace(true)
.WithTraceSink(null);
await Assert.ThrowsAsync<RouteSiteScopeException>(
() => helper.To("inst-1").Call("doWork"));
}
// --- Arch-review F11: WithBodyCapture ---
[Fact]
public async Task RouteCallRow_WithBodyCaptureOff_HasNoRequestOrResponseSummary()
{
SiteResolves("inst-1", "SiteA");
_router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
var writer = Substitute.For<ICentralAuditWriter>();
AuditEvent? written = null;
writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
var helper = new RouteHelper(_locator, _router, writer, null).WithBodyCapture(false);
var result = await helper.To("inst-1").Call("doWork", new { x = 1 });
Assert.Equal(99, result);
Assert.NotNull(written);
var row = written!.AsRow();
Assert.Null(row.RequestSummary);
Assert.Null(row.ResponseSummary);
}
[Fact]
public async Task RouteCallRow_WithBodyCaptureOn_ByDefault_HasRequestAndResponseSummary()
{
SiteResolves("inst-1", "SiteA");
_router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
var writer = Substitute.For<ICentralAuditWriter>();
AuditEvent? written = null;
writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
var helper = new RouteHelper(_locator, _router, writer, null);
await helper.To("inst-1").Call("doWork", new { x = 1 });
Assert.NotNull(written);
var row = written!.AsRow();
Assert.Contains("\"x\":1", row.RequestSummary);
Assert.Equal("99", row.ResponseSummary);
}
[Fact]
public async Task WithBodyCapture_SurvivesTheOtherWithCopies()
{
// Mirrors WithSiteScope_SurvivesTheOtherWithCopies (Task 3): the flag must not be
// dropped by the copies InboundScriptExecutor makes (WithDeadline /
// WithRequestAborted / WithParentExecutionId / WithTrace / WithSiteScope).
SiteResolves("inst-1", "SiteA");
_router.RouteToCallAsync("SiteA", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow));
var writer = Substitute.For<ICentralAuditWriter>();
AuditEvent? written = null;
writer.WriteAsync(Arg.Do<AuditEvent>(e => written = e), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
var helper = new RouteHelper(_locator, _router, writer, null)
.WithBodyCapture(false)
.WithDeadline(CancellationToken.None)
.WithRequestAborted(CancellationToken.None)
.WithParentExecutionId(Guid.NewGuid())
.WithTrace(true)
.WithTraceSink(null)
.WithSiteScope(Scope("SiteA"));
await helper.To("inst-1").Call("doWork", new { x = 1 });
Assert.NotNull(written);
var row = written!.AsRow();
Assert.Null(row.RequestSummary);
Assert.Null(row.ResponseSummary);
}
}
@@ -124,6 +124,31 @@ public class FlowDebugRunnerTests
Assert.Contains(_central, e => e.AsRow().Kind == AuditKind.RouteCall);
}
[Fact]
public async Task RunMethod_SiteClockBehindCentral_StillReturnsTheSinkEventsInEmissionOrder()
{
// Arch-review F5: the site's rows carry the SITE clock. A site 2 s behind central used to
// re-sort ahead of the always-on central RouteCall row.
var method = new ApiMethod("M", "x") { Id = 6, TimeoutSeconds = 5 };
_apiRepo.GetApiMethodByIdAsync(6, Arg.Any<CancellationToken>()).Returns(method);
_locator.GetSiteIdForInstanceAsync("inst", Arg.Any<CancellationToken>()).Returns("site-a");
var child = Guid.NewGuid();
var siteRun = ScadaBridgeAuditEventFactory.Create(AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
occurredAtUtc: DateTime.UtcNow.AddSeconds(-2), executionId: child, target: "work");
_router.RouteToCallAsync("site-a", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(((RouteToCallRequest)ci[1]).CorrelationId, true, 7, null, DateTimeOffset.UtcNow, child, [siteRun]));
_executor.RegisterHandler("M", ctx => ctx.Route.To("inst").Call("work"));
var result = await Runner().RunMethodAsync(_services.BuildServiceProvider(),
new FlowDebugRunMethodCommand(6, null), Deployer, CancellationToken.None);
Assert.True(result.Completed, result.Error);
Assert.Equal(3, result.Events.Count);
Assert.Equal(AuditKind.InboundRequest, result.Events[0].AsRow().Kind);
Assert.Equal(AuditKind.RouteCall, result.Events[1].AsRow().Kind);
Assert.Same(siteRun, result.Events[2]);
}
[Fact]
public async Task RunMethod_UnknownMethod_IsNotFound()
{
@@ -233,10 +258,200 @@ public class FlowDebugRunnerTests
Assert.Equal(FlowDebugRunner.NotFoundCode, empty.ErrorCode);
}
[Fact]
public void RoleGate_AllThreeCommands_AreDeployerOnly()
// --- Arch-review F2: a method-entry Live run is bound by the operator's site scope ---
//
// RunValueChange already called EnforceSiteScope on the target instance; a method
// Live run has no single target site up front, so the scope is pushed into the
// RouteHelper and enforced at route time (RouteTarget.ResolveSiteAsync), before any
// site traffic. The denial comes back as SITE_SCOPE_DENIED and the runner converts it.
/// <summary>Wires a method whose script routes to <paramref name="instanceCode"/>, resident at <paramref name="siteIdentifier"/>.</summary>
private ISiteRepository WireRoutedMethod(int methodId, string instanceCode, string siteIdentifier)
{
foreach (var cmd in new object[] { new FlowDebugRunMethodCommand(1, null), new FlowDebugRunValueChangeCommand(1, "a", "b"), new FlowDebugReplayCommand(Guid.NewGuid()) })
var method = new ApiMethod("M", "x") { Id = methodId, TimeoutSeconds = 5 };
_apiRepo.GetApiMethodByIdAsync(methodId, Arg.Any<CancellationToken>()).Returns(method);
_locator.GetSiteIdForInstanceAsync(instanceCode, Arg.Any<CancellationToken>()).Returns(siteIdentifier);
_router.RouteToCallAsync(siteIdentifier, Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToCallResponse(
((RouteToCallRequest)ci[1]).CorrelationId, true, 7, null, DateTimeOffset.UtcNow));
_executor.RegisterHandler("M", ctx => ctx.Route.To(instanceCode).Call("work"));
// The user's PermittedSiteIds are NUMERIC ids; the locator speaks site IDENTIFIERS,
// so the runner resolves one to the other through the repository.
var sites = Substitute.For<ISiteRepository>();
sites.GetSiteByIdAsync(1, Arg.Any<CancellationToken>()).Returns(new Site("Site A", "site-a") { Id = 1 });
_services.AddSingleton(sites);
return sites;
}
[Fact]
public async Task RunMethod_ScopedDeployer_RouteToOtherSite_IsUnauthorized_AndInboundRowIsWritten()
{
WireRoutedMethod(11, "inst", "site-b");
var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" });
await Assert.ThrowsAsync<SiteScopeViolationException>(() => Runner().RunMethodAsync(
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(11, null), scoped, CancellationToken.None));
// The site was never contacted...
await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default);
// ...but the denied attempt IS recorded against the operator's name.
Assert.Contains(_central, e => e.AsRow().Kind == AuditKind.InboundRequest && e.Actor == "ops");
}
[Fact]
public async Task RunMethod_ScopedDeployer_RouteWithinScope_Runs()
{
// The same scoped user, routing to a site their numeric id DOES resolve to.
WireRoutedMethod(12, "inst", "site-a");
var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" });
var result = await Runner().RunMethodAsync(
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(12, null), scoped, CancellationToken.None);
Assert.True(result.Completed, result.Error);
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
}
[Fact]
public async Task RunMethod_Administrator_IsNeverScoped()
{
WireRoutedMethod(13, "inst", "site-b");
var admin = new AuthenticatedUser("boss", "Boss", new[] { "Deployer", "Administrator" }, new[] { "1" });
var result = await Runner().RunMethodAsync(
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(13, null), admin, CancellationToken.None);
Assert.True(result.Completed, result.Error);
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
}
[Fact]
public async Task RunMethod_UnscopedUser_IsNeverScoped()
{
WireRoutedMethod(14, "inst", "site-b");
// Deployer with an EMPTY PermittedSiteIds = global scope, as EnforceSiteScope treats it.
var result = await Runner().RunMethodAsync(
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(14, null), Deployer, CancellationToken.None);
Assert.True(result.Completed, result.Error);
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
}
[Fact]
public void RoleGate_TheTwoRunCommands_AreDeployerOnly()
{
// arch-review F3: a Live run executes scripts and writes attributes, so it stays Deployer-only.
foreach (var cmd in new object[] { new FlowDebugRunMethodCommand(1, null), new FlowDebugRunValueChangeCommand(1, "a", "b") })
Assert.Equal(new[] { "Deployer" }, ManagementActor.GetRequiredRoles(cmd));
}
[Fact]
public void RoleGate_Replay_AlsoAcceptsTheOperationalAuditRoles()
{
// arch-review F3: Replay only reads the audit chain, and the Audit Log / Execution
// Chain drill-ins must work for an auditor who holds no Deployer role.
var actual = ManagementActor.GetRequiredRoles(new FlowDebugReplayCommand(Guid.NewGuid()));
Assert.NotNull(actual);
Assert.Equal(new[] { "Administrator", "Deployer", "Viewer" }, actual!.OrderBy(r => r, StringComparer.Ordinal));
}
// --- Arch-review F1: the Live result is the REDACTED, capped trace copy ---
//
// The runner collects sink events through the same FlowTraceCollector the site launcher
// uses, so central and site filter identically and a Live run shows exactly the rows a
// later Replay reads back out of the audit store.
/// <summary>Wires a method whose handler pushes <paramref name="events"/> straight at the trace sink.</summary>
private void WireSinkPushingMethod(int methodId, IEnumerable<AuditEvent> events)
{
var method = new ApiMethod("M", "x") { Id = methodId, TimeoutSeconds = 5 };
_apiRepo.GetApiMethodByIdAsync(methodId, Arg.Any<CancellationToken>()).Returns(method);
_executor.RegisterHandler("M", ctx =>
{
foreach (var e in events) ctx.Route.TraceSink!(e);
return Task.FromResult<object?>(null);
});
}
private static AuditEvent TraceRow(string? detailsJson) =>
ScadaBridgeAuditEventFactory.Create(
AuditChannel.ScriptTrace, AuditKind.ScriptRun, AuditStatus.Delivered,
executionId: Guid.NewGuid(), target: "work") with
{ DetailsJson = detailsJson };
[Fact]
public async Task RunMethod_SinkEvents_AreRedacted_WhenARedactorIsRegistered()
{
WireSinkPushingMethod(31, [TraceRow("{\"secret\":\"abc\"}")]);
_services.AddSingleton<IAuditRedactor>(new UpperRedactor());
var result = await Runner().RunMethodAsync(_services.BuildServiceProvider(),
new FlowDebugRunMethodCommand(31, null), Deployer, CancellationToken.None);
Assert.True(result.Completed, result.Error);
Assert.Equal(2, result.Events.Count); // inbound row + the one sink event
Assert.Equal("{\"SECRET\":\"ABC\"}", result.Events[1].DetailsJson);
Assert.False(result.Truncated);
// Review follow-up: the row the RUNNER synthesizes is redacted too — the central writer
// only redacts its own private copy, so the caller's copy would otherwise be the raw one.
var written = Assert.Single(_central, e => e.AsRow().Kind == AuditKind.InboundRequest);
Assert.Equal(written.DetailsJson!.ToUpperInvariant(), result.Events[0].DetailsJson);
Assert.NotEqual(written.DetailsJson, result.Events[0].DetailsJson); // the raw row still reaches the writer
}
[Fact]
public async Task RunValueChange_SynthesizedRootRow_IsRedacted_WhenARedactorIsRegistered()
{
var (_, _, puller) = WireValueChange();
puller.PullAsync(default!, default!, default, default, default).ReturnsForAnyArgs(Array.Empty<AuditEvent>());
_services.AddSingleton<IAuditRedactor>(new UpperRedactor());
var result = await Runner().RunValueChangeAsync(_services.BuildServiceProvider(),
new FlowDebugRunValueChangeCommand(5, "Status", "Running", WaitSeconds: 1), Deployer, CancellationToken.None);
var root = Assert.Single(result.Events);
var written = Assert.Single(_central);
Assert.Equal(written.DetailsJson!.ToUpperInvariant(), root.DetailsJson);
Assert.NotEqual(written.DetailsJson, root.DetailsJson);
}
[Fact]
public async Task RunValueChange_FailedWrite_ReturnsTheRedactedRootRow()
{
WireValueChange();
_router.RouteToSetAttributesAsync("site-a", Arg.Any<RouteToSetAttributesRequest>(), Arg.Any<CancellationToken>())
.Returns(ci => new RouteToSetAttributesResponse(((RouteToSetAttributesRequest)ci[1]).CorrelationId, false, "no such attribute", DateTimeOffset.UtcNow));
_services.AddSingleton<IAuditRedactor>(new UpperRedactor());
var result = await Runner().RunValueChangeAsync(_services.BuildServiceProvider(),
new FlowDebugRunValueChangeCommand(5, "Status", "Running", WaitSeconds: 1), Deployer, CancellationToken.None);
Assert.False(result.Completed);
Assert.Equal("WRITE_FAILED", result.ErrorCode);
var root = Assert.Single(result.Events);
var written = Assert.Single(_central);
Assert.Equal(written.DetailsJson!.ToUpperInvariant(), root.DetailsJson);
Assert.NotEqual(written.DetailsJson, root.DetailsJson);
}
[Fact]
public async Task RunMethod_OverCap_ReportsTruncated()
{
WireSinkPushingMethod(32, Enumerable.Range(0, 501).Select(_ => TraceRow("{}")).ToList());
var result = await Runner().RunMethodAsync(_services.BuildServiceProvider(),
new FlowDebugRunMethodCommand(32, null), Deployer, CancellationToken.None);
Assert.True(result.Truncated);
Assert.Equal(501, result.Events.Count); // inbound row + the 500 kept
}
/// <summary>Upper-cases <c>DetailsJson</c> so a test can see the filtered copy land.</summary>
private sealed class UpperRedactor : IAuditRedactor
{
public AuditEvent Apply(AuditEvent rawEvent)
=> rawEvent with { DetailsJson = rawEvent.DetailsJson?.ToUpperInvariant() };
}
}
@@ -23,6 +23,12 @@ public class RequiredRoleMatrixTests
private static readonly string[] AreaManagers = [Roles.Designer, Roles.Deployer];
private static readonly string[] SecuredWriteReaders = [Roles.Operator, Roles.Verifier, Roles.Administrator];
// arch-review F3: Flow Debugger Replay is an audit read, so it is open to the
// OperationalAudit roles {Administrator, Viewer} in addition to Deployer. Written out
// by hand (not read from AuthorizationPolicies.FlowDebuggerAccessRoles) to keep the
// freeze meaningful — a change to the policy's role set must fail this table first.
private static readonly string[] FlowDebuggerReplayers = [Roles.Deployer, Roles.Administrator, Roles.Viewer];
private static readonly Dictionary<string, string[]?> Expected = new(StringComparer.OrdinalIgnoreCase)
{
// ---- Administrator-only -----------------------------------------------------
@@ -138,10 +144,14 @@ public class RequiredRoleMatrixTests
["RetryParkedMessage"] = [Roles.Deployer],
["DiscardParkedMessage"] = [Roles.Deployer],
["DebugSnapshot"] = [Roles.Deployer],
// Flow Debugger (design 2026-09-05 §9): Live runs + Replay are Deployer, like Debug View.
// Flow Debugger (design 2026-09-05 §9 / D6, arch-review F3): a Live run executes
// scripts and writes attributes on operator input, so it stays Deployer-only.
// Replay only reads the audit chain, so it carries the OperationalAudit roles as
// well — the Audit Log / Execution Chain drill-ins are the feature's main entry
// for a past incident and an auditor must be able to follow them.
["FlowDebugRunMethod"] = [Roles.Deployer],
["FlowDebugRunValueChange"] = [Roles.Deployer],
["FlowDebugReplay"] = [Roles.Deployer],
["FlowDebugReplay"] = FlowDebuggerReplayers,
// ---- Two-person secured write ----------------------------------------------
["SubmitSecuredWrite"] = [Roles.Operator],
@@ -1264,6 +1264,44 @@ public class AuthorizationPolicyTests
Assert.False(await EvaluatePolicy(AuthorizationPolicies.RequireSecuredWriteAccess, principal));
}
// ─────────────────────────────────────────────────────────────────────
// Flow Debugger page access (arch-review F3 / design D6). Replay is an
// audit read, so opening the page is any-of {Deployer, Administrator,
// Viewer} — the OperationalAudit roles plus Deployer. The Live run
// controls are separately gated on Deployer by the page and, server-side,
// by the ManagementActor role matrix; Designer gets neither.
// ─────────────────────────────────────────────────────────────────────
[Theory]
[InlineData("Deployer")]
[InlineData("Administrator")]
[InlineData("Viewer")]
public async Task RequireFlowDebugger_ReplayCapableRoles_Satisfy(string role)
{
var principal = CreatePrincipal(new[] { role });
Assert.True(await EvaluatePolicy(AuthorizationPolicies.RequireFlowDebugger, principal));
}
[Theory]
[InlineData("Designer")]
[InlineData("Operator")]
[InlineData("Verifier")]
public async Task RequireFlowDebugger_OtherRoles_Denied(string role)
{
var principal = CreatePrincipal(new[] { role });
Assert.False(await EvaluatePolicy(AuthorizationPolicies.RequireFlowDebugger, principal));
}
[Fact]
public async Task RequireFlowDebugger_DoesNotImplyDeployment()
{
// A Viewer may open the page (Replay) but must not satisfy the policy the
// Live run controls are gated on.
var viewer = CreatePrincipal(new[] { Roles.Viewer });
Assert.True(await EvaluatePolicy(AuthorizationPolicies.RequireFlowDebugger, viewer));
Assert.False(await EvaluatePolicy(AuthorizationPolicies.RequireDeployment, viewer));
}
private static ClaimsPrincipal CreatePrincipal(string[] roles, string[]? siteIds = null)
{
var claims = new List<Claim>();
@@ -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()
{
@@ -3,14 +3,20 @@ using Akka.TestKit;
using Akka.TestKit.Xunit2;
using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Scripting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Abstractions;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.ScriptExecution;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
using ZB.MOM.WW.ScadaBridge.SiteEventLogging;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport;
using IAuditWriter = ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services.IAuditWriter;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
@@ -213,6 +219,151 @@ public class ScriptRunShedTests : TestKit, IDisposable
Assert.Equal(4, alarm.UnderlyingActor.RunsInFlight);
}, TimeSpan.FromSeconds(10));
}
// ---------------------------------------------------------------------------------------
// Arch-review F7: a TRACED run refused by either guard (the in-flight cap, or
// MinTimeBetweenRuns) used to emit nothing at all, so the Flow Debugger reported
// NO_TRIGGER_WITHIN_WINDOW — "no value-change trigger on this attribute, or no device echo"
// — which is wrong in both cases and sends the operator to check the wrong things. Each
// guard now leaves a Failed ScriptTrace/ScriptRun row naming the guard that refused it.
// ---------------------------------------------------------------------------------------
/// <summary>Audit writer fake: records every emitted row and can wait for one matching a predicate.</summary>
private sealed class CapturingAuditWriter : IAuditWriter
{
private readonly List<AuditEvent> _events = new();
public Task WriteAsync(AuditEvent evt, CancellationToken ct = default)
{
lock (_events) _events.Add(evt);
return Task.CompletedTask;
}
public AuditEvent[] Snapshot()
{
lock (_events) return _events.ToArray();
}
public AuditEvent? WaitFor(Func<AuditEvent, bool> predicate, TimeSpan timeout)
{
var deadline = DateTime.UtcNow + timeout;
while (DateTime.UtcNow < deadline)
{
lock (_events)
{
var hit = _events.FirstOrDefault(predicate);
if (hit is not null) return hit;
}
Thread.Sleep(25);
}
return null;
}
}
private static IServiceProvider AuditServices(ISiteEventLogger siteLog, IAuditWriter writer)
{
var services = new ServiceCollection();
services.AddSingleton(siteLog);
services.AddSingleton(writer);
return services.BuildServiceProvider();
}
private static ResolvedScript ValueChangeScript(string name, TimeSpan? minTimeBetweenRuns = null) => new()
{
CanonicalName = name,
TriggerType = "valuechange",
TriggerConfiguration = "{\"attribute\":\"Status\"}",
MinTimeBetweenRuns = minTimeBetweenRuns
};
private static bool IsFailedScriptRun(AuditEvent evt) =>
evt.Action == "ScriptTrace.ScriptRun" && evt.AsRow().Status.ToString() == "Failed";
[Fact]
public void TracedRun_ShedAtTheCap_EmitsAFailedScriptRunRow_ParentedToTheWriter()
{
var siteLog = new FakeSiteEventLogger();
var writer = new CapturingAuditWriter();
var health = new SiteHealthCollector();
var instance = CreateTestProbe().Ref;
var options = Options();
var actor = ActorOfAsTestActorRef<ScriptActor>(
Props.Create(() => new ScriptActor(
"Hot", "Inst1", instance, BlockingScript(),
ValueChangeScript("Hot"),
_sharedLibrary, options, NullLogger<ScriptActor>.Instance,
null, null, health, AuditServices(siteLog, writer), _scheduler, null)),
"shed-traced-" + Guid.NewGuid().ToString("N"));
// Fill the cap with four untraced runs, all blocked in their bodies.
for (var i = 0; i < 4; i++)
actor.Tell(new ScriptCallRequest("Hot", null, 0, $"corr-{i}"), ActorRefs.NoSender);
AwaitAssert(() => Assert.Equal(4, actor.UnderlyingActor.RunsInFlight), TimeSpan.FromSeconds(15));
// The fifth trigger is a TRACED value change stamped by its writing execution.
var parent = Guid.NewGuid();
actor.Tell(new AttributeValueChanged(
"Inst1", "Status", "Status", "Running", "Good", DateTimeOffset.UtcNow,
SourceExecutionId: parent, Trace: true));
var evt = writer.WaitFor(IsFailedScriptRun, TimeSpan.FromSeconds(10));
Assert.NotNull(evt);
var row = evt!.AsRow();
Assert.Equal(parent, row.ParentExecutionId);
Assert.NotNull(row.ExecutionId);
Assert.StartsWith("shed: MaxConcurrentRunsPerScript", row.ErrorMessage);
Assert.Equal("Hot", evt.Target);
Assert.Equal("Inst1", row.SourceInstanceId);
// The trigger detail survives, so the debugger can still name the attribute.
var extra = ScriptTraceExtra.TryParse(row.Extra)!;
Assert.Equal("valuechange", extra.TriggerKind);
Assert.Equal("Status", extra.TriggerAttribute);
// Still exactly four in flight — the shed run was never launched.
Assert.Equal(4, actor.UnderlyingActor.RunsInFlight);
}
[Fact]
public void TracedRun_SuppressedByMinTimeBetweenRuns_EmitsAFailedScriptRunRow()
{
var siteLog = new FakeSiteEventLogger();
var writer = new CapturingAuditWriter();
var health = new SiteHealthCollector();
var instance = CreateTestProbe().Ref;
var options = Options();
var actor = ActorOfAsTestActorRef<ScriptActor>(
Props.Create(() => new ScriptActor(
"Throttled", "Inst1", instance, CompileRaw("return null;"),
ValueChangeScript("Throttled", TimeSpan.FromMinutes(10)),
_sharedLibrary, options, NullLogger<ScriptActor>.Instance,
null, null, health, AuditServices(siteLog, writer), _scheduler, null)),
"throttled-traced-" + Guid.NewGuid().ToString("N"));
// First traced change runs (and emits a Delivered row); the second, back to back,
// is inside MinTimeBetweenRuns and never launches.
var parent = Guid.NewGuid();
for (var i = 0; i < 2; i++)
{
actor.Tell(new AttributeValueChanged(
"Inst1", "Status", "Status", $"Run{i}", "Good", DateTimeOffset.UtcNow,
SourceExecutionId: parent, Trace: true));
}
var evt = writer.WaitFor(IsFailedScriptRun, TimeSpan.FromSeconds(10));
Assert.NotNull(evt);
var row = evt!.AsRow();
Assert.Equal(parent, row.ParentExecutionId);
Assert.StartsWith("suppressed: MinTimeBetweenRuns", row.ErrorMessage);
// The first run still produced its ordinary Delivered row — the guard row is additive.
AwaitAssert(() => Assert.Contains(
writer.Snapshot(),
e => e.Action == "ScriptTrace.ScriptRun" && e.AsRow().Status.ToString() == "Delivered"),
TimeSpan.FromSeconds(10));
}
}
/// <summary>Test hook used to hold script runs in flight while the cap is exercised.</summary>
@@ -209,6 +209,39 @@ public class ScriptRuntimeContextTraceEmissionTests : TestKit
Assert.False(ctx.IsTraced);
}
/// <summary>
/// Arch-review F1: the collector applies the host redactor, so the inline copy the reply
/// carries is the FILTERED record — while the writer still receives the raw event and
/// redacts it itself (redaction is idempotent, so the stored row is identical).
/// </summary>
[Fact]
public async Task SetAttribute_WhenTraced_InlineCopyIsRedacted_WriterStillSeesTheRawEvent()
{
var probe = CreateTestProbe();
var writer = new CapturingAuditWriter();
var collector = new FlowTraceCollector(redactor: new UpperRedactor());
var ctx = CreateContext(probe.Ref, writer, traced: true, collector);
var call = ctx.SetAttribute("Setpoint", "42");
var cmd = probe.ExpectMsg<SetStaticAttributeCommand>(TimeSpan.FromSeconds(5));
probe.Reply(new SetStaticAttributeResponse(cmd.CorrelationId, InstanceName, "Setpoint", true, null, DateTimeOffset.UtcNow));
await call;
var written = Assert.Single(writer.Events);
var collected = Assert.Single(collector.Complete());
Assert.NotSame(written, collected);
Assert.Contains("\"channel\":\"ScriptTrace\"", written.DetailsJson);
Assert.NotEqual(written.DetailsJson, collected.DetailsJson);
Assert.Equal(written.DetailsJson!.ToUpperInvariant(), collected.DetailsJson);
}
/// <summary>Upper-cases <c>DetailsJson</c> so a test can see the filtered copy land.</summary>
private sealed class UpperRedactor : IAuditRedactor
{
public AuditEvent Apply(AuditEvent rawEvent)
=> rawEvent with { DetailsJson = rawEvent.DetailsJson?.ToUpperInvariant() };
}
[Fact]
public async Task SetAttribute_WhenTraced_FailedWrite_EmitsFailedRow_ThenThrows()
{
@@ -872,6 +872,11 @@ public class OperationTrackingStoreTests : IDisposable
kind: nameof(AuditKind.ApiCallCached), targetSummary: "ERP.B",
sourceInstanceId: "I", sourceScript: "S", sourceNode: "node-a");
// Writes are queued (single-writer pump, 2026-09-06): the verifier connection
// below bypasses the store, so drain the queue first or the UPDATE races the
// INSERTs and touches zero rows.
await store.FlushAsync();
// Force both rows to share one exact UpdatedAtUtc instant, formatted the same
// way the read normalises the cursor ('Z'-suffixed round-trip "o").
var boundary = DateTime.UtcNow;