docs: CLAUDE.md review-remediation facts + 2026-10-21 remediation record

docs/plans/2026-10-21-system-review-remediation.md

@@ -0,0 +1,55 @@
+# System review remediation (2026-10-21)
+
+Decision and execution record. Source: eight read-only parallel slice reviews of `main @ 513fa1aa` (findings in the review directory `00-SUMMARY.md` + `01..08`), excluding items already known from the 2026-09-09 arch review, the deferred-work register and CLAUDE.md. Remediation base for wave 2 docs: `1627513f`.
+
+## Scope
+
+8 slices (SiteRuntime; site data path DCL/S&F/event log; Communication + cluster; Deploy/Template/ScriptAnalysis/Transport; Management/InboundAPI/Security/ESG/Host; Audit/SiteCalls/Outbox/Notify/KPI/Health; Central UI + CLI; ConfigDB/Commons + gap inventory). ~107 findings: 4 High, ~30 Medium, ~73 Low. Recurring themes: non-collectible Roslyn loads, tracked whole-graph template reads for id/name lookups, uncached LDAP bind + N+1 role mapping, reconciliation paths that replay everything row-at-a-time, options validated but never read, sticky process-wide failure caches.
+
+## Owner decisions
+
+- **CLI auth = bearer per invocation.** One LDAP bind at `POST /auth/token`, then `Authorization: Bearer` on every management/audit/debug-stream call; Basic remains the fallback. Roles in a token are authoritative until expiry (<= `JwtExpiryMinutes`, 15); rotating `JwtSigningKey` invalidates all tokens; `StartupValidator` requires a >= 32-byte key on Central; `/auth/token` answers 404 under `DisableLogin`.
+- **`Database:MachineDataDb` removed** (a required boot key nothing read).
+- **Config audit stores summaries + optional retention** (script body = length + SHA-256; `AfterStateJson` capped 256 K; `ConfigAudit:RetentionDays` default 0 = keep).
+- **Bound Test Run gate:** Test Run against a deployed instance needs Deployer + site scope and writes an audit row (closes the Flow Debugger F2/F3 asymmetry).
+- **Overrides seed, not fire:** persisted static overrides loaded at actor start seed script state without firing scripts; alarms still evaluate.
+
+## Lanes (one line each)
+
+- **A SiteRuntime:** actor-name gate (`ActorNameRules`) + guarded `ActorOf` so a bad name can never restart the singleton (High); overrides republished/seeded on start; per-site serialised artifact apply with 10-min watchdog; metadata-only reconcile sweep; batched `Route.To().Get/SetAttributes`; DCL connection actor names ASCII-sanitised with collision rejection; startup warm/parallel compile.
+- **B Scripts:** compile-cache eviction counter/warning + configurable bound (`ScriptCompileCacheMaxEntries`), faulted compiles never cached, scheduler slot reuse, shed-log rate limiting, native-alarm deletes batched into the flush.
+- **C DCL:** same-adapter reconnect advances the session generation at issue time (High); MxGateway subscription ids `{session}:{handle}`; `IDataConnection.WriteBatchAndWaitAsync` removed; `MxAlarmStreamReopenDebounce` 200 ms; `ClientNamePrefix` now live; OPC UA discovery async and bounded by `OperationTimeoutMs`.
+- **D Store-and-forward / event log:** per-lane ranked paging, gauge re-seed; event-log reads off the write lock with `KeywordSearchMaxWindow`; cap-trim count refresh every 10th batch, one vacuum per tick.
+- **E Communication:** failback probe is a real readiness RPC (sentinel `$scadabridge-failback-probe`, 5-min probation); site-address refresh single-flight with deadline + sequence; streaming consumers re-resolve addresses on reopen; reconcile cancellation flows from the gRPC call; malformed address keeps the old channel (High); dead `SiteContacts` code deleted.
+- **F Script analysis / Template Engine:** compile gate no longer emits/loads assemblies; trust verdicts cached by code hash; `BlockingWaitAnalyzer` warning; `ActorNameRules` at save; lean template projections.
+- **G Deployment:** hash-only revision computation; single `FlattenSession` for bulk deploy pre-filter + prepare; reconcile fixed round trips with 3 retries; system artifact deployment records purged under `TerminalDeploymentRecordRetention`.
+- **H Site audit:** reconciliation pull serves only un-acked rows; new site-only `PushAbandoned`/`Rejected` states; `IngestAck.rejected_event_ids` (field 2); persisted high-water; set-based paged central ingest (High); paged purge; fail-fast writer; per-site state pruning.
+- **I Site calls / outbox / notify / KPI:** node-local `OperationTrackingCentralAck` ledger for un-acked pull; 128-row upsert chunks; conditional-aggregate KPI queries; outbox per-attempt timestamps, Pending/Retrying split claim, `DispatchSweepBudget` 2 min; SMTP `MaxConcurrentConnections` clamps Email parallelism; OAuth2 token cache eviction; KPI fold hour-at-a-time.
+- **J Host / Inbound / ESG:** single-flight method compile and warm-up service, relay nonce single-use, in-memory request buffering, `ApiMethodCache` pruning, JSON GET/DELETE query params, central definition cache (`DefinitionCacheTtl` 10 s), secret-store fault vs missing PSK, NodeOptions-bound ports.
+- **K Security / CLI:** bearer-per-invocation, `RoleMappingCache` (30 s, invalidated post-commit by the management handlers), 10 s display-state memo, dead refresh members removed from `JwtTokenService`.
+- **L Central UI:** blocking-wait warning as Monaco marker, bounded script-analysis payload/concurrency, Topology behind `PollGate`, lean projections, bounded caches/rows, safe-invoke fire-and-forget, negative TTL for a faulted pending-updates flight.
+- **M Management:** ListInstances SQL paging/prefix search, 409 `IMPORT_IN_PROGRESS` single-flight for large bodies, `CompileForPublish` once per save, site address + native-alarm-source name validation, terminal `PendingSecuredWrites` purge (`SecuredWriteTerminalRetention` 90 d).
+- **N Transport:** import validates site gRPC addresses and actor-safe names as blockers; compressed decrypted content pinned on the session and zeroed on every release; template library read once; `StaleInstanceIds` covers derived/composing templates; `Transport:SchemaVersionMajor` removed.
+- **O/P ConfigDB:** deterministic config-audit paging, `InstanceLocator` single projected join, dead repository members deleted, config-audit purge + `ConfigAuditOptions` binding, migration `ReviewRemediationIndexes`.
+- NuGet: `System.Security.Cryptography.Xml`/`Pkcs` and SignalR.Common pinned to 10.0.13.
+
+## Adversarial-review catches (fixed before merge)
+
+- **C reconnect race:** the first generation bump left a window where a stale prior-session subscribe completion could still land; the bump now happens when the reconnect attempt is issued.
+- **H central-fault refusals:** a central-side transient fault was being counted as a permanent refusal toward push abandonment; only row-intrinsic SQL errors are rejected now, and the served high-water is persisted so a prune cannot skip rows.
+- **A data-connection actor names:** the actor-name gate covered instances/scripts/alarms but not DCL connection actors; names are sanitised and collisions rejected at deploy.
+- **I outbox double-send:** a single claim over mixed Pending/Retrying rows could re-deliver a row mid-sweep; the claim is split by status and bounded by `DispatchSweepBudget`.
+- Also: failback probe sentinel changed to an illegal actor name; alarm aggregator reconcile no longer reopens during a pending backoff reconnect (double subscribe); `RoleMappingCache` invalidation after commit (lane K review M1).
+
+## Deferred to the register
+
+Rows 128-135 of `docs/plans/2026-07-08-deferred-work-register.md`: TLS production profile; `ExternalDefinitionLookupCache` invalidation; whole-graph load on template member add/update (04/P6 partial); idle `SiteStreamGrpcClientFactory` client eviction; compile-cache eviction Meter; dedicated rejected-rows health field; oversize audit row wedge (accepted); possibly redundant config-audit indexes. Row 36 annotated (same free-text `LIKE` shape on Instances/config audit; ListInstances now prefix-paged). Row 117 closed (already fixed by arch-review MG-6).
+
+## Rollout notes
+
+- **Not deployed:** the docker rig is rebuilt from main; QA (vq03/vq04) and prod (vd03) are not deployed.
+- **Migration:** `ReviewRemediationIndexes` adds `AuditLog` `IX_AuditLog_Kind_Occurred` and `IX_AuditLog_Actor_Occurred` (partition-aligned) plus three composite `AuditLogEntries` indexes. Production applies `docs/plans/sql/ReviewRemediationIndexes.sql` by hand (index builds on a large partitioned `AuditLog`: schedule off-hours). Not yet applied to QA/prod.
+- **Mixed version:** upgrade central first (new `IngestAck` field 2 and proto additions are additive; an old central ignores `rejected_event_ids` and an old site never rejects). Pair upgrades stay both-nodes-together for LocalDb sites.
+- **Config changes:** CLI hosts need a central that serves `/auth/token` (older centrals fall back to Basic automatically); Central must have a >= 32-byte `JwtSigningKey` or fails startup validation; remove `Database:MachineDataDb` and `Transport:SchemaVersionMajor` from appsettings (ignored if left).
+- **New keys (all defaulted):** `ScadaBridge:ConfigAudit:{RetentionDays 0, PurgeBatchSize 5000, MaxBatchesPerTick 200, PurgeInterval 1d}`; `Security:RoleMappingCacheSeconds 30`; `SiteEventLog:{KeywordSearchMaxWindow 7d, CapTrimCountRefreshBatches 10}`; `AuditLog` site writer/reconciler `{MaxRejectedPushAttempts 10, PurgeBatchSize 5000, PurgeVacuumPagesPerBatch 1000, MaxPagesPerTick 50}`; `SiteRuntime:ScriptCompileCacheMaxEntries 1024`; `NotificationOutbox:DispatchSweepBudget 2m`; `ManagementService:{SecuredWriteTerminalRetention 90d, SecuredWriteTerminalPurgeInterval 1h}`; ESG `DefinitionCacheTtl 10s` (central); DCL `MxAlarmStreamReopenDebounce 200ms`; `DeploymentManager:TerminalDeploymentRecordRetention` governs system artifact deployment record purge.
+- **Behaviour changes operators will notice:** `instance list --search` is prefix-only; Test Run against a deployed instance needs Deployer + site scope; deploys reject non-actor-safe names (rename before redeploy); a redeployed instance's persisted overrides no longer re-fire value-change scripts on start.