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

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

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.

Backlog review — 2026-10-08

Consolidated into #792. The retained tracker(s) preserve this issue’s implementation scope and acceptance criteria. Closing this separate scheduling item as not planned, not as completed.

The accepted decision and open PR #1049 retire --download-mode. Preserve validation/removal expectations there rather than a separate issue for the soon-to-be-removed option.