# Architectural Review Remediation Plan **Date:** 2026-08-14 · **Status:** PROPOSED · **Source:** 2026-08-14 performance/architecture review (eight-pass, all 27 projects + `ZB.MOM.WW.LocalDb`; report artifact `https://claude.ai/code/artifact/8986cd89-6ab6-48aa-973f-119fddd5cafb`). This plan resolves **every** finding from the review — the ten ranked findings plus all section-level Med/Low items — as an automated, multi-phase program executed by subagents, parallel where file-ownership makes it safe. It is written so that each work package (WP) is a self-contained subagent assignment: an executor agent should be able to complete a WP from its spec here plus the code, without further orchestrator input. --- ## 1. Execution model **Orchestrator:** the interactive Claude session (Fable) in this repo. It spawns one subagent per work package, merges results, runs phase gates, and commits. **Isolation:** every code-writing package runs in its **own git worktree** (`Agent` tool, `isolation: "worktree"`). Packages within a phase touch disjoint file sets (see the conflict matrix, §7), so merges are clean; the orchestrator resolves any residual conflict itself. Worktrees also make concurrent `dotnet build`/`test` safe (no shared `obj/`). **Per-package protocol.** Each executor agent must: 1. Read this plan's WP section and the referenced source files first; verify the finding still reproduces in code (line numbers may have drifted — the *description* is authoritative). 2. Implement per the spec. Repo rules apply: edit in place, no backup copies; design doc + code + tests + docker config travel together. 3. Build the touched projects and run the listed test suites in the worktree (`dotnet build ZB.MOM.WW.ScadaBridge.slnx`, `dotnet test` filtered to relevant projects). 4. Commit on the worktree branch with the message given in the WP spec. 5. Report back: commit SHA, test summary (counts, any failures), and any spec deviations with rationale. **Phase gates (orchestrator, serial).** After merging a phase's branches into the remediation branch: full `dotnet build ZB.MOM.WW.ScadaBridge.slnx` + full `dotnet test` must be green; for phases touching cluster runtime (2 and 3), rebuild the rig (`bash docker/deploy.sh`) and run the phase's live probes (§6). A red gate stops the program — the orchestrator fixes or reverts the offending package before proceeding. **Branching.** All work lands on `arch-review-remediation` (branched from `main` in Phase 0). One commit per package (the worktree commit, merged `--no-ff`), plus one docs commit in Phase 4. Cross-repo work in `~/Desktop/scadaproj` gets its own branch there (WP3.3). Nothing is pushed without the user. ### Model matrix | Model | Used for | Rationale | |---|---|---| | **fable** | Orchestration; design docs for structural seams (WP2.1a, WP3.1a, WP3.2a); final review synthesis | Hardest open design decisions and cross-package judgment | | **opus** | Concurrency-sensitive implementation (streams, DCL, actors, ingest, UI coalescing, LocalDb library) | Subtle correctness work; failure modes are silent | | **sonnet** | Mechanical/localized fixes, index migrations, schema tweaks, docs updates | Well-specified, verifiable by tests | | **opus** (code-reviewer agents) | Phase 4 adversarial diff review | Independent verification pass | Approximate agent budget: Phase 1 = 7 executors; Phase 2 = 6 executors + 1 design; Phase 3 = 2 design + 3 executors; Phase 4 = 6 reviewers + 1 docs agent. ~26 agents total. --- ## 2. Phase 0 — Preflight (orchestrator, serial, ~minutes) 1. `git checkout -b arch-review-remediation` (current `main`; note the pre-existing dirty `docs/requirements/Component-SiteRuntime.md` — stash or leave untouched, do not absorb it). 2. Baseline: full build + full test run; record pass/fail counts as the reference state. 3. Confirm the rig is deployable (`docker/deploy.sh` last known good) but do **not** redeploy yet. Gate: baseline green. If baseline has pre-existing failures, record them — packages are judged against the baseline, not absolute green. --- ## 3. Phase 1 — Quick wins (7 packages, ALL PARALLEL) Highest leverage per line changed. All packages are independent; the two silent-failure defects (WP1.1, WP1.2) lead. ### WP1.1 — Stream OK-completion reconnect `[opus]` **Findings:** #1 (High) — streams silently die at the 4-hour max lifetime; OK completion is invisible to reconnect logic. **Files:** `Communication/Grpc/SiteStreamGrpcClient.cs`, `Communication/Actors/SiteAlarmAggregatorActor.cs`, `Communication/Actors/DebugStreamBridgeActor.cs` (+ tests). **Changes:** - Add an `onCompleted` callback to the client subscribe surface (preferred over an error sentinel — callers can distinguish graceful end from fault). Invoke it when the `await foreach` ends without exception, including server OK completion at max lifetime and graceful site shutdown. - `SiteAlarmAggregatorActor`: on completion, `Tell` self a new `GrpcAlarmStreamCompleted(generation)` message → set `_streamDown = true`, mark `IsLive = false`, and let the reconcile tick reopen **without** consuming error-retry/backoff budget (completion is not a fault). Also observe the `Task.Run` subscription task itself (`ContinueWith` faulted → route into the existing error path) so no completion or fault is ever unobserved again. - Same treatment for the debug stream path in `DebugStreamBridgeActor`. - Generation-fence everything (the fencing pattern already exists in the actor — reuse it). **Tests:** unit test on the client (fake stream that completes OK → `onCompleted` fires, `onError` does not); actor test (stream completes OK → `IsLive` false → reopen on next reconcile tick → backoff counter unchanged). Regression: error path still burns retry budget. **Commit:** `fix(comms): reconnect on graceful stream completion — kills the 4h silent stream death` ### WP1.2 — Site audit DB placement + flush `[sonnet]` **Findings:** #2 (High) — `auditlog.db` on container overlayfs; dead `FlushIntervalMs`; fsync per event. **Files:** `AuditLog/Site/SqliteAuditWriterOptions.cs`, `AuditLog/Site/SqliteAuditWriter.cs`, all site-node `appsettings` under `docker/` and `docker-env2/` (+ tests). **Changes:** - Make `DatabasePath` **required on Site nodes** (drop the CWD-relative default; `ValidateOnStart`), mirroring the `LocalDb:Path` precedent — a healthy-looking node silently writing to overlayfs is the failure mode being closed. Update all rig site-node configs to `/app/data/auditlog.db`. Update any tests that relied on the implicit default to pass an explicit temp path. - Implement `FlushIntervalMs` as designed: the writer loop groups commits within the interval (batch already drains up to 256 — add the time-based commit coalescing). Additionally set `PRAGMA synchronous=NORMAL` on the audit connection — audit is best-effort by design (CLAUDE.md: "audit-write failure NEVER aborts the user-facing action"), so NORMAL's power-loss window is acceptable; say so in a code comment. - One-time migration note in the commit message: the existing container-local file is abandoned; already-forwarded rows are safe centrally, pending rows on the old path are lost once — accepted, this is the bug being fixed. (Cross-reference `docs/known-issues/2026-07-20-cached-telemetry-drain-hot-loop.md`, which this placement bug caused.) **Tests:** options validation test (site role boots ⇒ path required); writer test asserting multi-event single-transaction commit under trickle load. **Commit:** `fix(auditlog): site audit DB onto the data volume; required path + soft flush` ### WP1.3 — Conditional CDC registration `[opus]` **Findings:** #5 (High), partial — non-replicating nodes pay full CDC capture forever. (The library half — trigger cleanup API, O(1) backlog — is WP3.3.) **Files:** `Host/SiteLocalDbSetup.cs` (+ tests). Read `~/Desktop/scadaproj/ZB.MOM.WW.LocalDb/` and the site-a rig configs first to confirm the exact replication-config shape on both the active and passive peer. **Changes:** - Skip all ten `RegisterReplicated` calls when the node has **no replication configured** (neither `LocalDb:Replication:PeerAddress` nor `ApiKey` — verify the passive-side shape on the rig before choosing the predicate; the guard must be true on BOTH members of a replicated pair and false on site-b/site-c). - Preserve the load-bearing ordering documented in the file: when registration is skipped, the legacy migrator still runs (its rows simply don't replicate — consistent with an unreplicated node). - **Known residual, documented in-file:** a database file that was registered under an older build keeps its stale triggers until WP3.3 ships the library cleanup API; on the docker rig this is moot (volumes are recreated on redeploy of a schema change) — state this in a comment and in the commit message. **Tests:** setup test with and without replication config, asserting trigger presence/absence via `sqlite_master` (`name LIKE '%localdb%'` — confirm the library's trigger naming first). **Commit:** `perf(host): install CDC capture only when replication is configured` ### WP1.4 — SQL quick indexes + sliced purge `[sonnet]` **Findings:** S&F sweep O(N) scan (High); audit KPI window key-lookup; Notification KPI delivered-interval scan; unbounded terminal purge (all part of #7/#8 support). **Files:** `StoreAndForward/StoreAndForwardSchema.cs` (+ its versioned-upgrade path), `ConfigurationDatabase/Migrations/` (+ manual SQL script for production per repo convention), `ConfigurationDatabase/Repositories/NotificationOutboxRepository.cs`. **Changes:** - `sf_messages`: add `(status, created_at)` index so the ordered due-scan terminates at `LIMIT`; follow the schema's existing versioned-upgrade mechanism so existing site files get it. - `IX_AuditLog_OccurredAtUtc` → add `INCLUDE (Status)` (EF migration; remember the repo gotcha — **build first**, never `--no-build`, delete empty migrations if scaffolded). - Notifications: filtered index `(DeliveredAt) WHERE Status = 'Delivered'` (or the enum's stored value — check the column type) covering the delivered-last-interval KPI count. - `DeleteTerminalOlderThanAsync`: slice into batched deletes — copy the slicing pattern used twice elsewhere in the same layer (per-channel audit retention delete is one). **Tests:** migration applies cleanly (dev auto-apply path); a query-shape test is not required — Phase 4 live probes verify plans via `EXPLAIN`/showplan. **Commit:** `perf(sql): sweep/KPI covering indexes + sliced notification terminal purge` ### WP1.5 — Mechanical hot-path fixes `[sonnet]` **Findings:** InstanceActor nits (Med); per-call `JsonSerializerOptions` ×4 (Med). **Files:** `SiteRuntime/Actors/InstanceActor.cs`, `ExternalSystemGateway/ExternalSystemClient.cs`, `SiteRuntime/Repositories/SiteExternalSystemRepository.cs` (options instance only — the query redesign is WP2.6), `ManagementService/ManagementEndpoints.cs`, `CentralUI/.../ScriptAnalysisService.cs`. **Changes:** - `HandleSetStaticAttribute`: use the existing `_resolvedAttributeByName` `TryGetValue` instead of the LINQ scan (InstanceActor.cs:373). - `HandleTagValueUpdate`: precompute the parsed `DataType` into the resolved-attribute index entry at config-apply time; stop re-`Enum.TryParse`-ing per update. Intern/cache the quality strings (`Quality.ToString()` per update → static readonly map). - Static-override writes: coalesce per instance on a short timer, reusing the batched-flush pattern already present for native alarms in the same codebase (copy that shape, don't invent). - One `static readonly JsonSerializerOptions` per assembly at the four verified sites. **Tests:** existing InstanceActor suite must stay green; add a test pinning single-flush coalescing of N rapid static writes. **Commit:** `perf(runtime): O(1) attribute resolution, precomputed types, coalesced static writes, shared JSON options` ### WP1.6 — Transport fail-fast `[sonnet]` **Findings:** known-failure sends burn full Ask timeouts (Med, first half — the timeout stagger and double-hop are WP2.2). **Files:** `Communication/Grpc/GrpcSiteTransport.cs` (+ tests). **Changes:** a send to a site with no configured channel replies `Status.Failure` (with a descriptive exception) to the asker immediately instead of warn-and-drop. Audit every warn-and-drop in the file for the same pattern. **Tests:** Ask against an unconfigured site faults in <1s, not at the 30s timeout. **Commit:** `fix(comms): fail known-dead sends immediately instead of burning Ask timeouts` ### WP1.7 — Event-log writer batching + sliced retention `[sonnet]` **Findings:** per-event transactions in `SiteEventLogger` (part of the site_events High — the volume *policy* is WP3.2); unbatched `site_events` retention DELETE (Med). **Files:** `SiteEventLogging/SiteEventLogger.cs`, `SiteEventLogging/EventLogPurgeService.cs` (+ tests). **Changes:** - Writer loop: drain the existing batching channel into **one transaction per drain** (up to 256), exactly the `SqliteAuditWriter` shape — cite it in a comment. - Retention purge: batch the DELETE at 1000 rows/iteration like the cap purge next to it. **Tests:** writer test (N queued events → 1 transaction); purge test over >1000 expired rows. **Commit:** `perf(sitelog): batched event-log commits and sliced retention purge` **Phase 1 gate:** merge all seven, full build + test, then rebuild the rig (`docker/deploy.sh` — WP1.2 changed site configs) and run the Phase 1 live probes (§6). --- ## 4. Phase 2 — Seam rework (6 packages, PARALLEL after WP2.1a) ### WP2.1 — DCL batch seam `[design: fable → implement: opus]` *(largest package)* **Findings:** #3 (High, four collapsed findings) + MxGateway 2-RPC subscribe / no-backoff retry (Med) + quality-counter and alarm-fanout costs (Med). **Stage a — design memo** (fable, ~1 page, committed as `docs/plans/2026-08-XX-dcl-batch-seam-design.md`): the batch API shape on `IDataConnection` (`SubscribeBatchAsync(IReadOnlyList)` returning per-tag results; true bulk `ReadBatchAsync`/`WriteBatchAsync`), per-subscription monitored-item budget + sharding scheme, reconnect chunking/stagger parameters (configurable; defaults sized for 37,500 tags), re-seed deadline semantics, and what the MxGateway adapter can do **without** a cross-repo mxaccessgw change (fallback: client-side chunked pipelining; note the cross-repo bulk-RPC follow-up if the proto lacks one — check `~/Desktop/MxAccessGateway` first). **Stage b — implementation** (opus): - `RealOpcUaClient`: N×`AddItem` + one `ApplyChangesAsync` per batch; serialize all `ApplyChangesAsync` on the shared Subscription behind a `SemaphoreSlim` (batching makes contention rare; the lock makes it safe); shard monitored items across subscriptions above the per-subscription budget (default ~5,000, configurable). - `OpcUaDataConnection`: implement the batch interface; `ReadBatchAsync`/`WriteBatchAsync` become single OPC UA service calls (the MxGateway adapter's `ReadBulk`/`WriteBulk` is the in-repo reference). - `DataConnectionActor`: `HandleSubscribe`/`ReSubscribeAll` pass whole tag lists; reconnect becomes bounded chunked batches honoring the existing failover stagger; `SeedTagsAsync` becomes chunked batch reads with bounded parallelism and an **overall** deadline (no more 30s-per-tag serial worst case). Tag-resolution retry gains exponential backoff with a ceiling and batched probes. Quality counters flush on transitions or a short timer instead of per-change; alarm subscriber matching gets a source-prefix index and pushes the union filter to the gateway feed. **Tests:** the existing DCL suite (Become/Stash lifecycle, generation fencing) must stay green; new tests with a fake client: batch subscribe issues one apply per chunk; concurrent applies serialize; re-seed respects the deadline; backoff caps. **Commit:** `perf(dcl): batch subscribe/read/write seam, bounded reconnect, sharded subscriptions` ### WP2.2 — Central ingest & SQL set-based rework `[opus]` **Findings:** #6 (High, partition purge), #7 (High, row-at-a-time ingest), #8 (High, KPI scans — query-shape half), KPI rollup N+1 / read-path hygiene / no pooling (Med). **Files:** `AuditLog/.../AuditLogIngestActor.cs`, `ConfigurationDatabase/Repositories/AuditLogRepository.cs`, `SiteCallAudit/.../SiteCallAuditActor.cs` + `SiteCallAuditRepository.cs`, `ConfigurationDatabase/Repositories/NotificationOutboxRepository.cs`, `KpiHistoryRepository.cs`, `ConfigurationDatabase/ServiceCollectionExtensions.cs`, gRPC ingest entry in Communication (+ migration + manual SQL script). **Changes:** - **Ingest batching:** one TVP (or multi-row `INSERT … SELECT … WHERE NOT EXISTS` batch) per telemetry packet with per-row fallback on constraint violation; cached-telemetry entry = one transaction, not ~5 round trips. - **SiteCalls upsert:** single statement (`UPDATE … ; IF @@ROWCOUNT = 0 INSERT …` guarded for the newer-status predicate, or `MERGE` with the usual HOLDLOCK hygiene). - **Reconciliation off-mailbox:** the drain moves out of `ReceiveAsync` to the `PipeTo` + in-flight-guard shape — `NotificationOutboxActor` next door is the in-repo reference; ingest/queries/KPI asks must not queue behind post-outage catch-up. - **Timeout stagger + hop removal:** inner budgets strictly less than outer (site-ask 25s → ingest 20s → SQL 15s, or similar); the gRPC handler Asks the ingest singleton proxy directly, removing the redundant intermediate actor hop. - **Partition purge:** prefer making EventId uniqueness **partition-aligned** (`UNIQUE (EventId, OccurredAtUtc)`) so `SWITCH` needs no index drop at all; EventId is globally unique by construction (GUID minted at source), so the idempotency probe still seeks correctly — document this reasoning in the migration. If alignment proves incompatible with the probe shape, fall back to `ONLINE = ON` rebuild outside the switch transaction (Standard-edition caveat: check target SQL edition; if ONLINE is unavailable, alignment is the only acceptable option). - **KPI query shapes:** split the live-queue seeks from the delivered-interval count (filtered index from WP1.4); rollup fold preloads existing rows per window into a dictionary (kill the per-(series,hour) existence SELECT). - **Hygiene:** `AsNoTracking` + keyset paging on the Notifications read path (mirror the two sibling repos); targeted `ExecuteUpdate` for delivery-attempt status writes instead of detached full-row `Update()`; `AddDbContextPool` — **first verify** the context registers no scoped state/injected services that break pooling; if it does, fix or document why pooling is skipped. **Tests:** ingest idempotency under batch (duplicate EventIds in one packet + across packets); upsert newer-status semantics; reconciliation running while an ingest Ask completes promptly. **Commit:** `perf(central): set-based ingest, aligned partition purge, KPI query shapes, EF hygiene` ### WP2.3 — Wire efficiency `[opus]` **Findings:** #10 (Med, alarm seed waste + per-attempt re-fan-out), unbounded pre-snapshot buffers / lossy shared channel (Med), `PullAuditEvents` at-most-once (Low). **Files:** `Communication/Actors/SiteAlarmAggregatorActor.cs`, `SiteAlarmLiveCacheService.cs`, `SiteCommandDtoMapper.cs`, `DebugStreamBridgeActor.cs`, `Communication/Grpc/SiteStreamGrpcServer.cs`, site audit queue (`ISiteAuditQueue` impl) (+ tests). **Changes:** - **Alarms-only snapshot:** the seed travels the `SiteCommandService` query surface (`QueryReply.DebugViewSnapshot`), so add an additive alarms-only flag on the query DTO — verify whether the vendored proto is involved at all; if it is, follow the documented manual toggle-build-copy-untoggle regeneration, additive field numbers only. - Seed **once per successful (re)connect** plus the existing reconcile as the diff/backstop — not a full unconditional snapshot every 60s. Jitter the per-site aggregator timers so 10 sites don't fan out in lockstep; give the seed-retry leg backoff. - Cap the pre-snapshot buffers (drop-oldest + a dropped-count surfaced on the health report); hard timer on snapshot arrival that fails the session rather than buffering forever; stream events must not reset the receive-timeout that ends a stuck session. - `SubscribeSite` gets its own channel (larger, or wait-mode) instead of sharing the debug view's 1000-slot DropOldest — an alarm burst during a WAN stall must not silently drop transitions. Batch multiple events per proto message where the contract allows (additive). - `PullAuditEvents`: mirror `PullSiteCalls`' composite `(timestamp, id)` keyset cursor; flip rows to `Reconciled` only when the **next** pull's cursor proves receipt (at-least-once restored). **Tests:** cursor-proof flip semantics (fault between response and next pull → rows re-served); seed-once behavior across a simulated reconnect; buffer cap + counter. **Commit:** `perf(comms): alarms-only seed, capped buffers, at-least-once audit pull` ### WP2.4 — Central UI performance `[opus]` **Findings:** #8 (High, per-circuit KPI polling half), AlarmSummary/Health/DebugView per-circuit costs (High), push-triggered full reloads / serial Asks / no virtualization (Med), sandbox thread-parking (Low). **Files:** `CentralUI` — `Health.razor`, `AlarmSummary.razor`, `DebugView.razor`, `Deployments.razor`, a new process-level KPI snapshot service, sandbox host (+ tests where the project has them). **Changes:** - **Process-level memoized KPI snapshot service** (singleton, 5–10s TTL, single-flight so N circuits share one SQL round per interval) consumed by Health + Notification Outbox + Site Calls pages. This plus WP2.2's query shapes closes finding #8 from both ends. - AlarmSummary derives not-reporting from the alarm aggregator's own reconcile state instead of re-running the per-instance snapshot fan-out per circuit per 15s. - DebugView: coalesce renders at ~250ms; rebuild the trees behind a version stamp instead of per event; marshal off the gRPC thread once per coalesce window. - Deployments: apply the pushed delta (or debounce reloads ~500ms) instead of reloading all records + instances per push; server-side paging. - Health tick: `Task.WhenAll` with short per-tile timeouts so one hung singleton degrades one tile, not the tick. `Virtualize` on the flat alarm table. - Sandbox Test Runs: bound with a semaphore instead of parking thread-pool threads on sync accessors. - Preserve the existing visual design exactly — these are behavioral changes only, no restyling, no new component frameworks (repo rule). **Tests:** KPI cache single-flight test; render-coalescing unit test if the code shape allows. **Commit:** `perf(ui): shared KPI cache, live-cache-backed alarm summary, coalesced debug renders` ### WP2.5 — Deployment & authoring pipeline `[opus]` **Findings:** #9 (High, both halves), verdict-cache Clear() leak + unpaged management queries (Med). **Files:** `TemplateEngine/.../FlatteningPipeline.cs`, `TemplateEngineRepository.cs`, `TemplateService.cs`, `DeploymentManager/.../DeploymentService.cs`, `ManagementService/ManagementActor.cs`, `ScriptAnalysis/.../ScriptCompileVerdictCache.cs`, CLI (+ tests). **Changes:** - **Flatten-session cache:** within one deploy/validation session, memoize template chain and composition loads keyed by template id + version; hoist the three global queries (shared scripts, schemas, connections) once per session — `ArtifactDeploymentService` is the in-repo reference shape. - **`DeploySiteAsync`:** bulk orchestration over a site's instances with bounded parallel fan-out and per-instance timeouts, reusing the per-instance operation lock; surfaced via ManagementActor command + CLI (`instance deploy-site ` or consistent naming — follow the CLI's existing verb conventions); UI wiring optional, defer if it inflates scope. - **Staleness watermark:** per-template monotonic version bumped on any template-graph mutation; staleness detection compares watermarks before paying a full flatten. - **Authoring:** slim projections for acyclicity/collision checks (no script bodies); `AsNoTracking` on read-only walks; `ReconcileDescendantsAsync` receives the already-loaded graph instead of re-calling `GetAllTemplatesAsync`. - **Management queries:** `QueryDeployments` pages DB-side with a summary projection; `ListTemplates` pages DB-side without full script bodies; batch override apply becomes one read + one commit. - **Verdict cache:** LRU/segmented eviction instead of wholesale `Clear()` (the Clear re-opens the non-collectible `InteractiveAssemblyLoader` leak the cache exists to bound); add a retention policy for terminal deployment records (config, default generous). **Tests:** flatten count test (N instances, same template ⇒ chain loaded once); DeploySiteAsync fan-out with an injected slow site (bounded, others complete); verdict-cache eviction keeps hot entries. **Commit:** `perf(deploy): flatten-session caching, bulk DeploySiteAsync, paged management queries` ### WP2.6 — Cross-cutting misc `[sonnet]` **Findings:** site external-system resolution (Med), Inbound API per-request SQL (Low), unbounded S&F observer queue (Low), CLI timeout (Low), failback-probe heartbeat pollution (Low), site stream alarm-vs-attribute shared buffer (Med). **Files:** `SiteRuntime/Repositories/SiteExternalSystemRepository.cs`, InboundAPI method-lookup path, `StoreAndForward` observer queue, `SiteRuntime/Streaming/SiteStreamManager.cs`, CLI `HttpClient` setup, failback probe (+ tests). **Changes:** - External-system resolution: name-keyed lookup (the ID reverse-map scan goes); cache the parsed method list per system, invalidated on redeploy — honoring the documented "indexed query" contract. - Inbound API: short-TTL ApiMethod cache invalidated by the existing `ScriptArtifactChangeSubscriber` bus (the invalidation plumbing already exists — lean on it). - S&F observer queue: bound it DropOldest like its siblings (it is the one unbounded channel in the system) + a dropped counter. - `SiteStreamManager`: separate alarm publish source (or priority path) so attribute storms can't evict alarm transitions; drop counter on the site health report; skip publish at zero subscribers. - CLI `HttpClient`: explicit timeout (30s, config-overridable). - Failback probe: mark the probe heartbeat synthetic (flag or reserved SiteId) so it stops polluting liveness data — check what consumes heartbeats before choosing the mechanism. **Commit:** `perf(misc): cached hot-path lookups, bounded observer queue, alarm-priority stream path` **Phase 2 gate:** merge order WP2.2 → WP2.3 → WP2.1 → WP2.4 → WP2.5 → WP2.6 (Communication-heavy first — likeliest residual conflicts surface early). Full build + test; rig rebuild; Phase 2 live probes (§6). --- ## 5. Phase 3 — Structural (design-first; 3 tracks) Design stages run in parallel (fable); implementations start as each design lands. Defaults are pre-decided below so automation never blocks — the design memos may refine but not stall. ### WP3.1 — Script execution pool split `[design: fable → implement: opus]` **Findings:** #4 (High) + actor-per-execution overhead / compile-cache Clear (Med). **Stage a — design doc** (`docs/plans/2026-08-XX-script-pool-split-design.md`), deciding: - Trigger-expression evaluation leaves the blocking pool — default decision: evals are non-blocking by construction, so run them as plain async on the default dispatcher with a concurrency gate, no second dedicated pool. - Blocking-script pool sizing scales with deployed instance count (default: `max(8, instances/8)` capped at a config ceiling); stuck-script watchdog **replaces** lost threads. - Timeout budget includes queue wait (deadline captured at enqueue). - Per-script in-flight cap — default policy: bounded queue per script (cap 4), overflow **sheds newest with a site event + counter** (an alarm-triggered run that can't start within its deadline is stale anyway); design memo must justify or amend. - `ScriptExecutionActor` elimination: run the guarded task directly under `ScriptActor`, preserving supervision semantics (Stop-on-failure equivalence), telemetry, and the audit `ExecutionId`/`ParentExecutionId` threading — this is the riskiest part; the design memo maps every behavior the actor currently provides to its replacement. - `SiteScriptCompileCache`: LRU eviction; cache-miss compiles move off-dispatcher (the DeploymentManager shared-script path is the in-repo reference). **Stage b — implement + tests:** starvation regression test (8 blocked scripts ⇒ alarm eval still completes <2s); queue-time-inclusive timeout test; watchdog replacement test; supervision-parity tests for the actor removal. **Commit:** `perf(runtime): split trigger evals from the blocking pool; bounded, deadline-aware execution` ### WP3.2 — site_events volume policy `[design: fable → implement: sonnet]` **Findings:** #High site_events (policy half; mechanics done in WP1.7). **Stage a — design memo** deciding: per-run Started/Completed Info events become **sampled/opt-in** (default: off per-run, aggregate counters per interval instead; per-script opt-in flag for debugging); whether `site_events` should replicate at all (default recommendation: keep replicated — central event-log queries hit either node — but the memo must check what actually reads `site_events` on the peer before confirming; if nothing does, deregister it and use clock-based local purges that never enter the oplog, coordinating with WP3.3's dereg API). **Stage b:** implement per memo; update `Component-SiteEventLogging` doc (Phase 4 collects). **Commit:** `perf(sitelog): sampled per-run events; site_events replication policy` ### WP3.3 — LocalDb library (CROSS-REPO: `~/Desktop/scadaproj`) `[opus]` **Findings:** #5 (library half), replication row-count batching (Med), library-level costs (Low). **Files:** `~/Desktop/scadaproj/ZB.MOM.WW.LocalDb/` on its own branch; then the consuming update here. **Changes (library):** - `DeregisterReplicated(table)` / cleanup API: idempotently drops capture triggers (and optionally prunes that table's oplog rows) — consumed by `SiteLocalDbSetup` when replication is unconfigured, closing WP1.3's stale-trigger residual. - Backlog depth becomes O(1) (`MAX(seq) − last_acked_seq` or a maintained counter) — it currently runs an O(backlog) `COUNT(*)` per minute and per metrics scrape. - Replication batching by **summed bytes** (budget under the 4 MB gRPC cap, row-count as a secondary cap) + ack coalescing; the rig's `MaxBatchSize=16` pin can then be retired. - `LwwApplier` prepare-once-rebind per table; drop the unused `__localdb_oplog_hlc` index (handle existing files in the schema-version upgrade). **Changes (this repo, after the library lands):** bump the package/project reference; call the cleanup API from `SiteLocalDbSetup` when unregistered; relax the rig `MaxBatchSize` pin to the byte budget; update the CLAUDE.md LocalDb bullet and the **umbrella `../scadaproj/CLAUDE.md` ScadaBridge entry in the same change** (repo rule). **Tests:** library suite for dereg idempotency, O(1) depth, byte-budget batching; ScadaBridge integration: site-b boots with zero triggers and zero oplog growth. **Commits:** library commit in scadaproj + `chore(deps): LocalDb — dereg API, byte-budget replication` here. **Phase 3 gate:** full build + test in BOTH repos; rig rebuild; live probes. --- ## 6. Phase 4 — Verification, review, docs (mixed parallel) ### Live probes (orchestrator + rig, accumulate per phase gate) - **Stream lifetime:** set max stream lifetime to ~2 min on the rig; verify the alarm stream reconnects within one reconcile tick of OK completion and `IsLive` reflects the gap (WP1.1). - **CDC:** on site-b/site-c nodes, `sqlite_master` shows no capture triggers; oplog table empty and not growing (WP1.3/WP3.3). Site-a pair still converges (replication regression). - **Audit path:** site `auditlog.db` present under `/app/data`; survives `docker compose rm`+recreate of one site container (WP1.2). - **Query plans:** `EXPLAIN QUERY PLAN` on the S&F due-sweep (index-terminated); SQL Server showplan on the notification KPI and audit KPI queries (seeks, no full scans) (WP1.4/WP2.2). - **Deploy timing:** bulk-redeploy a site on the rig before/after; record wall clock (WP2.5). - **Failover drill:** `docker/failover-drill.sh` still passes (~25s takeover) — protects against regressions from the actor/timeout changes. - **Load validation:** the deferred target-scale load test (deferred-work register #25) is the final proof the ceilings moved — schedule as its own follow-on; this plan's exit criterion is the probes above, not #25. ### Adversarial review `[6 parallel code-reviewer agents, opus; fable synthesis]` One reviewer per area over the full remediation diff (site runtime, DCL, comms, site persistence, central SQL, UI/deploy), each instructed to try to **refute** the fixes (regressions, changed semantics, missed call sites, test gaps). Confirmed findings are fixed by targeted follow-up agents before the docs commit. ### Documentation propagation `[sonnet]` - Component docs touched: `Component-DataConnectionLayer`, `Component-SiteRuntime`, `Component-CentralSiteCommunication`, `Component-AuditLog`, `Component-SiteEventLogging`, `Component-StoreAndForward`, `Component-NotificationOutbox`, `Component-SiteCallAudit`, `Component-TemplateEngine`, `Component-DeploymentManager`, `Component-CentralUI`, `Component-ClusterInfrastructure` (only where behavior/config changed — no cosmetic edits). - CLAUDE.md Key Design Decisions: stream-completion reconnect, conditional CDC, required site audit DB path, DCL batch seam, `DeploySiteAsync`, script-pool split, LocalDb byte-budget replication (retire the MaxBatchSize=16 note). - Umbrella `../scadaproj/CLAUDE.md`: LocalDb entry (done with WP3.3). - Close/annotate any related `docs/known-issues/` entries; README component table needs no change (no components added/removed). - Final `git diff` review by the orchestrator, then the docs commit. **Push only on user request.** --- ## 7. Parallelism & conflict matrix | Phase | Package | Projects touched | Conflicts within phase | |---|---|---|---| | 1 | WP1.1 | Communication (client+actors) | WP1.6 same project, different files — merge-safe | | 1 | WP1.2 | AuditLog(site), docker configs | none | | 1 | WP1.3 | Host | none | | 1 | WP1.4 | ConfigurationDatabase, StoreAndForward | none | | 1 | WP1.5 | SiteRuntime, ESG, ManagementService, CentralUI | single-line edits, none | | 1 | WP1.6 | Communication (transport) | see WP1.1 | | 1 | WP1.7 | SiteEventLogging | none | | 2 | WP2.1 | DataConnectionLayer | none | | 2 | WP2.2 | AuditLog(central), SiteCallAudit, ConfigurationDatabase, NotificationOutbox | none (WP2.3 owns Communication) | | 2 | WP2.3 | Communication, site audit queue | none | | 2 | WP2.4 | CentralUI | none | | 2 | WP2.5 | TemplateEngine, DeploymentManager, ManagementService, ScriptAnalysis, CLI | none | | 2 | WP2.6 | SiteRuntime(streaming+repos), InboundAPI, S&F, CLI(http) | CLI touched by WP2.5 (commands) and WP2.6 (HttpClient) — different files; merge WP2.5 first | | 3 | WP3.1 | SiteRuntime(scripts) | none | | 3 | WP3.2 | SiteEventLogging, SiteRuntime(one call site) | coordinate with WP3.1 merge order (3.1 first) | | 3 | WP3.3 | scadaproj (separate repo) + Host/config here | in-repo half lands after 3.1/3.2 | ## 8. Risks & mitigations - **Vendored proto drift** (WP2.3): additive fields only, never reuse numbers; follow the documented manual regeneration toggle. Mitigation: the executor verifies whether the change is DTO-only before touching proto at all. - **Partition-aligned uniqueness** (WP2.2) changes the enforcement scope of EventId uniqueness from global to per-partition. GUID collision across partitions is not a real risk, but the ingest probe's semantics must be re-tested for duplicate delivery across a month boundary. - **Pooled DbContext** (WP2.2): pooling breaks contexts with per-scope injected state — verify before switching; skipping with a documented reason is an acceptable outcome. - **ScriptExecutionActor removal** (WP3.1) is the highest-semantic-risk change: supervision, telemetry, and audit-correlation parity are explicit design-memo obligations, and it ships in its own commit so it can be reverted independently. - **Rolling-upgrade caveat** (WP3.3): site pairs already require stop/start together (repo rule); the LocalDb schema-version bump rides that existing constraint — call it out in the topology guide. - **EF migration gotcha:** always build before `dotnet ef migrations add`; delete empty scaffolds. - **Merge conflicts:** worktree isolation + the matrix above; residual conflicts are resolved by the orchestrator, never by re-running an executor blindly. ## 9. Exit criteria 1. Full solution build + tests green in both repos; baseline test count not reduced. 2. All §6 live probes pass on the rig; failover drill unchanged (~25s). 3. Adversarial review pass complete with all CONFIRMED findings resolved. 4. Docs propagated (component docs, both CLAUDE.mds, known-issues) with no stale cross-references. 5. One reviewed commit per package on `arch-review-remediation`; branch ready for the user to merge/push; target-scale load test (#25) scheduled as the follow-on validation.