Decide: one shape for the --json top-level error (scan and get emit both a string and a {code, message} object)

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.

Kind: decision. Source: review 2.8 and R4; register C14.

Question

scan, get and rollback aren't on the unified envelope yet. Their top-level error key has no fixed type: within one command, it is sometimes a bare string and sometimes a {code, message} object, depending on which failure fired. A consumer can't read .error without checking its type first. Which shape should the legacy commands emit?

Options:

Always {code, message} (recommended). This matches EnvelopeError, which the migrated commands already use, so every string site gets a stable code. It is a breaking change for consumers that read .error as a string, so it needs a MAJOR note in CLI_CONTRACT.md.
Keep error as a string and add a sibling errorCode everywhere. get's lock failure already does this. It's additive, but the object-shaped sites would then have to flip back to strings, which also breaks consumers.
Move scan, get and rollback onto Envelope now. This finishes the v3.0 migration in one MAJOR step. It's the largest change, and it overlaps C11 (the run_scan split).

Whichever option is chosen, one emitter per command should own the shape, so a new failure path can't pick its own.

Problem (main @ `045d7ec`)

get:
- string: report_error emits {"status":"error","error":"<msg>"};
- string + code: report_lock_failure emits {"status":"error","errorCode":…,"error":"<msg>"};
- object: vendor failure and L3801-L3802 emit "error": {"code","message"}.
scan:
- object: embedded-VEX failure;
- string: emit_discovery_error_json, L1281-L1282 and L2062-L2063.`` The doc comment of the first says it keeps "one consistent scan-error schema".
rollback: string only, from emit_rollback_error.
The migrated commands use EnvelopeError { code, message }. CLI_CONTRACT.md "Migration status (v3.0)" still lists scan, get and rollback as ⏳.

Impact

Every PR-bot or dashboard consumer has to type-check .error. Each new failure path picks a shape ad hoc, which is how get ended up with three. This is also the root of the untyped error codes (C13): the string sites carry no code at all.

Proposed change (after the decision)

Add one fn emit_legacy_error(cmd, code, message) per legacy command, or a shared one in json_envelope.rs.
Route every site above through it, and delete report_error, emit_rollback_error and emit_discovery_error_json's ad-hoc assignment.
Assign a code to each string site, reusing existing codes where they exist.

Size and scope

Option 1 is about 150 production lines across get.rs, scan/mod.rs, rollback.rs and json_envelope.rs, plus test updates and a contract note. The per-patch patches[*].error strings and rollback's results[*].error are out of scope: only the top-level key is.

Acceptance criteria

An owner picks an option.
Each of scan, get and rollback emits a single top-level error type on every failure path, with a test per path that asserts the type.
CLI_CONTRACT.md documents the shape and the change (MAJOR if option 1 or 3).
The existing get/scan/rollback JSON tests stay green or are updated in the same PR.

Dependencies

Feeds C13 (typed error-code registry). Option 3 overlaps C11 (run_scan split).
Not blocked by anything.

[agent] Triage: priority:p3 (CLI JSON contract). This is a decision issue, so it keeps agent:needs-human until an owner picks an option. Agents won't claim it before then.

Generated by Claude Code

[agent] The audit routine (CLI and core) found a second dimension of this decision: what --json prints on a usage error (exit 2). Register row C53. Main @ 9c43dfc.

Each self-enforced exit-2 site picks its own stdout behavior, and there is no shared usage-error helper. The same refusal, --global --mode vendored, gives three different stdouts on three commands. I ran each case twice on a debug build under env -i, with identical results both times:

Invocation (`--json`)	stdout	stderr
`scan --global --mode vendored` (L1634-L1637)	empty	`Error: …`
`scan --mode hosted --apply` (same site)	empty	`Error: …`
`remove --preserve-state --skip-rollback` (L321-L327)	empty	`Error: …`
`rollback` with a bad glob (L1010-L1015)	empty (by reading)	`Error: …`
`get --global --mode vendored lodash` (L2559-L2562, via `report_error`)	`{"status":"error","error":"<string>"}`, no code	nothing
`vendor --global` (L728-L740)	full `Envelope`, `error.code: global_scope_unsupported`	nothing
`repair --offline --download-only` (L49-L57)	full `Envelope` with a code	nothing
`vex --json` with no `-O` (L298-L309)	full `Envelope`, `json_requires_output`	nothing

Clap's own parse errors print nothing on stdout. scan's comment calls that the intended rule ("Cross-mode combinations are usage errors (exit 2), which print no JSON envelope even under --json, like clap's own"). But get, vendor, repair and vex don't follow it. CLI_CONTRACT.md documents each case on its own (the vendor envelope at L129, "stderr only" for scan PATHs at L157), not as one rule. A script that runs --json and parses stdout gets a parse failure from scan, remove and rollback, and a document from the others.

Suggested addition to the options above: whichever shape wins, also decide one rule for exit-2 usage errors. Either:

(a) none of them write stdout, as clap does (simplest; vendor, repair, vex and get change), or
(b) every self-enforced one writes the chosen error shape with a code (scan, remove and rollback change; clap's errors stay stdout-free).

Then route them through one usage_error(cmd, json, code, msg) -> i32 helper, so a new conflict can't choose its own channel. Of the ~17 return 2 sites in commands/, 4 write JSON today.

Generated by Claude Code

audit-core: C35 decision #966, C53 into #704, Part 2 refresh

Option 1 is clearly the correct choice. Implement proposed solution.

[agent] Decision recorded: option 1. On every --json failure, the top-level error is a {code, message} object on every command, the same shape as EnvelopeError. This is a breaking change (MAJOR) that ships in v5.0, before the stable release. A PR will follow.

What changes (main @ db83f014)

One emitter per command, no bare strings.
- A shared helper in json_envelope.rs sets status: "error", sets error: {code, message} and removes any top-level errorCode.
- get: report_error (13 call sites), report_lock_failure and the hand-built objects at get.rs:2491, :2544, :3462, :3830 and :3931 all go through it. The nested-apply run error (get.rs:2443) moves from errorCode + string into error. selection_required (get.rs:1078) keeps its status, and its error becomes {code: "selection_required", message}.
- scan: these go through it:
  - emit_json_error/emit_json_error_with_code (scan/hosted.rs:66-90)
  - emit_discovery_error_json (scan/mod.rs:761)
  - print_zero_error_envelope (:1478)
  - the all-batches-failed object (:2279, api_batch_failed)
  - policy_error_json (scan/policy.rs:468)
  - the embedded-VEX and vendor failures, which are already objects
  Scan's error results keep their counts and redirect keys.
- rollback: emit_rollback_error takes a code, and the inline objects at rollback.rs:1127, :1266 and :1986 go through it.
Codes. Existing codes are reused where they fit:
- patch_fetch_failed, manifest_not_found, manifest_invalid, manifest_unreadable, manifest_write_failed
- lock_held and lock_io
- api_batch_failed, socket_yml_invalid, socket_yml_ambiguous, global_scope_unsupported, invalid_args, rollback_failed
New codes are added only where nothing fits, for example a blob write failure, a patch with no applicable files, an offline refusal, an invalid identifier and a path glob that doesn't parse. Each new code goes into the contract's top-level code table.
Exit-2 usage errors (C53). The issue offered two rules. This takes rule (b), which follows from option 1: under option 1, get's report_error usage path already prints a coded object, and vendor, repair and vex already print coded envelopes. A new usage_error(cmd, json, code, msg) -> i32 replaces the stderr-only sites in scan (scan/mod.rs:1554-1702), remove (remove.rs:326) and rollback (rollback.rs:1014). It also replaces the bespoke ones in get, vendor, repair and vex. --global --mode vendored will give error.code: "global_scope_unsupported" on scan, get and vendor. Clap's parse errors still print nothing on stdout. Human stderr text is unchanged. If you'd rather have rule (a), where usage errors write no stdout at all, say so before the PR lands.
Guard test. No return 2; under commands/ outside usage_error. The only exceptions are list.rs's source rank and the hidden hosted-bundle harness.

Tests

One test per failure path in scan, get and rollback, asserting error is an object with a non-empty code and no top-level errorCode.
Usage-error tests for every self-enforced exit-2 site, asserting the stdout shape under --json.
Existing tests that read .error as a string or read the top-level errorCode are updated: the covgap_commands_{get,rollback,scan_hosted,scan_mod} suites, e2e_socket_yml_policy, cli/output_modes_e2e and the in-crate tests in get.rs, scan/hosted.rs and scan/vendor_flow.rs.

Docs

CLI_CONTRACT.md:
- "JSON output shapes" states the rule for every command.
- The "Migration status" bullets note that scan, get and rollback now share the error shape.
- The socket.yml "Error output" example, the rollback JSON table and get's lock-failure text lose the sibling errorCode.
- The top-level code table lists the codes for scan, get, rollback and usage errors.
- The exit-code 2 row states the stdout rule.
docs/migrating-to-v5.md gets a "JSON output" entry: read .error.message / .error.code instead of .error / .errorCode, and scan, remove and rollback usage errors now print JSON.
CHANGELOG.md is not touched.

Not in scope: moving these commands onto Envelope (option 3, which overlaps C11) and the typed code registry (C13, #930). The new codes will feed C13.

Generated by Claude Code