Repository navigation
fix(cli): exit 2 for remaining flag validation errors - #6639
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits
The exit-code conversions hold up. I ran 15 of them as an agent in a scaffolded project, and each exited 2 with a usage_error envelope naming the problem. The touched suites pass locally (2,287 tests). Keeping project- and machine-dependent checks at exit 1 (--aspect-ratio vs the composition, --gpu --docker on arm64) is right; an agent can't fix those by retyping the command.
- blocker —
Commentsis red: the comment ratchet rejects raised comment shares incommands/check.ts(+1 line),commands/cloudrun.ts(+2),utils/jsonError.ts(+1) andutils/variables.ts(+2). Cut those lines, or move the explanation into a name.
Scope and notes
This PR is stacked, so CI doesn't run; I tested locally. Not from this PR, but in the stack (#6622): render --json writes its human header and progress bar to stdout. A failure after that point, such as render --variables '{bad' --json, exits 2 with no envelope, because the boundary sees that stdout was already written. Also not from this PR: snapshot --frames abc and --frames -3 are accepted silently (exit 0, with the default frames).
— Genesis
25132f4 to
b4f433c
Compare
280f7dc to
dab4b83
Compare
b4f433c to
cf67718
Compare
The base branch was changed.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Flag and argument checks that commands make themselves still exited 1 and reported runtime_error under --json. They now take the existing usage-error path (failUsage / CliUsageError): exit 2 and a usage_error envelope. This covers lambda and cloudrun render/render-batch (--width/--height, --fps, positive-number, enum and --output-resolution flags, unknown subcommands), capture, feedback, benchmark --runs, tts, init, snapshot, compare, grade-compare, check, normalize-audio, media-treatment, play/present --port, models, docs, browser and telemetry unknown subcommands, cloud render conflicting inputs, cloud delete without --no-confirm, and inline --variables. A few checks now run before project or stack lookup so a bad flag is reported first. Runtime failures (missing files, network, render errors) still exit 1. An unattended spinner now writes its start and progress text as plain stderr lines, not only the outcome, so tools watching init can see "Transcoding". Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…p added comments - `snapshot --frames abc` (or 0, -3, 2.5) is now a usage error: exit 2, and a usage_error envelope under --json. It used to fall back to 5 frames silently. - An unattended spinner prints its start line once and its outcome, but no longer one line per progress update, which flooded stderr during remove-background and the Chrome download. - Removes the comment lines this branch added, so the comment ratchet passes. - The render --json test now expects exit 2 and usage_error for bad inline --variables. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
cf67718 to
ed900a5
Compare
Edit accuracy: accurate 271 (base branch 271), smooth 182 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What
Flag and argument checks that commands make themselves now exit 2 and, under
--json, answer with ausage_errorenvelope. Before this they exited 1 and reportedruntime_error, even though the docs say a usage error exits 2. This uses the existingfailUsage()/CliUsageErrorpath from earlier in the stack.Also, an unattended spinner now writes its start message as one plain line on stderr, next to its outcome. Before, only the final outcome was written, so a tool watching
initoutput never saw the "Transcoding" start line. Progress updates stay silent, so a longremove-backgroundrun or a Chrome download does not print a line per update.Why
Agents and scripts branch on the exit code. A bad flag should read as "fix your command" (2), not "the run failed" (1).
Related work
Follow-up to #6622 (stacked on
fix/cli-agent-mode-json).How
Converted to exit 2 /
usage_error:lambda renderandrender-batch: missing--width/--height, invalid--fps, positive-integer flags (--width,--height,--chunk-size,--max-parallel-chunks,--target-chunk-frames,--max-concurrent,--wait-interval-ms,--memory),--format/--codec/--quality/--chrome-source/--video-arch,--output-resolution,lambda deploy --concurrency,lambda sites <bad verb>, unknownlambdasubcommandcloudrun renderandrender-batch: the same checks, plus--max-instances/--timeouton deploy,cloudrun sites <bad verb>, and unknowncloudrunsubcommand. Render flags are now checked before the stack state is read and before any upload. Before this, inrender-batcha bad flag only showed up as N failed renders.capture:--capture-budget/--max-screenshots, a URL together with--local-file, and an invalid URLfeedback: out-of-range--rating, no rating and no comment,--file-issuewithout a rating, bad--sourcebenchmark --runs(now checked before the project lookup)tts: no text given,--speedout of rangeinit:--template(renamed),-V,--videotogether with--audiosnapshot: conflicting--at/--range/--composition,--reviewclashes,--range/--atparse, empty--composition,--keepname collision, and an invalid--frames(abc,0,-3,2.5). Before this, a bad--frameswas silently replaced with 5 frames.compare(fewer than 2 paths,--labels,--at,--cols) andgrade-compare(--gradesvs--luts, empty--luts), viafailCaught, which now keeps aCliUsageErroras a usage errorcheck:--range,--frame-check,--layout,--caption-zone(options are now parsed before the project lookup)normalize-audio:--tolerance,--lufs, the same element for reference and targetmedia-treatment:--selector-index,--grading/--apply/--clearcombinations,--capabilitiescombinations,--fileoutside the project, missing--selector,--analyzecombinationsplayandpresent:--port,--user-data-dirwithout--browser-path,--remote-debugging-portand its dependenciesmodels,docs,browserandtelemetrysubcommands or topicscloud renderwith conflicting inputs, andcloud deletewithout--no-confirmwhen it cannot prompt (same aspublish/upgradewithout--yes)--variablesgiven twice or as bad inline JSON (an unreadable or bad--variables-filestays exit 1)Still exit 1: missing or unreadable files, bad batch-file contents, missing stack state, network and render failures. Some flag checks stay at exit 1 on purpose, because they depend on the project or the machine and retyping the command will not fix them. Two examples:
render --resolutionwhose aspect ratio does not match the composition. The flag is valid; it conflicts with what the project contains.render --gpu --dockeron an arm64 machine. The same command works on x86_64; it fails because of the host.Test plan
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments"
cli.agentMode.test.ts: a new table runs about 50 of the converted checks as an agent would. Each must exit 2. Commands with--jsonmust also answer with ausage_errorenvelope that names the problem.Updated unit tests for capture, feedback, check, init, media-treatment and play/present
--port. The spinner test inui/prompts.test.tsnow checks that start and outcome are each written as one plain line and progress writes nothing.snapshotSampling.test.tscovers the--framesparse, and the agent-mode table includessnapshot --frames abcand--frames -3.In
packages/cli: typecheck, oxlint, oxfmt check andfallow auditpass. In the full CLI suite, 6688 tests pass and 11 fail. The 11 are Chrome-launch tests (*.process.test.ts,snapshot.lateImages.browser) that time out on this machine. They fail the same way on the base branch.🤖 Generated with Claude Code