ScadaBridge/docs/plans/2026-08-14-arch-review-remediation-plan.md

# 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<TagSubscription>)` 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 <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 <version> — 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.