refactor: centralize known CLI command checks - #1057
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Review pass: no actionable blockers found. This matches #1043 by moving known-command recognition into the command catalog and using the shared predicate from parser unknown-command handling plus the CLI fall-through. The AST-backed drift test covers cli.ts command comparisons, including property-access forms, so it should fail if dispatch learns a literal command the parser does not know. Checks are green. I marked this ready-for-human. Note: #1055 appears to be a duplicate solution for the same issue; prefer this PR unless maintainers choose otherwise. |
thymikee
added
the
ready-for-human
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
Review: LGTM — preferred over #1055, which it supersedes. 155/155 parser/CLI tests locally, typecheck clean.
This one wins the duplicate resolution on design: the known-but-unhandled fall-through gets a distinct user-facing message ("Command is registered but no CLI handler accepted it"), so the #1043 drift bug self-describes at the moment someone hits it, instead of masquerading as a user typo. The type-predicate export (isKnownCliCommandName) consumed by both the parser early-check and cli.ts is exactly the single-source-of-truth the issue asked for.
I folded #1055's one advantage into this branch (160f418): the fall-through now also emits a cli_known_command_unhandled error-level diagnostic, so the drift is visible in telemetry, not only to the unlucky user. With that, #1055 is fully subsumed and should close.
160f418 to
4b54e86
Compare
Folds #1055's telemetry into this branch: known-command fall-through now emits cli_known_command_unhandled at error level alongside the distinct user-facing message, so catalog/dispatch drift is visible in diagnostics as well as to the user who hits it.
4b54e86 to
e8f23fa
Compare
|
1 participant
Summary
Centralizes known CLI command detection in the command catalog and reuses it from parser unknown-command handling and the CLI defensive fall-through.
Adds regression coverage for catalog/help/internal command recognition plus an AST-backed drift test that fails when cli.ts dispatch checks mention a command the parser does not recognize.
Closes #1043
Touched files: 4. Scope stayed within CLI command recognition/parser/dispatch contracts.
Validation
Focused parser/help coverage passed: pnpm exec vitest run src/utils/tests/args.test.ts src/tests/cli-help.test.ts --project unit.
Static quick checks passed: pnpm check:quick.
Formatting passed: pnpm format.