diff --git a/docs/plans/2026-09-10-management-api-cli-parity-review.md b/docs/plans/2026-09-10-management-api-cli-parity-review.md new file mode 100644 index 00000000..11af2f03 --- /dev/null +++ b/docs/plans/2026-09-10-management-api-cli-parity-review.md @@ -0,0 +1,165 @@ +# Management Actor, API, and CLI parity review + +**Date:** 2026-09-10 +**Reviewed revision:** `9ee402bb` +**Status:** Findings recorded; remediation not started. + +## Scope and method + +Source-level comparison of management command records, automatic HTTP command +registration, ManagementActor dispatch (including inherited record types), CLI +command construction and registration, and management audit REST endpoints. +No live mutations were performed and no tests were rerun for this review. + +The CLI's 19 registered command groups and previously passing 475-test suite do +not establish complete actor/API parity. The inventory found: + +| Inventory | Count | +|---|---:| +| Registered management command contracts | 151 | +| Contracts handled by ManagementActor, including inherited commands | 149 | +| Registered contracts referenced in CLI command source | 130 | +| Handled contracts without a direct CLI mapping | 19 | +| Registered contracts without an actor dispatch handler | 2 | + +The 130 figure measures source references, not independently verified end-to-end +behavior or complete option coverage. The four audit REST endpoints (query, +export, tree, and backfill-source-node) have CLI counterparts. The internal +deployment-configuration fetch endpoint is machine-to-machine infrastructure and +is not counted as a missing operator command. + +## Missing CLI operations + +All 19 commands below are handled by the actor and reachable through the generic +`POST /management` API, subject to server-side authorization. Each remains open; +the secured-write limitation is already documented and may be retained as an +explicit product decision rather than implemented. + +| ID | Capability | Missing command contracts | Status | +|---|---|---|---| +| GAP-01 | Shared-schema library (5) | `ListSharedSchemasCommand`, `GetSharedSchemaCommand`, `CreateSharedSchemaCommand`, `UpdateSharedSchemaCommand`, `DeleteSharedSchemaCommand` | Open | +| GAP-02 | Secured-write lifecycle (3) | `SubmitSecuredWriteCommand`, `ApproveSecuredWriteCommand`, `RejectSecuredWriteCommand` | Open; documented listing-only CLI scope | +| GAP-03 | Individual notification editing (3) | `RenameNotificationListCommand`, `AddNotificationRecipientCommand`, `DeleteNotificationRecipientCommand` | Open | +| GAP-04 | Provider configuration creation (2) | `CreateSmtpConfigCommand`, `CreateSmsConfigCommand` | Open | +| GAP-05 | Template inspection and editing (3) | `GetResolvedTemplateMembersCommand`, `MoveTemplateToFolderCommand`, `UpdateTemplateNativeAlarmSourceCommand` | Open | +| GAP-06 | Instance override management (2) | `DeleteInstanceAttributeOverrideCommand`, `ListInstanceNativeAlarmSourceOverridesCommand` | Open | +| GAP-07 | Data-connection relocation (1) | `MoveDataConnectionCommand` | Open | + +Evidence: + +- [ManagementActor dispatch](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs) +- [CLI root registration](../../src/ZB.MOM.WW.ScadaBridge.CLI/Program.cs) +- [CLI command implementations](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/) +- [Management command contracts](../../src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/) + +`CreateSmtpConfigCommand` and `CreateSmsConfigCommand` inherit their update +contracts and are dispatched through the corresponding update handlers. They +are not unhandled API contracts. The handlers distinguish the create subtype; +sending an ordinary update with ID zero is not equivalent. + +## Behavioral and contract findings + +Severity is a review estimate. These findings overlap the operation inventory; +do not add the row counts together as unique missing commands. + +### PAR-01 — Deployment deadlines differ between API and CLI + +**Severity:** Medium · **Status:** Open + +`deploy instance`, `instance deploy`, `deploy artifacts`, and +`site deploy-artifacts` use the default 30-second CLI request deadline. The API +classifies their contracts as long-running and allows five minutes by default. +The CLI can report a timeout while the deployment continues, encouraging a +retry of work that is still applying. `deploy site` already avoids a shorter +client deadline by allowing the server to own the overall time budget. + +**Recommended resolution:** Align these commands with the server-owned deadline +policy and add delayed-response regression coverage for every CLI alias. + +**Evidence:** [CommandHelpers](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs), +[DeployCommands](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DeployCommands.cs), +[InstanceCommands](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs), +[SiteCommands](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/SiteCommands.cs), +[ManagementEndpoints](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementEndpoints.cs). + +### PAR-02 — Notification replacement is not equivalent to individual editing + +**Severity:** Medium · **Status:** Open · **Related:** GAP-03 + +The existing CLI whole-list update reaches `HandleUpdateNotificationList`, which +deletes existing recipients and recreates them. It cannot faithfully replace +rename-only or individual-recipient editing while preserving recipient IDs and +names. The actor already provides dedicated commands for those operations. + +**Recommended resolution:** Expose rename and recipient add/delete commands and +verify unrelated recipients retain their identities and names. + +**Evidence:** [NotificationCommands](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs), +[ManagementActor.HandleUpdateNotificationList](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs). + +### PAR-03 — SMS retry and timeout options are missing + +**Severity:** Low · **Status:** Open + +The actor/API accept `ConnectionTimeoutSeconds`, `MaxRetries`, and `RetryDelay` +on `UpdateSmsConfigCommand`. CLI payload construction supplies none of these +fields, so scripts cannot configure them through `notification sms update`. +Omitted values preserve stored settings; this is a capability gap, not evidence +that normal CLI updates reset these settings. + +**Recommended resolution:** Add corresponding optional CLI flags and verify +explicit values and omission behavior against the API contract. + +**Evidence:** [CLI NotificationCommands](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs), +[notification contracts](../../src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/NotificationCommands.cs), +[ManagementActor.HandleUpdateSmsConfig](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs). + +### PAR-04 — Registry accepts two contracts the actor cannot dispatch + +**Severity:** Low · **Status:** Open + +The registry automatically includes every concrete `*Command` type in the +management namespace. `ReadTagValuesCommand` and `ResolveRolesCommand` therefore +resolve at the HTTP boundary but have no ManagementActor dispatch handler. + +- `ReadTagValuesCommand` is used by the UI's binding tester, which routes directly + to a site. There is no corresponding management handler or CLI operation. + Its existing contract lacks the site identifier needed for central routing. +- `ResolveRolesCommand` is intentionally retired: the HTTP endpoint authenticates + and resolves roles before dispatch. Restoring the old handler is not the fix. + +**Recommended resolution:** Make HTTP eligibility explicit. Exclude retired or +site-only contracts, or add a separately authorized, site-scoped management +operation where operator access is desired. Preserve the retired role-resolution +flow's security restriction. + +**Evidence:** [ManagementCommandRegistry](../../src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/ManagementCommandRegistry.cs), +[ManagementActor](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs), +[ReadTagValuesCommand](../../src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/ReadTagValuesCommand.cs), +[BindingTester](../../src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/BindingTester.cs). + +### PAR-05 — Tests do not enforce complete surface parity + +**Severity:** Low · **Status:** Open + +Command-tree tests verify existing groups and selected operations, but do not +require every supported actor command to map to a CLI command or an explicit +exemption. The current passing suite can coexist with the gaps above. + +**Recommended resolution:** Maintain a checked command-to-CLI mapping with +documented exemptions. Verify registry-to-dispatch coverage, account for record +inheritance, and test representative payload fields and deadline policies. + +**Evidence:** [CommandTreeTests](../../tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/CommandTreeTests.cs), +[ManagementCommandRegistryTests](../../tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Messages/ManagementCommandRegistryTests.cs). + +## Suggested order + +1. Fix deployment deadline parity and expose non-replacing notification edits. +2. Add the missing schema, configuration, template, instance, and connection operations. +3. Decide whether secured-write CLI scope should expand; retain an explicit exemption if not. +4. Correct HTTP registry eligibility and add SMS options. +5. Add a parity gate so subsequent actor/API additions require CLI support or a recorded exemption. + +This is a bounded surface comparison, not an exhaustive security audit or a +field-by-field review of all 130 CLI-referenced contracts.