ScadaBridge/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md

# Follow-up tracker — template-inheritance UI gaps + CLI/validation footguns (2026-06-24 session)

**Status:** RESOLVED · **Found:** 2026-06-24 · **Context:** live ops session on `wonder-app-vd03` (CvdReactor / Z28061 / Z28061Sim) — renaming the template, adding the LeakTest module, and adding MoveInType to the MESReceiver children.
**Components:** Central UI (#9), Template Engine (#1), CLI (#19), Configuration Database (#17), Deployment Manager (#2)

**Resolved:** #3 (collision detector) and #7 (sandbox compile surface) on branch `fix/followups-3-7`; #1 + #2 (inherited-member propagation & resync) on branch `fix/followups-1-2`; #4 + #5 + #6 + #8 (CLI ergonomics + structured deploy validation error) on branch `fix/followups-4-5-6-8` (all 2026-06-24). All items resolved.

Issues are listed worst-first. Severities are author estimates. None caused data loss; the runtime/flattened config and deployed instances are correct.

---

## 1. Template editor omits inherited-but-unmaterialized base attributes (user-reported)
**Severity:** Medium · **Components:** Central UI (#9), Template Engine (#1) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-1-2`)**

**Fix:** shared root cause with #2 — see #2's fix. Once a template's inherited rows are materialized (auto-propagation going forward, or the Resync action for already-stale templates), the editor's editable Attributes/Alarms/Scripts tabs list them. The "base changed" banner is now actionable: it carries a **Resync inherited members** button (`TemplateEdit.razor`) that calls `TemplateService.ResyncInheritedMembersAsync` and reloads. The read-only "Effective inherited set" preview is retained.


**Symptom:** On `/design/templates` → `LeftMESReceiver`, the **Attributes** tab does not list `MoveInType`. Same for `RightMESReceiver`. (Also missing from the list: all `MoveOut*` and `ScanStateCmd`.)

**Root cause:** `MoveInType` (String), the 12 `MoveOut*`, and `ScanStateCmd` live on the **base** `MESReceiver` template (id 3, 26 attrs). The derived children `LeftMESReceiver`/`RightMESReceiver` (ids 5/6) only have **12** materialized `IsInherited` rows (the `MoveIn*` basics) — the base attributes added *after* these children were derived were never materialized as child rows. The editor's main Attributes tab lists only the template's stored rows (`TemplateEdit.razor:187` → `GetAttributesByTemplateIdAsync(Id)`; count badge at `:415`), so the unmaterialized inherited members are invisible there.

**Not a runtime bug:** the flattener resolves the full chain, so `LeftMESReceiver.MoveInType` *is* in the flattened deploy config and is bound + live on the instances (`''` / `'na'`, Good). The page also has an "Effective inherited set" read-only preview (M9-T26b, `TemplateEdit.razor:465-491`) + a staleness banner (`:292-301`) that *do* surface these — but the main, editable Attributes list does not.

**Suggested fix:** in the Attributes tab, render the resolved inherited set (with inherited badges) rather than only materialized rows — or auto-materialize missing inherited rows on load. See #2 (shared root cause).

---

## 2. Derived templates carry incomplete/stale `IsInherited` row sets
**Severity:** Medium · **Components:** Template Engine (#1), Configuration Database (#17) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-1-2`)**

**Fix:** root-cause fix — derived templates' stored inherited rows are now kept in sync two ways. (1) **Auto-propagation:** adding/updating/removing a member on a template now reconciles its entire derived subtree (`TemplateService.ReconcileDescendantsAsync`, called from every member-mutating path incl. native-alarm-source CRUD in the ManagementActor). (2) **Resync:** `ResyncInheritedMembersAsync` (CLI `template resync-members`, management `ResyncInheritedMembersCommand`, Designer-gated, audited; UI banner button) repairs a template + its subtree on demand — materializing missing placeholders, re-syncing drifted ones, removing orphans, across attributes/alarms/scripts/native sources. `BuildDerivedTemplate` also now materializes native-source placeholders at compose time (previously omitted, which made any inherited native source perpetually stale). Authored overrides are never touched. Covered by `TemplateServiceTests` (materialize / drift-update / orphan-remove / override-untouched / base-cascade / multi-type / propagation / end-to-end add). Documented in `Component-TemplateEngine.md` → "Inherited-Member Propagation & Resync".


**Symptom:** `LeftMESReceiver`/`RightMESReceiver` (parent=3) have 12 stored attribute rows vs the base's 26. By contrast `LeftReactorSide`/`RightReactorSide` (parent=7) mirror the full 61. So derived row-sets are inconsistent.

**Root cause:** when a base attribute is added after a child is derived, the child's stored `IsInherited` placeholder rows are not auto-resynced. There is a staleness banner but no surfaced one-click "resync inherited members" action (or it exists and was never run for these children). This is the data root cause of #1.

**Suggested fix:** a "resync inherited members" command (CLI + UI) that materializes missing base members onto derived children; consider running it automatically when a base attribute is added.

---

## 3. Collision detector blocks adding attributes/compositions to ANY derived template
**Severity:** Medium-High · **Components:** Template Engine (#1) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-3-7`)**

**Fix:** `CollectDirectMembers` now takes a `skipInherited` flag and skips `IsInherited` placeholder rows for the direct-template and inherited-parent walks (where the inheritance walk already re-adds those members under the parent's origin), while keeping them for the composed-module walk (the sole representation of a derived module's inherited members). Covered by `CollisionDetectorTests` (`DerivedTemplateWithInheritedPlaceholders_NoFalseCollision`, `MultiLevelInheritedPlaceholders_NoFalseCollision`, `DerivedTemplate_GenuineCollisionStillDetected_DespiteInheritedPlaceholder`) and the end-to-end `TemplateServiceTests.AddAttribute_ToDerivedTemplateWithInheritedPlaceholders_Succeeds`. Documented in `Component-TemplateEngine.md` → Naming Collision Detection.


**Symptom:** `template attribute add --template-id 5 --name MoveInType ...` fails with **13** "Naming collision" errors — the new attribute *plus all 12 pre-existing inherited rows*. Same class of failure when adding a composition to a derived template (hit earlier when trying to add `LeakTest` to `LeftReactorSide`).

**Root cause:** `CollisionDetector.CollectDirectMembers` (`CollisionDetector.cs`) emits every `template.Attributes` row — including `IsInherited` placeholders — with origin = child name, while `CollectInheritedMembers` emits the same names with origin = `parent '…'`. Two distinct origins for the same canonical name ⇒ reported as a collision. `AddAttributeAsync` (`TemplateService.cs:294`) and `AddCompositionAsync` (`:869`) both run `DetectCollisions`, so there is effectively **no supported API path to extend a derived template** — and the error message is misleading (it blames pre-existing inherited attrs, not the member you're adding).

**Workaround used this session:** add the feature module to the **base** template instead (LeakTest → base `ReactorSide` (7)); the flattener propagates it to all derivations.

**Suggested fix:** `CollectDirectMembers` should skip `IsInherited` rows (or the grouping should treat an inherited row and its parent source as the same origin), so only genuine cross-origin duplicates are flagged.

---

## 4. CLI `instance set-bindings` cannot set `DataSourceReferenceOverride`
**Severity:** Medium · **Components:** CLI (#19) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)**

**Fix:** `--bindings` now accepts an optional **third element** per entry —
`[attributeName, dataConnectionId, dataSourceReferenceOverride]` — so the CLI can set the
per-instance reference override that the wire contract (`ConnectionBinding`) already
carried. A string sets it; a JSON `null` or an omitted third element leaves it unset
(template default). `TryParseBindings` accepts 2- or 3-element entries and rejects a
non-string/non-null third element or 4+ elements with a clean validation error. The
`--bindings` help and CLI README now document the full-replace behaviour (omitting the
override on a re-bind clears any previously-set one). Covered by
`InstanceArgumentParsingTests` (three-element / explicit-null / wrong-type / four-element).

**Symptom:** `instance set-bindings --bindings` only accepts `[attributeName, dataConnectionId]` pairs (`InstanceCommands.cs` → `ConnectionBinding(name, connId)` 2-arg). The override is sent as `null`, and because `SetConnectionBindingsAsync` upserts `DataSourceReferenceOverride = b.DataSourceReferenceOverride` (`InstanceService.cs:340`), using the CLI on an attribute that already has an override would **wipe** it.

**Workaround used this session:** raw `POST /management` with `{"command":"SetConnectionBindings","payload":{...,"dataSourceReferenceOverride":"…"}}` — the wire contract `ConnectionBinding(AttributeName, DataConnectionId, DataSourceReferenceOverride?)` does carry the field; only the CLI omits it.

**Suggested fix:** add an optional 3rd element / `--ref-override` to the CLI bindings input.

---

## 5. CLI `template update` is full-replace, not partial
**Severity:** Low · **Components:** CLI (#19), Template Engine (#1) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)**

**Fix:** `TemplateService.UpdateTemplateAsync` now uses **leave-unchanged** semantics for
optional fields (fixed server-side, so every client benefits): a `null` description keeps
the stored value (pass `""` to explicitly clear it), and a `null` `parentTemplateId` keeps
the existing parent. The parent remains immutable — a non-null value that differs from the
current parent is still rejected — but omitting it (the CLI default) is now a no-op instead
of tripping the immutability guard, which previously made `template update` fail on any
derived template unless `--parent-id` was re-passed. CLI `--description`/`--parent-id` help,
the `UpdateTemplateCommand` doc, and the CLI README document the semantics. Tests:
`UpdateTemplate_OmittedParentAndDescription_LeavesUnchanged`,
`UpdateTemplate_EmptyDescription_ClearsIt` (the prior `UpdateTemplate_ClearParent_Fails`
was repurposed, since a null parent now means leave-unchanged rather than clear-and-fail).

**Symptom:** omitting `--description` on `template update` overwrites the stored description to NULL (`TemplateService.cs:124-125` assigns Name+Description unconditionally). Renaming a template silently drops its description unless you re-pass it.

**Suggested fix:** treat omitted optional fields as "leave unchanged" (nullable-not-provided vs explicit-null), or warn when a non-empty description would be cleared.

---

## 6. (Minor) CLI `template list`/`get` table output dumps every attribute
**Severity:** Low · **Components:** CLI (#19) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)**

**Fix:** `template list`/`get` **table** output is now a compact projection — id / name /
description / parentTemplateId / isDerived plus member **counts** (`#attrs`, `#alarms`,
`#scripts`, `#comps`, `#nativeAlarms`) — via a new `TemplateTableProjection.ProjectSummary`
fed through an optional `tableProjector` seam on `CommandHelpers.ExecuteCommandAsync`/
`HandleResponse`. A `--detail` flag restores the full table dump. **JSON output is
deliberately left untouched** (always the full payload) so machine consumers are unaffected
— the projector only runs on the table path. Covered by `TemplateTableProjectionTests`
(array/object projection, counts, non-JSON passthrough, size-shrink sanity check).

**Symptom:** `--format table template list` emitted ~171 KB (the full attribute set per template inline), unusable in a terminal. `--format json` is fine.

**Suggested fix:** a compact table projection (id/name/desc/#attrs/#comps) for list/get; reserve full attribute dumps for an explicit `--verbose`/`--detail` flag.

---

## 7. Central UI script editor false-flags batch/wait helpers (sandbox compile surface out of sync)
**Severity:** Medium · **Components:** Central UI (#9), Script Analysis (#25) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-3-7`)**

**Fix:** `SandboxAttributeAccessor` now mirrors the runtime `AttributeAccessor` — added `WriteBatchAndWaitAsync`, both `WaitAsync` overloads, and both `WaitForAsync` overloads with matching signatures. They throw a clearly-labelled `ScriptSandboxException` if exercised in Test Run (the central sandbox has no device-batch/event-waiter transport), but they now resolve at compile time so the editor stops false-flagging valid scripts. A reflection parity test isn't feasible across the CentralUI→SiteRuntime boundary (Central UI does not reference Site Runtime by design), so the guard is representative-script "diagnose clean" tests in `ScriptAnalysisServiceTests` (`InstanceScript_BatchAndWaitHelpers_DiagnoseClean`, `ChildInstanceScript_WriteBatchAndWait_DiagnoseClean`), consistent with how the inbound `Database`/`WaitForAttribute` and `Notify` surfaces are guarded. Documented in `Component-ScriptAnalysis.md` → Parity guard.


**Symptom:** In the template script editor (`/design/templates/{id}` → Scripts → Edit → Code), a script that calls `Attributes.WriteBatchAndWaitAsync(...)` (or on a child, `Children["X"].Attributes.WriteBatchAndWaitAsync(...)`) shows a red compile error:
`'SandboxAttributeAccessor' does not contain a definition for 'WriteBatchAndWaitAsync' ... (CS1061)`. Confirmed on `CvdReactor.MesMoveIn`; the same false error hits the base `MESReceiver.MoveIn`/`MoveOut`, which also use the helper.

**Root cause:** the editor validates against the Central UI's own sandbox surface (`CentralUI/ScriptAnalysis/SandboxScriptHost.cs`). Its `SandboxAttributeAccessor` only defines `this[string]`, `GetAsync`, `SetAsync`, `Resolve` — it is **missing `WriteBatchAndWaitAsync`, `WaitAsync`, and `WaitForAsync`** (none are defined anywhere in the CentralUI sandbox surface). The real runtime `AttributeAccessor` (`SiteRuntime/Scripts/ScopeAccessors.cs`) and the deploy-gate `ScriptCompileSurface` (`ScriptAnalysis`) both define them — so `template validate` reports the script clean and it deploys/runs fine. The error is purely the in-editor validator.

**Impact:** misleads authors and can block saving from the UI for any script using the batch-write/wait helpers, even though the script is valid. Authoring such scripts currently has to go through the management API (as was done for `MesMoveIn`).

**Suggested fix:** bring `SandboxAttributeAccessor` (and the sandbox composition/children accessors) to parity with the runtime `AttributeAccessor` — add `WriteBatchAndWaitAsync` / `WaitAsync` / `WaitForAsync` with matching signatures. Ideally enforce surface parity with a test, like the `RoslynScriptCompiler` "representative real script" corpus already does for `ScriptCompileSurface`. Three surfaces (runtime, deploy-gate `ScriptCompileSurface`, UI `SandboxScriptHost`) currently drift independently — consider collapsing to one source of truth.

---

## 8. Deploy-time unbound-binding validation returns one giant semicolon-joined error string
**Severity:** Low · **Components:** Template Engine (#1), Deployment Manager (#2) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)**

**Fix:** new `ValidationResult.SummarizeErrors()` (Commons) returns a grouped, capped
summary: a leading total count, one line per `ValidationCategory`, and within a category a
per-**module** rollup (canonical name up to its last dot) with counts and a `… and N more
module(s)` cap. `DeploymentService` now uses it for the `Pre-deployment validation failed:`
message and logs the full per-entry list via `LogWarning` so nothing is lost. Entity-less
findings (e.g. script-compile errors) fall back to a capped message list. Documented in
`Component-DeploymentManager.md` → "Validation Error Reporting". Covered by
`ValidationResultSummaryTests` (count header, module rollup, breadth cap, root grouping,
message fallback, mixed categories).

**Symptom:** Deploying an instance whose data-sourced attributes aren't all bound fails with a single error that concatenates one clause per attribute: `Pre-deployment validation failed: Attribute 'LeftReactorSide.LeakTest.DeltaVac' has a data source reference but no connection binding; Attribute 'LeftReactorSide.LeakTest.ResultType' has …; …`. For 50–194 unbound attrs (e.g. Z28062's unbound LeakTest members) it's a wall of text that's hard to scan in a CLI/UI toast.

**Root cause:** `ValidateConnectionBindingCompleteness` emits one clause per unbound attribute and joins them into a flat string; there is no grouping or count.

**Suggested fix:** return a structured/summarized error — leading count (`52 attributes are unbound`) + grouped-by-module breakdown (or a capped list with "…and N more") — instead of the flat semicolon-joined dump. Keep the full list available in a detail/expandable view or the deploy log.