Description

Mikola Lysenko (mikolalysenko)

opened on Oct 3, 2026

Issue body actions

[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:

1. 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.
2. 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.
3. 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.

Activity

1. Mikola Lysenko (mikolalysenko)

   added

   agent:needs-human

   arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)Filed by a scheduled architecture audit routine (see the architecture review discussion)

   refactorStructural change: duplicated code or logic, missing abstraction, layering, dead codeStructural change: duplicated code or logic, missing abstraction, layering, dead code

   priority:p3

   agent:triaged

   on Oct 3, 2026
2. mikolalysenko commented on Oct 3, 2026

   

   Mikola Lysenko (mikolalysenko)

   on Oct 3, 2026

   CollaboratorAuthor

   More actions

   [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
3. mikolalysenko commented on Oct 6, 2026

   

   Mikola Lysenko (mikolalysenko)

   on Oct 6, 2026

   CollaboratorAuthor

   More actions

   [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
4. Mikola Lysenko (mikolalysenko)

   added a commit that references this issue on Oct 6, 2026

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

   e785943
5. mikolalysenko commented on Oct 7, 2026

   

   Mikola Lysenko (mikolalysenko)

   on Oct 7, 2026

   CollaboratorAuthor

   More actions

   Option 1 is clearly the correct choice. Implement proposed solution.
6. mikolalysenko commented on Oct 7, 2026

   

   Mikola Lysenko (mikolalysenko)

   on Oct 7, 2026

   CollaboratorAuthor

   More actions

   [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
7. Mikola Lysenko (mikolalysenko)

   added

   agent:claimed

   and removed

   agent:needs-human

   on Oct 7, 2026
8. Mikola Lysenko (mikolalysenko)

   added a commit that references this issue on Oct 7, 2026

   maintainer-triage: record 2026-10-07 decisions

   fcbcde7
9. mikolalysenko commented on Oct 7, 2026

   

   Mikola Lysenko (mikolalysenko)

   on Oct 7, 2026

   CollaboratorAuthor

   More actions

   [agent] Implemented in #1027.

   ---

   Generated by Claude Code
10. Mikola Lysenko (mikolalysenko)

    added a commit that references this issue on Oct 7, 2026

    Add shared JSON error helpers and usage_error (#704)

    b056b60
11. 2 remaining items
12. Mikola Lysenko (mikolalysenko)

    added 12 commits that reference this issue on Oct 7, 2026

    Document the single --json error shape in the contract and v5 guide (#…

    0fb1060

    Test the coded --json error on usage, offline and rollback failures (#…

    b30c9b3

    Fix stale errorCode wording in the contract and test docs (#704)

    1824b60

    Fix CI: keep the forced identifier off stderr, align tests with #704

    70c3229

    Document the apply --check copy rules and manifest-path exemptions

    b7b4854

    Add shared JSON error helpers and usage_error (#704)

    a59afe4

    Make every scan/get/rollback --json error a {code, message} object (#704

    8d316df

    Update tests and backtest script for the {code, message} error shape (#…

    e4b5819

    Document the single --json error shape in the contract and v5 guide (#…

    ba92cea

    Test the coded --json error on usage, offline and rollback failures (#…

    15ba36f

    Fix stale errorCode wording in the contract and test docs (#704)

    619a666

    Fix CI: keep the forced identifier off stderr, align tests with #704

    a458ba9
13. github-merge-queue

    added a commit that references this issue on Oct 8, 2026

    Stop CI gates passing on missing paths, unverified agent patches and …

    6fcc51b
14. Mikola Lysenko (mikolalysenko)

    closed this as completedin #1027on Oct 8, 2026
15. Mikola Lysenko (mikolalysenko)

    removed

    agent:claimed

    on Oct 11, 2026

Issue actions