Commit Graph

151 Commits

Author SHA1 Message Date
Joseph Doherty 8645138274 test(site-runtime): flush the tracking-store queue before the boundary-timestamp test's direct UPDATE
The single-writer queue (d91135f9) made RecordEnqueueAsync return on enqueue, so the
test's verifier-connection UPDATE raced the pump and touched zero rows whenever the pump
lost. Every other verifier-connection test already flushed; this one was missed.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 14:23:47 -04:00
Joseph Doherty 105541b655 fix(site-runtime): shed or rate-limited traced runs emit a Failed ScriptRun row (arch-review F7)
A traced run refused before launch — by the per-script in-flight cap
(MaxConcurrentRunsPerScript) or by MinTimeBetweenRuns — emitted no ScriptTrace row at
all, so the Flow Debugger reported NO_TRIGGER_WITHIN_WINDOW ("no value-change trigger on
this attribute, or no device echo"), which is wrong in both cases and sends the operator
to check the wrong things.

Both guards now leave a Failed ScriptTrace/ScriptRun row naming the guard that refused
the run, parented to the writing execution and carrying the trigger detail. The new
ScriptRunLauncher.EmitSuppressedRun reuses EmitScriptRun with elapsedMs 0 and no
collector (a shed run has no reply to ride on — the row reaches the debugger via
PullTraceEvents / Replay), resolving the writer and identity providers from a fresh
scope because it runs on the actor thread. Best-effort, never throws, no-op untraced.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 14:22:11 -04:00
Joseph Doherty 05b97b596d Merge branch 'worktree-agent-a5b63f3a0707e4e46' into fix/flow-debugger-arch-review 2026-09-06 14:15:58 -04:00
Joseph Doherty e79452d2de fix(site-runtime): Flow Debugger inline trace copy is redacted and byte-capped at the collector (arch-review F1, site half)
Every emit site hands FlowTraceCollector the same AuditEvent it hands the
writer, but redaction and payload capping lived inside the writers, which
return a NEW filtered record — so the inline copy that rides
ScriptCallResult.TraceEvents / RouteToCallResponse.TraceEvents was the RAW
event: no per-target SQL-parameter redaction, no header scrubbing, no 8 KB /
64 KB truncation. Live and Replay of the same execution could therefore show
different payloads, and the 500-event-only cap left the reply free to breach
the 4 MB gRPC receive limit — which would fail a real call, since the same
reply carries the script's return value.

The collector now applies the host IAuditRedactor itself (outside the lock;
the writers keep their own idempotent Apply) and enforces a byte budget
(DefaultMaxBytes = 1 MB over the redacted DetailsJson) alongside the event
cap, flagging Truncated when either trips. It moves to Commons
(Types/Debugging) so the central FlowDebugRunner can share it.

The redactor is a singleton, so ScriptRunLauncher resolves it from the ROOT
provider and keeps constructing the collector before the try — the traced
ScriptRun row is emitted from the catch blocks too, including the
shed-at-dequeue path that throws before the DI scope exists.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 14:15:38 -04:00
Joseph Doherty 50bf9ac74a fix(site-runtime): trace arm is cleared when the traced DCL write fails (arch-review F6)
The Flow Debugger trace arm was recorded when a traced write was ISSUED, not
when the DCL acknowledged it. A write the device rejected (or that timed out)
left the arm standing for the whole TraceArmWindowSeconds (30 s), so the next
unrelated subscription update on that attribute was stamped with the debugger's
execution id and spawned a traced, parented run — the debugger drew a "device
echo" of a write that never landed.

The write's continuation runs off the actor's dispatcher, so it cannot touch
_traceArms directly; it Tells self a new DisarmTrace message instead, handled in
a Receive<>. The disarm is keyed on the writer's execution id, so a late failure
cannot steal an arm a newer traced write has since taken.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 14:12:09 -04:00
Joseph Doherty d91135f960 perf(site-runtime): operation tracking store writes off the script thread (single-writer queue, batch per transaction)
A Flow Debugger live run (execution 1130c01f) spent 108 of a 176 ms site script run
between the cached DB write completing and Notify.Send starting: the immediate-
completion telemetry awaited two autocommit SQLite transactions on the consolidated
LocalDb file (RecordAttemptAsync + RecordTerminalAsync), on the script thread, each a
WAL fsync on a Docker volume; the submit row was a third. The audit half of the same
forwarder was already a channel write, so the tracking half was the whole cost.

OperationTrackingStore is now a single-writer queue: Record*Async enqueue an ordered
work item and return; one background pump drains the bounded channel (8192, wait
backpressure - tracking never drops) and applies each batch (<=256) in ONE transaction,
falling back to per-row apply when a batch fails so a poison row cannot take its
neighbours. Preserved invariants: FIFO across enqueue/attempt/terminal; read-your-
writes (GetStatusAsync and ReadChangedSinceAsync barrier on every write queued before
them, skipped when idle) so Tracking.Status, the telemetry actor's status join and
central's PullSiteCalls never see a stale row; timestamps stamped when recorded;
PurgeTerminalAsync returns after the purge lands; DisposeAsync drains the queue before
closing; sync Dispose keeps its documented in-flight-writes-fail semantics. New public
FlushAsync() is the explicit barrier. Accepted: a queued-but-unapplied row is lost on a
hard crash (ms window); S&F buffer and audit rows are unaffected.

No interface, forwarder or ScriptRuntimeContext change. Tests: write returns while
another connection holds the DB write lock and lands once released (fails on the old
store: 31 s); reads see 100 ordered attempts + terminal; DisposeAsync drains 200
queued rows; write after dispose throws ObjectDisposed. Existing tests that poke the
SQLite file right after a store call now FlushAsync first.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 12:25:58 -04:00
Joseph Doherty ec9011429c fix(site-runtime): trace sink is guarded — a throwing observer can neither fail a call nor skip the audit write
Review of Task 3: the sink invoke sat before WriteAsync/ForwardAsync inside the
best-effort try, so a throwing sink would have silently dropped the durable
row. GuardTraceSink wraps the delegate in every helper ctor (and Collect
guards EmitTrace); pinned by a throwing-sink test. ScriptRunLauncher no longer
allocates a collector for a traced run with no reply target (Nobody) — the
list would never be read. FlowTraceCollector.Complete pinned at unit level;
Snapshot documented as a non-closing peek.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 01:57:09 -04:00
Joseph Doherty f394f58405 feat(site-runtime): outbound API/DB/notification audit rows ride the flow-trace collector
ScriptRuntimeContext caches a trace sink (null on an untraced run, so the
production path is byte-identical) and threads it into the ExternalSystem /
Database / Notify helpers and AuditingDbConnection→AuditingDbCommand (onAudit).
All eight outbound emit sites hand the SAME AuditEvent instance to the sink
first, then to the writer/forwarder. FlowTraceCollector gains Completed +
Complete(): ScriptRunLauncher closes the collector where it builds the reply
snapshot, so un-awaited calls finishing after the run returned never mutate a
list the reply was built from. Cap (500) unchanged. Plan 2026-09-06 D5.

Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
2026-09-06 01:51:19 -04:00
Joseph Doherty 2c86adb782 feat(site-runtime): route handlers forward Trace and return child ExecutionId + trace events
Flow Debugger (design 2026-09-05 §5.2/§5.4): RouteInboundApiCall passes Trace on the
ScriptCallRequest and maps ScriptCallResult.ExecutionId/TraceEvents onto
RouteToCallResponse; RouteInboundApiSetAttributes passes Trace on SetStaticAttributeCommand.

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:43:33 -04:00
Joseph Doherty 01be39019e feat(site-runtime): InstanceActor trace arm stamps the device echo of a traced write; Trace rides on published changes
Flow Debugger (design 2026-09-05 §5.3): a traced SetStaticAttributeCommand on a data-sourced
attribute records (attribute → writer, expiry) for TraceArmWindowSeconds (new option,
default 30, validated > 0); the next DCL echo for that attribute is published with the
writer's SourceExecutionId + Trace whatever its value, and the arm is consumed once. Static
writes publish Trace directly. Untraced writes never arm (pinned).

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:43:33 -04:00
Joseph Doherty eca1434160 feat(site-runtime): traced runs emit ScriptRun rows, return ExecutionId + trace events; traced value-change runs inherit the parent
Flow Debugger (design 2026-09-05 §5.1/§5.4): ScriptRunLauncher emits one ScriptRun row per
traced run on every exit path (OccurredAtUtc = run start), replies with the run's
ExecutionId (always) and the collector snapshot (traced only). ScriptActor forwards
ScriptCallRequest.Trace and, for a value-change trigger fired by a traced stamped change,
parents the run to the writer with trigger detail in Extra; every other trigger stays a root.

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:43:33 -04:00
Joseph Doherty 93489cd7a2 feat(site-runtime): ScriptRuntimeContext emits ScriptTrace AttributeWrite/AttributeWait rows and inherits the Trace flag
Flow Debugger (design 2026-09-05 §5.1/§5.2): the context is the single funnel for
SetAttribute / WriteBatchAndWait / WaitInternal, so it emits the write/wait rows (collector
first, then the best-effort IAuditWriter whose redactor applies the payload cap), stamps
Trace on SetStaticAttributeCommand and ScriptCallRequest, and hands the flag + sink to the
inline CallShared child context. Untraced contexts emit nothing (pinned).

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:43:32 -04:00
Joseph Doherty febb268d89 feat(site-runtime): FlowTraceCollector
Per-run inline sink for ScriptTrace audit events (Flow Debugger design 2026-09-05 §5.4):
cap 500, Truncated flag, lock-based; the audit rows stay complete beyond the cap.

Claude-Session: https://claude.ai/code/session_01GSLkkHKz9JrAtmJjmQK3cP
2026-09-05 23:42:39 -04:00
Joseph Doherty afb5180ce0 docs+test(site-runtime): Lever 1 final-review polish — document that the Good reset does not release an in-flight request and that the guard is inert once interval ≥ ceiling; FlappingGood asserts the retry landed during the burst instead of timing Thread.Sleep; NativeAlarmActor (same unguarded amplifier) + cancel-race residue recorded as follow-ups
Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV
2026-08-28 15:10:21 -04:00
Joseph Doherty 0589486f4e fix(site-runtime): ConnectionQualityChanged(Good) only resets the tag-subscribe backoff while it is stretched — a flapping connection can no longer defer the retry it was meant to hasten
Review finding on ab08ecd3: each Good re-armed the watchdog from 'now', so Goods arriving faster than the floor starved the retry. Pinned by FlappingGood_DoesNotDeferTheRetry (12 Goods over 1.2 s; naive re-arm measured 1.43 s, guarded ~0.3 s).

Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV
2026-08-28 14:57:30 -04:00
Joseph Doherty ab08ecd3d3 feat(site-runtime): ConnectionQualityChanged(Good) resets tag-subscribe backoff to the floor (Lever 1); lock in superseded-reply drop
Task 4 review fix-ups: RepeatedFailures gaps[1] lower bound 600→500 (two compounding jitter draws bottom out at 512 ms); SubscribeToDcl also clears _tagPathToAttributes and its prune is documented as defensive (PreStart is the only caller).
2026-08-28 14:50:04 -04:00
Joseph Doherty b6b401c879 feat(site-runtime): tag-subscribe failure reply frees the in-flight slot, backs off (×2 ±20%, capped) and Success resets to floor (Lever 1)
Also from the Task 3 review: FailedSubscribeResponse_IsRetried guards the failure path again (large ceiling); HandleSubscribeTagsResponse reads state instead of get-or-create; SubscribeToDcl prunes stale retry state on re-entry; doc comments refreshed.

Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV
2026-08-28 14:36:22 -04:00
Joseph Doherty 029d39fca2 feat(site-runtime): tag-subscribe in-flight guard — one outstanding request per connection until TagSubscribeInFlightCeilingMs (Lever 1)
FailedSubscribeResponse_IsRetried and LostSubscribeResponse_IsRetried_AndSuccessStopsRetrying now pin the in-flight ceiling to the retry interval: a lost reply is re-sent only once the outstanding request is presumed lost, by design.

Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV
2026-08-28 14:24:35 -04:00
Joseph Doherty 2f471dd7a1 feat(site-runtime): pure NextTagSubscribeInterval backoff step (double, ±20% jitter, clamp) 2026-08-28 14:20:19 -04:00
Joseph Doherty 6f7c6ad877 feat(site-runtime): TagSubscribeRetryMaxIntervalMs + TagSubscribeInFlightCeilingMs options (Lever 1, validated) 2026-08-28 14:19:50 -04:00
Joseph Doherty 65b3c23239 perf(siteruntime): projected single-row deployed-config reads (P1-4)
HandleEnsureInstanceStarted, HandleDeploymentStateQuery, and HandleEnable each
called GetAllDeployedConfigsAsync — materializing every deployed config,
including every ConfigJson blob — just to FirstOrDefault one row. A
volume-restore reconcile healing hundreds of instances turned that into
hundreds of full-table reads.

Add SiteStorageService.GetDeployedConfigAsync(name) (single-row, full
projection incl. ConfigJson, for callers that (re)start an Instance Actor)
and GetDeployedConfigMetadataAsync(name) (single-row, ConfigJson-free
projection, for callers that only need DeploymentId/RevisionHash/IsEnabled).
Switch the three call sites; leave decisions/semantics identical.

HandleEnsureInstanceStarted gets its own _deployedConfigLoader seam,
deliberately separate from the existing _configLoader (still used by the
boot-time load, which legitimately needs every row) so a test overriding one
cannot accidentally gate the other. Generation fencing (StoreGenerationAtRead,
Medium wave T1) is untouched: the generation is still captured on the actor
thread before the read dispatches.

Tests: new SiteStorageServiceTests lock in that both projections return the
same values GetAllDeployedConfigsAsync().FirstOrDefault(...) did. The three
row-101 ensure-started race tests (ReconcileStartsInstanceTests) are ported
to the new seam and still pass, proving generation fencing survives the
projection. Proof of failure: since this is a pure I/O-cost fix with no
observable behavior change, the new/changed tests were confirmed to fail to
even compile against the unfixed code (stash/unstash) — GetDeployedConfigAsync,
GetDeployedConfigMetadataAsync, and the deployedConfigLoader ctor param don't
exist there.
2026-08-19 16:41:03 -04:00
Joseph Doherty e63ffd90ee fix(siteruntime): stamp the ensure-started heal with the store-write generation + order the redeploy override reset ahead of the successor (arch-review 2026-08-19 P1-1 + P1-2)
P1-1 — the row-67 ensure-started self-heal re-read the durable row off-thread but
captured no _storeWriteGeneration and never ran IsStoreSnapshotStale, unlike the boot
load and the reconcile sweep. A Disable or Delete committing inside that window left the
result carrying an enabled config that passed every guard (neither command sets any of
the three pipeline markers ShouldAttemptEnsureStart re-checks), so the heal started an
instance the operator had just stopped — PERMANENTLY, since the sweep iterates store rows
only and has no runtime→store stop leg. The generation is now captured at dispatch,
threaded on EnsureInstanceStartedResult and checked on landing, and the comment claiming
the stored row already covered this is corrected. The heal reads through _configLoader,
the same seam the boot load and sweep use (identical production call), so a test can hold
the read open across a committed disable/delete.

P1-2 — the "static overrides reset on redeployment" invariant was racy in both
directions. ApplyDeployment created the successor Instance Actor first and only then
dispatched the clear, queued behind a full config-JSON write, so the successor's PreStart
override load routinely won and applied the PREDECESSOR's overrides — keys absent from
the new config included — to in-memory state for the actor's lifetime while the disk
ended cleared; a later restart then silently flipped the values. The clear is now
dispatched BEFORE CreateInstanceActor and handed to the actor as a gate its override load
chains behind; the persistence task joins the same task, so a failed clear still fails the
deploy exactly as before. Leg 2: the predecessor's PostStop flush was fire-and-forget and
could commit up to one coalescing window AFTER the clear (the Deployment Manager gates the
clear on Terminated), durably resurrecting overrides — PostStop now waits for it, bounded
at 5s with a Warning on overrun.

Tests: new RedeployStaticOverrideResetTests (5) pins both legs plus the gate mechanism and
first-deploy behaviour; three new cases in ReconcileStartsInstanceTests pin the discarded
disable/delete heals and that an unraced heal still starts. InstanceActorTests'
StaticOverride_ResetOnRedeployment comment claimed to simulate an ordering the production
path did not have — corrected.
2026-08-19 14:49:24 -04:00
Joseph Doherty fd4cb30273 fix(siteruntime): arm stuck-script watchdog + health/summary on alarm runs (arch-review 2026-08-19 P1-3)
Alarm on-trigger runs (ScriptRunLauncher.RunAlarmScriptAsync) bypassed the
stuck-script watchdog, the script-error health counter, and
ScriptRunSummaryRecorder entirely, even though they run on the same bounded
ScriptExecutionScheduler pool as instance scripts. A wedged alarm body cost a
pool thread forever with no site event and no detach/replace, and a timed-out
or failed on-trigger run was invisible to health monitoring and the run
summary.

Bring the alarm path to parity with the instance-script path (RunScriptAsync):
capture the worker slot/run stamp in the first synchronous segment and arm
ArmStuckScriptWatchdog with the same detach-and-replace semantics and gauge;
resolve ScriptRunSummaryRecorder/ISiteEventLogger from the same DI provider and
thread RecordStarted/RecordCompleted/RecordFailed/RecordTimedOut; increment
ISiteHealthCollector.IncrementScriptError on both the timeout and general
failure catch blocks. LaunchAlarmScript/RunAlarmScriptAsync now take
healthCollector and serviceProvider parameters, passed from AlarmActor's
existing _healthCollector/_serviceProvider fields. No change to the
enqueue-anchored deadline, the shed cap, or "a stop does NOT cancel an
in-flight run" semantics.

Adds AlarmScriptWatchdogTests (previously zero coverage of LaunchAlarmScript):
wedged-worker detach + replacement, timeout-at-dequeue health counter
increment, and successful-completion summary recording. All three fail
against the pre-fix code (verified via stash) and pass after.
2026-08-19 14:49:24 -04:00
Joseph Doherty f02e116688 fix(siteruntime): extend the row-101 stale-snapshot guard to the instance-config half
ReadStoreSnapshotAsync's snapshot is read off the actor thread and consumed only
when its piped message lands. Row 101 guarded the DEFINITIONS half of that race;
the CONFIG half was left to machinery whose reach is time-limited, and both legs
of the instance reconcile sweep were reachable through it:

- Row-72 leg: a read dispatched before a deploy's persist, landing after the
  deploy completed AND after the pipeline released ownership, carries the
  revision the deploy replaced. TryRestartStaleInstance then "repairs" the
  instance BACKWARD, re-deploying the snapshot's own superseded ConfigJson.
  InstanceIsOwnedByADeployPipeline covers only in-flight deploys, and the
  per-revision claim has never seen the old revision, so nothing stopped it.
- Row-78 leg: a read dispatched before a delete/disable commits still shows an
  enabled row while the actor is already stopped, so StartStoredInstance
  resurrects it — and delete clears _sweepAttemptedRevisions, freeing the claim.

Both verified real by neutralising the freshness check and re-running the new
tests (the fail-before site event literally reads "running revision rev-2 is
stale, applying stored revision rev-1"; the deleted instance came back).

_definitionsWriteGeneration becomes _storeWriteGeneration, covering both tables
(the snapshot is one consistent read and is discarded whole, so one counter is
the right shape); DefinitionsGenerationAtRead -> StoreGenerationAtRead on both
internal records. The generation is now bumped at BOTH ends of every mutation —
when the off-thread write is dispatched and when its completion message lands —
because the config paths advance actor state before the write, so an
initiation-only bump would let an in-window read look fresh. Disable, delete and
the init-failure rollback have no actor-thread completion message of their own,
so a minimal StoreWriteCompleted self-message carries the bump for them.

CDC-replicated writes, snapshot resyncs, restored files and
SiteReconciliationActor's fetch deliberately do NOT bump: discovering exactly
those is the sweep's reason to exist.

A stale sweep result is now dropped WHOLE (no overlay apply, no start, no
restart). A stale boot load cannot be dropped — startup must finish — so it
re-dispatches LoadDeployedConfigs(); a fifth consecutive superseded read is
applied anyway with a Warning, and a late one arriving after the boot completed
is dropped rather than looping against the S5 retry timer.

Tests: three fail-before regressions (backward restart, resurrection, boot
re-read) plus two guard twins pinning that the legitimate row-72 restart and
row-78 start still fire on a current-generation snapshot. The row-101
definitions tests keep passing; FreshEmptySweepSnapshot's hardcoded generation
moves 1 -> 2 now that one artifact apply bumps at both ends.

Docs: Component-SiteRuntime.md's stale-snapshot paragraph now describes the
whole snapshot (both halves, both consumers, the boot re-dispatch, the
CDC-no-bump rule), the components mirror follows, and the arbitration memo gains
§11 extending §10 to the config half.
2026-08-18 05:38:00 -04:00
Joseph Doherty 0d3a90a9fd fix(siteruntime): guard the DCL arbitration overlay against stale store snapshots — register row 101
An off-thread snapshot read racing an artifact apply could land after the
apply populated the overlay; its stale absences then tombstoned and stopped
just-applied LIVE connections, which nothing re-creates until an instance
redeploy (the removal evicts the hash slot the overlay push is gated on).
Each read now captures a definitions-write generation at dispatch; a stale
result is discarded whole (additions too — a stale row would reconfigure a
live connection backward). Deterministic repro fails before the fix; a
fresh-generation guard test pins that row-90 disappearance still fires.
2026-08-17 07:03:26 -04:00
Joseph Doherty 8a9a8d2951 merge(agent-c): row 100 — SiteScriptCompileCache becomes an instance-scoped sealed class owned by the ScriptCompilationService DI singleton (zero production Clear() callers existed, so Clear() is deleted outright); row-91 serialized collection relaxed and delta assertions restored to exact counts; new registration test pins the load-bearing singleton lifetime 2026-08-17 06:20:17 -04:00
Joseph Doherty 7c1268d85c feat(dcl+siteruntime): reconfigure a LIVE data connection in place — register row 97
`DataConnectionManagerActor.HandleCreateConnection` is create-only (warns and
returns for a name it already holds), so `EnsureDclConnection`'s documented
"a changed config re-issues the create so the DCL adopts it" was never true on
ANY node: a corrected endpoint/credential/backup/failover-count took effect only
when the connection was next built from scratch (node restart, singleton
failover, remove-then-re-add), while the DM's hash cache recorded it as applied.
Rows 89/90 made that honest with a Warning; this closes it.

New `ReconfigureConnectionCommand` (Commons, same shape as the create):

- `DeploymentManagerActor.EnsureDclConnection` now has three outcomes off its one
  per-name hash slot — unchanged -> nothing at all, new -> create, CHANGED ->
  reconfigure — and logs "Reconfigured DCL connection {Connection}
  (protocol={Protocol}) in place" at Information in place of the old Warning. The
  winner-resolution / arbitration / tombstone logic is untouched; only what
  happens after a change is detected changed.
- `DataConnectionManagerActor` FORWARDS the command to the owning connection
  actor, falling back to a create for a name it does not hold, so the command is
  safe whatever the manager's state.
- `DataConnectionActor` handles it in ALL THREE states and never stashes it — a
  connection pointed at a wrong or dead endpoint lives in Connecting/Reconnecting,
  which is exactly when a correction arrives; stashed it would wait on the broken
  configuration forever. Handling replaces protocol/primary/backup/failover-count,
  re-arms endpoint selection to Primary (the new config's primary is the intent,
  whatever the old config had failed over to), and calls the new shared
  `SwapAdapter` helper — now used by both failover sites too, so the swap exists
  once: unhook + fire-and-forget dispose -> _factory.Create -> rewire -> bump
  _adapterGeneration LAST. `BecomeReconnecting(afterReconfigure: true)` then
  reconnects immediately (StartSingleTimer replaces any armed reconnect, so no
  racing attempts) with the unstable-disconnect accounting and failover check
  suppressed — the link was dropped on purpose, and counting it as an unstable
  cycle could trip a failover onto the NEW backup before the new primary had been
  tried once. ReSubscribeAll/ReSubscribeAllAlarms then re-issue everything from
  the untouched subscription registries. Idempotent: a reconfigure carrying the
  configuration already in force is a no-op.

Stop-and-recreate was rejected as destructive: subscriptions live inside
DataConnectionActor and InstanceActor cancels its subscribe-retry timer on first
success, so a rebuilt actor would have no subscribers and nothing would ever
re-subscribe.

Tests: new DataConnectionReconfigureTests (7; 5 fail / 2 pass against a
behavioural revert). DclConnectionArbitrationTests gains two row-97 tests and its
CDC-arrival test now asserts the second push is a reconfigure; its ping-pong
guard also asserts zero reconfigures, since a flip would now show up there.
SiteRuntime-010 renamed to ..._ReissuesReconfigureCommand. 3 fail against a
reverted EnsureDclConnection. DataConnectionLayer.Tests 327/327,
SiteRuntime.Tests 643/643, solution build 0 warnings.

Docs: Component-DataConnectionLayer.md gains an "In-place reconfiguration"
section under Endpoint Redundancy; Component-SiteRuntime.md, and the
docs/components/ mirrors, updated; the arbitration decision memo gains an
appended §9 postscript resolving its residual 1 (residuals 2 and 3 stay open).
2026-08-17 06:19:13 -04:00
Joseph Doherty 84b405ff5f refactor(siteruntime): scope SiteScriptCompileCache to the ScriptCompilationService singleton — close register row 100
The compile cache was a `static` class, so its entry table and its Hits/Count
counters were one process-global blob every test class in the SiteRuntime
assembly wrote into. Row 91 last wave neutered the resulting flakes (per-test
GUID keys + one-sided counter bounds), but the shared mutable state itself
remained: any test calling Clear() evicted entries other parallel classes had
just inserted, and a Clear() from a class outside the serialized collection
would degrade the others to recompiles and re-open the row-91 flake class.

PREMISE CORRECTION: the register speculated production clears "per-instance or
per-deployment script entries" and offered ClearKeys(prefix) as the natural fit.
There are NO production callers of Clear() at all — grep of src/ finds only
GetOrAdd (ScriptCompilationService.CompileCore). Clear() existed solely so
tests could reset the global. A key-scoped API would therefore have been new
production surface built for nobody, so the instance-scoped seam is the fix and
Clear() is deleted outright rather than migrated.

- SiteScriptCompileCache: `internal static class` -> `internal sealed class`;
  Cache/EvictionLock/_hits/_accessSequence become instance fields. LRU batch
  eviction, the double-checked sweep and the ConcurrentDictionary.ToArray()
  snapshot are byte-for-byte the same policy. Clear() removed.
- ScriptCompilationService owns one cache. Public ctor (the DI path) builds its
  own; a new internal ctor takes one for tests. Production N4 sharing is
  unchanged and now rests on the DI SINGLETON registration — the one service
  instance is threaded to DeployCompileValidator and to every Instance Actor —
  so that lifetime is load-bearing and is pinned by a new registration test.
- Tests: all three Clear() callers migrated off it. Each test now owns its cache,
  which lets the assertions get STRONGER — the row-91 workarounds are retired:
  Hits/Count are exact again (no >= baseline bounds), the per-test GUID key
  token is gone, and ForeignEntrySlack (512 entries of allowance on the bound)
  drops to the racing-thread count in the concurrency test. New test
  SeparateCaches_DoNotShareEntriesOrCounters pins the scoping directly: filling
  one cache past MaxEntries leaves another's entry, hit count and identity
  intact. DeploymentWarmThenGateTests additionally asserts Count == 1 now that
  its counter is private.
- The [Collection("SiteScriptCompileCache")] serialization from row 91 is
  RELAXED (removed from all three classes) — they no longer touch any shared
  state, so there is nothing left to serialize.
- Docs: Component-SiteRuntime.md gains the ownership/singleton note and drops
  the Clear() reference; DeploymentManagerActor / DeployCompileValidator /
  ServiceCollectionExtensions comments corrected to say where the cache lives.

Build: 0 warnings. SiteRuntime.Tests 643/643, three consecutive full-parallel
runs. AuditLog.Tests correlation classes (other ScriptCompilationService
consumers) green.
2026-08-17 06:03:58 -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 26e3a1b9a6 test(siteruntime): close register row 91 — compile-cache asserts become key-scoped deltas
SiteScriptCompileCacheTests.GetOrAdd_SameCodeAndGlobals_ComputesOnce asserted
Assert.Equal(1, SiteScriptCompileCache.Hits) after a Clear() on what is a
process-wide static cache. The [Collection("SiteScriptCompileCache")] attribute
serializes this class against ScriptCompilationServiceTests and
DeploymentWarmThenGateTests, but NOT against the rest of the assembly: every
class that compiles a script — Instance/Script actor tests, deployment tests —
goes through the same GetOrAdd in parallel and bumps the same global Hits/Count.
Failed ~1 run in 4 under a full parallel solution run, green in isolation.

Fixed with the register's row-74 pattern (98c94588): assert the DELTA
attributable to this test's OWN keys, never an absolute global value. Two habits
carry it.

1. Every test now keys its entries on a per-test-instance guid token (the new
   Code() helper embeds it as a comment), so no other class in the assembly can
   collide with or serve them. That makes the factory-call counts and
   Assert.Same identity checks exact and fully owned — and those are the real
   claims: "computed once" and "the second lookup returned that instance".
2. The process-global counters are only ever asserted as one-sided bounds
   (>= baseline + n), since foreign traffic can only add. There is no concurrent
   Clear() to make a delta go negative: all three classes that call Clear() are
   in the serialized collection.

Four assertions changed shape, and two got stronger rather than weaker.
GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries asserted Count == 2 and
Hits == 0 — both only true while nothing else was compiling. "No cross-globals
hit" is now asserted where it is observable and ours: the second lookup HAD to
compute, the two results are not the same instance, and re-getting either serves
its own entry rather than the other globals surface's. Overflow_StaysWithinTheBound
gained an eviction probe over its own keys — a bound with foreign slack is not
proof a sweep ran; a recompute of an entry we inserted is.
Overflow_EvictsOldestBatch_… had Assert.Equal(hitsBefore + 1, Hits), the same
absolute-delta shape, now a lower bound alongside the sharp Assert.Same.

DeploymentWarmThenGateTests carried the identical latent shape one line before
its own delta assertion — Assert.Equal(0, Hits) immediately after Clear() —
and is converted to a baseline snapshot too.

SiteRuntime.Tests: 633/633 green, twice, at full parallelism (43 s, 42 s).
2026-08-17 05:27:22 -04:00
Joseph Doherty c57e74e660 fix(siteruntime): arbitrate DCL connection config — definitions store wins (rows 89 + 90)
Closes deferred-work-register rows 89 and 90, and corrects their premise twice.

THE RULE
The `data_connection_definitions` store wins wherever it has an opinion; an
instance's flattened `Connections` entry is a deploy-time snapshot and is
authoritative only for names the store has never owned (the deploy-only site,
where that table is legitimately empty).

PREMISE CORRECTION 1 — the peer's DCL is EMPTY, not stale
`DeploymentManagerActor` is a `site-{SiteId}`-scoped singleton and the only
sender of Create/RemoveConnectionCommand, to its OWN node's `dcl-manager`
(which exists on every node but is populated only where the singleton runs).
So the standby serves nothing. The real defect is that `_createdConnections`
is in-memory per singleton incarnation: a restart or failover rebuilds every
connection from the flattened snapshots and silently discards a newer
connection-only artifact deploy. Row 89 is therefore create-time, not
message-delivery, and NOT replication-specific — the same loss happens on an
unreplicated single-node restart.

PREMISE CORRECTION 2 — a changed config never reached a LIVE connection either
`DataConnectionManagerActor.HandleCreateConnection` is create-only (warns and
returns on a name it holds), so `EnsureDclConnection`'s documented "re-issues
the create so the DCL adopts the new configuration" has never been true on any
node. Now surfaced with a Warning saying exactly that, once per changed config.
Not fixed here: stop-and-recreate drops every tag subscription and InstanceActor
cancels its subscribe-retry timer on first success, so nothing re-subscribes.

MECHANISM
`data_connection_definitions` gains its first production reader. It is read on
the SAME off-thread task as the deployed configs (`ReadStoreSnapshotAsync`), by
the boot load and every sweep tick, into an actor-thread-confined arbitration
overlay applied BEFORE the boot batch and BEFORE the sweep's repair loop. A
CDC-delivered definition change converges within one tick on whichever node runs
the singleton — no new cross-node message. A failed definitions read yields null
and leaves the overlay untouched (distinct from an empty list, which is a real
answer); clearing it would reintroduce the ping-pong through the error path.

THE PING-PONG, KILLED BY CONSTRUCTION
§7.4 of the sweep memo rejected a store-level pass because two sources would
flip the single per-name hash slot, recreating the actor every tick. That is a
property of WHERE the hash is computed: every ensure path now resolves the
winner BEFORE hashing, so only one configuration can ever occupy the slot. The
artifact-deploy path writes its rows into the overlay in the same pass, closing
the same flip arriving through the other door.

REMOVAL (row 90)
Both removal paths share `RemoveConnectionCentrally` and record an in-memory
tombstone, so a centrally removed connection is not resurrected by the next
instance start — a pre-existing bug. Tombstones are deliberately not persisted;
a definition row reappearing clears one.

TESTS
tests/.../Actors/DclConnectionArbitrationTests.cs — 8 tests. Fail-before
(behavioral revert: overlay inert + tombstone check disabled): 5 fail, 3 pass;
the 3 passers are the two deploy-only regression guards and the ping-pong guard.

DOCS
New decision memo docs/plans/2026-08-17-dcl-connection-arbitration-decision.md
(topology, rule, ping-pong mechanism, tombstones, residuals); Component-SiteRuntime,
Component-DataConnectionLayer + their docs/components mirrors; the §7.4 residual in
the sweep memo marked closed with its premise corrected.
2026-08-17 05:22:42 -04:00
Joseph Doherty 524bdbbe90 test(siteruntime): gate ReconcileStartsInstanceTests on the DM's first instance-count report (register row 83)
Row 67's suite still opened every test with a bare Task.Delay(500) after
constructing the Deployment Manager. Each test then writes its
deployed_configurations row -- and a row that lands before the boot-time load
reads storage is picked up by that load, which means the test passes while
proving the BOOT LOAD works rather than the startup-reconcile path it names. A
sleep only makes that race unlikely; on a loaded machine the load can still be
in flight when it expires and the test silently changes what it proves instead
of failing.

The row-78 fix session hit exactly this trap and closed it in
InstanceSweepSelfHealTests by gating on the first
ISiteHealthCollector.SetInstanceCounts report -- the edge the manager raises
unconditionally the moment its load completes, including for an empty site.
This applies the same gate here.

The probe is now a shared TestSupport helper (DeploymentManagerStartupProbe,
added with row 85) rather than a private nested class, so both suites use one
implementation.

Also pins InstanceSweepIntervalSeconds = 0 for this suite: with the sweep armed
its periodic self-heal would independently start the same instances and the
suite would stop distinguishing the row-67 mechanism from the row-78 one.

The remaining Task.Delay calls in the file are deliberate and left alone -- they
precede assertions that NOTHING happened (a disabled instance stayed stopped, a
redundant ensure-started changed nothing), which cannot be awaited on. The one
delay that was a readiness wait rather than an absence wait is now an
AwaitAssertAsync on the instance actually being routable.
2026-08-17 03:48:12 -04:00
Joseph Doherty 8f2c2813ea fix(siteruntime): self-healed and newly-enabled instances now get their DCL connections (register row 85)
An Instance Actor started by a self-heal came up with NO DCL connection actors,
so every tag it subscribed was answered "Unknown connection: <name>" by the DCL
manager indefinitely. Proven live 2026-08-16 by the row-78 close-out probe: the
sweep correctly started the replication-seeded instances, then every subscribe
failed for 6+ min until a second pair restart.

Root cause is narrower than the register assumed. A DCL connection actor is
never created from data_connection_definitions -- that table has no production
reader at all (GetAllDataConnectionDefinitionsAsync is called only by a test).
It exists only as a side effect of starting an instance, from the Connections
map inside that instance's own flattened config, via EnsureDclConnections.

That ensure was the CALLER's job and only two of the four callers made it:

  CreateInstanceActorBatch (boot)   ensured  OK
  ApplyDeployment (deploy)          ensured  OK
  StartStoredInstance               MISSING  <- row-67 ensure-started + row-78 sweep
  HandleEnableResult                MISSING  <- enabling a disabled instance

The two misses are the two paths added after the original pair -- the signature
of a convention living at call sites instead of in the funnel.

It was never replication-specific: the boot load reads only ENABLED configs, so
a node booting with an instance disabled never creates its connections either,
and enabling it afterwards hit the identical dead end on an ordinary
unreplicated site.

Fix: EnsureDclConnections moves into CreateInstanceActor, already documented as
the single Instance Actor creation funnel (it is where the running revision is
stamped for the same reason), and the two caller-side calls are removed. Placed
before the already-exists guard so the two paths that already called it keep
their exact previous semantics. No new message, option or DCL change -- it
reuses EnsureDclConnection's existing per-name hash guard, so steady state is
silent. Structural rather than another guard: a fifth start path cannot forget.

Deliberately NOT done, with rationale in the memo: a sweep pass over
data_connection_definitions. It would not have fixed row 85 (that table is empty
on a site that has only ever received instance deploys), and re-hashing from it
would fight the deploy path over the same per-name hash slot, recreating
connection actors and dropping live subscriptions on a loop. Two residual
sibling gaps (connection-only artifact deploy replicating to a peer that never
adopts it, and the mirror-image removal case) are recorded in the memo and the
component docs as open, not silently skipped.

Tests: SelfHealCreatesDclConnectionTests, 7 tests asserting on the
CreateConnectionCommand messages sent to a recording DCL stand-in. Verified
fail-before against a behavioral revert (call removed from the funnel, restored
at the two original sites): 4 fail / 3 pass, the 3 being steady-state silence
plus two refactor guards that must pass on both sides.

Docs: sweep design memo gains section 7; Component-SiteRuntime,
Component-DataConnectionLayer, docs/components/SiteRuntime and
docs/components/DataConnectionLayer updated (the deploy-flow step ordering in
docs/components/SiteRuntime was stale as written).
2026-08-17 03:48:01 -04:00
Joseph Doherty e606b24a55 fix(siteruntime): instance reconcile sweep closes config/runtime divergence (register rows 78, 72)
The site's deployed_configurations rows are the durable truth about what should be
running; the set of live Instance Actors is what IS running. Nothing kept the two in
agreement after boot, and nothing surfaced a disagreement.

Row 78 (availability, proven live 2026-08-15) -- deployed_configurations is one of
LocalDb's ten replicated tables, so on a replicated pair a config can appear in a
node's file with no local actor involved at all: the CDC sync applier writes the row
directly. Live sequence: site-a-b's reconcile fetched 4 configs at its solo boot; the
pair was then restarted with a fresh site-a-a file and snapshot replication seeded
site-a-a's table within ~5s -- AFTER its boot-time load had read an empty table, and
invisibly to its own reconcile (central rightly reported "0 fetched", because central's
view of the site was correct). Enabled instances sat dead with their configs present
until yet another restart. Row 67's EnsureInstanceStartedCommand only covers configs
the reconcile itself FETCHES; this is the same gap through a side door.

Row 72 (silent divergence) -- for a STALE ReconcileGapItem the pass writes the newer
config via StoreDeployedConfigIfNewerAsync, a pure file write, while the running
Instance Actor keeps executing the previously compiled config until the next restart
or redeploy. Nothing said "stored revision != running revision".

Both are closed by one periodic sweep owned by the Deployment Manager singleton
(ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds, default 60, validated >= 0;
0 disables). Each tick re-reads the durable rows off-thread and, on the actor thread,
for every ENABLED stored config either starts a missing Instance Actor (row 78) or --
when the running one's revision no longer matches the stored one -- re-applies the
stored config through the ordinary redeploy path (row 72).

A sweep was chosen over a LocalDb change hook deliberately: a hook covers only the
replication arrival path, and only on the half of the fleet running with replication
enabled, whereas the sweep is arrival-path-agnostic (replication seeding, post-boot
snapshot resync, a hand-restored file, any future path). It is also not available --
ILocalDb 0.3.0 exposes no change-notification or applied-rows callback, so a hook would
mean a new upstream capability in scadaproj plus an in-process bus into this singleton,
a cross-repo change to close a site-local gap.

Row 72's decision is to CONVERGE, not merely warn. The stored row is what central
successfully deployed; an actor serving the config central believes was replaced is the
actual fault, and a surface-only warning leaves the operator to issue by hand exactly
the redeploy the sweep would have performed. Reusing HandleDeploy means the compile
gate, warm-then-gate ordering, redeploy-supersede, the static-override / native-alarm
reset and all-or-nothing-per-instance are the production ones, not a second
implementation -- and a stop still does NOT cancel an in-flight script run, exactly as
for a central redeploy.

Guards, shared with row 67 via the extracted InstanceIsOwnedByADeployPipeline predicate
and the shared StartStoredInstance helper:
- a DISABLED instance is never started (config row, no actor is its correct state);
- the sweep defers to any deploy pipeline owning the instance (compile warm in flight,
  predecessor still terminating, deployment awaiting its persistence/init join), so it
  can never interleave with a real redeploy -- that window has the config stored and no
  actor, which an unguarded sweep would answer by creating a child whose actor name is
  still registered;
- inert until boot-time Instance Actor creation has finished, so a tick between
  staggered batches does not "heal" instances the boot load is about to create;
- ONE repair attempt per (instance, revision) pair -- this is what stops a periodic
  self-heal becoming a restart loop for an actor that dies during init, or for a stored
  config the compile gate rejects (a rejection leaves the running instance untouched:
  the gate returns before anything is stopped);
- reads never overlap, so a slow read cannot stack ticks into duplicate repairs.

A sweep that finds nothing is completely silent -- no log above Trace, no site event.
A repair emits the same Warning `deployment` site-event pattern as row 67 with the
trigger named ("deployment-manager sweep self-heal") so it is distinguishable from a
startup-reconcile heal; a stale-revision restart names BOTH revisions and a rejected
one adds an Error event. The synthetic redeploy replies to the singleton itself rather
than NoSender, because dead letters are a monitored site health metric.

CreateInstanceActor gains a revisionHash parameter and is the single creation funnel
that records it, so the stale check covers every path that can start an Instance Actor;
a missing entry reads as "unknown", never "stale", so a future path bypassing the funnel
degrades to no-op rather than to spurious restarts.

Fail-before by behavioral revert (sweep timer left unarmed): 5 of 10 new tests fail,
including both live symptoms -- the row-78 repro ("A replication-seeded config for an
enabled instance left no running Instance Actor") and the row-72 repro, whose snapshot
still shows the rev-1 value 111 while the store holds rev-2's 222. The 5 negative
guards (disabled / in-flight redeploy / matching revisions / silent-when-healthy /
interval-zero) are green in both states, as they must be.

SiteRuntime 626/626, Host 505/505, AuditLog 383/383.

Design note: docs/plans/2026-08-15-instance-sweep-self-heal-design.md
2026-08-16 04:16:06 -04:00
Joseph Doherty 744ff36a7b merge: startup reconcile starts the healed InstanceActor (register row 67) — EnsureInstanceStartedCommand to the DM singleton, idempotent, disable-wins, Warning site event on self-heal 2026-08-15 17:56:21 -04:00
Joseph Doherty e8a90f18dc merge: external-system retry policy end-to-end (register row 27) — MaxRetries/RetryDelay now travel artifact→gRPC→site schema→S&F enqueue, CLI --max-retries/--retry-delay-seconds, ArtifactDiff compares them 2026-08-15 17:56:21 -04:00
Joseph Doherty 7a7d345b89 test: replace wall-clock sleeps used as synchronization — register row 40
Six sleeps across four test files stood in for an observable edge. Each preceded an
ABSENCE assertion (or "settled" before one), so they failed toward false-pass rather than
flake: the assertion could not distinguish "the mechanism ran and correctly stayed quiet"
from "the mechanism had not run yet".

ScriptDeadlineAtEnqueueTests (two sleeps, the worst offender)
  - :105 Thread.Sleep(2s) — now two explicit edges: the run must be observably QUEUED
    (scheduler.QueueDepth >= 1, never checked before), then the wait polls until the 1 s
    budget has elapsed, bailing early if the run leaves the queue, and asserts it is STILL
    queued afterwards. The test now proves the design claim it was named for — queue wait
    burns the run's budget — instead of assuming it.
  - :128 Thread.Sleep(2s) + Assert.Equal(0, DetachedThreadCount) — replaced by a positive
    control: a second script that genuinely wedges a worker, on the same scheduler, same
    1 s timeout + 1 s grace, armed strictly later. Its stuck report is the edge; once it
    lands, every watchdog armed at or before it has provably finished its grace delay. The
    absence is then asserted per script name, and the detach counter is pinned at 1 (the
    wedged run) rather than 0 — which a counter that never moves also satisfied.

DeploymentWarmThenGateTests (:117 plus two sibling Task.Delay(500) startup waits)
  - The manager calls SetInstanceCounts unconditionally when its startup config load
    completes, so a report counter on the test collector is the startup edge. The
    empty-site zero is now the manager's own observation instead of an untouched field.

SiteAlarmLiveCacheServiceTests (:195, :216)
  - Both sampled IsLive once after a fixed sleep. Now they poll continuously across three
    linger windows on a new teardown observable (the mock client counts subscription
    cancellations), so a stop at any instant inside the window is caught, and each closes
    with a positive control proving that counter moves.

DataConnectionActorTests (:435)
  - A Task.Delay(200) "settle" after an AwaitCondition on a connect counter. The counter
    only proves ConnectAsync was CALLED; the actor is still Connecting until it processes
    the piped result. Now waits for the Connected health publication that BecomeConnected
    emits, so the subscribe under test provably runs against Connected rather than being
    silently stashed. (The stash path keeps its own coverage in
    WP6_ConnectingState_StashesSubscribeRequests.)

Negative controls (product broken in the working tree, old vs new test compared):
  - Stuck-script watchdog disabled entirely: OLD test passed 2/2, NEW test failed. The
    whole watchdog could be deleted and the old assertions were satisfied.
  - Startup reporting a phantom instance count and taking longer than 500 ms: OLD passed
    2/2, NEW failed (expected 0, actual 1).
  - Linger cancellation disabled + the remaining-viewer guard dropped, so the stream is
    torn down under a live viewer while IsLive keeps reporting true:
    Resubscribe_Within_Linger OLD passed 2/2, NEW failed. (Last_Viewer_Leaving caught this
    particular break in the old form too, via a later assertion — its strengthening is the
    teardown observable and the positive control, not a false-pass fix.)
  - DataConnectionActorTests :435 has no false-pass demo: the sleep guarded no assertion
    there, and the change is determinism plus proving the claimed state.

Also fixes a latent race these timing changes exposed in
InstanceActorChildAttributeRaceTests: it resolved child actors single-shot immediately
after construction, but ResolveOne throws until the child's cell is published on the
parent AND the cell's Actor property stays null until the child's constructor has run on
its own mailbox thread. Neither is ordered against the test thread, so it failed both ways
under full-suite load (~1 run in 6 once the sleeps above stopped padding the schedule;
14/14 green on the unmodified tree, so it was latent, not new). Resolution now retries to
a bounded deadline; the assertion is unchanged.

Verification: SiteRuntime.Tests 8/8 green (605 tests), Communication.Tests 3/3 (717),
DataConnectionLayer.Tests 3/3 (319). Test-only change — no src/ modifications.
2026-08-15 17:06:51 -04:00
Joseph Doherty 41a5ecf14a feat(external-systems): deploy per-system retry policy to sites + CLI/management surface (register row 27)
An external system's MaxRetries/RetryDelay existed on the central entity but never
reached a site and had no management surface, so every ExternalSystem.CachedCall
buffered with the S&F engine default (DefaultMaxRetries 50 x DefaultRetryInterval
30s ~= 25 min to park) whatever central had configured. The sibling
DatabaseConnectionArtifact had always carried its equivalents.

The consumption end was already wired: ExternalSystemClient.CachedCallAsync already
read system.MaxRetries/RetryDelay and passed them to EnqueueAsync with the 0-as-unset
convention. Only the transport and persistence were missing.

Commons
- ExternalSystemArtifact gains MaxRetries/RetryDelay as optional positional params
  (0 / TimeSpan.Zero = unset), matching DatabaseConnectionArtifact rather than going
  nullable, so it maps 1:1 onto the entity and the proto3 scalars.
- Create/UpdateExternalSystemCommand gain nullable MaxRetries/RetryDelay, following
  the existing TimeoutSeconds "omit to preserve" convention. Both additive.

Central
- ManagementActor create maps both; update preserves-if-null.
- ArtifactDeploymentService projects them onto the artifact.

Wire
- site_command.proto ExternalSystemArtifactDto gains max_retries=7 /
  retry_delay=8 (google.protobuf.Duration), mirroring DatabaseConnectionArtifactDto.
  Additive: a pre-existing site decodes them as 0/absent, which IS the unset value,
  so old behaviour is preserved. Vendored C# regenerated via docker/regen-proto.sh
  sitecommand; csproj Protobuf item re-commented, no message types added or removed.

Site
- external_systems gains max_retries / retry_delay_ms via AddColumnIfMissing,
  DEFAULT 0 = unset. Storage upsert, repository SELECT/mapping (new ordinals 6/7
  appended so existing ones are untouched), apply path, and the legacy migrator's
  explicit column list all carry them.

Replicated-table decision: external_systems IS in SiteLocalDbSetup.ReplicatedTables,
so this was checked against the AckTime/native_alarm_state precedent. Real columns
are correct here, not a JSON bag: the table has no general-purpose config column
(auth_configuration and method_definitions are both semantically specific),
timeout_seconds already set the additive-column precedent on this exact table, and
sibling database_connections stores the same two values as plain columns. Verified in
LocalDb source that RegisterReplicated re-reads PRAGMA table_info every boot and its
install script is DROP-then-CREATE, so capture triggers rebuild over the new column
set automatically, and SiteStorageSchema.Apply runs before registration on the same
boot. The only cost is the ReplicatedTable.ComputeDigest change, which the sync
handshake compares fail-closed - a mixed-version pair refuses to sync loudly instead
of diverging silently. Both-nodes-together is already the standing site-pair rule, so
no new operational constraint; recorded in topology-guide.md.

Also fixed: ArtifactDiff.CompareExternalSystem never compared MaxRetries/RetryDelay,
so a bundle import that changed a system's retry budget previewed as "Identical" -
harmless while the values were inert, a live behaviour change now.

No EF migration: the central ExternalSystemDefinitions columns already exist
(InitialSchema). Central UI already had both form fields.

Tests: site storage round-trip + unset-default, site apply path, central artifact
projection, proto round-trip (via the shared sample), management create/preserve,
CLI option parse + alias, and diff reporting. Fail-before verified on the two core
tests (expected 7 got 0 on the read path; expected 9 got 0 on the apply path).

Docs: Component-ExternalSystemGateway, Component-StoreAndForward, Component-CLI,
components/StoreAndForward, components/SiteRuntime, deployment/topology-guide,
CLI README (which was also missing --timeout-seconds entirely).
2026-08-15 12:56:12 -04:00
Joseph Doherty 482165cbac fix(siteruntime): startup reconcile starts the healed InstanceActor (register row 67)
SiteReconciliationActor's node-STARTUP fetch healed the config FILE but never the
RUNTIME. Instance Actors are created from exactly two paths -- the Deployment
Manager's boot-time SQLite load and an explicit deploy/enable command -- and the
reconcile pass runs AFTER that boot load has already read storage. So a config
reconcile restored started no actor: an enabled, successfully-deployed instance
whose config was missing at boot (lost volume, first boot after a restore,
rehomed data dir) stayed dead until the NEXT restart, whose load finally picked
it up. Proven live on site-a 2026-08-15: "Reconcile: fetched + stored config"
with no "InstanceActor started" after it, and a debug snapshot answering
instanceNotFound.

The pass now raises a fire-and-forget EnsureInstanceStartedCommand (new Commons
Deployment message) for every ENABLED instance it healed, routed through the
deployment-manager singleton PROXY so Instance Actors stay under the singleton no
matter which node's reconcile found the gap.

The handler is deliberately advisory:
- idempotent -- no-op when the actor is already running, which both nodes'
  reconcile actors legitimately trigger for the same instance;
- enabled-state-aware -- it re-reads the durable row, which is the authority on
  both the config body and enabled/disabled, so a disable landing after the
  fetch still wins and the instance stays stopped;
- it defers to any in-flight deploy/redeploy/warm for the instance, which keeps
  the deploy gate's ordering guarantees and the all-or-nothing-per-instance rule
  intact -- this never applies a config, it only starts one already persisted;
- a missing row is a quiet no-op, the normal case on an unreplicated site whose
  peer node healed its own file.

The heal is no longer silent: a successful reconcile start emits a Warning log
line and an operational `deployment` site event, because reaching it means the
node booted without a config for an instance that should have been running.
2026-08-15 12:43:25 -04:00
Joseph Doherty 9fb52153fd fix(test): order three more unsynchronized assertions behind the observables they follow
Deferred flake-pattern sweep of tests/ for the class fixed in c4caebe9 and
cfa6acbf — a bounded wait on observable A followed by a bare assert on an
observable B that the product only reaches strictly after A. Three clear
instances, each reproduced deterministically by delaying only the later step
and each re-verified green with that same delay still injected.

AlarmOnTriggerRuns_ShedAtTheSameCap_WithAnAlarmScopedSiteEvent gated on the
rate-limited shed site event and then asserted the shed COUNT bare.
AlarmActor.ShedAlarmRun increments the counter and only then emits the event,
and the event fires on the first shed only — so the gate observed Flap(4)'s
shed and ordered nothing with respect to Flap(5)'s, which is a separate
mailbox message with no observable of its own (an alarm on-trigger run has no
Ask caller to reply to, unlike ScriptActor.ShedRun, whose sibling test is
correctly ordered by its ScriptCallResult and is left alone). Deferring
Flap(5) by 2 s failed it with "Expected: 2 / Actual: 1". The count is now
ACCUMULATED across polls rather than re-read, because
SiteHealthCollector.CollectReport DRAINS the interval counters — a poll loop
that simply re-read it would consume the first shed and never reach 2.

EndToEnd_GrpcStubError_RowStays_Pending_NextTick_Succeeds gated on the
central row arriving and then asserted bare that the site SQLite row had left
Pending. SiteAuditTelemetryActor pushes via IngestAuditEventsAsync (which is
what writes the central row) and calls MarkForwardedAsync only after parsing
the ack. Delaying just that post-push step failed it with
"Assert.DoesNotContain() Failure: Filter matched in collection".

PreSnapshotBuffer_IsCapped_DropsOldest_AndCountsTheDrops gated on
"Count >= cap" and then asserted "Count == cap + 1" bare — a gate strictly
weaker than the assertion it guards, so it ordered nothing with respect to
the last event of a FlushBuffer loop that delivers one at a time. Parking
that loop after its 19,999th delivery failed it with
"Expected: 20001 / Actual: 20000".

Also hardens GrpcCentralTransportTests.WaitUntil, which returned silently on
timeout; today's single caller re-asserts immediately, so this only sharpens
the message rather than fixing a live flake.

Cleared with evidence, not guessed: SiteAlarmLiveCacheService's LingerStop
removes the site entry inside one lock, so IsLive and GetCurrentAlarms flip
atomically; and SiteReconciliationActor walks response.Gap with a sequential
foreach in which the asserted "Gone" log precedes the awaited "Good" row, the
inverse of this class.

Test-only; every ordering named above is correct as written.
2026-08-15 03:37:35 -04:00
Joseph Doherty ca30d17f94 fix(test): replace the SandboxTests wall-clock cancellation pins with a deterministic edge
Sandbox_LongRunningScript_TimesOut and Sandbox_InfiniteLoop_CancelledByToken
armed a CancellationTokenSource for a fixed 100 ms / 500 ms and hoped it
expired while the script body was still running. That is a race between a
fixed timer and a fixed amount of work, not a synchronization, and it is
wrong in both directions.

Too slow a timer relative to the work and the script FINISHES first, so
nothing is cancelled and ThrowsAnyAsync fails with "No exception was thrown".
Measured on this machine: the bounded 100M-iteration loop takes 298 ms
against the 100 ms pin — a 3x margin that a faster host closes, and that any
move off the scripting default OptimizationLevel.Debug would close outright.
Reproduced deterministically by shrinking the loop to 1M iterations: it ran
in 76 ms and the test failed with exactly that message while all 27 siblings
passed.

Too fast a timer and the token is already cancelled before the body is
entered — Roslyn's runner throws OperationCanceledException up front
(verified with a pre-cancelled token) — so the test goes GREEN without the
script's own in-loop ThrowIfCancellationRequested ever being reached. That
vacuous pass is the worse half: it asserts nothing while looking healthy.

Both now cancel deterministically. The script invokes an Action handed in
through Parameters from inside its own loop; when that call returns the token
is already cancelled ON THE SAME THREAD, so the next in-loop check is
guaranteed to observe it, with ~10,000 checks still ahead of it. No wall
clock, no host-speed or scheduler dependence — and the 600 ms of sleeping
goes away. Sandbox_UncancelledScript_RunsToCompletion is added as the
negative control: the same script with the signal wired to a no-op must run
every iteration and return the closed-form sum, which is what establishes
that the sibling's OCE is caused by the cancellation.

Verified: the injection that killed the old test passes with the fix, and
suppressing the cancellation entirely still fails it with the identical
message, so the claim is unchanged in force. Test-only; the sandbox's
cancellation behaviour is correct as written.
2026-08-15 03:37:18 -04:00
Joseph Doherty 950c54c5fc fix(runtime): review findings — recursion-safe run cap, atomic detach counter, summary edge cases, per-row event-log fallback 2026-08-14 23:42:29 -04:00
Joseph Doherty c254d0740e perf(sitelog): sampled per-run events; interval run summaries; site_events replication policy pinned
Implements WP3.2 stage (b) per docs/plans/2026-08-15-site-events-policy-design.md.

- Per-run instance-script Started/Completed Info site events are now off by
  default (SiteRuntimeOptions.PerRunScriptEvents=false) instead of firing on
  every run, closing the dominant site_events writer. Gated at the ScriptRunLauncher
  call sites (moved there from ScriptExecutionActor by WP3.1). Error-level events
  (timeout/failure/stuck-watchdog/recursion-limit) remain unconditional.
- ScriptRunSummaryRecorder accumulates per-(instance, script) run counters and a
  new site-only ScriptRunSummaryFlushService emits one aggregate "script" Info
  site event per ScriptRunSummaryIntervalSeconds (default 300s), top-50-script
  breakdown with an "others" rollup, zero-activity intervals emit nothing.
- Per-script opt-in via PerRunScriptEventScripts ("Instance/Script" exact or
  "Instance/*" wildcard), matched by the new pure ScriptRunEventPolicy. All three
  options are read from IOptionsMonitor<SiteRuntimeOptions> per run, so the
  policy is hot-togglable without a restart.
- Fixed the stale "event log is not replicated" comment at AkkaHostedService.cs
  (~905): site_events IS registered in SiteLocalDbSetup.ReplicatedTables — the
  singleton is what makes queries always hit the actively-written copy;
  replication is what gives the singleton history to read after a failover
  (memo Decision (b)). site_events replication itself is unchanged (still
  registered) and already pinned by
  tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbCdcRegistrationTests.cs.
- Updated Component-SiteEventLogging.md (Volume Policy section, corrected
  Storage/replication rationale) and Component-SiteRuntime.md (Script Run
  Launch + Error Handling sections).
2026-08-14 22:56:08 -04:00
Joseph Doherty c4fc1f8ecd perf(runtime): split trigger evals from the blocking pool; bounded, deadline-aware execution 2026-08-14 22:36:15 -04:00
Joseph Doherty 4cd1441984 Merge branch 'worktree-agent-aae48b78656e5a4e0' into arch-review-remediation
# Conflicts:
#	src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Repositories/SiteExternalSystemRepository.cs
2026-08-14 21:16:11 -04:00
Joseph Doherty a212283104 perf(misc): cached hot-path lookups, bounded observer queue, alarm-priority stream path
WP2.6 (arch-review remediation, cross-cutting misc):
- SiteExternalSystemRepository: name/ID-indexed ExternalSystemDefinitionCache replaces
  the fetch-all + reverse-map scan on every by-ID/method lookup; loaded once per
  redeploy, invalidated by DeploymentManagerActor after HandleDeployArtifacts applies
  external-system changes. Static JsonSerializerOptions for method-list parsing.
- Inbound API: short-TTL ApiMethodCache fronts the per-request ApiMethod repository
  fetch; invalidated by name via the existing ScriptArtifactChangeSubscriber/
  IScriptArtifactChangeBus pipeline, self-healing via TTL for changes the bus
  doesn't cover (e.g. Management API edits).
- StoreAndForward: the cached-call audit-observer queue — the one unbounded channel
  left in the system — is now bounded (ObserverQueueCapacity, default 10,000) with
  DropOldest overflow and a dropped-notification counter.
- SiteStreamManager: alarm state changes now travel a dedicated publish
  source/broadcast hub, isolated from the (far higher-volume) attribute path, so an
  attribute storm can no longer evict a pending alarm transition; the alarm hand-off
  queue is bounded with a drop counter surfaced on the site health report
  (SiteStreamAlarmDropCount via the new SiteStreamAlarmDropReporter), and publishing
  is skipped entirely at zero subscribers on either path.
- CLI ManagementHttpClient: explicit 30s HttpClient.Timeout on the shared
  construction (was the 100s framework default), overridable via
  SCADABRIDGE_HTTP_TIMEOUT_SECONDS.

Deviation: the failback-probe heartbeat item is NOT included — its only viable
surface (CentralChannelProvider.cs / heartbeat consumers) lives entirely in the
Communication project, explicitly off-limits to this work package this phase.

Tests: SiteRuntime.Tests (550), InboundAPI.Tests (278), StoreAndForward.Tests (133),
CLI.Tests (390), HealthMonitoring.Tests (97) — all green after full solution build.
2026-08-14 20:59:43 -04:00
Joseph Doherty 125055d998 perf(runtime): O(1) attribute resolution, precomputed types, coalesced static writes, shared JSON options 2026-08-14 19:57:26 -04:00
Joseph Doherty 71284adcc4 fix(scripts): memoize missing-assembly resolution on the site compile path 2026-08-12 16:38:04 -04:00
Joseph Doherty 5a781c706c fix(scripts): build Roslyn ScriptOptions once per process, not per compile
ScriptOptions.WithReferences(Assembly[]) resolves each assembly through
MetadataReference.CreateFromFile, which does not cache: every call mints a
fresh AssemblyMetadata -> PEReader -> NativeHeapMemoryBlock holding an
unmanaged copy of the assembly metadata that nothing disposes. Building the
options per compile therefore leaked native memory permanently — invisible to
the GC, to gcdump and to the managed allocation counters, so the working set
grew while the GC heap did not.

Diagnosed from a live dump of a wonder-app-vd03 Site node: 2,885 MB working
set 78 min after a cold start, only 150 MB live GC heap, ~2,469 MB on the
default process heap across ~6,700 undisposed AssemblyMetadata instances
against 473 DLLs on disk.

Three sites, all hoisted to static readonly:
- SiteRuntime ScriptCompilationService (the dumped one)
- InboundAPI InboundScriptExecutor — same defect on the central node; method
  compiles recur on every re-registration and revision change
- CentralUI ScriptAnalysisService — CreateFromFile per sandbox run

ScriptAnalysis RoslynScriptCompiler also builds options per call but draws
from the static ScriptTrustPolicy.DefaultReferences, so it mints no metadata
and is left alone.

Guarded by reference-equality on the artifact rather than by watching memory:
a bytes-watching test would be flaky, and the leak is native so the managed
counters cannot see it at all. The test is proven to fail before the fix.

This does NOT close the AddTemplateScript OOM — that path was shown twice not
to compile scripts. It explains how a long-running node reaches a native
memory state where a large allocation fails with gigabytes free, which is a
lead worth re-testing, not a closure.
2026-08-12 09:51:08 -04:00
Joseph Doherty d0af884760 feat(scripts): add the Alarms.CurrentAsync() read accessor for site scripts
MES alarm-status API §5.2 (docs/plans/2026-06-30-mes-alarm-status-api.md,
Phase 1 tasks 2-4). Site `Call` scripts had NO way to read alarm condition
state: the `Alarm` global exists only inside an on-trigger handler and
describes the one alarm that fired, and native mirrored conditions were
reachable only from the Debug View. That gap blocked the CvdReactor
SimpleAlarmStatus/AlarmStatus scripts entirely -- they cannot be written
without it. `Alarms.CurrentAsync()` closes it.

The data was already local: the script runs inside its own Instance Actor's
context, so this is a LOCAL Ask -- the same mechanism attribute reads use, no
cross-cluster hop. A dedicated GetAlarmSnapshotRequest/Response is used rather
than reusing DebugSnapshotRequest, which would materialise every attribute
value on every alarm poll; both are served from the same
BuildAlarmStatesSnapshot(), so the script view and the operator's Debug View
can never disagree.

Deliberate shape decisions:
  - NOT scope-prefixed, unlike Attributes. Alarm identity is not a
    scope-relative attribute name (computed alarms are keyed by configured
    name, native conditions by a source-supplied reference), so prefixing
    would hand a composed script a silently truncated list.
  - Read-only. Native alarms are a read-only mirror of the source (no
    ack-back), so no acknowledge/shelve operation is exposed.
  - Placeholder rows are NOT pre-filtered: a caller must be able to tell
    "binding configured and quiet" from "binding unknown". The documented
    filter is `Active && !IsConfiguredPlaceholder`.
  - ScriptAlarm lives in Commons so the runtime accessor and the compile-only
    surface project to the SAME type -- a script binding at the design-time
    gate binds identically at the site. Condition is the authority for
    active/acked/severity, so one filter expression works across computed and
    native alarms.

Mirrored on BOTH design-time surfaces. ScriptCompileSurface is covered by the
reflection parity guard (AlarmsAccessor added to its mirror pairs). The Central
UI Test-Run SandboxScriptHost is the third, hand-maintained mirror that the
parity test cannot reach (Central UI does not reference Site Runtime); without
it the design page would false-flag CS1061 on scripts the deploy gate accepts.
It throws a labelled ScriptSandboxException at run time -- there is no central
route to per-instance alarm state, and returning an empty list would read as
"nothing is in alarm", which is worse than an error.

ScriptTrustPolicy needs NO change, and the reason is structural rather than
incidental: the trust boundary is a deny-list over API roots, not an allow-list
of context members. A test pins that no ForbiddenScopes entry prefixes the
Commons script-surface namespace, so a future deny-list entry cannot silently
make ScriptAlarm untouchable.

Tests: 6 accessor cases (Ask contract, full native projection incl. AckTime,
unacked, computed-alarm derivation, placeholder visibility, scope-independence),
2 InstanceActor snapshot cases incl. equality with the Debug View row set, the
full MES script shape compiling against ScriptCompileSurface, 2 trust cases,
and a sandbox diagnose-clean case reading every projected ScriptAlarm field.
2026-08-01 13:12:30 -04:00