An invalid --download-mode fails apply and repair with exit 1 but is accepted by every other command

[mikolalysenko]

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

Kind: bug. Source: new finding, register C45.

Problem

--download-mode / SOCKET_DOWNLOAD_MODE is a free String in GlobalArgs (args.rs#L153-L162). Its sibling enums are validated by clap: --vendor-source has value_parser = parse_vendor_source and --maven-config has value_parser = ["auto", "none"] (args.rs#L168-L179), so a bad value is a usage error with exit 2.

--download-mode is instead parsed late, in two places, with DownloadMode::parse (blob_fetcher.rs#L31-L43):

• fetch_stage.rs#L284 (agent apply, and the nested apply in scan/get);
• repair.rs#L479-L480.

Everything else ignores the value. CLI_CONTRACT.md documents it as an enum (diff | file, "package was removed and is rejected") at CLI_CONTRACT.md#L56.

Proof (debug CLI at 045d7ec, run twice). I used a project with an empty manifest ({"patches":{}}), --offline --json, and SOCKET_DOWNLOAD_MODE=Bogus or --download-mode bogus|package:

command	exit	result
apply	1	status: error, code: apply_failed, "unknown download mode 'bogus'"
repair	1	code: repair_failed, same message
apply --check	0	success
rollback, list, vendor, vendor --check	0	success
apply --vendor-source bogus (control)	2	clap usage error

So the same typo is:

• a runtime failure under a generic command code (apply_failed, not a usage error) for apply and repair, even when there's nothing to download;
• silently accepted by the other commands;
• carried raw into telemetry (download_mode in telemetry.rs#L743-L755).

For scan and get, a bad value surfaces only in the nested apply, after the patch has been fetched and saved to the manifest. I inferred this from the code path and didn't execute it, because it needs the API.

Symptoms

None filed.

Impact: small. A CI job that exports a misspelled SOCKET_DOWNLOAD_MODE gets exit 1 (a "patch failure") from apply instead of a usage error, and a partially completed scan/get.

Proposed change

• Make the field typed: pub download_mode: DownloadMode, with value_parser = DownloadMode::parse (keep it case-insensitive and keep the blob alias), so clap rejects a bad value with exit 2 for every command.
• Delete the two runtime DownloadMode::parse(..).map_err(..)? calls in fetch_stage.rs and repair.rs, and pass the enum through get's DownloadParams/nested-apply plumbing instead of the String.
• Telemetry takes DownloadMode::as_tag().

Size and scope

• Files: args.rs, fetch_stage.rs, repair.rs, get.rs (the download_mode: String fields), scan/mod.rs, and the telemetry.rs signature.
• Size: ~60 production lines plus test fixture updates (download_mode: "diff".to_string() → DownloadMode::Diff).
• Out of scope: changing the default. That is the separate C25 decision.

Acceptance criteria

• socket-patch <cmd> --download-mode bogus exits 2 with a clap usage error for every subcommand, and so does SOCKET_DOWNLOAD_MODE=bogus.
• --download-mode package stays rejected, with the "was removed; use diff or file" text.
• FILE, Diff and blob still parse, as they do today.
• No DownloadMode::parse call remains outside the clap value parser.
• Regression test: parse --download-mode bogus for each subcommand in args.rs tests.
• The existing test_download_mode_parse and the fetch_stage/repair tests stay green.

Dependencies

None. It is compatible with the C25 decision (a --download-mode default change), and makes it easier.

[mikolalysenko]

[agent] Correction to the Source line: the review already notes that --download-mode is an unvalidated String (Part 2, "Every global flag is silently accepted by every command"). It lists "validate --download-mode at parse time" inside the MAJOR R8 bundle (per-command flags).

This issue is the narrow, non-breaking slice of that work. It adds the execution proof that apply and repair fail a typo with exit 1 and a generic apply_failed/repair_failed code, while every other command accepts it.

Rejecting an invalid value with exit 2 matches the enum the contract already documents, so it needs no contract change. Rejecting flags a command doesn't use stays in R8 (register C34/C35).

---

Generated by Claude Code

[mikolalysenko]

[agent] Triaged: priority:p3 (cross-cutting CLI). It's related to #793/#794 (global-flag plumbing), but the cause is separate: download_mode is an unvalidated String and gets parsed late. It can be fixed on its own. No open PR covers it.

---

Generated by Claude Code