fix(cli): reject a swallowed --json when catalog's string flags have no value

[miga-heygen]

Summary

catalog --query --json (a missing --query value) silently parsed as query: "--json", json: false instead of erroring. citty's parser (node:util.parseArgs, strict: false) consumes the next argv token as a type: "string" option's value, even when that token is another declared flag. A consumer piping stdout got a human-readable table instead of JSON, with exit code 0.

Control case keeps working: catalog --query "kinetic type" --json still emits parseable JSON on stdout.

Fix

guardSwallowedFlagValues(cmd, path, rawArgs) in packages/cli/src/utils/reject-unknown-flags.ts scans the raw argv for a string/enum flag followed by -- or by a spelling of one of the command's own flags, and fails with Missing value for --<flag>: ... use --<flag>= or move --<flag> to the end. It scans raw argv because the parsed args cannot tell --query --json from the legitimate --query=--json.

The guard is shared: wrapCommand in packages/cli/src/utils/command-failure-tracking.ts runs it for every leaf command right after the existing unknown-flag check, so every command's string flags get the protection, not just catalog's. Two commands keep their documented behaviour through per-path exceptions:

• check --frame-check is documented to work bare or followed by another flag. The guard rewrites a bare --frame-check to --frame-check= and re-parses the args, replacing check's own raw-args preprocessing.
• upgrade --project already recovers a swallowed value itself, so the guard skips it.

The exceptions are keyed by full command path, so skills check does not inherit check's rewrite. The error is printed once and marked as presented, so --json callers get an empty stdout instead of a usage dump. init --example's hand-rolled check is removed in favour of the shared one.

Behaviour notes:

• citty drops every --no-<flag> before parsing, so the guard looks past them: --output --no-docker --json is rejected naming --json, a trailing --output --no-docker is fine, and check rewrites --frame-check --no-browser-gpu <dir> to --frame-check= as before.
• The error tells a value-taking flag to pass a value (--output <value> or --output=<value>); check's --frame-check keeps its "use --frame-check=" hint.
• Under --json, a swallowed-value error now goes to stderr with empty stdout and exit 1, the same as an unknown flag. Scripts that parsed a JSON error object from stdout for this case (e.g. check --json --layout --strict) get nothing on stdout now.

Test plan

• reject-unknown-flags.test.ts: the swallow is rejected; the equals form, an unrecognised dash value, a trailing bare flag and boolean args are accepted; -- gets its own wording; check's rewrite, upgrade's skip and the path scoping each have a case; end-to-end through citty's real runCommand and the real wrapCommand (the reported catalog --query --json repro, a positional ahead of the flags, nested skills check); stdout stays empty on the built CLI.
• check.test.ts: a dash-prefixed --frame-check value fails once, not twice, and --json after a --layout value is no longer swallowed.
• reject-unknown-flags, command-failure-tracking, check and init test files pass 3 runs in a row; breaking the upgrade skip or the re-parse fails the matching tests.

[somanshreddy]

APPROVE at 3ba621b4 — the mechanism is correct and both opt-out exceptions are right. Two things worth putting on record (neither blocking): the PR description materially under-describes the blast radius, and the completeness of the exception maps is the load-bearing assumption. Independent manual pass (codex disabled per workspace cap — my own source verification, flagged for honesty).

Heads-up: the diff is broader than the summary. The summary says "deliberately opt-in per command, not merged into the global gate … wired into catalog.ts's own run()." The actual change is the opposite shape, and catalog.ts isn't touched at all. guardSwallowedFlagValues is wired into the global wrapCommand gate (command-failure-tracking.ts) right beside assertKnownFlags, so it now runs for every leaf command. The per-command guards it replaces — check's normalizeFrameCheckRawArgs and init's --example requires a value — are removed and folded into it. "Opt-in per command" now refers to the two exception maps, not to where the guard runs. The code is correct; I'm flagging this so the merge isn't decided on a catalog-only mental model of the blast radius.

Mechanism verified at source:

• Triggers only for a command's own type:"string"|"enum" flags (stringValueFlagOwners) — booleans never swallow, and the test pins that. Fires only when the next token is a recognized flag (looksLikeSwallowedFlag: next === "--" or a known-flag spelling), so an ordinary dash-value like --query -out.mp4 is accepted while --query --json is rejected. --query=--json is always accepted because ownableFlagSpelling bails on any token containing = — the one case a raw-args scan can distinguish that node:util.parseArgs collapses.
• Double-print is correctly split: throwSwallowedFlagError (from inside wrapCommand, before any run() try/catch exists) prints and marks the error presented:true so executeCli won't re-dump usage; swallowedFlagUsageError (returned for check's parseFrameCheck, which is caught by check's own run()) is a plain non-printing error so check prints it exactly once. The "prints exactly once, not doubled" test pins this.
• Both exceptions are correctly chosen. SWALLOW_REWRITE_FLAGS = {check: frame-check} rewrites --frame-check --json → --frame-check= (bare = defaults per check's grammar) and the wrapper updates both ctx.rawArgs and ctx.args, which matters because check re-derives args from ctx.rawArgs. SWALLOW_IGNORE_FLAGS = {upgrade: project} leaves rawArgs untouched — verified correct against upgrade.ts:resolveProjectArgs, which deliberately recovers a swallowed --project value (project.startsWith("-") → treats --check as the flag, falls back to cwd). Guarding either of those would have been the regression the PR set out to avoid.

The one thing I could not fully close — non-blocking: the exception maps assume frame-check and upgrade/project are the only string/enum flags with legitimate bare-then-flag semantics across the whole CLI. I verified those two are correct and spot-checked, but did not exhaustively enumerate every command's string flags (a startsWith("-") code search came back empty, but that search is unreliable, so I'm not treating it as proof). Resting on the author's stated adversarial iteration (the body notes an earlier global-gate draft where review already caught frame-check, and upgrade/project was found too — evidence of real enumeration, not a one-case patch) plus green CI across all command suites. If a third bare-optional string flag exists without test coverage, --thatflag --knownflag would start erroring; worst case is a clearer error on a rare idiom, not silent breakage.

Tests are thorough and real (throw / accept-equals / accept-dash-nonflag / trailing-bare / boolean-ignored / ---terminator / rewrite / the exact catalog repro, plus check's --layout swallow and the frame-check single-print). All required CI green; BLOCKED is branch protection awaiting the stamp. Home to merge.

_{Provenance: my own source pass at 3ba621b4 (reject-unknown-flags.ts, command-failure-tracking.ts, check.ts, init.ts, upgrade.ts + tests) + exception-wiring trace; codex disabled (owner cap). Did not run the suite locally (env); resting on source verification + green required checks. Recommend the author sync the PR description to the shipped (global) design.}

[github-actions]

Edit accuracy: accurate 2061 (base branch 2061), smooth 1599 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

[jrusso1020]

Approve at 5816dfe6. A string flag that swallows the next option now fails with a clear message. I found no false rejection: a sweep of 1,432 documented invocations (1,170 from tracked .md/.mdx files, 262 from help strings and skills) is never rejected or rewritten. The check and upgrade exceptions keep their behaviour.

Should-fix

1. auth login --api-key is a third flag documented to work with no value. commands/auth/login.ts:65-68 says to pass --api-key with no value to read the key from stdin, as in echo "$HEYGEN_API_KEY" | hyperframes auth login --api-key.
   • Now auth login --api-key --json or --api-key --device fails with "Missing value for --api-key … pass one: --api-key or --api-key=".
   • This isn't a regression, since main took the next flag as the key itself. But the hint steers people to put a secret on the command line, where it lands in shell history.
   • Add "auth login": new Set(["api-key"]) to SWALLOW_REWRITE_FLAGS (utils/reject-unknown-flags.ts, around line 104). Tested: --api-key --json then reads stdin as documented, and --api-key --device reaches the command's own "Choose either --device or --api-key" error.
2. One exception table instead of two. Moving upgrade --project from SWALLOW_IGNORE_FLAGS into the rewrite table behaves identically. I tested --project --check, --json, -y, bare, and --project dir --json. That makes both the skip table and upgrade.ts:175-181's own recovery removable, leaving one list: string flags documented to work with no value.

Nits

• A trailing flag with nothing after it is still silent: render p --output falls back to the default path, and catalog --query lists everything. The body says so, but "a string flag left without its value now fails" overstates it; it fails only when the next token is a flag or --.
• The enum branch in stringValueFlagOwners is dead. None of the 381 string args across 62 commands is an enum, and citty validates enums before run.
• The wording value "--json" appears to have swallowed the next option reads backwards: the flag swallowed it. It also always names the long flag when the user typed -o or --out.
• Under --json, a swallowed value gives exit 1 with plain text on stderr and nothing on stdout, which matches unknown-flag errors. check callers used to get {"ok":false,…} on stdout in this case. If that matters, one JSON error shape at wrapCommand would cover every usage error.
• Not from this PR: the older unknown-flag check rejects negative values written with a space (render --fps -1, keyframes --angle -45, check --at -1, render --output -out.mp4), while the = form works. That's worth a follow-up, since normalize-audio --lufs defaults to -16.

Checked (built CLI, head vs main)

• Now rejected (main silently took the next token as the value):
  • catalog --query --json: main exited 0 with query="--json";
  • render p --output --json, -o --json, -o -w, --output --no-docker --json (the error names --json);
  • tts -o --voice, media-use resolve -p --json, cloud render --title --json, feedback --search-miss --json.
• Unchanged:
  • catalog --query=--json --json, --query -, --json before or after;
  • render p --output --no-docker out.mp4, --output=-out.mp4;
  • a repeated --output, --help on any command, tts -- --hello;
  • upgrade --project with --check/--json/-y/bare;
  • check d --frame-check --json.
• Improved:
  • check d --frame-check --caption-zone … --json now works (main: "unexpected extra argument"), because ctx.args is re-parsed;
  • render p --output -- x gives a clear missing-value error.
• Tests:
  • The focused suites (reject-unknown-flags, command-failure-tracking, check, init, upgrade) pass 169/169.
  • The full CLI suite has 4,589 passed. The 2 failures are 30 s browser-launch timeouts under load, and they're green in CI.
  • Typecheck, oxlint and oxfmt are clean on the 7 changed files.
• Mutants: all 8 are caught: guard off (10 tests fail), --no-x look-past removed (2), check exception (5), upgrade exception (1), ctx.args not re-parsed (1), --frame-check --no-x rewrite (1), the already-printed marker (2), and -- handling (1). The upgrade exception's test hardcodes the path, so a wrong path key wouldn't be caught.
• CI: all 11 required checks pass at this head.

— Rames