docs: reconcile stale cluster/security/notification docs; close register row 117, add rows 128-135 (review 2026-10-21 wave 2)
This commit is contained in:
@@ -29,7 +29,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 33 | ~~**`LocalDbOplogBacklog` has no threshold, KPI history, or dashboard treatment**~~ **RESOLVED 2026-08-15 (`8da60f57`).** Both halves shipped: sampled into KPI history via `SiteHealthKpiSampleSource` (conditionally — 12 samples unreplicated / 14 replicated, so a series gap truthfully means "not replicated") plus a backlog trend chart on the site health panel, and a SUSTAINED threshold badge (`LocalDbBacklogThreshold` 1000 held for `LocalDbBacklogSustainedFor` 15 min, anchored by `LocalDbBacklogAboveSince` in the aggregator) — a bare instantaneous threshold would fire on every replicated site every day from the documented purge burst. Null backlog is a deliberate third branch: it leaves the anchor unchanged (neither evidence of a drain nor of a backlog), so unreplicated sites can never anchor and can never be badged. NOTE the row's premise was false — the `SiteAuditBacklog` "configurable threshold badge" it told us to mirror was described twice in `Component-HealthMonitoring.md` but NEVER BUILT; both stale doc claims corrected, and the new badge was modeled on the real Metrics-stale badge instead. | Found by R7 while documenting the purge burst | Closed 2026-08-15. | — |
|
||||
| 34 | ~~**`site_events` purge burst is unbounded in the oplog and unmeasured at scale**~~ **RESOLVED 2026-08-16 (`d9a35846`) — measured, and paced.** Measured with the REAL `EventLogPurgeService` over a real `RegisterReplicated` LocalDb file + a loopback sync pair at the rig's `MaxBatchSize: 64`: 393 B/row incl. indexes ⇒ a 1 GB event log holds **~2.73 M rows**, and expiring them in one tick took 285 s and queued **2 731 947 oplog rows = 2.7x LocalDb's `MaxOplogRows` default (1 M)** — enough to make the maintenance tick prune the oplog and flag a FULL ten-table snapshot resync, bought by a routine retention purge. The decisive nuance: the *resident* backlog is produced−minus−drained, so a healthy peer blunts it (391 k peak on loopback, drained 6.0 min) — the old safety margin was a function of peer throughput and vanished with the peer down. Fix: `MaxPurgedRowsPerTick` (default **250 000**, `0` = unlimited escape hatch), one budget SHARED across retention + cap trim; a ceiling-limited tick reschedules after `PurgeCatchUpInterval` (**5 min**, validated `> 0` and `<= PurgeInterval`) so worst-case retention converges in ~55 min instead of 11 days; purge loop is now a delay loop anchored to the END of the previous purge; rate-limited Warning + counter `scadabridge.site.event_log.purge_ceiling_hits`. No purge-exemption path added. Row-33 interplay verified empirically (500 k drained in 46 s vs the 15-min dwell). Fail-before: `PurgeBudget` neutralised → 6 of the new tests fail, all 19 pre-existing pass. Measurement + decision record: `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. Side-finding recorded as row 81 (storage cap unenforceable on a replicated node). | Found by R7 | Closed 2026-08-16. | — |
|
||||
| 35 | ~~**`DeploymentRecords` index does not cover the deployment-list sort key**~~ **RESOLVED 2026-08-15 (`be29bf47`).** Migration `20260815165452_AddDeploymentRecordListSortIndex` + hand-written `docs/plans/sql/AddDeploymentRecordListSortIndex.sql`: composite `IX_DeploymentRecords_DeployedAt_Id (DeployedAt DESC, Id DESC)`; the old single-column index DROPPED as strictly redundant (`Id` is the clustered PK and was already in the old index's leaf as the row locator — same leaf width, now a declared ordered key; keeping it would cost a second index on every insert/status transition for no read benefit). Serves the three reads sharing the key (`QueryDeploymentListPageAsync`, `QueryDeploymentSummariesAsync`, `GetCurrentDeploymentStatusAsync`). `Id` in the key is also a CORRECTNESS matter: a bulk site deploy stamps many rows in one `DeployedAt` instant, so without the tie-break keyset pages can repeat or drop rows. Recorded caveat: the optimizer may already have served the ordering via a backward scan of the old index, so the "sorts per page" premise was possibly optimizer-dependent — the composite makes it deterministic and self-documenting; an actual plan capture against the rig would quantify the before/after. The ConfigurationDatabase suite exercises the descending composite as real DDL via SQLite `EnsureCreated()`, proving provider portability. | Found by R3 (`35ce1413`) | Closed 2026-08-15. | — |
|
||||
| 36 | **Deployments free-text search is unindexable by construction** | Found by R3; re-verified 2026-08-17 (Low-tail sweep) | Four `LIKE '%term%'` predicates (instance name, deployment id, revision hash, user) — leading wildcards preclude index seeks, so a search is a scan of the join (`DeploymentManagerRepository.cs:100-104`; only searched column with any index is `DeploymentId`, unseekable under a leading wildcard). Acceptable at the purge-bounded row count. **2026-08-17 refresh: the row undercounted the cost by half** — `BuildListQuery` is shared with `GetDeploymentStatusCountsAsync`, so one search = TWO scans of the join (list + status tiles), mitigated by the UI's trailing-edge debounce (one pair per pause, not per keystroke). Anchoring the id/hash legs was evaluated and REJECTED as a micro-fix: `DeploymentListFilter.Search`'s doc comment promises substring matching and a 4-case `[Theory]` deliberately pins mid-string matches — changing it is a contract renegotiation, not an optimization. Fix if hot: renegotiate to prefix on id/hash legs, or full-text. *(Low)* | Search latency complaints on the Deployments page |
|
||||
| 36 | **Deployments free-text search is unindexable by construction** | Found by R3; re-verified 2026-08-17 (Low-tail sweep) | Four `LIKE '%term%'` predicates (instance name, deployment id, revision hash, user) — leading wildcards preclude index seeks, so a search is a scan of the join (`DeploymentManagerRepository.cs:100-104`; only searched column with any index is `DeploymentId`, unseekable under a leading wildcard). Acceptable at the purge-bounded row count. **2026-08-17 refresh: the row undercounted the cost by half** — `BuildListQuery` is shared with `GetDeploymentStatusCountsAsync`, so one search = TWO scans of the join (list + status tiles), mitigated by the UI's trailing-edge debounce (one pair per pause, not per keystroke). Anchoring the id/hash legs was evaluated and REJECTED as a micro-fix: `DeploymentListFilter.Search`'s doc comment promises substring matching and a 4-case `[Theory]` deliberately pins mid-string matches — changing it is a contract renegotiation, not an optimization. Fix if hot: renegotiate to prefix on id/hash legs, or full-text. *(Low)* **2026-10-21 note:** the same leading-wildcard `LIKE` shape exists on Instances and `AuditLogEntries` free-text search. `ListInstances` is no longer one of them — it pages, site-scopes and **prefix**-searches in SQL as of the 2026-10-21 review remediation (`baab6ac7`; CLI `instance list --search` is now a prefix match) — but the config-audit viewer filters and the Deployments page remain scans. | Search latency complaints on the Deployments page or the config audit log |
|
||||
| 37 | ~~**Two paged reads of `DeploymentRecords` with different contracts**~~ **RESOLVED 2026-08-17 (merge `ad0100f2`, agent commit `9f67147e`).** `QueryDeploymentSummariesAsync` and `DeploymentRecordSummary` are DELETED; the CLI/HTTP `QueryDeployments` handler now rides `QueryDeploymentListPageAsync` + `DeploymentListFilter` — one row shape (`DeploymentListRow`), one scoping mechanism (site-id via the query join), total count available to both surfaces (`DeploymentListPage` now also owns `MaxPageSize` + `DefaultPageSize = 50`). Rows 86–88 were resolved with the consolidation, as this row anticipated. Row 36's 4-case substring-search Theory untouched and still passing. Remaining UI-vs-CLI asymmetry (`GetDeploymentStatusCountsAsync` status tiles are UI-only) is deliberate, not drift. **Live-probed on the rig:** CLI `deploy status --status Success --page-size 5` returns the consolidated page — rows now carry `siteId` + `instanceUniqueName`, and the response carries `totalCount` (11) the CLI never had. | Found by R3; consolidation shipped with rows 86–88 | Closed 2026-08-17. | — |
|
||||
| 38 | ~~**OtOpcUa: `RoslynVirtualTagEvaluatorTests` racing-clear test is load-fragile**~~ **RESOLVED 2026-08-18 in OtOpcUa (`34829ea0` + `837d8a17`, branch perf/followups-r3).** Rebuilt on the assert-on-retry-exhaustion option, renamed `…never_leaks_a_raw_disposed_failure`. Two findings beyond the brief: (1) the DOMINANT load-failure mode was the test's own 45 s `WaitAsync` budget — every clear forces a recompile, so the fixed 12×400 workload's wall-clock tracked spare CPU; reproduced RED 4/4 at 4-way self-concurrency, fixed by duration-bounding the loops (10 s window OR 400 iters), GREEN 4/4 and 8/8 after. (2) A second flake vector: `TimedScriptEvaluator`'s 5 s clock starts at enqueue, so pool starvation surfaces "script timed out" from healthy evaluations — now its own bounded bucket, not counted as a leak; failure-reason literals promoted to shared consts. Honest caveat recorded: the disposed race itself does not reproduce on the arm64 macOS box (zero ODEs even with the product retry disabled), so the raw-escape assertion is the load-independent guarantee. Commit stats cross-checked from this repo. | Found by R4; fixed cross-session by the OtOpcUa agent 2026-08-18 | Closed 2026-08-18. | — |
|
||||
| 39 | ~~**OtOpcUa: late-enabled replication cannot baseline pre-existing rows**~~ **RESOLVED 2026-08-18 in OtOpcUa (`3e62f161` + review round `df533916`, branch perf/followups-r3).** ScadaBridge's WP1.3/WP3.3 pattern ported: registration follows replication config — configured ⇒ `RegisterReplicated(..., baselineExistingRows: true)` (probed live on 0.2.1: seeds `__localdb_row_version` at HLC 0), unconfigured ⇒ boot-time `DeregisterReplicated` self-heal, pair-symmetric; review round added deregister-preserves-data + partial-flip-fails-loudly tests. Green: `LocalDbSetupTests` 15/15, Host.IntegrationTests 226 pass/0 fail. **Stayed pinned to LocalDb 0.2.1 by deliberate choice** — the 0.2.1 API already carries both needed methods, and the v2→v3 one-way bookkeeping upgrade makes a 0.3.x bump a both-nodes-together operation; recorded caveat in OtOpcUa's CLAUDE.md: on 0.2.1 the snapshot-resync flag is per-DATABASE, so a late enable snapshots the whole file rather than per-table (0.3.1 available; bump logged as a deliberate future change in OtOpcUa's own follow-up register). Commit stats cross-checked from this repo. | Found by R4; fixed cross-session by the OtOpcUa agent 2026-08-18 | Closed 2026-08-18. | — |
|
||||
@@ -109,7 +109,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 114 | *Never issued.* The 2026-09-09 step-logs close-out minted two rows numbered **113** — this one and the "other actors' log lines" follow-up. The duplicates were renumbered **115** / **116** at final review (I2) and 114 was left unused rather than reshuffling every number after it; register numbers are permanent identifiers, cited from code comments and `CLAUDE.md`. | — | — | — |
|
||||
| 115 | **Other actors' log lines are not captured for an execution — only the script's own `Log.*` and the runtime lines inside a scoped helper** *(Low)*. The Flow Debugger's step logs (2026-09-09) capture what the script logs and what `ScriptRuntimeContext`'s own logger emits while a run is traced, because those two go through one seam. What the *Instance Actor*, the *DCL*, the alarm actors or the S&F engine log while working on the same execution never reaches the graph: there are **zero** `LogContext.PushProperty` calls in the repo, `Enrich.FromLogContext()` is not configured on the Serilog pipeline (`TraceContextEnricher` stamps W3C ids only), and `site_events` has no execution-id column. Capturing them is a separate enrichment initiative: push `ExecutionId` (and instance/script) onto the ambient `LogContext` at run start, enable `Enrich.FromLogContext()`, add an execution-id column + index to `site_events`, then either sink matching entries into `ScriptLog` rows or query the site event log by execution id. | Step-logs design `docs/plans/2026-09-09-flow-debugger-step-logs-design.md` §2 + §9 ("Out of scope") | Out of proportion to the ask: it changes the logging pipeline of every actor on a site node, adds a replicated-table column (`site_events` is one of LocalDb's ten `ReplicatedTables`, so the change is a both-nodes-together schema edit), and risks a volume increase on untraced runs unless the enrichment is itself gated. The script-authored lines cover the debugging case the Flow Debugger exists for. | First investigation where the answer sits in what the Instance Actor or the DCL logged during a traced run and the operator cannot correlate it by hand. |
|
||||
| 116 | **`CompileSurfaceParityTests.AccessorMirrorPairs` cannot guard the `(ScriptLogger, CompileLog)` pair — the theory has no per-member exclusion mechanism** *(Low)*. The arity theory asserts the compile mirror exposes at least as many overloads as the runtime accessor for **every** public instance method, and `ScriptLogger` has one the mirror must NOT have: `FlushDropped()`, the run-end closing-row flush, which is called by `ScriptRunLauncher`, not by a script. Adding the pair today would fail the theory for a member that is correctly absent, so the pair is left out and `Log`'s four methods are guarded only by the top-level name-superset check plus the hand-written surface tests. The fix is an exclusion seam on the theory — an optional `IReadOnlySet<string> ignoredRuntimeMethods` per row (or an `[NotScriptReachable]` marker attribute on `FlushDropped`, which also documents the intent at the declaration) — after which the pair joins `AccessorMirrorPairs`. | Found while wiring Task 15 of `docs/plans/2026-09-09-flow-debugger-step-logs.md`; test `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/CompileSurfaceParityTests.cs` | No drift is possible without a code change to `ScriptLogger`, and a missing `Log` method on the mirror surfaces immediately as a `CS1061` at the design-time gate for any script that calls it. The seam is a test-infrastructure change that touches every existing row's signature. | The next public method added to `ScriptLogger` (or to any runtime accessor whose mirror must deliberately omit a member) — at that point build the exclusion seam and add the pair. |
|
||||
| 117 | **Flow Debugger site trace pull: `FlowDebugRunner.WriteAndPollAsync` fetches ONE bounded page with a fixed `since`** *(Low)*. The write-and-poll loop asks the site for `FlowDebugRunner.PullTraceEventsMax` rows per poll, always `since` the run start, and the site serves `ORDER BY OccurredAtUtc ASC LIMIT` — so the page is the OLDEST rows in the window, never a continuation of the previous one. Step logs made that reachable: one traced run can now emit up to 201 `ScriptLog` rows, so a handful of chatty runs inside the window can fill the page and push a later spawned run's `ScriptRun` row past the end of it, at which point the loop never sees the awaited run and reports a false `NO_TRIGGER_WITHIN_WINDOW` for a run that did happen. Final review I1 raised the cap 500 -> 2 000 (the site's own ceiling, `SiteStreamGrpcServer.MaxTraceEventsPerPull`), which makes it ~10x harder but does not remove it. The real fix is to PAGE — advance `since` to the last-seen `OccurredAtUtc` between polls (keeping the already-known ids so nothing is double-counted) — or to exclude `ScriptLog` from the awaited-run poll entirely and pull the log rows once, after the run has settled. | Final review of `docs/plans/2026-09-09-flow-debugger-step-logs.md` (I1); `src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs` `WriteAndPollAsync` | 2 000 is the site's hard ceiling, so the headroom cannot be bought again with a bigger number; paging needs the poll loop to track a cursor and the awaited-run predicate to survive a partial view, which is more than the raise was worth at review time. | The first `NO_TRIGGER_WITHIN_WINDOW` reported for a run whose Replay of the same root DOES show the spawned `ScriptRun`. |
|
||||
| 117 | ~~**Flow Debugger site trace pull: `FlowDebugRunner.WriteAndPollAsync` fetches ONE bounded page with a fixed `since`**~~ **RESOLVED 2026-09-09 (`9ee402bb`, arch-review 2026-09-09 MG-6; closed in this register 2026-10-21 — found stale by the 2026-10-21 system review).** `WriteAndPollAsync` now advances a `(SinceUtc, AfterEventId)` keyset cursor between polls and only treats a page as progress when the cursor moved, so a chatty run can no longer push a spawned run past the page end. Original description kept for traceability: *(Low)*. The write-and-poll loop asks the site for `FlowDebugRunner.PullTraceEventsMax` rows per poll, always `since` the run start, and the site serves `ORDER BY OccurredAtUtc ASC LIMIT` — so the page is the OLDEST rows in the window, never a continuation of the previous one. Step logs made that reachable: one traced run can now emit up to 201 `ScriptLog` rows, so a handful of chatty runs inside the window can fill the page and push a later spawned run's `ScriptRun` row past the end of it, at which point the loop never sees the awaited run and reports a false `NO_TRIGGER_WITHIN_WINDOW` for a run that did happen. Final review I1 raised the cap 500 -> 2 000 (the site's own ceiling, `SiteStreamGrpcServer.MaxTraceEventsPerPull`), which makes it ~10x harder but does not remove it. The real fix is to PAGE — advance `since` to the last-seen `OccurredAtUtc` between polls (keeping the already-known ids so nothing is double-counted) — or to exclude `ScriptLog` from the awaited-run poll entirely and pull the log rows once, after the run has settled. | Final review of `docs/plans/2026-09-09-flow-debugger-step-logs.md` (I1); `src/ZB.MOM.WW.ScadaBridge.ManagementService/FlowDebug/FlowDebugRunner.cs` `WriteAndPollAsync` | 2 000 is the site's hard ceiling, so the headroom cannot be bought again with a bigger number; paging needs the poll loop to track a cursor and the awaited-run predicate to survive a partial view, which is more than the raise was worth at review time. | The first `NO_TRIGGER_WITHIN_WINDOW` reported for a run whose Replay of the same root DOES show the spawned `ScriptRun`. |
|
||||
| 118 | **Pre-existing bUnit flake: `CentralUI.Tests.Pages.NotificationReportDetailModalTests.CloseButton_DismissesModal` (~10 %)** *(Low)*. Fails intermittently with `UnknownEventHandlerIdException` — the test resolves the close button once and clicks it after a re-render has already retired that element's handler id. Not caused by the step-logs work: the file is untouched since `40944a0d`. The fix is a one-liner — re-`Find` the button immediately before the click (the pattern the other modal tests already use) rather than reusing an element captured earlier in the test. | Observed during the step-logs full-suite runs, 2026-09-09 | It is a test-only race with a known cause and a known fix; it was out of scope for a remediation pass whose rule was to touch only the files under review. | Next time `CentralUI.Tests` is worked on, or the first time the flake blocks a gate run. |
|
||||
| 68 | ~~**`deploy.sh` run from a git worktree silently rehomes every node's bind-mount data dir**~~ **RESOLVED 2026-08-15 (guard added in the same close-out).** The compose files' bind mounts are relative (`./site-a-node-a/data`), so a deploy from a worktree checkout mounts the worktree's copies — the rig keeps running and looks healthy, but its SQLite stores split from the main checkout's and everything written there is orphaned by the next deploy from main. Bit the rows-46–53 close-out live: the seeding agent's `rig-probe-1` deploy persisted into worktree-homed dirs and "vanished" after the orchestrator's redeploy (no tombstone, no product bug — LWW/tombstone forensics ruled out deletion; the row sat in `.claude/worktrees/…/docker/site-a-node-a/data/site-localdb.db` the whole time). Both `docker/deploy.sh` and `docker-env2/deploy.sh` now refuse to run from a `.claude/worktrees/` path (override: `SCADABRIDGE_ALLOW_WORKTREE_DEPLOY=1`). | Found during the rows-46–53 close-out | Closed 2026-08-15. | — |
|
||||
| 119 | **Port the QA/prod MES receiver scripts onto the reset-aware `WriteBatchAndWaitAsync` overload (Gitea #36)** *(Medium)*. Template scripts CvdReactor MesMoveIn (13) / MesMoveOut (14) / MesSampleData (36) and base MESReceiver MoveIn/MoveOut (15, 16, 25, 26, 37, 38) still reset `<X>CompleteFlag` with a plain write and rely on `Task.Delay(1000)`; QA carries a config-only `WaitAsync(false, 10 s)` patch on 13/14 (deployment record 110). Rewrite: `var hs = await rx.Attributes.WriteBatchAndWaitAsync(values, "<X>Flag", true, "<X>CompleteFlag", true, 25 s, "<X>CompleteFlag", false, 10 s); if (!hs.Matched) return new { WasSuccessful=false, ErrorText = hs.ResetFailed ? "Timeout waiting for <X>CompleteFlag to reset" : "Timeout waiting for <X> to complete", BatchID=0 };` and delete the delay. Requires the #36 build on the target central + sites first (the overload does not exist on older sites → deploy-gate compile failure, by design). | Gitea #36, design `2026-09-10-handshake-reset-echo-design.md` §8 | Repo-only scope was the user's decision for the #36 session; the script bodies live only in the QA/prod databases. | Next QA deployment of a build ≥ this change (write a deployment record; then prod). |
|
||||
@@ -121,6 +121,14 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 125 | **Pending-updates counts have no Health-dashboard tile or CLI surface** *(Low)*. The aggregate (`PendingUpdatesSnapshot.StaleInstanceCount` / `StaleArtifactSiteCount`) is consumed only by the Central UI Sites page and rail indicator; there is no Health dashboard KPI tile and no `deploy`/`site` CLI command surfacing it — `site list` shows only the raw `artifactRevisionHash`/`artifactsDeployedAt` fields on each site, not whether they are current. | `docs/plans/2026-09-14-pending-updates-design.md` §3.2 ("Not in scope") | Out of scope for the UI-focused ask; the CLI/Health surfaces are a separate consumer of the same `IPendingUpdatesCache`. | An operator wants the pending-updates counts without opening the Central UI (headless monitoring, CLI scripting). |
|
||||
| 126 | ~~**TODO — roll the two outstanding migrations to the QA pair vq03/vq04**~~ **DONE 2026-09-28** — QA vq03/vq04 upgraded `2b9a7c3c` → `b3021e7c`, 55 → 57, artifacts deployed (record `docs/deployment/deployment-records/2026-09-28-wonder-app-vq03-vq04-b3021e7c.md`). *(deployment, closed)*. **DEV vd03 DONE 2026-09-28** (`fe89363b`, 55 → 57, record `docs/deployment/deployment-records/2026-09-28-wonder-app-vd03-fe89363b.md` — the `sb-mig-55to57.sql` there is the same script QA needs). QA still runs `2b9a7c3c` with the DB at migration 55 (`20260909203046_EnforceGlobalInstanceNamesAndProtectProtocolConfiguration`); `main` (`ac833f5a`, pushed 2026-09-14) carries two more: **56** `20260911135501_AddAlarmMesReportable` (computed-alarm `IsMesReportable` flag) and **57** `20260914173147_AddSiteArtifactRevision` (`Sites.ArtifactRevisionHash`/`ArtifactsDeployedAt`, both nullable). Both are additive `AddColumn`s — no data upgrade, no coordinated step like the 2026-09-10 protocol-config encryption. Recipe (same as the 55 rollout): generate ONE from→to delta script `dotnet ef migrations script EnforceGlobalInstanceNamesAndProtectProtocolConfiguration AddSiteArtifactRevision --project src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase --startup-project src/ZB.MOM.WW.ScadaBridge.Host -o sb-mig-55to57.sql` (NOT idempotent, identical for vd03 and the QA pair), take the pre-upgrade DB backup, stop the pair TOGETHER, apply via `sqlcmd -C -b -I` on the SQL host, publish/swap the `ac833f5a` binaries, start the pair, confirm the Central boot log shows 0 pending migrations and no "Applying migration" (the script already applied them). After upgrade every site reads *Artifacts never deployed* until one *Deploy Artifacts to All Sites* — run it as the last step. Write the deployment record `docs/deployment/deployment-records/<date>-wonder-app-{vd03,vq03-vq04}-ac833f5a.md`. | Pending-updates finish (2026-09-14) | ~~Blocked by the 2026-09-11 remote-access hold~~ — **unblocked 2026-09-28**: `winrm` is re-enabled for deployment activities (CLAUDE.md). vd03 done; QA done 2026-09-28. | Closed 2026-09-28. |
|
||||
| 127 | **TODO — roll out site secrets via artifact deploy to vd03 + QA (vq03/vq04)**. The gRPC secrets hub is retired in ScadaBridge as of `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` (merged to `main` @ `23a139a4` 2026-09-28, plus the same-day follow-ups: per-site secret-store outage handling, removal audit rows, per-node mirror status on the health report, artifact-deploy CLI exit codes; rig-only so far); vd03 and QA still run the pre-feature build with `Secrets:Replication:Enabled=false`, so upgrading them will **not** fail the boot (the retirement's `StartupValidator` rejection only fires on `Enabled=true`). **Upgrade impact:** once upgraded, any site whose data connections carry `secret:` references fails its **whole** artifact deploy (scripts, external systems, DB connections too) and reads *Out of date* until every referenced secret exists in the central store — so before upgrading vd03/QA, check whether their connection configs use `secret:` references and whether their site pairs run LocalDb replication, and do the prerequisites in the same window. Prerequisites, in order: (1) provision the `zbsecrets` schema/store in the environment's configuration SQL Server (the same server `ConfigurationDb` already points at); (2) set `Secrets:Store:Provider=SqlServer` on **both** central nodes of the environment; (3) verify the KEK (`ZB_SECRETS_MASTER_KEY` / master-key file) is identical across **every** node — both centrals and every site node — since a mismatch is caught only at deploy time by `DeployedSecretsMirror`'s per-secret verify, not at boot (after the first deploy a mismatch on either node — the standby included — shows as a *Secret mirror failing* badge on that site's Health card); (4) confirm every site pair that uses secrets runs LocalDb replication — on an unreplicated pair only the node active at deploy time gets the secrets, so a failover leaves the new active node without them until the next *Deploy Artifacts*; the upgrade also adds two replicated site tables (`deployed_secrets`, `secret_mirror_status`), so stop and start each site pair together as usual; (5) recreate, at the new central shared store, any secret an operator previously hand-seeded directly into a site's local `secrets.db` (those never round-tripped through central and would otherwise be silently absent from the referenced-secrets scan). Then run *Deploy Artifacts* against every affected site as the rollout's last step. | `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` §4 *Rollout*; `docs/deployment/topology-guide.md` → *Secrets: shared store, per-site deploy, and the KEK* | Deployment record, not a code change — vd03/QA config is user-held and their rollout schedule is independent of the branch merging to main. | User asks for the vd03/QA secrets window. |
|
||||
| 128 | **No TLS production profile: Traefik, Akka remoting and the gRPC h2c listeners are all plaintext** *(Medium for a non-lab network; accepted for the lab)*. Only the gRPC command/control and stream boundaries carry per-site PSK auth; nothing is encrypted and Akka remoting is unauthenticated (see CLAUDE.md *Security*). A deployable TLS profile — Traefik entrypoint certs, Akka `enable-ssl` + secure cookie, gRPC over h2/TLS with the PSK retained — has never been designed end to end. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (carried from prior reviews) | Trusted-network lab/plant deployment is the stated assumption; TLS is purely additive to the PSK design and needs no protocol change, but needs cert distribution for 8+ nodes. | First non-lab deployment (any network not wholly inside the plant security boundary). |
|
||||
| 129 | **`ExternalDefinitionLookupCache` is not invalidated on external-system / DB-connection edits** *(Low)*. The central short-TTL definition cache (review 05/P2) is never `InvalidateAll()`'d by the management edit paths because ManagementService has no reference to ExternalSystemGateway; the node that took the edit and its peer both see it only after `DefinitionCacheTtl` (default 10 s). | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane M) | A 10 s staleness window on a rarely-edited definition is harmless; wiring needs a new project reference or a bus event. | A report of an edit visibly not taking effect for an inbound-method call, or any increase of `DefinitionCacheTtl`. |
|
||||
| 130 | **`TemplateService` member add/update still loads the whole template analysis graph** *(Low)*. 04/P6 is only partly done: the read paths use lean projections, but member add/update still materialises the full analysis graph because no repository method loads just a template's inheritance chain + composition closure. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lanes F/G, 04/P6 partial) | Designer-rate operation; the new chain-only query is a repository + service refactor with its own correctness risk. | Template-edit latency complaints on a large library, or the library growing past a few hundred templates. |
|
||||
| 131 | **`SiteStreamGrpcClientFactory` never evicts idle `(siteId, endpoint)` clients** *(Low)*. Clients are keyed per site and endpoint and live until disposed with their session/factory; an endpoint that is removed or renamed in the site's address pair leaves its client (and channel) resident. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane E item 6) | Bounded by the number of address edits over a process lifetime; a rename is rare. | Sites whose gRPC addresses change often, or a channel/handle leak seen in a long-running central. |
|
||||
| 132 | **Site compile-cache eviction counter is not exported as a Meter** *(Low)*. `SiteScriptCompileCache` counts evictions and logs a rate-limited Warning (review 01/SR-M2) but the counter is not a `System.Diagnostics.Metrics` instrument, and `ZbTelemetryOptions.Meters` is an allowlist (`SiteServiceRegistration.ObservedMeters`), so exporting it also needs the Host allowlist entry. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane B) | The Warning log already surfaces thrash; a Meter is observability polish. | Wanting to alert on compile-cache thrash from Prometheus. |
|
||||
| 133 | **Dedicated health field for site audit rejected rows** *(Low)*. Permanent push rejections (`IngestAck.rejected_event_ids`) are counted only by the `scadabridge.site.audit.rejected_rows` counter and the `Rejected` `ForwardState`; there is no `SiteHealthReport` field/dashboard badge. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane H) | Rejections only come from row-intrinsic SQL errors and should be near-zero; the counter is sufficient for now. | The first non-zero `rejected_rows` seen in production, or an operator asking for it on the Health dashboard. |
|
||||
| 134 | **An oversize `AuditLog` row (larger than the gRPC message cap) wedges both push and pull** *(Low, accepted)*. A single row bigger than the 4 MB gRPC cap cannot be shipped by either the telemetry push or the reconciliation pull, so the queue head stalls. Unreachable at the default caps (8 KB / 64 KB payload caps, `InboundMaxBytes` 1 MiB) — it needs an operator to raise caps past the transport limit. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane H) | Defaults keep every row far below the limit; guarding means a skip/park state for unshippable rows. | Any deployment that raises `InboundMaxBytes` or the payload caps toward 4 MB. |
|
||||
| 135 | **Config-audit single-column indexes are possibly redundant** *(Low)*. `ReviewRemediationIndexes` added composite `AuditLogEntries` indexes `(EntityType, Timestamp DESC)`, `(User, Timestamp DESC)` and `(Timestamp DESC, Id DESC)`; the older single-column indexes on timestamp/user/entity type may now be dead weight on a write-heavy table. | 2026-10-21 system review (`docs/plans/2026-10-21-system-review-remediation.md`) (lane P / O) | Dropping needs a measured before/after on a populated table, not an assumption. | Measure index usage (`sys.dm_db_index_usage_stats`) after the migration has run in QA/prod for a few weeks, then drop the unused ones. |
|
||||
|
||||
## Resolved (verified against the code 2026-07-10)
|
||||
Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability.
|
||||
|
||||
Reference in New Issue
Block a user