Success now requires BOTH persistence commit AND an InstanceActorInitialized
readiness signal from the actor's PreStart — persistence can commit before the
actor's async init has run or failed, so persistence alone must not report
Success. An actor that dies during init never signals readiness; the Terminated
fallback fails that deployment and rolls back the optimistic state. The
persisted-row rollback is deferred until the store commits so it cannot race the
optimistic write.
Deviation from plan Task 15: the plan's swallow-only guard did not handle the
persistence-first ordering (empirically the store commits before the Terminated
signal, so Success was reported for a dead actor). Added the readiness handshake
to make the join deterministic.
Validation runs on the actor thread (not off-thread as the plan sketched): piping
the verdict back to self reorders concurrent deploys relative to each other and to
delete/disable, breaking the redeploy-supersede FIFO ordering (SR020/SR029). A deploy
is an infrequent admin command, so a brief synchronous Roslyn compile is acceptable.
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.
Fixes the 8 findings from the 2026-06-24 re-review (commit c42bb485), with a
regression test per Medium finding:
- DataConnectionLayer-029 (Med): HandleAlarmSubscribeCompleted now mirrors the
tag-path re-check — if a feed is already stored for the source, release the
redundant just-created subscription instead of overwriting + leaking the first
one (the double-subscribe window DCL-023 reopened). +regression test.
- InboundAPI-031 (Med): remove WaitForAttribute's local 5s grace backstop (tighter
than the CommunicationService Ask's timeout+IntegrationTimeout round-trip budget,
so a slow-but-valid timed-out 'false' got cancelled into a 500). Link only the
client-abort + explicit caller tokens; the lower layer owns the backstop. +test.
- SiteRuntime-032 (Med): derive the deployed count from an authoritative set of
deployed config names (HashSet) instead of a map-presence-gated int, so deleting
a DISABLED instance decrements correctly (SiteRuntime-029's gate leaked it).
+deploy->disable->delete regression test.
- StoreAndForward-028 (Med): reset _bufferedCount in StopAsync alongside the
register-guard so a same-instance Stop->Start re-seeds from a clean base (no ~2N
gauge double-count). +restart regression test.
- AuditLog-017 (Low): test the OnIngestAsync scope-resolution guard (actor survives,
replies empty, counts the failure) — no longer unpinned.
- CentralUI-037 / ScriptAnalysis-009 / SiteRuntime-033 (Low): doc-comment + spec
fixes (Database-throws in the inbound sandbox; baseReferences param wording;
native-alarm cap return-to-normal + per-condition NativeAlarmDropped eviction).
Targeted suites green: SiteRuntime 5, StoreAndForward 6, InboundAPI 31,
DataConnectionLayer 10, AuditLog 5, ScriptAnalysis 40, CentralUI ScriptAnalysis 52.
Remediation from the full per-module code review at 4307c381 (findings recorded
separately in code-reviews/).
Highs fixed:
- DeploymentManager-025/SiteRuntime-031: stop broadcasting notification lists + SMTP
configs (incl. credentials) to sites; site purges already-persisted rows on apply
(enforces the central-only delivery design; clears plaintext SMTP creds at rest).
- DataConnectionLayer-023: guard the native-alarm subscribe path against the
mid-flight-unsubscribe adapter-feed leak (mirrors the DCL-021 tag-path fix).
- SiteEventLogging-024: normalize From/To query bounds to UTC (the -016 fix the
audit trail claimed but never committed).
- KpiHistory-001: add an in-flight guard to the recorder sample tick.
- ScriptAnalysis-001: harden the trust analyzer's TPA-absent fallback (resolve
forbidden anchors in the minimal reference set; warn on degraded mode) — anchors
added to validation references only, never the compile gate.
(InboundAPI-026 left to the feat/ipsen-movein effort per owner decision.)
Medium/Low: DM-026 deterministic deploy-status tiebreaker; SR-027/028/029/030
native-alarm leak/phantom-active/delete-during-redeploy fixes; AL-013/014/016;
TE-024 (folder-mutation audit rows now persisted)/025; SF-025 gauge-provider
clear-on-stop; ESG-025/026; SEC-023/024/025; SCA-007/008/009; plus doc/test
accuracy COM-023/024, HOST-025/026, HM-024/025, NS-027/028.
Full-solution build 0 warnings; ~3560 tests across 18 touched suites green.
- Replace placeholder-loop comment with the double-render guard explanation
- Use _alarmTimestamps.GetValueOrDefault(binding, DateTimeOffset.MinValue) so the
placeholder timestamp is stable/idempotent across snapshot calls (was UtcNow)
- Add dcl.ExpectMsg<SubscribeAlarmsRequest>() drain in Snapshot_QuietNativeBinding_EmitsPlaceholder
and Snapshot_NativeBindingWithLiveCondition_NoPlaceholder to consume the DCL message
the NativeAlarmActor sends at startup
InstanceActor.BuildAlarmStatesSnapshot now adds an IsConfiguredPlaceholder
row per configured native source binding that currently has no live
condition, so the Debug View tree can show the binding node even when
quiet. A binding is "quiet" when no retained AlarmStateChanged carries its
NativeSourceCanonicalName (DV-1).
Kind derivation: reuses the exact nativeKind value already computed via
ResolveNativeKind(nativeSource.ConnectionName) at the NativeAlarmActor
creation site and stored in a new _nativeAlarmKinds dictionary -- the
accurate per-binding kind (NativeOpcUa vs NativeMxAccess), not the
NativeOpcUa default.
Tests: Snapshot_QuietNativeBinding_EmitsPlaceholder,
Snapshot_NativeBindingWithLiveCondition_NoPlaceholder.
Add two additive init-only fields to AlarmStateChanged so the Debug View can
nest live native conditions under their configured source-binding node:
- NativeSourceCanonicalName (binding canonical name, e.g. "Motor1.MotorAlarms")
- IsConfiguredPlaceholder (quiet-binding placeholder flag; default false)
Flow on BOTH cross-process paths:
- Live: proto AlarmStateUpdate fields 22/23 -> StreamRelayActor packs ->
SiteStreamGrpcClient unpacks (regenerated SiteStreamGrpc/Sitestream.cs).
- Snapshot (Newtonsoft): record defaults carry through; no special handling.
NativeAlarmActor.Emit now stamps NativeSourceCanonicalName = _source.CanonicalName.
Additive-only: no existing positional constructor or wire frame changed.
Tests: StreamRelayActorTests round-trips both fields pack->unpack;
NativeAlarmActorTests asserts the emitted event carries the binding canonical name.
A routed inbound-API call (Route.To(inst).Call(script)) runs the script on
the Site and returns its value to Central inside RouteToCallResponse, which
crosses the Central<->Site PROCESS boundary. A script's natural
'return new { ... }' is a compiler-generated anonymous type that Akka's
cross-process serializer cannot reconstruct on the receiving node, so the
reply was silently dropped and the caller's Route.To().Call() Ask timed out
at 30s with 'Script execution timed out' -- even though the script completed
and all device writes committed.
DeploymentManagerActor.RouteInboundApiCall now projects the routed return
value to a plain CLR graph (Dictionary/List/string/long/double/bool/null)
via a JSON round-trip before placing it in RouteToCallResponse. The graph
round-trips the wire and re-serializes to the same JSON shape the inbound
API expects for the HTTP body / ReturnDefinition validation.
Diagnosed live: IpsenMESMoveIn writes committed + site_events showed the
IpsenMoveIn script completed in ~0.6s, yet the inbound POST returned 500 at
30s; Central's Akka serializer logged 'Writing value of type
<>f__AnonymousType0`1 as Json' at the timeout moment.
379/379 SiteRuntime tests green.
- NJ-3: widen per-row catch to Exception (an STJ encode failure can't abort startup); drop dead null-guard already excluded by the SQL filter
- NJ-4: capture logger/instanceName in locals for the fire-and-forget normalize continuation (match the sibling pattern in this actor)
- NJ-5: emit a warn-log when a malformed List value is imported verbatim; thread an optional ILogger<BundleImporter> to the sync re-import site
Inject ISiteEventLogger into ScriptRuntimeContext (additive optional ctor
param, defaulted null, all existing callers source-compatible). Add a single
private EmitRecursionLimitEventAsync helper that fires-and-forgets a
"script"/Error site event; called at both recursion guard sites (CallScript
at ~:332 and ScriptCallHelper.CallShared at ~:499). ScriptExecutionActor
threads the already-resolved siteEventLogger singleton into the context;
AlarmExecutionActor leaves it null (no siteEventLogger wired there).
Existing _logger.LogError + throw behaviour unchanged.
Tests: RecursionLimitSiteEventTests — 5 tests covering both CallScript and
CallShared (ISiteEventLogger.LogEventAsync called once with category "script",
severity "Error"; null logger path does not throw).
Joseph Doherty