dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

Complete management API and CLI command parity

Browse Source
This commit is contained in:
Joseph Doherty
2026-09-10 00:51:45 -04:00
parent 477e2abe97
commit 2750bd2809
38 changed files with 1637 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/2026-09-10-management-api-cli-parity-review.md
+95 -21
View File
@@ -2,17 +2,17 @@
**Date:** 2026-09-10
**Reviewed revision:** `9ee402bb`
**Status:** Findings recorded; remediation not started.
**Status:** Remediation in progress on `codex/management-cli-parity`.
## Scope and method
## Original review 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:
At the reviewed revision, 19 registered command groups and the passing 475-test
suite did not establish complete actor/API parity. The baseline inventory found:
| Inventory | Count |
|---|---:|
@@ -31,24 +31,24 @@ 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.
`POST /management` API, subject to server-side authorization. Remediation adds
all 19, including the full secured-write lifecycle. The descriptions below retain
the original findings; status cells and completion evidence track the fixes.
| 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 |
| GAP-01 | Shared-schema library (5) | `ListSharedSchemasCommand`, `GetSharedSchemaCommand`, `CreateSharedSchemaCommand`, `UpdateSharedSchemaCommand`, `DeleteSharedSchemaCommand` | Completed |
| GAP-02 | Secured-write lifecycle (3) | `SubmitSecuredWriteCommand`, `ApproveSecuredWriteCommand`, `RejectSecuredWriteCommand` | Completed; full lifecycle exposed |
| GAP-03 | Individual notification editing (3) | `RenameNotificationListCommand`, `AddNotificationRecipientCommand`, `DeleteNotificationRecipientCommand` | Completed |
| GAP-04 | Provider configuration creation (2) | `CreateSmtpConfigCommand`, `CreateSmsConfigCommand` | Completed |
| GAP-05 | Template inspection and editing (3) | `GetResolvedTemplateMembersCommand`, `MoveTemplateToFolderCommand`, `UpdateTemplateNativeAlarmSourceCommand` | Completed |
| GAP-06 | Instance override management (2) | `DeleteInstanceAttributeOverrideCommand`, `ListInstanceNativeAlarmSourceOverridesCommand` | Completed |
| GAP-07 | Data-connection relocation (1) | `MoveDataConnectionCommand` | Completed |
Evidence:
- [ManagementActor dispatch](../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs)
- [CLI root registration](../../src/ZB.MOM.WW.ScadaBridge.CLI/Program.cs)
- [CLI root registration](../../src/ZB.MOM.WW.ScadaBridge.CLI/CliApplication.cs)
- [CLI command implementations](../../src/ZB.MOM.WW.ScadaBridge.CLI/Commands/)
- [Management command contracts](../../src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/)
@@ -57,14 +57,14 @@ 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
## Original 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
**Severity:** Medium · **Status:** Completed
`deploy instance`, `instance deploy`, `deploy artifacts`, and
`site deploy-artifacts` use the default 30-second CLI request deadline. The API
@@ -84,7 +84,7 @@ policy and add delayed-response regression coverage for every CLI alias.
### PAR-02 — Notification replacement is not equivalent to individual editing
**Severity:** Medium · **Status:** Open · **Related:** GAP-03
**Severity:** Medium · **Status:** Completed · **Related:** GAP-03
The existing CLI whole-list update reaches `HandleUpdateNotificationList`, which
deletes existing recipients and recreates them. It cannot faithfully replace
@@ -99,7 +99,7 @@ verify unrelated recipients retain their identities and names.
### PAR-03 — SMS retry and timeout options are missing
**Severity:** Low · **Status:** Open
**Severity:** Low · **Status:** Completed
The actor/API accept `ConnectionTimeoutSeconds`, `MaxRetries`, and `RetryDelay`
on `UpdateSmsConfigCommand`. CLI payload construction supplies none of these
@@ -116,7 +116,7 @@ explicit values and omission behavior against the API contract.
### PAR-04 — Registry accepts two contracts the actor cannot dispatch
**Severity:** Low · **Status:** Open
**Severity:** Low · **Status:** Completed
The registry automatically includes every concrete `*Command` type in the
management namespace. `ReadTagValuesCommand` and `ResolveRolesCommand` therefore
@@ -140,7 +140,7 @@ flow's security restriction.
### PAR-05 — Tests do not enforce complete surface parity
**Severity:** Low · **Status:** Open
**Severity:** Low · **Status:** Completed
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
@@ -163,3 +163,77 @@ inheritance, and test representative payload fields and deadline policies.
This is a bounded surface comparison, not an exhaustive security audit or a
field-by-field review of all 130 CLI-referenced contracts.
## Implementation plan and progress
The requested remediation implements all 19 CLI mappings, including secured-write
submit/approve/reject through existing server authorization. The HTTP registry
will exclude site-only ReadTagValues and retired ResolveRoles contracts; their
internal types remain intact. Existing update semantics remain compatible.
| Work package | Files / scope | Verification | Status |
|---|---|---|---|
| Notifications | CLI NotificationCommands; focused notification tests; GAP-03/04, PAR-02/03 | Capture real command payloads, preserve named recipients, create and optional SMS settings | Completed |
| Template/instance/connection commands | CLI TemplateCommands, InstanceCommands, DataConnectionCommands; GAP-05/06/07 | Parse and HTTP-envelope regressions for every added operation | Completed |
| Schema and secured writes | New SharedSchemaCommands, SecuredWriteCommands; GAP-01/02 | Real command payloads, role enforcement remains server-side | Completed |
| Deadline and registry policy | CommandHelpers, Commons registry, API tests; PAR-01/04 | Delayed responses for all deploy aliases; rejected HTTP-ineligible contracts | Completed |
| Parity gate and integration | CLI root, command mapping tests, CLI docs; PAR-05 | Exhaustive registry/dispatch/CLI mapping with explicit internal exclusions | Completed |
| Completion | Full solution build/tests, local deployment/read-only smoke, report updates, merge/commit/push | Fresh verification evidence recorded below | In progress |
Each package follows failing regression tests, minimal implementation, focused
verification, and code review. Builds/tests use a shared lock in this worktree to
avoid concurrent dotnet output races. The final merge and git operations are
owned by the primary agent.
### Progress checkpoint
- Notification/Smtp/Sms focused suite: 52 passed, including 19 new parse/HTTP cases.
- Editing parity: 15 passed, covering all six added operations and an existing alias.
- Deployment deadline regression: all four aliases now accept the server's response
after 31 seconds; the same test failed with the old 30-second client deadline.
- HTTP registry regressions: 11 passed; ReadTagValues and ResolveRoles no longer resolve.
- Production-root parity probe: all 149 HTTP-eligible contracts actually dispatched
to a loopback recorder. Removing the schema group produces missing coverage.
- Independent review approved the editing and notification mappings. Additional
actor preservation coverage and secured-write execution-failure exit handling
are being verified before final completion.
### Independent review follow-ups
- Added email and SMS actor-dispatch preservation tests: rename, add, and delete
leave the existing named recipient and ID untouched (2 passed).
- Secured-write approval now treats HTTP 200 with `status: Failed` as CLI failure,
and rejects malformed or unexpected approval outcomes. Schema/secured focused
tests: 33 passed, including existing listing behavior and new outcome checks.
- Fixed a pre-existing blind spot in the actor dispatch gate: its sentinel now
passes through the configuration transaction before throwing on handler
dependencies. A negative control detects an unsupported command (2 passed).
- New CLI tests that invoke Console-writing actions share the Console collection
to avoid racing tests that redirect process-global output.
All implementation packages are present. Final solution verification, canonical
local deployment, and integration remain in progress.
### Final pre-integration verification
- Full solution build: zero warnings and errors.
- Full solution run (excluding deployed Playwright): 9,551 passed, one failure in
the UI toast unobserved-exception test. Root cause was unrelated incomplete
JavaScript success stubs in UI fixtures; no product UI behavior changed.
- Fixed 15 successful void stubs across 11 fixtures to complete explicitly. Two
direct regression tests failed before and passed after. Full CentralUI rerun:
**1,418/1,418 passed**; the original toast assertion remains unchanged.
- Replacing the initial UI result with the corrected full UI suite gives
**9,554 passed, zero outstanding failures or skips** across 29 test suites.
Key suites: CLI **543**, ManagementService **665**, Commons **852**,
Communication **766**, Integration **118**, Host **598**.
- Documentation links and `git diff --check` passed. All GAP-01 through GAP-07 and
PAR-01 through PAR-05 are complete. Canonical deployment and final merge/push
evidence will be added after integration.
Local verification logs: `/tmp/parity-full-build.log`, `/tmp/parity-full-tests.log`,
`/tmp/parity-ui-red.log`, `/tmp/parity-ui-green.log`, `/tmp/parity-ui-full.log`.
These logs are local evidence, not committed artifacts.