fix: show 'unknown command' for invalid subcommands#441
Conversation
…id subcommands When a user types `kimi nonexistent`, the CLI previously reported "too many arguments" because Commander treated the unrecognized word as an excess positional argument. Allow excess arguments on the main command and check them in the action handler, producing a clear "unknown command 'nonexistent'" error with a list of available commands. Closes MoonshotAI#440
🦋 Changeset detectedLatest commit: 3f86303 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0cccd4013
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .description('The Starting Point for Next-Gen Agents') | ||
| .version(version, '-V, --version') | ||
| .allowUnknownOption(false) | ||
| .allowExcessArguments(true) |
There was a problem hiding this comment.
Keep excess-argument checks on subcommands
Because Commander’s .command() copies inherited settings from the parent (cmd.copyInheritedSettings(this)), and that copied set includes _allowExcessArguments, setting this before registering subcommands disables the default excess-argument validation for every subcommand. In the inspected CLI registrations this means commands like kimi upgrade extra or kimi provider list extra now execute and silently ignore the stray operand instead of failing, which is risky for side-effecting subcommands; keep this suppression scoped to the root fallback or reset it on subcommands.
Useful? React with 👍 / 👎.
Replace `allowExcessArguments(true)` (which inherited to all
subcommands) with an explicit `argument('[unknown...]')` on the
root command. Subcommands define their own arguments independently
and retain Commander's default excess-argument validation.
|
Good catch — replaced |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Sorry, we currently do not accept PRs submitted in batches using AI |
Related Issue
Resolve #440
Problem
kimi nonexistentreportserror: too many arguments. Expected 0 arguments but got 1.instead of saying the command is unknown. Commander treats the unrecognized word as an excess positional on the root command, which has noargument()defined.What changed
Add
argument('[unknown...]')on the root command only and check the captured value in the action handler. Subcommands keep Commander's default excess-argument validation (catching the codex review feedback —allowExcessArguments(true)at the root would inherit to subcommands and silently acceptkimi upgrade extra). Also enableshowSuggestionAfterError()for better hints on misspelled options.Error becomes:
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.Verification
pnpm -C apps/kimi-code exec vitest run test/cli/options.test.tsNote: PR #442 by @Eric-Song-Nop (the issue reporter) addresses the same issue with a similar approach. Happy to defer to that PR if preferred.