37 KiB
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:
- 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).
- Implement per the spec. Repo rules apply: edit in place, no backup copies; design doc + code + tests + docker config travel together.
- Build the touched projects and run the listed test suites in the worktree
(
dotnet build ZB.MOM.WW.ScadaBridge.slnx,dotnet testfiltered to relevant projects). - Commit on the worktree branch with the message given in the WP spec.
- 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)
git checkout -b arch-review-remediation(currentmain; note the pre-existing dirtydocs/requirements/Component-SiteRuntime.md— stash or leave untouched, do not absorb it).- Baseline: full build + full test run; record pass/fail counts as the reference state.
- Confirm the rig is deployable (
docker/deploy.shlast 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
onCompletedcallback to the client subscribe surface (preferred over an error sentinel — callers can distinguish graceful end from fault). Invoke it when theawait foreachends without exception, including server OK completion at max lifetime and graceful site shutdown. SiteAlarmAggregatorActor: on completion,Tellself a newGrpcAlarmStreamCompleted(generation)message → set_streamDown = true, markIsLive = false, and let the reconcile tick reopen without consuming error-retry/backoff budget (completion is not a fault). Also observe theTask.Runsubscription task itself (ContinueWithfaulted → 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 →
onCompletedfires,onErrordoes not); actor test (stream completes OK →IsLivefalse → 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
DatabasePathrequired on Site nodes (drop the CWD-relative default;ValidateOnStart), mirroring theLocalDb:Pathprecedent — 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
FlushIntervalMsas designed: the writer loop groups commits within the interval (batch already drains up to 256 — add the time-based commit coalescing). Additionally setPRAGMA synchronous=NORMALon 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
RegisterReplicatedcalls when the node has no replication configured (neitherLocalDb:Replication:PeerAddressnorApiKey— 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 atLIMIT; follow the schema's existing versioned-upgrade mechanism so existing site files get it.IX_AuditLog_OccurredAtUtc→ addINCLUDE (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 viaEXPLAIN/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_resolvedAttributeByNameTryGetValueinstead of the LINQ scan (InstanceActor.cs:373).HandleTagValueUpdate: precompute the parsedDataTypeinto 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 JsonSerializerOptionsper 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
SqliteAuditWritershape — 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+ oneApplyChangesAsyncper batch; serialize allApplyChangesAsyncon the shared Subscription behind aSemaphoreSlim(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/WriteBatchAsyncbecome single OPC UA service calls (the MxGateway adapter'sReadBulk/WriteBulkis the in-repo reference).DataConnectionActor:HandleSubscribe/ReSubscribeAllpass whole tag lists; reconnect becomes bounded chunked batches honoring the existing failover stagger;SeedTagsAsyncbecomes 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 EXISTSbatch) 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, orMERGEwith the usual HOLDLOCK hygiene). - Reconciliation off-mailbox: the drain moves out of
ReceiveAsyncto thePipeTo+ in-flight-guard shape —NotificationOutboxActornext 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)) soSWITCHneeds 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 toONLINE = ONrebuild 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); targetedExecuteUpdatefor delivery-attempt status writes instead of detached full-rowUpdate();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
SiteCommandServicequery 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.
SubscribeSitegets 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: mirrorPullSiteCalls' composite(timestamp, id)keyset cursor; flip rows toReconciledonly 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.WhenAllwith short per-tile timeouts so one hung singleton degrades one tile, not the tick.Virtualizeon 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 —
ArtifactDeploymentServiceis 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);
AsNoTrackingon read-only walks;ReconcileDescendantsAsyncreceives the already-loaded graph instead of re-callingGetAllTemplatesAsync. - Management queries:
QueryDeploymentspages DB-side with a summary projection;ListTemplatespages 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-collectibleInteractiveAssemblyLoaderleak 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
ScriptArtifactChangeSubscriberbus (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.
ScriptExecutionActorelimination: run the guarded task directly underScriptActor, preserving supervision semantics (Stop-on-failure equivalence), telemetry, and the auditExecutionId/ParentExecutionIdthreading — 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 bySiteLocalDbSetupwhen replication is unconfigured, closing WP1.3's stale-trigger residual.- Backlog depth becomes O(1) (
MAX(seq) − last_acked_seqor 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=16pin can then be retired. LwwApplierprepare-once-rebind per table; drop the unused__localdb_oplog_hlcindex (handle existing files in the schema-version upgrade). Changes (this repo, after the library lands): bump the package/project reference; call the cleanup API fromSiteLocalDbSetupwhen unregistered; relax the rigMaxBatchSizepin to the byte budget; update the CLAUDE.md LocalDb bullet and the umbrella../scadaproj/CLAUDE.mdScadaBridge 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 replicationhere.
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
IsLivereflects the gap (WP1.1). - CDC: on site-b/site-c nodes,
sqlite_mastershows no capture triggers; oplog table empty and not growing (WP1.3/WP3.3). Site-a pair still converges (replication regression). - Audit path: site
auditlog.dbpresent under/app/data; survivesdocker compose rm+recreate of one site container (WP1.2). - Query plans:
EXPLAIN QUERY PLANon 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.shstill 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 diffreview 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
- Full solution build + tests green in both repos; baseline test count not reduced.
- All §6 live probes pass on the rig; failover drill unchanged (~25s).
- Adversarial review pass complete with all CONFIRMED findings resolved.
- Docs propagated (component docs, both CLAUDE.mds, known-issues) with no stale cross-references.
- 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.