feat(cli+templateengine+deploymanager): resolve follow-ups #4/#5/#6/#8 — CLI ergonomics + structured deploy validation error
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).
This commit is contained in:
Joseph Doherty
@@ -1,9 +1,9 @@
|
||||
# Follow-up tracker — template-inheritance UI gaps + CLI/validation footguns (2026-06-24 session)
|
||||
|
||||
**Status:** PARTIALLY 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)
|
||||
**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` (2026-06-24). Open: #4, #5, #6, #8.
|
||||
**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.
|
||||
|
||||
@@ -56,7 +56,17 @@ Issues are listed worst-first. Severities are author estimates. None caused data
|
||||
---
|
||||
|
||||
## 4. CLI `instance set-bindings` cannot set `DataSourceReferenceOverride`
|
||||
**Severity:** Medium · **Components:** CLI (#19)
|
||||
**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.
|
||||
|
||||
@@ -67,7 +77,19 @@ Issues are listed worst-first. Severities are author estimates. None caused data
|
||||
---
|
||||
|
||||
## 5. CLI `template update` is full-replace, not partial
|
||||
**Severity:** Low · **Components:** CLI (#19), Template Engine (#1)
|
||||
**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.
|
||||
|
||||
@@ -76,7 +98,16 @@ Issues are listed worst-first. Severities are author estimates. None caused data
|
||||
---
|
||||
|
||||
## 6. (Minor) CLI `template list`/`get` table output dumps every attribute
|
||||
**Severity:** Low · **Components:** CLI (#19)
|
||||
**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.
|
||||
|
||||
@@ -102,7 +133,17 @@ Issues are listed worst-first. Severities are author estimates. None caused data
|
||||
---
|
||||
|
||||
## 8. Deploy-time unbound-binding validation returns one giant semicolon-joined error string
|
||||
**Severity:** Low · **Components:** Template Engine (#1), Deployment Manager (#2)
|
||||
**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.
|
||||
|
||||
|
||||
@@ -64,6 +64,19 @@ flowchart TD
|
||||
class step4 start
|
||||
```
|
||||
|
||||
### Validation Error Reporting
|
||||
|
||||
When step 2 fails, the returned error is a **grouped, capped summary** rather than a flat
|
||||
semicolon-joined dump (followup #8). `ValidationResult.SummarizeErrors()` (Commons) leads
|
||||
with the total error count, then lists one line per `ValidationCategory`; within a
|
||||
category, entity-scoped findings (notably the unbound connection-binding case, which can
|
||||
produce 50–194 entries for a richly data-sourced instance) are rolled up by **module** —
|
||||
the attribute's canonical name up to its last dot — with per-module counts, and the breadth
|
||||
is capped with a `… and N more module(s)` suffix. The complete per-entry list remains on
|
||||
`ValidationResult.Errors` and is written to the deploy log (`LogWarning`) so operators can
|
||||
still see every clause when needed. This keeps the UI/CLI failure toast scannable while
|
||||
preserving full detail for diagnosis.
|
||||
|
||||
## Deployment Identity & Idempotency
|
||||
|
||||
- Every deployment is assigned a unique **deployment ID** and includes the flattened configuration's **revision hash** (from the Template Engine).
|
||||
|
||||
Reference in New Issue
Block a user