Skip to content

refactor: derive command identity from descriptors#1151

Merged
thymikee merged 2 commits into
mainfrom
fix/adr0008-cli-client-backed-derived
Jul 8, 2026
Merged

refactor: derive command identity from descriptors#1151
thymikee merged 2 commits into
mainfrom
fix/adr0008-cli-client-backed-derived

Conversation

@thymikee

@thymikee thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Derive PUBLIC_COMMANDS, INTERNAL_COMMANDS, local CLI names, and platform dispatch command coverage from CommandDescriptor facets.

This removes the live hand-maintained command catalog tables and the manual DispatchCommand union. Dispatch-only aliases such as read and swipe-preset are now explicit descriptor aliases, command-family metadata is coherence-guarded against the descriptor CLI catalog, and ADR 0008/CONTEXT now record the completed registry state.

Follow-up: public Node-client typed-result narrowing is tracked in #1153 because several remaining methods need public contract moves before they can safely stop returning CommandRequestResult.

Validation

  • pnpm format
  • pnpm check:quick
  • pnpm check:layering
  • pnpm exec vitest run src/core/command-descriptor/tests/parity.test.ts src/core/command-descriptor/tests/timeout-policy.test.ts src/core/command-descriptor/tests/post-action-observation.test.ts src/commands/tests/command-surface-metadata.test.ts src/cli/commands/tests/client-backed.test.ts src/utils/tests/command-schema-guards.test.ts
  • pnpm check:fallow --base origin/main

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.6 MB 1.6 MB -2.7 kB
JS gzip 507.8 kB 506.3 kB -1.5 kB
npm tarball 610.1 kB 606.9 kB -3.2 kB
npm unpacked 2.2 MB 2.1 MB -11.3 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.0 ms 27.5 ms +1.5 ms
CLI --help 53.5 ms 54.0 ms +0.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/cli-help.js -4.6 kB -1.4 kB
dist/src/sdk-batch.js +1.8 kB -332 B
dist/src/tv-remote.js +239 B +209 B
dist/src/cli.js +24 B +8 B
dist/src/client.js 0 B -8 B

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Reviewed PR #1151 against ADR 0003/0008 and the command surface -> CLI router -> client invocation path. The change removes the catalog-owned client-backed predicate, derives routing from the command-family executable metadata used by runCommand, and leaves daemon registry ownership/predicates untouched. CI is green across 21/21 reported checks. No actionable blockers found.\n\nNote: the monitor guidance referenced plans/perfect-shape.md, but that file is absent from both origin/main and the PR ref, so this review used the PR body plus ADR 0003/0008 as source context.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee thymikee changed the title refactor: derive client-backed cli routing refactor: derive command identity from descriptors Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

The previous ready-for-human signal is stale after the new 1245094 commit. This PR is currently DIRTY against the base and only partial checks are reported, so I removed ready-for-human. I’ll re-review once the branch is clean and checks have completed.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee thymikee force-pushed the fix/adr0008-cli-client-backed-derived branch from 1245094 to 7158fc3 Compare July 8, 2026 12:29
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed updated head 7158fc36 after the descriptor-rooted command identity change. The PR now derives public/internal/local CLI catalog names and the platform dispatch command set from descriptor facets, keeps daemon policy behind the existing daemon registry/predicate interface, and explicitly models dispatch-only aliases like read / swipe-preset without exposing them as CLI commands. Targeted parity/coherence tests cover catalog projections, dispatch handler registration, command-family metadata, schema-only local commands, and CI is green across 21/21 checks. No actionable blockers found.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee thymikee force-pushed the fix/adr0008-cli-client-backed-derived branch from 7158fc3 to c59054d Compare July 8, 2026 13:26
@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

New head c59054d is still running checks, so I removed the stale label for now. I’ll re-review/relabel once the new run is clean and the updated diff is ready for maintainer judgment.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Correction to the prior status comment: new head c59054d is still running checks, so I removed the stale ready-for-human label for now. I will re-review/relabel once the new run is clean and the updated diff is ready for maintainer judgment.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed updated head c59054d after checks completed. The latest delta keeps command identity descriptor-owned by deriving catalog records from descriptor catalog entries and by modeling dispatch-only aliases as named non-public descriptors. The dispatch handler map remains exhaustively typed from descriptor dispatch facets, and new parity tests guard descriptor-only commands from falling into the public catalog plus generic-route commands from missing dispatch coverage. CI is green across 21/21 checks. No actionable blockers found.

Note: monitor guidance still references plans/perfect-shape.md, but that file is absent from origin/main and this PR ref, so this review used the PR body plus ADR 0003/0008 context.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee thymikee force-pushed the fix/adr0008-cli-client-backed-derived branch from c59054d to 0beb726 Compare July 8, 2026 13:58
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

New head 0beb726 is still running iOS smoke, so I removed the stale ready-for-human label for now. I will re-review/relabel once the current run is fully clean.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-checked current head 0beb726 after the delayed iOS smoke completed. The only delta since the prior ready review is ADR wording plus a comment update in src/core/command-descriptor/derive.ts; descriptor/runtime behavior is unchanged, and CI is green across 21/21 checks. No actionable blockers found.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee thymikee force-pushed the fix/adr0008-cli-client-backed-derived branch from 0beb726 to 83b3646 Compare July 8, 2026 15:33
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-checked current head 83b3646. The new delta makes catalog.group explicit on every descriptor, removes the implicit public fallback, and tightens the descriptor-only parity test accordingly. That moves the command identity model in the intended direction: new descriptors now have to declare public/internal/local/dispatch identity instead of becoming public by omission. CI is green across 21/21 checks. No actionable blockers found.

@thymikee thymikee merged commit 9dabe5b into main Jul 8, 2026
21 checks passed
@thymikee thymikee deleted the fix/adr0008-cli-client-backed-derived branch July 8, 2026 15:55
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-08 15:55 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant