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) and #9 (inherited compositions in the templates tree) 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.

---

## 9. Templates tree omits inherited compositions under derived composed-members (user-reported)
**Severity:** Low-Medium · **Components:** Central UI (#9) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)**

**Fix:** `Templates.razor` → `BuildCompositionLeavesFor` now renders the **effective** composition set (own + inherited) via a new `EffectiveCompositionsFor` that walks the inheritance chain (leaf→root, child wins on `InstanceName`), mirroring `TemplateResolver.ResolveAllMembers`. 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 they're suppressed on inherited nodes). Because the same inherited row can now appear under several derived members (LeakTest under both LeftReactorSide and RightReactorSide), composition nodes use a path-qualified `KeyOverride` (`t:{owner}/c:{id}/…`) so TreeView selection/expansion keys stay unique; the recursion is cycle-guarded. Covered by `TemplatesPageTests.Renders_InheritedComposition_UnderDerivedComposedMember`.

**Symptom:** On `/design/templates`, the base `ReactorSide` node expands to show its composed `↳ LeakTest`. But under `CvdReactor`, the composed members `LeftReactorSide` / `RightReactorSide` (derived from `ReactorSide`) render as **flat leaves with no LeakTest** — even though they inherit it. Observed live on `wonder-app-vd03`.

**Root cause:** `BuildCompositionLeavesFor(owner)` recursed only over `owner.Compositions` (the template's **own** rows). A derived template's inherited composition row lives on its ancestor, and `TemplateComposition` has no `IsInherited` placeholder (unlike attributes/alarms/scripts/native-sources, which `BuildDerivedTemplate`/the #1/#2 reconcile materialize) — so the derived child's own `Compositions` is empty and the recursion found nothing. Same "derived templates don't surface inherited members" family as #1/#2, but for compositions, which the #1/#2 fix did not cover. Deploy/flatten was always correct (`TemplateResolver.ResolveAllMembers` walks the chain), so this was display-only.

**Note:** this is a Central UI change — it shows on `wonder-app-vd03` only after that host is redeployed with the new build.