Add StoreDeployedConfigIfNewerAsync to SiteStorageService — an atomic
conditional upsert using SQLite's ON CONFLICT … WHERE clause so the
standby node only overwrites a deployed_configurations row when the
incoming deployed_at is strictly newer. The active-node path
(StoreDeployedConfigAsync) stays unguarded. Four deterministic tests
cover: no-row insert, older-overwrites, newer-is-noop, equal-is-noop;
all seed rows with explicit DateTimeOffset values via direct SQL to
avoid wall-clock timing dependencies.
Akka.Actor.AskTimeoutException does not derive from System.TimeoutException,
so the isTimeout check in DeployInstanceAsync's catch block missed it and
routed it to the generic "Deployment error:" branch. This broke the
DeploymentManager-006 reconciliation query (query-before-redeploy), which
keys off the "Communication failure:" prefix to detect a prior timeout-induced
failure. Add AskTimeoutException to the pattern; add a covering regression test.
Add CommunicationService.RefreshDeploymentAsync — the typed send method
for the small notify-and-fetch wire message (RefreshDeploymentCommand).
Mirrors DeployInstanceAsync exactly: SiteEnvelope + Ask<DeploymentStatusResponse>
bounded by DeploymentTimeout. CentralCommunicationActor needs no change
(HandleSiteEnvelope is fully generic — all SiteEnvelope messages forward
to /user/site-communication without a per-type switch). Adds a parallel
routing test asserting the envelope reaches the site ClusterClient.
The templates tree rendered a derived/composed member (e.g. LeftReactorSide,
derived from ReactorSide) as a flat leaf, omitting compositions it inherits
from its base (e.g. LeakTest composed onto ReactorSide). BuildCompositionLeavesFor
recursed only over a template's OWN composition rows; an inherited composition
row lives on the ancestor, and TemplateComposition has no IsInherited placeholder
(unlike attributes/alarms/scripts/native-sources), so the child's own Compositions
was empty. Same 'derived templates don't surface inherited members' family as
followups #1/#2, but for compositions. Deploy/flatten was always correct
(TemplateResolver.ResolveAllMembers walks the chain) — display-only.
Fix:
- BuildCompositionLeavesFor now renders the EFFECTIVE composition set (own +
inherited) via EffectiveCompositionsFor, which walks the inheritance chain
(leaf->root, child wins on InstanceName), mirroring the resolver.
- Inherited slots are flagged (TemplateTreeNode.IsInherited), badged 'inherited'
in the label, and their context menu offers only 'Open composed template'
(Rename/Delete edit the ancestor's slot, so suppressed on inherited nodes).
- The same inherited row can appear under several derived members (LeakTest under
both LeftReactorSide and RightReactorSide), so composition nodes use a
path-qualified KeyOverride to keep TreeView selection/expansion keys unique;
recursion is cycle-guarded.
Tests: +1 bUnit (TemplatesPageTests.Renders_InheritedComposition_UnderDerivedComposedMember);
CentralUI suite 867 green; full solution builds 0/0.
Docs: Component-CentralUI.md (effective composition set in tree); known-issues
tracker #9 recorded + resolved.
Note: CentralUI change — shows on wonder-app-vd03 only after that host is redeployed.
Closes the four remaining items in the 2026-06-24 template-inheritance/CLI
follow-up tracker.
#4 — CLI `instance set-bindings` can now set DataSourceReferenceOverride.
`--bindings` accepts an optional 3rd element per entry:
[attributeName, dataConnectionId, dataSourceReferenceOverride]. A string
sets the override; a JSON null or an omitted 3rd element leaves it unset
(template default). TryParseBindings accepts 2- or 3-element entries and
rejects a non-string/non-null 3rd element or 4+ elements with a clean
error. Previously the CLI sent the override as null and silently wiped any
existing one (only a raw POST /management could set it).
#5 — `template update` is partial, not full-replace (fixed server-side so all
clients benefit). UpdateTemplateAsync now uses leave-unchanged semantics:
a null description keeps the stored value (pass "" to clear); a null
parentTemplateId keeps the existing parent. Parent stays immutable — a
non-null differing value is still rejected — but omitting --parent-id is
now a no-op instead of failing every derived-template update.
#6 — compact `template list`/`get` table output + `--detail`. Table output is
now id/name/description/parent/derived + member counts (#attrs/#alarms/
#scripts/#comps/#nativeAlarms) via TemplateTableProjection, fed through a
new optional tableProjector seam on CommandHelpers. `--detail` restores the
full dump. JSON output is left untouched (always full) so machine consumers
are unaffected — the projector only runs on the table path.
#8 — structured deploy-time validation error. New ValidationResult.SummarizeErrors()
(Commons) returns a grouped, capped summary: leading total count, one line
per ValidationCategory, and a per-module rollup (canonical name up to its
last dot) with counts + "... and N more module(s)" caps. DeploymentService
uses it for the "Pre-deployment validation failed" message and logs the full
per-entry list via LogWarning. Replaces the flat semicolon-joined dump that
became a wall of text for instances with 50-194 unbound attributes.
Tests: +8 Commons (SummarizeErrors), +8 CLI (4 binding 3-element / 4 table
projection), +2 net TemplateEngine (partial-update). Affected suites green:
Commons 587, CLI 341, TemplateEngine 447, DeploymentManager 101,
ManagementService 230, CentralUI 866; full solution builds 0/0.
Docs: Component-DeploymentManager.md "Validation Error Reporting"; CLI README
(set-bindings 3-element form, template update leave-unchanged, list/get
--detail); UpdateTemplateCommand doc; known-issues tracker #4/#5/#6/#8 resolved
(all 8 items now closed).
Derived templates store IsInherited placeholder rows mirroring inherited
members, but a base member added/changed/removed AFTER a child was derived
never reached the child — leaving the editor's editable tabs incomplete (#1)
and stored rows drifted from the resolved set (#2).
Fix (one order-independent reconcile, two entry points):
- Auto-propagation: every attribute/alarm/script add/update/delete now
reconciles the template's derived subtree (TemplateService.ReconcileDescendantsAsync),
hooked into all member-mutating paths incl. native-alarm-source CRUD in the
ManagementActor.
- Resync: ResyncInheritedMembersAsync repairs a template + its subtree on
demand — materialize missing placeholders, re-sync drifted ones, remove
orphans, across attributes/alarms/scripts/native sources. Exposed as
management ResyncInheritedMembersCommand (Designer-gated, audited) → CLI
`template resync-members` → a Resync button on the editor's staleness banner.
Reconcile drives off TemplateInheritanceResolver (same precedence + HiLo merge
as deploy), only ever touches IsInherited placeholders (never an authored
override), and matches the staleness comparison keys so the banner clears.
BuildDerivedTemplate now also materializes native-source placeholders at
compose time (previously omitted → any inherited native source was perpetually
stale).
Tests: +8 TemplateServiceTests (materialize / drift-update / orphan-remove /
override-untouched / base-cascade / multi-type / direct-propagate / end-to-end
add) + 1 ManagementService test fix (native-source add resolves TemplateService).
Affected suites green: TemplateEngine 446, ManagementService 230, CentralUI 866,
CLI 333, Transport 127, ConfigurationDatabase 307; full solution builds 0/0.
Docs: Component-TemplateEngine.md "Inherited-Member Propagation & Resync";
CLI README `template resync-members`; known-issues tracker #1/#2 resolved.
#3 — CollisionDetector counted a derived template's IsInherited placeholder
rows as a distinct origin from the parent members the inheritance walk
re-adds, reporting a spurious "Naming collision" for every inherited row and
blocking any attribute/composition add to a derived template. CollectDirectMembers
now skips IsInherited rows on the direct-template and inherited-parent walks;
it keeps them for the composed-module walk, where placeholders are the sole
representation of a derived module's inherited members (that walk does not
climb the composed template's parent chain).
#7 — SandboxAttributeAccessor (Central UI Test-Run host) omitted
WriteBatchAndWaitAsync / WaitAsync / WaitForAsync, so the editor false-flagged
valid instance scripts with CS1061 even though `template validate` and the
deploy gate accept them. Added the five overloads mirroring the runtime
AttributeAccessor; they throw a labelled ScriptSandboxException if run in
Test Run (the central sandbox has no device-batch / event-waiter transport).
Tests: +3 CollisionDetector unit + 1 end-to-end TemplateService (derived add
now succeeds); +2 ScriptAnalysisService diagnose-clean. Each new test verified
to fail without its fix with the exact user-facing symptom. Full suites green
(TemplateEngine.Tests 438, CentralUI.Tests 866).
Docs: Component-TemplateEngine.md (inherited-placeholder collision rule),
Component-ScriptAnalysis.md (third sandbox surface + its compile-clean guard),
known-issues tracker #3/#7 marked resolved and the minor note promoted to #8.
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.
Resolves InboundAPI-026/027/028/029 (+ newly-surfaced -030).
- 026: authorize the scoped Database helper in the design doc; SQL-injection
protection is parameter binding (values never concatenated); allow writes via
ExecuteAsync; drop the false 'read-only' claim. Named connections only.
- 027: async ADO.NET end-to-end (no .GetAwaiter().GetResult()); honour the method
deadline token on ExecuteScalarAsync/ExecuteReaderAsync/ExecuteNonQueryAsync +
a CommandTimeout backstop derived from the method timeout.
- 028: negative-path tests (null-gateway, deadline cancellation, parameterization)
+ e2e Database + WaitForAttribute cases through the real endpoint.
- 029: WaitForAttribute is bounded by its WAIT timeout (per-wait CTS + client-abort
+ explicit token), NOT the method deadline (spec §6) — a long wait may outlive the
method timeout; WithRequestAborted threads the raw client-abort token separately.
- 030: Central UI compile-surface mirrors (InboundScriptHost / SandboxInboundScriptHost)
gained the Database member (drifted since the runtime helper was added) so the
authorized async API type-checks at the design-time gate.
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.
Code-review finding UI-Med-2: the design doc + delivery adapter treat FromNumber and
MessagingServiceSid as either-or, but the entity ctor, EF schema, UI and CLI all hard-
required FromNumber — so a Messaging-Service-only Twilio config (a normal production
setup) could not be created. Bring the implementation into line with the spec:
- Commons: SmsConfiguration.FromNumber -> string? (ctor fromNumber optional);
UpdateSmsConfigCommand.FromNumber -> string?.
- ConfigurationDatabase: FromNumber.IsRequired(false) + migration SmsFromNumberOptional
(ALTER COLUMN nullable, idempotent; Down backfills '' — harmless, MsgSid keeps it
deliverable) + regenerated model snapshot.
- Transport: SmsConfigDto.FromNumber -> string? (round-trips a Messaging-Service-only config).
- CentralUI: form validation requires AccountSid + at-least-one-of(FromNumber, MsgSid);
nullable create/edit paths; From-number help text.
- CLI: --from-number no longer Required; BuildUpdateSmsConfigCommand validates the either-or.
- Adapter: From branch null-forgiving (guarded by the existing incomplete-config check).
Tests: ManagementActor MsgSid-only persists null FromNumber; CLI MsgSid-only builds +
neither-throws + contract (--from-number not Required); CentralUI MsgSid-only save.
Findings from the per-module code review of the SMS feature (code-reviews/):
- ManagementService (High): UpdateSmsConfig + UpdateSmtpConfig were Designer-gated
while both /notifications/{sms,smtp} pages enforce RequireAdmin — a Designer
blocked in the UI could still rotate a production credential via CLI. Moved both
to the Administrator arm so the actor gate matches the UI.
- ManagementService (Medium): UpdateSmsConfig treated --auth-token "" as a value,
silently clearing the stored Twilio token. Guard on IsNullOrWhiteSpace so empty ==
omitted (SMTP Credentials keeps its null-only guard — empty is valid for no-auth).
- CentralUI (Medium): NotificationLists recipient badge rendered Name <Email>
unconditionally, showing "Name <>" for SMS lists. Now type-aware (phone for SMS).
- ConfigurationDatabase (Medium): AddSmsNotifications.Down() backfilled NULL emails
to '' — silent data loss for SMS-only recipients. Added a pre-drop guard that
refuses rollback while such rows exist.
- NotificationOutbox (Low): SMS body truncation could split a surrogate pair at the
cap boundary; back off one code unit to stay well-formed.
- Commons (Low): NotificationRecipient public ctor name-guard now matches the
ForEmail factory (IsNullOrWhiteSpace). Documented SmsConfiguration.MaxRetries/
RetryDelay as RESERVED (dispatcher reuses the shared SMTP-derived retry policy).
Implement the central ConfigurationDatabase side of SMS notifications:
- NotificationConfiguration: EmailAddress now nullable (SMS-only recipients
carry a PhoneNumber, no email); add PhoneNumber nvarchar(32); add
SmsConfigurationConfiguration (AuthToken sized as the encrypted column,
mirroring SmtpConfiguration.Credentials; timeout/retry mapped REQUIRED for
ctor-default round-trip fidelity).
- ScadaBridgeDbContext: add SmsConfigurations DbSet, encrypt AuthToken at rest
via EncryptedStringConverter, and cover SmsConfiguration in the schema-only
secret-write guard.
- NotificationRepository: implement the four INotificationRepository SMS-config
methods (resolves the 4x CS0535), mirroring the SMTP methods' stage-only /
separate-SaveChangesAsync discipline.
- Migration AddSmsNotifications: idempotent (guarded) ALTER EmailAddress nullable,
ADD PhoneNumber, CREATE SmsConfigurations; Down reverses cleanly (backfills
NULL emails before restoring NOT NULL).
S1 made NotificationRecipient.EmailAddress nullable + added SmsConfiguration
and four INotificationRepository SMS methods, breaking compilation beyond the
intentionally-deferred central NotificationRepository.
Fix 1 (CS8620/CS8604 nullable EmailAddress projections, email-only paths):
- NotificationOutbox EmailNotificationDeliveryAdapter: filter non-null emails
- DeploymentManager ArtifactDeploymentService: filter non-null emails
- Transport EntitySerializer: filter non-null emails into NotificationRecipientDto
Fix 2 (CS0535): stub the four SMS-config methods on SiteRuntime
SiteNotificationRepository (central-only — NotSupportedException, matching the
existing 'Managed via artifact deployment from Central' write-path pattern).
Doc nits: reword NotificationRecipient private ctor and SmsConfiguration.AuthToken
comments.
The central ConfigurationDatabase.NotificationRepository compile break is left
as-is (S2 implements those four methods).
Pre-existing render-crash (introduced by eb4bce3e): a @* *@ comment between
attributes inside the site-health-trends <select> start tag rendered as an
invalid attribute name in a real browser (InvalidCharacterError on setAttribute),
tearing down the SignalR circuit whenever Site Health Trends renders. bUnit's
virtual DOM masked it; the live Playwright smoke (KpiTiles) caught it. Comment
moved outside the start tag.
- Root context menu now has tabindex/focus + Escape-key close (OnRootMenuKeyDown) mirroring the node menu
- Opening root menu calls _tree.DismissNodeContextMenu(); opening node menu fires OnNodeContextMenuOpened → DismissRootContextMenu so only one menu is ever visible
- Add FolderContextMenu_MoveUp_IsDisabled_OnFirstSibling and FolderContextMenu_MoveDown_IsDisabled_OnLastSibling bUnit tests
InjectAnalysisKind returns null on a null/empty trigger-config, so passing
--trigger-kind strict WITHOUT --trigger-config silently dropped the kind on
template script add/update and template alarm update. The CLI now detects that
combination (TriggerKindWillBeIgnored) and prints a clear warning to stderr,
then continues (warn-and-continue: the kind is advisory metadata, not a required
field, so the entity is still created — just without the requested analysis kind).
The --trigger-kind help text on all three commands now documents that it requires
--trigger-config, as does the CLI README.
This commit also carries the shared CLI command-builder file (Commands/TemplateCommands.cs)
and README, which the same builders edit for both #257 and the #54 flag additions —
the #54 message contracts/handler/UI/tests landed in the preceding commit.
- TriggerKindWillBeIgnored predicate + WarnIfTriggerKindIgnored stderr warning,
wired into script add/update and alarm update SetActions.
- Shared option descriptions document the --trigger-config requirement.
- Adds the #54 CLI flags (--min-time-between-runs, --execution-timeout-seconds) and
TryParseMinTimeBetweenRuns to the same builder file.
- Tests: TemplateTriggerKindIgnoredTests pins the warn predicate.
Joseph Doherty