fix(cli): one error envelope and exit 2 for every usage error - #424
Open
ankitranjan7 wants to merge 1 commit into
Open
fix(cli): one error envelope and exit 2 for every usage error#424ankitranjan7 wants to merge 1 commit into
ankitranjan7 wants to merge 1 commit into
Conversation
Unknown subcommands, unknown options, missing arguments, missing required options, and excess arguments all rendered differently: a plain line, a plain line plus a YAML envelope, or the same line printed twice. Exit codes were 1 or 2 depending on which, and `-f/--format`/`--json` were ignored entirely. Commander structural failures now route through the same envelope path as CliError: exit 2, a specific `error.code`, one `error:` line plus a `help:` line for humans, and a JSON/YAML envelope when a machine format is requested. `applyUnknownOptionContract` captures Commander's `writeErr` so the default `outputError` write can no longer reach stderr ahead of the handler, which removes the duplicated line. The `web fetch` fast path and the hosted runner use the same contract, so local and hosted bytes stay identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
🟠 Maintainer review suggested — low confidenceThe automated review could not reach a fully supported conclusion. Limitations
This review is advisory and does not block merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The same class of typo produced three different error renderings and two different exit codes, so nothing could parse a webcmd failure reliably.
webcmd adapter list restprinted a plain line and a YAML envelope withcode: UNKNOWN, exit 1.webcmd adapter path -f json x/yprinted the identical line twice with no envelope, exit 2.webcmd web fetch --jsonignored--jsoncompletely, exit 1.What changed
unknownCommand,unknownOption,missingArgument,missingMandatoryOptionValue,excessArguments, andinvalidArgumentmap toUSAGE_ERROR_CODESinsrc/command-surface.tsand carry anErrorEnvelopeonCommanderStructuralError.UNKNOWN_COMMAND,UNKNOWN_OPTION,MISSING_ARGUMENT,MISSING_OPTION,EXCESS_ARGUMENTS,INVALID_ARGUMENT) instead of exit 1 withUNKNOWN.applyUnknownOptionContractnow callsconfigureOutput({ writeErr })alongsideexitOverride, capturing what Commander's defaultoutputErrorused to write directly to stderr before the throw. Output the handler does not own is replayed verbatim.-f/--formatand--jsonare honoured on structural errors viarequestedMachineFormatinsrc/output.ts. Humans still get plainerror:+help:lines; only an explicitjson/yamlrequest produces an envelope.help:extends the existing unknown-option treatment to unknown subcommands (lists valid subcommands) and missing arguments (restates the usage line).web fetchand hosted mode use the same contract.runWebFetchCommandbypassedcli.tsentirely and never installed the contract;handleProgramParseError/reportCliErrormoved tosrc/cli-error-report.tsso that fast path can reuse them without importing the full command tree.src/hosted/runner.tspreferserr.envelopeover the legacyUNKNOWN/exit-1 fallback, keeping local and hosted bytes identical.commander.help/commander.versionare untouched —COMMANDER_DISPLAY_CODESstill short-circuits before any envelope work, andwebcmd adapter --helpstill exits 0 with empty stderr.Before / After
Tests
npm run typecheckclean.npx vitest run --project unit: 154 files, 2713 passed, 1 skipped, 0 failed (baseline onorigin/mainwithdist/built: 2703 passed, 0 failed).npm run buildclean, and every command above was re-run against the builtdist/src/main.js.New coverage in
src/cli-error-report.test.ts: each reproduce case asserts exit 2, exactly oneerror:line on stderr, no envelope for humans, and a valid parsed JSON envelope under--json(plus a YAML case for-f yaml, andadapter --helpstill exit 0 / empty stderr).Assertions changed deliberately, all of them encoding the old inconsistency:
src/hosted/runner.test.ts— six structural cases moved fromexitCode: 1+ trailingcode: UNKNOWNenvelope toexitCode: 2+help:line (missing positional, missing required option ×3, excess positional, unknown site command).src/hosted/root-command-surface.test.ts—completionwith no shell,listwith an excess argument, and unknown-subcommand-of-a-known-site now assert the exit-2 usage bytes for both local and hosted.Deliberately out of scope:
commander.optionMissingArgument(option '--x <v>' argument missing) still exits 1 with the legacy envelope — it was not in the listed set and touching it churns a wider band of hosted parity tests. It is the obvious follow-up.🤖 Generated with Claude Code