Commit Graph

20 Commits

Author SHA1 Message Date
Joseph Doherty 530a096111 fix(auditlog): advance the served-rowid mark under the write lock so a purge clamp is never overwritten stale-high (review 2026-10-21 follow-up L1) 2026-10-22 09:58:46 -04:00
Joseph Doherty b0a08bb63b fix(auditlog): explicit permanent rejections on the ingest ack (IngestAck.rejected_event_ids, field 2) with a dedicated counter; reconciler ignores transient faults for abandonment; persist the served high-water mark; prune only on a non-empty enumeration (review 2026-10-21 06/PERF-1, 06/IMPL-1 adversarial follow-up) 2026-10-22 09:00:48 -04:00
Joseph Doherty 9e7e19c3c1 fix(auditlog): reconciliation pull serves only unacked rows (PushAbandoned/Rejected push-exit states), central pages within a tick with set-based inserts; refused push rows leave the queue after N attempts (review 2026-10-21 06/PERF-1, 06/COR-3, 06/IMPL-1) 2026-10-22 09:00:48 -04:00
Joseph Doherty 1da5d0bac4 fix(auditlog): page the site audit purge, bound incremental_vacuum per page, fail fast instead of parking writers on a full queue (review 2026-10-21 06/MEM-1) 2026-10-22 09:00:48 -04:00
Joseph Doherty 0ffd4bdbea docs(src): fix CommentChecker findings across src/ — 1,032 → 5 (all false positives)
Comment/XML-doc changes only, no code logic:
- strip project-tracking references (WP*, P*-*, row-NN, #NN, Task N) from
  comments, keeping the explanatory rationale
- add missing summaries, <param>, <returns>, <typeparam> and <inheritdoc/>
- replace misused <inheritdoc/> with real docs; where <inheritdoc/> was
  combined with useful remarks, move the rationale into // comments
- drop stale replicationActor <param> on DeploymentManagerActor ctor

Remaining 5 are false positives (CWE-1236 x2, failures-1, year-0001, M365).
Build 0 errors / 0 warnings.
2026-10-07 14:12:25 -04:00
Joseph Doherty 9ee402bbcd Resolve all 46 low-severity architecture review findings 2026-09-09 22:56:36 -04:00
Joseph Doherty 7e61552d82 feat(wire): attribute updates carry source_execution_id/trace; PullTraceEvents RPC + site read-by-execution-ids
Flow Debugger (design 2026-09-05 §5.3/§5.4): AttributeValueUpdate fields 7/8 stamped by
StreamRelayActor and parsed by SiteStreamGrpcClient (older frames => null/false). New
PullTraceEvents unary RPC on SiteStreamService (same PSK-gated prefix as PullAuditEvents):
a point read of the site audit SQLite by ExecutionId OR ParentExecutionId
(ISiteAuditQueue.ReadByExecutionIdsAsync, json_extract on the codec's camelCase keys, new
IX_audit_event_occurred index, 64-id / 2000-row caps) that moves no cursor and flips no
ForwardState. Central IPullTraceEventsClient/GrpcPullTraceEventsClient mirror the audit pull
client (endpoint resolution, NodeB failover, fault => empty) and register beside it.

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:52:50 -04:00
Joseph Doherty 6c11362d9f fix(auditlog)+fix(siteeventlog): complete audit write tasks only after commit; run event-log retention on standby (arch-review 2026-08-19 P4-2 + P4-4)
P4-2: SqliteAuditWriter.FlushBatch completed each pending row's Task
(TrySetResult) inside the per-row insert loop, before transaction.Commit()
ran. A later failure in the same batch (another row's insert throwing, or
the commit itself failing) rolled the whole transaction back, but the
earlier rows' Tasks had already completed successfully -- the caller and
FallbackAuditWriter (whose ring-buffer fallback exists precisely for this
outage) both saw false success while the rows were actually gone from the
database; the outer catch's TrySetException became a silent no-op against
an already-completed TaskCompletionSource. Every row's Completion is now
deferred and only resolved (TrySetResult for the whole batch, or
TrySetException for the whole batch) after transaction.Commit() succeeds
or the transaction rolls back, mirroring SiteEventLogger.FlushBatch.

P4-4: EventLogPurgeService gated the daily site_events purge on the
active-node check, early-exiting on the standby, on the stale premise that
"the standby receives no writes". DataConnectionActor runs on both site
nodes and writes site_events regardless of active/standby status (register
rows 89/90/97), so an unreplicated standby (the rig's site-b/site-c
posture) grew its site_events file forever with no retention and no cap.
The gate is removed; every node now purges its own file every tick. Safe
on a replicated pair because the retention/cap deletes are CDC-captured
and converge under LWW regardless of which node issues them; required on
an unreplicated pair because each node's file is independent. The
active-node check is retained only to annotate the purge tick's log line.

Tests: new SqliteAuditWriterCommitFailureTests forces a genuine
non-swallowed mid-batch INSERT failure via a real SQLite trigger + custom
scalar function on the writer's own connection (reflection seam), proving
(a) a row whose own insert succeeded still faults when a later row in the
same batch fails, and (b) FallbackAuditWriter rings every faulted row and
increments the failure counter for each -- both assertions fail against
the pre-fix code (proof-of-failure verified by temporarily reverting the
fix). EventLogPurgeServiceTests' standby case is inverted to assert the
purge now runs (not skips) on a standby node.
2026-08-19 14:49:24 -04:00
Joseph Doherty fd5e023d08 fix(comms): review findings — consumer-based debug orphan net, foreign-cancel triad, honest onConnected, served-row-exact retirement, full-rate reconcile
F1 (HIGH) DebugStreamBridgeActor: the 5-minute orphan net measured the MAILBOX
(SetReceiveTimeout), and once stream events were correctly marked
INotInfluenceReceiveTimeout nothing recurring reset it — the snapshot lands once
and GrpcStreamStable once — so every healthy session self-terminated at ~6 min
with a false "Site disconnected". Replaced with a periodic self-tick
(ConsumerLivenessCheckInterval, 30s) over a consumer-last-seen stamp renewed only
by DebugStreamConsumerAlive, which DebugStreamService Tells on a shared timer to
every session still in its registry (holding a session there IS "a consumer is
attached" — both the Blazor view and the SignalR hub release it on
dispose/disconnect, and it works headless). Reverting the wrapper was rejected: it
would restore the quiet-instance orphan bug.

F2 (MED) SiteStreamGrpcClient: the RpcException(Cancelled) filter now requires
cts.IsCancellationRequested. A peer-originated / channel-dispose Cancelled fired
none of onError/onCompleted/onConnected, leaving SiteAlarmAggregatorActor with
_streamDown=false forever (IsLive stuck true, reconcile reopen guard never fired).

F3 (MED) SiteStreamGrpcClient: a header TIMEOUT is no longer reported as
connected — that shape is exactly what an unreachable site produces, and it
cleared _streamDown, consumed _seedOnConnect and launched a full snapshot fan-out
at a dead site. AwaitHeadersAsync returns bool; the first received event is the
fallback connected signal, fired at most once from headers OR first event.

F4 (LOW-MED) SqliteAuditWriter.MarkReconciledUpToAsync: the blanket below-cursor
UPDATE retired late-stamped inserts that were never served (then age-purged —
silent loss). The flip is now bounded by insertion order: a Pending row retires
only if its rowid is at or below the high-water mark of rows this instance has
served from ReadPendingSinceAsync (clamped on purge, since SQLite reuses rowids);
Forwarded rows are exempt (central ACKed them over the push path). At-least-once
is unchanged.

F5 (LOW) Documented the liveness dependency (a served row never covered by a later
cursor stays Pending forever; PurgeExpiredAsync never purges Pending) in
ISiteAuditQueue + Component-AuditLog.md, and added a cheap site-health signal:
SiteAuditBacklogReporter logs a rate-limited warning when the existing
oldest-pending metric exceeds 24h.

F6 (MED) SiteAlarmAggregatorActor: _fanoutSinceLastTick was armed by the
reconcile's OWN fan-out, so steady state ran fan-out→skip→fan-out→skip — one
reconcile per 2x interval (120s), halving the not-reporting refresh and the alarm
reconcile backstop. The skip is now armed only by connect/failover-driven seeds
(initial, _seedOnConnect, and a re-seed queued behind one).

Tests: Communication.Tests 691 passed (+13), AuditLog.Tests 382 passed (+5).
2026-08-14 23:52:25 -04:00
Joseph Doherty 2ce0ad7ed1 perf(comms): alarms-only seed, capped buffers, at-least-once audit pull 2026-08-14 21:10:19 -04:00
Joseph Doherty 2e4e41a8f7 fix(auditlog): site audit DB onto the data volume; required path + soft flush
Closes WP1.2 of the arch-review remediation plan (finding #2, High):
SqliteAuditWriterOptions.DatabasePath defaulted to CWD-relative "auditlog.db",
which on the docker rig resolves onto the container's ephemeral overlayfs
(not the mounted /app/data volume), silently discarding the pending audit
forward-state backlog on every recreate; nothing in docker/ or docker-env2/
overrode it; FlushIntervalMs was validated but never read by the writer loop
(one commit per event even at trickle rate); and no PRAGMA synchronous was
set (SQLite's FULL default fsyncs every commit).

- DatabasePath now has no default (mirrors ZB.MOM.WW.LocalDb's LocalDbOptions.Path)
  and is required pre-host for Site nodes only, via a new StartupValidator raw-config
  check (top-level "AuditLog:SiteWriter:DatabasePath", NOT nested under ScadaBridge:
  AddAuditLog binds that section off the configuration root). SqliteAuditWriterOptionsValidator
  deliberately does NOT check DatabasePath itself, because AddAuditLog runs its
  ValidateOnStart on both Central and Site composition roots but only Site nodes
  ever resolve the writer — checking it there would fail Central's boot too.
- All 8 site-node appsettings under docker/ and docker-env2/ now set
  AuditLog:SiteWriter:DatabasePath to /app/data/auditlog.db (mounted volume,
  survives container recreate, same convention as LocalDb:Path); the local-dev
  base appsettings.Site.json sets ./data/auditlog.db to match.
- The writer loop now honors FlushIntervalMs: after draining the immediately
  available burst, it keeps the transaction open (bounded by FlushIntervalMs
  from the first event) waiting for more trickle-rate events before committing,
  instead of flushing (and fsyncing) per event.
- PRAGMA synchronous = NORMAL on the write connection — audit is best-effort by
  design (CLAUDE.md: "Audit-write failure NEVER aborts the user-facing action"),
  so NORMAL's narrower power-loss window is an acceptable trade for far fewer
  fsyncs; WAL mode still guarantees no corruption.
- Tests: StartupValidator site-required/blank/central-exempt cases; writer
  trickle-load single-transaction coalescing + beyond-interval separate-transaction
  regression (new FlushCountForTests seam); options-validator doc updates reflecting
  the moved responsibility. Full suite runs green: AuditLog.Tests 368/368,
  Host.Tests 480/480.

One-time migration note: the existing container-local auditlog.db (wherever it
landed under CWD) is abandoned by this change, not migrated — already-forwarded
rows are safe centrally (AuditLog is the durable copy), and any still-Pending
rows on the abandoned path are lost once. This is the exact bug being fixed, not
a new loss: those rows were already living outside the mounted volume and would
not have survived the next container recreate regardless. Cross-reference
docs/known-issues/2026-07-20-cached-telemetry-drain-hot-loop.md, which this
placement bug caused.
2026-08-14 20:13:31 -04:00
Joseph Doherty acd8a6b8b5 chore(data-layer): low-severity cleanups — SQLite param chunking, KPI metric catalog, computed-column predicate guard 2026-07-09 09:32:04 -04:00
Joseph Doherty 731ee69797 feat(audit-log): add site SQLite PurgeExpiredAsync honoring the ForwardState invariant
Retention purge deletes Forwarded/Reconciled rows older than the cutoff and
reclaims pages via incremental_vacuum; Pending rows are never purged on age
alone (would lose audit data that never reached central). Temp-table purge set
avoids the SQLite 999-param limit; runs under _writeLock. (PLAN-04 Task 1, S1/U2)
2026-07-09 06:11:54 -04:00
Joseph Doherty 9cff87fe85 docs(comments): strip internal task/milestone/bundle bookkeeping from code comments
Remove project bookkeeping citations from shipped code comments across the
solution: hyphenated task IDs (WP-14, StoreAndForward-025), milestone/task/
issue refs (M3, Task 4, Audit Log #23, #21), Bundle X task-bundle labels,
and C/D/K/S/T phase labels.

Comment text only — no code logic, string/log literals, or XML-doc structure
changed. Genuine descriptions are preserved (only the citation is stripped),
and technical lookalikes are retained (UTF-8, SHA-256, T00:00:00, M365,
UTC-5, pre-C4/pre-C5 schema versions). Flagged by the new CommentChecker
TaskReferenceInComment / TrackingReferenceInComment checks plus targeted
grep passes; full solution builds clean, append-only guard tests pass.
2026-07-07 11:03:26 -04:00
Joseph Doherty eabf270d71 docs: complete XML doc coverage (returns, summaries, inheritdoc)
Resolve all 622 issues flagged by the enhanced CommentChecker: add missing
<returns> tags (incl. the standard phrasing on non-generic Task methods),
add missing <summary> tags, and replace misused/redundant <inheritdoc/> on
members that override or implement nothing with real documentation.
Documentation-only — no behavior change; solution builds clean.
2026-06-03 11:39:32 -04:00
Joseph Doherty 1737d15f04 fix(audit): ScadaBridge C4 review — enable PRAGMA foreign_keys + MarkForwarded state guard (no Reconciled demotion) + test (Task 2.5) 2026-06-02 13:23:36 -04:00
Joseph Doherty 946d3e2aef feat(audit): ScadaBridge C4 — site SQLite two-table (audit_event canonical + audit_forward_state sidecar), forwarding on sidecar, IsCachedKind drain split (Task 2.5) 2026-06-02 13:11:20 -04:00
Joseph Doherty c27b2c3d5f fix(audit): ScadaBridge C3 review — safe enum-parse (fallback) in SqliteAuditWriter.MapRow + AuditEventDtoMapper.FromDto (Task 2.5) 2026-06-02 12:55:07 -04:00
Joseph Doherty db707bb0de feat(audit)!: ScadaBridge C3 — swap to canonical ZB.MOM.WW.Audit.AuditEvent across seams/emitters/DTO/redactor wiring; transitional 24-col storage shim (Task 2.5) 2026-06-02 12:37:50 -04:00
Joseph Doherty 7b0b9c7365 refactor: rename ScadaLink → ZB.MOM.WW.ScadaBridge (code + projects + namespaces)
Solution + 23 src projects + 26 test projects renamed; folders, csproj,
namespaces, and ScadaLinkDbContext/ScadaBridgeDbContext class updated.
ActorSystem "scadalink" → "scadabridge", Akka seed-node URLs migrated.
SQL roles/logins, LDAP domains, CLI command name, and CLI config dir
(~/.scadalink → ~/.scadabridge) also renamed.

Build green; 5 Host.Tests fail awaiting SQL login rename in next commit.
Pre-existing StaleTagMonitor timing flakes unchanged.

Rename script committed at tools/rename-to-scadabridge.sh.
2026-05-28 09:37:45 -04:00