Commit Graph

109 Commits

Author SHA1 Message Date
Joseph Doherty b95ee5a658 feat(audit): archive retention and superseded stale-archive cleanup (ScadaBridge#109)
Owner decisions recorded: the archive destination stays a directory/UNC
(no object-storage client; share + permissions named at enablement),
parquet-tools-only reading (no re-import), unsigned like the chain (#107).

AuditLog:Archive:RetentionDays (default 0 = never delete; else must exceed
AuditLog:RetentionDays, counted from the archived month's end) and
DeleteSupersededStaleArchives (default true) run after every purge tick.
A retried purge's stale Archived archive is deleted only when a later,
verifying, Purged archive of the same partition provably holds every row:
same prior anchor and the stale chain-end hash at its chain-end sequence
(row or tombstone), every unchained row present by EventId; otherwise it is
reported KeptStale. A sole copy left at Archived is never touched. Deletion
removes only the archive name's own three files in its folder, manifest
first, never a manifest-supplied path. `audit archive cleanup [--dry-run]`
= CleanupAuditArchives (Administrator).

AuditLogArchiveCleanupTests (MSSQL) cover retried purge, kept stale copy,
sole copy, retention and the path guard; each safety rule mutation-checked.
Follow-ups filed: #113, #114, #115.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 23:33:04 -05:00
Joseph Doherty c4d7ec9b96 feat(audit): off-box chain-head export, verify-heads, maintenance login (ScadaBridge#107)
Item 1 (owner decision: no signing). AuditLog:HashChain:HeadExport (default
off): both centrals periodically read every month's chain head (LastSeq,
LastHash, purge anchor, chained rows, tombstones; one snapshot under the
chain lock SHARED) and append the changed heads to a sink the DB login cannot
write - File (append-only JSONL per node per day, local or UNC) or Syslog
(RFC 5424 over UDP / TCP octet counting). `audit verify-heads --path` reads
those records back (file or syslog-collector lines) and the new
VerifyAuditChainHeads command checks each against the database: Matches /
Extended / Purged, or Mismatch / Regressed / MissingHead / MissingLink /
CountMismatch (exit 3). AuditChainHeadStoreTests forges a chain with the
production hasher: verify-chain says Intact, verify-heads says Mismatch.

Item 3. AuditLog:MaintenanceConnectionString (${secret:}-capable, unset =
today's single login) runs the chain job, chain verify / head export, the
partition switch-out + archive and the per-channel purge on
AuditMaintenanceContext (keyed audit-maintenance repository/archive, purge
actor prefers them). AuditMaintenanceLoginTests runs the hardened setup on
SQL Server: writer (datareader/datawriter/audit_writer) ingests and is
refused chaining and deleting; the chainer+purger login does the full cycle.
Two-login grants documented in Component-AuditLog.md.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 23:16:05 -05:00
Joseph Doherty 5e7d5389d4 feat(audit): flip AuditLog clustered key to (OccurredAtUtc, EventId) (ScadaBridge#111)
Variant B2 of the #78 benchmark: migration FlipAuditLogClusteredKey drops the
twelve non-clustered indexes, re-adds PK_AuditLog as (OccurredAtUtc, EventId)
on ps_AuditLog_Month, adds the aligned unique UX_AuditLog_EventId_Occurred
(EventId, OccurredAtUtc) for the idempotency probe, and rebuilds the twelve
indexes unchanged. Guarded (re-runnable) Up, inverse-rebuild Down; production
scripts docs/plans/sql/FlipAuditLogClusteredKey{,.rollback}.sql.

The partition-switch purge staging table now reads the live clustered key
order from sys.index_columns, so it switches either shape (old binaries
hard-code the old order: ship with the binaries, both centrals stopped).

Evidence: 300k rows, page 1 90,532 -> 34 logical reads (ordered backward
clustered scan under Top), keyset page 90,532 -> 46, probe 14 -> 14;
FlipAuditLogClusteredKeyMigrationTests pins indexes, grants, Down/Up, the
staging key (mutation: hard-coded old key fails SWITCH) and the read ratio.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 22:52:16 -05:00
Joseph Doherty a4806576af feat(pending-updates): short-circuit an unchanged fleet on a DB rowversion watermark (#62)
Owner decision (b), 2026-11-09. PendingUpdatesEvaluator re-flattened every
deployed instance on each cache-TTL expiry even when nothing had changed.

- Migration AddConfigGraphRowVersions: a non-concurrency rowversion column
  + index on the 18 configuration tables the evaluation reads (Templates +
  5 member tables, Instances + 4 override/binding tables, DataConnections,
  DeployedConfigSnapshots, Sites, SharedScripts, ExternalSystemDefinitions,
  ExternalSystemMethods, DatabaseConnectionDefinitions). SQL Server only in
  the model; SQLite test contexts are untouched. Script:
  docs/plans/sql/AddConfigGraphRowVersions.sql. Joins the coordinated
  central-pair migration window (#107 item 4).
- IConfigGraphWatermarkReader / ConfigGraphWatermarkReader: ONE query of
  MAX(RowVersion) + COUNT_BIG(*) per table (count catches deletes, which
  raise no rowversion). rowversion is engine-bumped for any writer, so a
  peer central node's edit moves it — not the per-process watermark MG-3
  removed.
- The secret store (outside the DB) gets its own stamp: SHA-256 over
  ListAsync(includeDeleted: true) metadata.
- Evaluator re-serves the last CLEAN snapshot while watermark + secret
  stamp are unchanged; every PendingUpdatesFullEvaluationEvery'th (default
  10, validated >= 1, 1 = off) is full regardless — the safety net for a
  same-KEK re-wrap or an input table missing from the set. Always full when
  the token is unavailable/unreadable, after an evaluation that could not
  compute a site's artifact hash, on Re-check, and after a deploy completion.
  It only re-serves a real earlier answer, never manufactures one.

Tests: SQL Server (real migrated DB) — token stable without writes, moves
for a peer-connection INSERT/UPDATE and a non-max DELETE, moves for a local
EF write, a detached Update still saves (not a concurrency token), and a
guard that every table the evaluator's repository reads touch is covered.
Evaluator — reuse without flatten (re-served answer still shows the stale
instance), watermark move, 4 secret changes, every-Nth, 5 no-token modes,
unclean evaluation not reused, cache Re-check and deploy completion force
full. Mutation-proven: token compare, clean check, cache force flag, row
count, and an uncovered read each turn tests red.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 22:38:55 -05:00
Joseph Doherty 7be4f19861 feat(audit): Parquet cold archive of expired AuditLog partitions before the purge (ScadaBridge#42)
Audit roadmap T2 (alog.md §15.2): with AuditLog:Archive:Enabled (default off)
the purge actor writes each expired monthly partition to Parquet under an
absolute AuditLog:Archive:Directory before switching it out, so audit rows
outlive RetentionDays.

- Parquet.Net 6.1.0 (fully managed; NuGetAudit clean) in central package
  management.
- AuditArchiveParquet: data file = the ten stored columns, OccurredAtUtc as a
  UTC microsecond timestamp plus exact OccurredAtUtcTicks, and ChainSeq/RowHash;
  tombstone file = the month's per-channel purge tombstones. Buffered row
  groups; the files are plain Parquet (DuckDB/pyarrow read them).
- AuditLogPartitionArchive (IAuditLogArchive): reads the partition's rows,
  chain heads and tombstones in one transaction under the chain lock SHARED,
  writes {Directory}/{YYYY}/auditlog-{YYYY-MM}-{stamp}Z.parquet,
  .tombstones.parquet and a .manifest.json (counts, SHA-256 per file, each
  month's head and prior anchor) via tmp + rename, manifest last. Archives are
  never overwritten.
- Archive before purge: SwitchOutPartitionAsync gains an internal guarded
  overload. Under the EXCLUSIVE chain lock it re-counts the partition and
  re-reads each month's LastSeq and THROWs 51002/51003 (nothing purged) unless
  both still equal the archive's, so the switch's checkpoint anchor is exactly
  the archive's chain end. The manifest is then marked Purged.
- Fail closed: a partition that cannot be archived is not purged; the failure
  takes the existing AuditLogPurgeFailedEvent / PurgeFailures path. Success
  publishes AuditLogArchivedEvent.
- Verify: file hashes and counts against the manifest, the chain recomputed
  from the archived rows and tombstones (AuditChainVerifier), and the chain end
  against the database anchor (Matches / Superseded / NotPurged / Mismatch).
- AuditLogArchiveOptionsValidator: Directory required when enabled and must be
  absolute; RowGroupSize, timeouts bounded.

No migration: the purger role already reads AuditLog, the head and tombstones.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 17:12:12 -05:00
Joseph Doherty fd4be80913 feat(audit): SHA-256 hash chain over the central AuditLog (ScadaBridge#41)
Audit roadmap T1 (alog.md §11.4): one chain per UTC calendar month of
OccurredAtUtc. Each chained row carries ChainSeq (gap-free within its month)
and RowHash = SHA-256(prev.RowHash || canonical(row)); the v1 canonical form
(AuditChainHasher) is pinned by a known-answer vector computed independently.

- Migration AddAuditLogHashChain: nullable ChainSeq/RowHash on dbo.AuditLog
  (metadata-only add), AuditLogChainHead (per-month chain end + partition-purge
  checkpoint anchor), AuditLogChainTombstone, two partition-aligned filtered
  indexes, scadabridge_audit_chainer role with a column-level UPDATE grant.
  Production script docs/plans/sql/AddAuditLogHashChain.sql.
- AuditLogHashChain chains unchained rows oldest month first under an
  exclusive sp_getapplock (zero timeout, so a dual-active central pair still
  yields one chain), with an optimistic head check and a ChainSeq IS NULL
  row-count check that roll a pass back instead of leaving a gap. The ingest
  path is untouched.
- Purges keep the chain verifiable: the partition switch records the month's
  checkpoint anchor and drops its tombstones; the per-channel DELETE writes a
  tombstone per chained row in the same transaction (OUTPUT INTO). Both, and
  the SourceNode backfill, hold the chain lock; the backfill skips chained
  rows. The switch staging table gains the two new columns.
- AuditChainVerifier reports the first break (partition, ChainSeq, EventId,
  kind) for edited, deleted, reordered rows and tail truncation.
- AuditLogHashChainService (central, default OFF via AuditLog:HashChain).

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 16:33:11 -05:00
Joseph Doherty 47f3a703bf perf(search): prefix-by-default Deployments and config-audit search; bounded Contains opt-in (ScadaBridge#50)
Deployments free-text search and the config-audit EntityName filter no longer
emit LIKE '%term%' by default. Prefix (LIKE 'term%') is the default; substring
is an explicit opt-in (DeploymentSearchMode.Contains / entityNameContains)
always bounded by a DeployedAt/Timestamp window (explicit, else 30 days).
Migration PrefixSearchIndexes adds DeployedBy, RevisionHash and EntityName
indexes. Deployments page gains a 'Match anywhere' checkbox.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 15:42:56 -05:00
Joseph Doherty f98369a786 feat(native-alarms): per-binding IsMesReportable on native alarm source bindings (ScadaBridge#59)
TemplateNativeAlarmSource.IsMesReportable + tri-state InstanceNativeAlarmSourceOverride
.IsMesReportableOverride flatten onto ResolvedNativeAlarmSource; NativeAlarmActor stamps every
mirrored condition and InstanceActor the quiet-binding placeholder, so ScriptAlarm,
Alarms.MesReportableAsync() and the #60 wire fields carry it. Additive to the 900-999 severity band.
Migration AddNativeAlarmSourceMesReportable. Management commands, CLI --mes-reportable, Central UI
(template dialog checkbox, instance override select), Transport DTOs/importer/diff and the revision
hash (true-only, so existing hashes are unchanged) carry it.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 15:09:54 -05:00
Joseph Doherty 7848a3fe7c fix(transport): case-insensitive remaining library lookups; base-only template reads for import targets (ScadaBridge#95)
- Preview matches template folders, shared scripts, external systems, SMTP
  (by Host) and SMS (by AccountSid) with the #93 ToNameLookup comparer
  (OrdinalIgnoreCase, first wins, never throws), matching SQL Server's
  case-insensitive collation, so a differently-cased target row is reported as
  a conflict instead of New.
- Apply-side in-memory lookups do the same: ApplyTemplateFoldersAsync, the
  template folder-id map, ResolveFolderParentEdgesAsync, ApplySmtpConfigsAsync
  and ApplySmsConfigsAsync. An Overwrite now updates the existing row instead of
  adding a duplicate (folder names are only unique per parent, so the old
  ToDictionary could also throw).
- New ITemplateEngineRepository.GetBaseTemplatesWithChildrenAsync filters
  IsDerived in SQL. Preview and the Overwrite load in ApplyTemplatesAsync use it,
  so a derived slot template that shares a bundled name is never the import
  target. GetTemplatesWithChildrenAsync is unchanged for the id-keyed callers
  (export closure, rewire passes), which can need derived rows.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 14:30:17 -05:00
Joseph Doherty 6070cc2b9d perf(templates): resync and resolved-members read only the chain/subtree (ScadaBridge#89)
The last two whole-library template reads are gone. Both fed the
inheritance-only TemplateInheritanceResolver, which walks one template's
parent chain and never follows compositions.

- TemplateService.ResyncInheritedMembersAsync (operator repair, CLI + editor)
  now reads #86's GetTemplateReconcileClosureAsync (derived subtree + the
  template's chain, tracked). When nothing derives from the template the
  closure is empty, so it falls back to the new tracked
  GetTemplateChainAsync - resyncing a leaf reads only its own chain.
- ManagementActor.HandleGetResolvedTemplateMembers now reads
  GetTemplateChainAsync(id, trackChanges: false): the chain alone, untracked,
  with script bodies (the resolver projects script code, so #66's body-free
  analysis closure would not be equivalent).

The chain walk is factored out of TemplateReconcileClosure into
CollectChainAsync (one load per level, stops on parent cycles and dangling
parents) and shared by both.

Evidence:
- TemplateChainReadTests (#86 fixture): for every template plus an unknown
  id, the closure-backed resync issues exactly the writes, audit row and
  result of the whole-graph resync with GetAllTemplatesAsync wired to throw;
  the resolver answers identically from the chain and the whole graph.
  17 of these fail against the old service; 13 fail with the leaf chain
  fallback removed (mutation-checked).
- TemplateChainRepositoryTests (SQLite, recorded SQL): a 4-deep chain loads
  4 templates / 8 attributes instead of 46 / 92, one split-query set per
  level, untracked on request and tracked for the resync.
- ResolvedTemplateMembersReadTests: the actor answers from the untracked
  chain read and never calls GetAllTemplatesAsync (fails on the old actor).

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 13:30:26 -05:00
Joseph Doherty c29b99c63b perf(templates): descendant reconcile loads only the subtree + chain (ScadaBridge#86)
TemplateService.ReconcileDescendantsAsync - run after every base member
add/update/delete, template and native-alarm-source alike - loaded the whole
tracked template graph (every template, member and script body) on each call,
which after #66 was the dominant cost of those paths. It now calls the new
ITemplateEngineRepository.GetTemplateReconcileClosureAsync: the same tracked,
fully-included shape as GetAllTemplatesAsync, restricted to the edited
template's derived subtree plus its own inheritance chain. The reconcile is
inheritance-only (TemplateInheritanceResolver never follows compositions) and
every descendant's chain runs through the edited template, so that set is
everything it reads.

The walk (Commons TemplateReconcileClosure, the descendant-direction twin of
#66's TemplateAnalysisClosure) loads one children batch per subtree depth and
one batch per ancestor level, terminates on parent cycles and stops at
dangling parents. A template with no children - the common case - costs one
empty probe and loads nothing.

Evidence:
- TemplateReconcileClosureTests: for every template of a graph with a deep
  subtree, an ancestor above the edited template, a composed module,
  look-alike unrelated trees, a parent cycle and a dangling parent, the
  closure-backed reconcile issues exactly the writes (add/update/delete,
  audit, save) of the old whole-graph reconcile, and the resolver answers
  identically for every descendant. Fails with the old service (whole-graph
  load wired to throw) and with the ancestor walk removed (mutation-checked).
- TemplateReconcileClosureRepositoryTests (SQLite, recorded SQL): 5 templates
  / 10 attributes instead of 47 / 94, tracked with script bodies; a leaf costs
  one probe query and tracks nothing.
- TemplateServiceTests: attribute add + update propagate two levels down with
  GetAllTemplatesAsync wired to throw; existing propagation tests now run
  against the closure via ServeReconcileClosureFromFullGraph.

ResyncInheritedMembersAsync (operator action) still reads the whole graph.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 13:10:31 -05:00
Joseph Doherty 4918210b78 build: adopt CentralPackageTransitivePinningEnabled, drop per-project SQLitePCLRaw pins (ScadaBridge#79)
Enables transitive pinning so the central SQLitePCLRaw.lib.e_sqlite3 2.1.12
pin applies everywhere (41 projects verified at 2.1.12). Required raising the
central floors: Newtonsoft.Json 13.0.4 and Microsoft.Extensions.* 10.0.13.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 13:05:48 -05:00
Joseph Doherty 792f16c2d5 perf(templates): member add/update loads only the chain + composition closure (ScadaBridge#66)
Attribute/alarm/script add (collision check) and update (parent-lock lookup)
used GetAllTemplatesForAnalysisAsync, materialising every template, attribute,
alarm and script in the library per edit. They now call the new
ITemplateEngineRepository.GetTemplateAnalysisClosureAsync: the same untracked,
body-free analysis projection, restricted to the template's inheritance chain
plus the composition closure of every chain member - exactly what
TemplateResolver and CollisionDetector walk (a composed module's own parent
chain is never climbed, so it is not loaded). Adds use the edited template's
closure, updates the parent's.

The walk (Commons TemplateAnalysisClosure) loads one batch per closure depth,
requests each id once, terminates on cycles and skips dangling edges.

Evidence:
- TemplateAnalysisClosureTests: for every template of a graph with chains,
  nested + diamond compositions, a derived module, cycles, dangling edges and
  colliding unrelated templates, ResolveAllMembers / DetectCollisions /
  FindMemberByCanonicalName answer identically from the closure and the whole
  graph (mutation-checked).
- TemplateAnalysisClosureRepositoryTests (SQLite, recorded SQL): 6 templates /
  12 attributes loaded instead of 47 / 94, in 4 queries (one per depth),
  untracked, script bodies never selected.
- TemplateServiceTests: two new tests with the whole-graph load wired to throw
  (fail on the old service); every existing add/update test now runs against
  the closure via AnalysisClosureMock.

ReconcileDescendantsAsync still loads the tracked full graph; not in scope.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 12:52:29 -05:00
Joseph Doherty 46ab5594ec fix(site-call-audit): an unknown-status row no longer aborts the rest of a batched page (review 2026-10-21 06/PERF-2 follow-up) 2026-10-22 09:50:51 -04:00
Joseph Doherty 5f7f889a96 fix(notification-outbox): keyset Pending back-fill + distinct ids so a mid-claim Retry cannot double-deliver (review 2026-10-21 06/PERF-5 follow-up) 2026-10-22 09:50:51 -04:00
Joseph Doherty 1627513fdb feat(configdb): ReviewRemediationIndexes migration — AuditLog (Kind, Actor) + AuditLogEntries composite indexes
AuditLog indexes are raw SQL ON ps_AuditLog_Month so they stay partition-aligned for the switch purge.
2026-10-22 09:42:02 -04:00
Joseph Doherty 88273693f9 fix(site-call-audit): reconciliation pull serves only un-acked tracking rows (node-local ack ledger), batched page upsert, prune removed sites' cursors (review 2026-10-21 06/PERF-2 + lane H follow-up) 2026-10-22 09:38:48 -04:00
Joseph Doherty e69be1654f fix(kpi-history): fold raw samples one hour at a time; only a unique-key violation is the benign fold race (review 2026-10-21 06/PERF-6, 08/P4, 08/C3) 2026-10-22 09:38:48 -04:00
Joseph Doherty e7fb71c2de fix(site-call-audit): KPI snapshots as single conditional-aggregate queries (review 2026-10-21 06/PERF-3, 08/P3) 2026-10-22 09:38:48 -04:00
Joseph Doherty 811a561885 fix(notification-outbox): per-attempt timestamps, Pending/Retrying split claim + sweep budget, SMTP MaxConcurrentConnections clamp (review 2026-10-21 06/COR-1, 06/PERF-5, 06/IMPL-2) 2026-10-22 09:38:48 -04:00
Joseph Doherty d495494074 fix(management): purge terminal PendingSecuredWrites rows on the list sweep after a retention window (review 2026-10-21 08/G3) 2026-10-22 09:32:14 -04:00
Joseph Doherty c6e4c7820a fix(configdb): InstanceLocator single untracked projected join (review 2026-10-21 08/P7) 2026-10-22 08:33:09 -04:00
Joseph Doherty 1d00d5f469 fix(configdb): deterministic config-audit paging (review 2026-10-21 08/P5) 2026-10-22 08:33:09 -04:00
Joseph Doherty 036f341a9d fix(configdb): opt-in config-audit retention purge + after-state size cap (review 2026-10-21 08/G1) 2026-10-22 08:33:09 -04:00
Joseph Doherty 7aacc49043 fix(deployment): detach each instance's settled rows from the bulk deploy's shared unit of work after Finalize (review 2026-10-21 04/M3) 2026-10-22 08:22:42 -04:00
Joseph Doherty 32e63eaa96 fix(deployment): bulk-deploy up-to-date pre-filter uses batch hash/status reads and the operation's shared flatten session; completion log reports pre-filter cost (review 2026-10-21 04/P1) 2026-10-22 08:22:42 -04:00
Joseph Doherty df67fabe68 fix(deployment): pending-updates tick uses a slim instance projection, hash-only flatten, one untracked mismatch snapshot read and a per-tick secret lookup memo (review 2026-10-21 04/P2) 2026-10-22 08:22:42 -04:00
Joseph Doherty 669822772e fix(deployment): read every external system's methods in one query for the artifact snapshot (review 2026-10-21 04/P7) 2026-10-22 08:22:41 -04:00
Joseph Doherty 7c83a4aa94 fix(reconcile): batch startup reconcile into fixed round trips, bound central work by QueryTimeout, retry failed site passes with back-off (review 2026-10-21 03/P-1) 2026-10-22 08:22:41 -04:00
Joseph Doherty d6a9a79c27 fix(deployment): purge SystemArtifactDeploymentRecords in the terminal-record retention sweep (review 2026-10-21 08/G2) 2026-10-22 08:22:41 -04:00
Joseph Doherty da5a049077 fix(deployment): pending-deployment supersede/purge/stage deletes never read ConfigurationJson; drop stale deferred-TODO comment (review 2026-10-21 08/P6, 08/C2) 2026-10-22 08:22:41 -04:00
Joseph Doherty bc1aca0ee6 feat(instances): GetInstancesPagedAsync SQL-side filter/order/page 2026-10-22 06:06:27 -04:00
Joseph Doherty 6bcca4b344 feat(templates): GetTemplateStubsAsync untracked identity/edge projection 2026-10-22 06:06:27 -04:00
Joseph Doherty 513fa1aa16 docs(tests): fix CommentChecker findings across tests/ — 13,392 → 0 (repo-wide scan clean)
Comment/XML-doc changes only:
- add summaries to ~10k test methods, fixtures, fakes and helpers
  (+ <param> on theories, <returns> on async Task methods,
  <inheritdoc/> on interface implementations the checker can resolve)
- strip ~2.6k project-tracking references (WP*, P*-*, row-NN, Task N,
  #NN, Component-0NN) from comments, keeping the explanatory text;
  reword false positives in comments only
- real docs on fakes implementing BCL/library interfaces
  (IDisposable, ILogger, IOptionsMonitor, IAsyncLifetime)
- reword the last 5 src/ false positives (CWE-1236, failures-1,
  year-0001, M365)

Only non-comment edits: four one-line nested classes split across lines
so members can carry docs. Build 0 errors / 0 warnings.
2026-10-07 15:05:49 -04:00
Joseph Doherty dd202e5dde feat(sites): record the artifact revision last deployed to a site
Site.ArtifactRevisionHash / ArtifactsDeployedAt (nullable, migration
AddSiteArtifactRevision) so the Sites page can tell whether a site's
system-wide artifacts are current. Not transported. Adds the
GetDeployedRevisionHashesAsync projection for the fleet-wide staleness batch.

Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB
2026-09-14 13:33:59 -04:00
Joseph Doherty 30a23458e9 feat(config-db): AddAlarmMesReportable migration (TemplateAlarms.IsMesReportable, InstanceAlarmOverrides.IsMesReportableOverride)
Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB
2026-09-11 09:59:00 -04:00
Joseph Doherty 9ee402bbcd Resolve all 46 low-severity architecture review findings 2026-09-09 22:56:36 -04:00
Joseph Doherty 9a87d7a328 Resolve all 33 medium-severity architecture review findings
Second slice of the 2026-09-09 whole-system review remediation
(docs/plans/2026-09-09-arch-review.md §10), on top of the High slice 146a1323.

Runtime/devices: SR-2 alarm on-trigger integration scope; SR-3 correlated NativeAlarmActor
subscribe retry; SR-4 immutable audited DB target; SR-5 ordered InstanceActor stop/flush;
SR-12 rehydration fence; SR-13 S&F gate recheck per delivery; DC-4 no OPC UA security
downgrade; DC-5 alarm snapshot for every new subscriber; ORCH-1 sync-dispose joins the pump.

Management/UI: MG-2 site scope on audit tree + Flow Debugger Replay; MG-3 stale-probe memo
removed; MG-4 server-owned bulk-deploy deadline; UI-5 sandbox runs in a killable child
process (new CentralUI.SandboxWorker project); UI-14 bounded sandbox output/IPC.

Edge/audit: EI-1 relay audit ownership; EI-2 SkipBodyCapture suppresses error text; EI-3/EI-11
error-field redaction + caps; EI-4 single relay deadline, pre-execution-only retry; EI-5
authenticated relay provenance; EI-10 body cap before audit read.

Persistence/contracts: DL-1 global Instance.UniqueName unique index + conflict translation;
DL-2 DataConnection protocol config encrypted at rest (migration
20260909203046_EnforceGlobalInstanceNamesAndProtectProtocolConfiguration — coordinated
central upgrade, old binaries cannot read protected rows); DL-3 bounded Replay paging;
DL-6 cross-site Overwrite of a deployed instance rejected; XC-4 proto descriptor contract test.

Config/hygiene: XC-1 central listener isolation by local port; XC-2/XC-3 inert SMTP + cron
keys removed from rig settings; CM-2 env2 bootstrap guard enabled; DC-6 deploy site exit code;
DC-7 notifier retries only explicit standby rejection; XC-5 ClusterClient purged from docs.

Gates recorded in the report: build 0/0, 9 397 passed / 0 failed, rig redeployed on all
eight nodes with the migration applied. 46 Low findings remain open.
2026-09-09 22:05:30 -04:00
Joseph Doherty 146a13238f Resolve all high-severity architecture review findings 2026-09-09 16:13:58 -04:00
Joseph Doherty 95773aa9fd feat(site-call-audit): additive TrackedOperationId query filter for ?correlationId= deep links
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 11:00:03 -04:00
Joseph Doherty 5497ffd181 feat(notification-outbox): additive NotificationId query filter for ?correlationId= deep links
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 10:59:49 -04:00
Joseph Doherty 30faee0d0e feat(audit): replay/pull include outbound API, DB and notification rows
GetTraceEventsAsync widens its channel filter to ScriptTrace ∪ ApiOutbound ∪
DbOutbound ∪ Notification ∪ (ApiInbound & InboundRequest) (plan D6); SecuredWrite,
Cluster and InboundAuthFailure rows in the same tree stay excluded. The site
SqliteAuditWriter.ReadByExecutionIdsAsync already had no channel predicate — pinned
by a test. Interface doc updated to match.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 01:47:05 -04:00
Joseph Doherty 9300bc1a4f feat(audit): GetTraceEventsAsync returns the ScriptTrace chain for Replay
Flow Debugger (design 2026-09-05 §7.1): IAuditLogRepository.GetTraceEventsAsync walks any
entry id to its root and down the Edges/Chain CTE exactly as GetExecutionTreeAsync, then
returns the chain's ScriptTrace rows plus the ApiInbound.InboundRequest row via the canonical
row mapper, ordered (OccurredAtUtc, EventId). Test doubles implement the new member with a
NotSupported stub.

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:59:08 -04:00
Joseph Doherty 1cea01d924 fix(notifications): list sort index + CAS operator Retry/Discard (P5-3, P5-4)
P5-3: add IX_Notifications_Created_NotificationId (CreatedAt DESC,
NotificationId DESC) — the Central UI Notification Outbox list page sorts on
exactly that pair, but no existing index leads unconditionally on CreatedAt,
so an unfiltered/lightly filtered page fell back to a full clustered-index
scan + sort on every COUNT + page request. Mirrors the Medium wave's
IX_SiteCalls_Created_TrackedOperationId migration + SQL twin pattern
faithfully, including the handwritten docs/plans/sql script.

P5-4: operator Retry/Discard was read-check-write keyed on NotificationId
only — a detached PipeTo task interleaving with the dispatcher's own write
(or a second concurrent operator action) could land between the read and
the write and get silently clobbered by the unconditional UpdateAsync,
stamping a Delivered row Discarded with stale fields. Adds
INotificationOutboxRepository.TryUpdateFromParkedAsync, a single-statement
compare-and-swap (WHERE Status == Parked) mirroring
SecuredWriteRepository.TryMarkApprovedAsync's CAS shape. NotificationOutboxActor's
RetryAsync/DiscardAsync now use the CAS write and, on a lost race, re-read
to report "notification already progressed to <Status>" instead of silently
overwriting whatever is there now.

Proof of failure: reverting only the actor's CAS usage while keeping the
repository addition reproduces the silent-overwrite bug — 7 tests fail,
including two new deterministic (non-sleep) race-proof tests that mock the
exact GetByIdAsync -> lost-CAS -> re-read sequence the finding describes.
P5-3 is perf-only; verified via migration/SQL-twin parity instead (no
runtime proof-of-failure is meaningful for an index).
2026-08-19 16:41:04 -04:00
Joseph Doherty 61a14bccab fix(management): CAS-guard secured-write reject (WHERE Status='Pending') so reject can never falsify an executed two-person write (arch-review 2026-08-19 P6-1 High)
Reject read the row as Pending, then landed a blind full-row UpdateAsync. A
concurrent approve could win its own Pending->Approved CAS in that gap and enter
the multi-second WriteTagAsync relay; the reject's update then stamped
Status/ExecutedAtUtc/ExecutionError to Rejected mid-relay, so the two-person-control
record said Rejected for a write that physically executed at the plant. Works
cross-node (both centrals share one MS SQL).

- ISecuredWriteRepository/SecuredWriteRepository: new TryMarkRejectedAsync,
  mirroring TryMarkApprovedAsync exactly (conditional UPDATE ... WHERE
  Status='Pending', writes only the columns the reject transition owns). No schema
  change, no migration, no rowversion column.
- HandleRejectSecuredWrite: uses the CAS; on loss re-reads the row and returns the
  status that actually won (same UX shape as approve's lost-race error), and emits
  the SecuredWriteReject audit row ONLY on a won CAS.
- Audited the other lifecycle writes: approve/reject/expiry (inline + the
  opportunistic list sweep) now ALL transition only out of Pending, so winning the
  approve CAS gives that leg exclusive ownership of the row and its post-relay
  Executed/Failed update stays safe as an unconditional write. Documented in place.
- Tests: repository-level approve-vs-reject races in both orders (exactly one wins,
  loser false, row reflects the winner only) plus reject-vs-expiry; handler-level
  lost-race reject (conflict error, no write, no audit row) and approve on an
  already-rejected row (no CAS, no relay, no audit).
2026-08-19 13:54:45 -04:00
Joseph Doherty 106e467dfd merge(agent-c): rows 91/95/96 — compile-cache tests on key-scoped delta assertions; Watch/Stop/ExpectTerminated at all 23 fast-tick actor creation sites in the three remaining AuditLog.Tests classes; per-suite xunit maxParallelThreads:4 for the four MSSQL-fixture suites (matches the row-82 semaphore slots) + Test Parallelism Policy doc 2026-08-17 05:28:51 -04:00
Joseph Doherty d2d4db492b test(infra): close register row 96 — per-suite xunit parallelism caps for the resource-heavy suites
maxParallelThreads was unset everywhere and only 3 of ~20 test projects carried
an xunit.runner.json at all, so xunit ran Environment.ProcessorCount collections
at once (14 on the dev box) in every suite. That is the general load multiplier
behind the row-82/91/95 flake family.

Policy: per-suite caps, NOT a global one. Pure unit suites stay uncapped for
speed; a suite gets a config only when a test CLASS costs a real external or
OS-level resource, so ProcessorCount of them at once is a resource question
rather than a CPU question. Four suites qualify, all capped at
maxParallelThreads: 4.

- ConfigurationDatabase.Tests, AuditLog.Tests, SiteCallAudit.Tests — verified as
  the three that actually take MsSqlMigrationFixture (the latter two by project
  reference). Every MSSQL-backed test class means a CREATE DATABASE plus the
  ~100-migration chain against the one shared dev SQL Server. 4 is chosen to
  MATCH the default slot count of row 82's cross-process sp_getapplock migrate
  semaphore, so the in-process and cross-process limits agree rather than
  fighting.
- Host.Tests — classes boot real ASP.NET Core hosts and real ActorSystems;
  Akka.Remote binds real sockets, Kestrel binds real ports, gRPC dials them.

Threads are capped, collections are NOT serialized. IntegrationTests
(parallelizeAssembly/parallelizeTestCollections false) and
CentralUI.PlaywrightTests (maxParallelThreads 1) already carry stricter
pre-existing settings and are left alone — the Playwright suite drives a browser
against a live rig, where shared UI state, not CPU, is the constraint.

Each capped csproj gains
<Content Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />;
the SDK does not copy it automatically and without it the file is silently
ignored. Verified by moving the copied file out of bin/ to take an uncapped
baseline: ConfigurationDatabase.Tests ran at 216 % CPU uncapped vs 145 % capped,
so the cap demonstrably takes effect.

Cost is close to nil — these suites are latency-bound on an external resource,
not CPU-bound (14-25 % utilisation for the two slowest). Standalone, uncapped ->
capped: AuditLog 383 tests 1m52s -> 1m51s; Host 520 tests 4m05s -> 4m08s;
ConfigurationDatabase 396 tests 10s -> 12s; SiteCallAudit 66 tests 8s -> 8s. All
green.

docs/test_infra/test_infra.md gains a Test Parallelism Policy section: the two
axes (in-process collection threads = this cap; cross-process solution-level
dotnet test = unbounded in general, bounded on the DB-migration path by the
sp_getapplock semaphore), the per-suite table with reasons, the rules (cap
threads rather than serialize; a cap is not a substitute for test isolation; the
Content copy is required), and the default for new projects (no config until the
project acquires a per-class external resource). test_infra_db.md cross-links it
from the semaphore bullet.
2026-08-17 05:27:52 -04:00
Joseph Doherty 9f67147e7b refactor(deployments): one paged read of DeploymentRecords + close its three drifts
Deferred-work register rows 37, 86, 87, 88 — the structural duplication and the
three behavioral divergences it had accumulated, fixed together.

Row 37 (structural). `IDeploymentManagerRepository.QueryDeploymentSummariesAsync`
is DELETED. It and `QueryDeploymentListPageAsync` were two paged reads of the same
insert-only table with different contracts — raw offset vs page number, no total vs
total, no instance join vs joined, instance-id-list scoping vs site-id scoping. The
CLI/HTTP `QueryDeployments` handler now goes through `QueryDeploymentListPageAsync`
with a `DeploymentListFilter`, exactly like the Central UI page. The
`DeploymentRecordSummary` projection went with it (`DeploymentListRow` is the single
row shape); its `MaxPageSize` const moved to `DeploymentListPage`, which also gains
`DefaultPageSize`.

Row 86 (Medium, authorization — the headline). `SiteScopeService.ResolveAsync` was
claims-only, so an Administrator carrying `SiteId` claims saw the whole fleet through
the CLI (`ManagementActor.EnforceSiteScope` has always exempted Administrator) and
only their granted sites on the Deployments page. One user, one dataset, two answers.
Administrator = system-wide is the intended semantic on both sides; the UI was the
defect. The role claim is read explicitly with an OrdinalIgnoreCase comparison rather
than via `IsInRole`, which compares claim VALUES ordinally and only resolves when the
identity carries the matching roleType — the two sides must not disagree on a
differently-cased role name. An Administrator's site claims are not collected at all,
so the invariant "system-wide => empty permitted-id set" holds for every caller.
Designer is NOT exempt: the class doc claimed "any Admin/Design user" is system-wide,
which was true of neither side, so the doc was corrected rather than the bypass
widened.

Row 87 (Low). The scoped branch used to resolve the site grant into an instance-id
list by loading EVERY instance in the system, non-transactionally, on every scoped
query; the grant now rides as `SiteIdScope` and is resolved by the query's own join.
An unparseable permitted-site id is DROPPED, never ignored — dropping can only narrow
a grant, discarding it would widen it to fleet-wide. And `PageSize <= 0` no longer
means UNLIMITED down at the repository (`--page-size 0` materialized the whole
retention window): it means "unspecified" and takes the CLI's own default of 50,
capped at 1000. Bounding without telling would be silent truncation, so the handler
now returns a `DeploymentQueryResult` envelope carrying `totalCount` plus the page
coordinates actually applied — with a CLI table projection (mirroring row 57's
`AuditConfigTableProjection`) that unwraps it and appends a "showing N of M" note.

Row 88 (Low, UX). An unrecognised `--status` fell back to "no filter", so a typo like
`--status Sucess` returned every status — a complete-looking answer to a question the
caller never asked. It is now a `ManagementError` naming the valid values. Parsing
stays case-insensitive; a numeric string is rejected too, since `Enum.TryParse`
accepts "7" and yields an undefined value (guarded with `Enum.IsDefined`). Loud
failure over silent full results, per row 46.

Tests: 7 new `SiteScopeServiceTests` pinning Administrator = system-wide (verified
failing 6/7 against the pre-fix service, the Designer pin passing both ways); the
ManagementActor deployment suite migrated onto the consolidated query with new pins
for the status error, case-insensitive parse, numeric rejection, page-size default/
cap/floor, empty-scope-on-garbage-grant, and "reads no instances at all"; a repository
test for site-scope+status composing over a bounded page; and 6 CLI tests for the new
table projection.
2026-08-17 05:15:18 -04:00
Joseph Doherty 75195fc512 merge(agent-c): row 82 — cross-process create+migrate throttle (sp_getapplock counting semaphore), honest command timeouts, deterministic actor stops in the two leaking AuditLog test classes 2026-08-17 04:09:36 -04:00
Joseph Doherty da23f2dcae test(mssql): close register row 82 — throttle cross-process create+migrate, stop leaked test actors
AuditLog.Tests flaked under parallel full-solution runs. Three distinct
mechanisms, all load-sensitive and all green in isolation.

1. MsSqlMigrationFixture create+migrate saturation (the register's headline).
   Database NAMING was already isolated (a guid per fixture), but nothing
   bounded how many fixtures ran CREATE DATABASE + the ~100-migration chain at
   once. xunit parallelises processor-count collections PER PROCESS, and a
   solution-level `dotnet test` starts several MSSQL-backed processes together,
   so the dev container saw dozens of simultaneous fresh-database migrations.
   That is resource saturation, not object-level locking, and it pushed
   individual DDL statements past EF's default 30s command timeout — every
   failure had the identical fingerprint the register recorded:
   MsSqlMigrationFixture..ctor -> Migrator.MigrateAsync -> SqlException
   "Execution Timeout Expired" (error 258), which xunit surfaces as
   "Class fixture type ... threw in its constructor" against every test in the
   class.

   Fix: a machine-wide counting semaphore over the create+migrate window, built
   from sp_getapplock on master (ScadaBridgeTestMigrate/0..N-1, default 4 slots,
   SCADABRIDGE_MSSQL_TEST_MIGRATE_SLOTS to override). An xunit collection cannot
   do this — the contention is cross-PROCESS, so the arbiter has to live on the
   shared server. The gate connection is unpooled (a pooled connection defers the
   implicit release of a session-scoped app lock until reuse) and releases
   explicitly; a crashed process drops its connection and SQL releases the lock,
   so the gate cannot wedge the suite, and a deadline miss proceeds ungated
   rather than failing. Migration commands also get a 300s timeout and
   CREATE/DROP DATABASE 120s: under a bounded queue the server is busy but
   progressing, so waiting is the correct response. Plus a once-per-process
   best-effort sweep of ScadaBridgeAuditMigTest_* databases older than 2h so a
   killed run does not leave strays, and the connectivity probe now retries
   (3 attempts) — a busy server misread as an absent one would silently turn
   every MSSQL test into a Skip, which reads green and is worse than a red.

2. PartitionPurgeTests leaked AuditLogPurgeActors. Each ticks every 100ms and
   switches out the Jan-2026 partition; they were only stopped implicitly by
   TestKit Dispose -> Shutdown, which is timeout-bounded and does not fail when
   it expires. Under load a previous test's actor was still ticking against the
   shared per-class fixture DB, so PerChannelOverride_... found its seeded
   Jan-15-2026 row already gone (Expected: 1, Actual: 0).

3. NotifyDispatcherAuditTrailTests leaked NotificationOutboxActors, same shape.
   The dispatcher claims EVERY due row, not just the test's siteId, so a leaked
   actor delivered the next test's notification through its own exhausted
   QueuedOutcomeAdapter — a transient outcome, leaving the row in Retrying
   (Expected: Delivered, Actual: Retrying) and audit counts short.

   Both fixed by stopping the actor inside the test that created it
   (Watch/Stop/ExpectTerminated), removing the dependency on shutdown timing.

Evidence. Reproducer: 10 concurrent ConfigurationDatabase.Tests processes
against the one dev SQL Server — before, 10/10 processes failed (17-77 failures
each, 100% the ctor->MigrateAsync timeout); after, 3 consecutive runs 10/10
green, 395/395 each. Realistic shape (3x AuditLog.Tests + 1x
ConfigurationDatabase.Tests concurrent): first post-fix run still showed the
NotifyDispatcher leak, then 3 consecutive clean runs. Standalone: AuditLog.Tests
383/383, ConfigurationDatabase.Tests 395/395 (12s, unchanged from baseline),
SiteCallAudit.Tests 66/66. No orphan databases or stuck app locks left behind.

docs/test_infra/test_infra_db.md documents the ephemeral per-test-class
databases, the throttle and its env override, the timeouts, and the stray sweep.
2026-08-17 04:08:45 -04:00