fix: report unknown command before flag-support validation - #1038
Conversation
When an unknown command is provided with flags, the parser was validating flag support before checking if the command exists, resulting in misleading "Flags are not supported for command X" errors instead of "Unknown command: X". This fix reorders the validation to check for unknown commands first, reporting more helpful error messages. Additionally, when the unknown command is 'tap', the error now includes a hint suggesting 'press or click' as alternatives. Closes #1036
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
Review: LGTM with two non-blocking notes. Verified locally (cli-help + parser suites pass) and confirmed the reorder does what #1036 asked: unknown command now wins over flag validation, with the tap → press/click suggestion.
-
This adds a second source of command-knowledge. The early check derives "known" from the catalog (
listCliCommandNames+INTERNAL_COMMANDS+ hardcodedhelp), while the authoritative check remains the cli.ts:408 fall-through after all runners decline. I verified today's special-cased commands (auth/connection/mcp/proxy/web) are all inLOCAL_CLI_COMMANDS, so nothing regresses — but a future command wired only into cli.ts would now be rejected at parse time. Worth a one-line comment onisCommandKnownpointing at the cli.ts fall-through as the thing it must stay in sync with (or, later, extracting one shared helper). -
includes(command as any)×2 — prefer widening the array instead of narrowing the value:(listCliCommandNames() as readonly string[]).includes(command).
Known accepted gap: tap --help still takes the old path (deliberate flags.help skip) and reports without the alias suggestion — fine, but worth stating in the PR body so it reads as a decision rather than an omission.
thymikee
added
the
ready-for-human
Review follow-ups: widen the command lists to readonly string[] instead of narrowing the value, and document that isCommandKnown must stay in sync with the cli.ts dispatch fall-through.
|
Applied the review nits directly on the branch (6df7d4e): dropped both |
|
1 participant
Summary
Fixes unknown-command detection to run before flag-support validation in the CLI parser. This resolves the bug where
agent-device tap e3 --session fooreported "Flags --session are not supported for command tap" instead of the more helpful "Unknown command: tap".Before/After
Before:
After:
Changes
finalizeParsedArgs(src/cli/parser/args.ts) to check if the command is known before validating flag supporttap→press or click)--helpflag to allow graceful error messages from cli.tsCloses #1036