Decide: make --download-mode file the default and retire the diff download path

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

Kind: decision. Source: review Part 7.4 and R10; register C25.

Question

--download-mode diff is the default, but on a cold cache it downloads every diff archive and then every blob anyway. Should file become the default, and should the diff download path be retired?

Options:

A. Make file the default now (a MAJOR per CLI_CONTRACT.md#L1578). Keep diff as an accepted value that behaves like file for one major, then delete the diff machinery. Recommended.
B. Keep diff as the default, but stop the redundant blob top-up. Fetch blobs only for the files whose diff is missing or for created files, as is already done when every archive is cached. That keeps all the diff code and fixes only the bytes.
C. No change.

Problem (verified at `045d7ec`)

Default: default_value = "diff" (args.rs#L153-L162).
Cold cache: in fetch_stage.rs#L314-L319,`` diff mode downloads whenever any archive is missing. Then the top-up in fetch_stage.rs#L377-L398 sets `blob_scope = manifest` whenever `missing_diff_archives` was non-empty, and fetches every blob still missing from the stage. The diff fetch writes no blobs, so that is all of them.
When diff saves bytes: only when a user has committed .socket/diffs but not .socket/blobs. Then just the blobs of created files are fetched (files_diffs_cannot_cover).
Code that exists only for diff:
- patch/diff.rs (99 production lines, bspatch);
- patch/package.rs (332 production lines, diff archive tarball reader);
- fetch_missing_diff_archives and get_missing_archives in api/blob_fetcher.rs;
- resolve_from_diff / AppliedVia::Diff in patch/apply.rs;
- the diff branches of fetch_stage.rs (patches_without_source, files_diffs_cannot_cover, the top-up, deferred failures) and repair.rs;
- the qbsdiff dependency in both crates.
Overlap with other work: diff archives are fetched sequentially with no retry (Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676 tracks retry), and the streaming work in Stream patch blob and diff downloads to disk (#571) #607 has to handle both artifact kinds.

Impact: every first apply on a fresh CI checkout pays an extra round-trip per patch for archives it then backs up with blobs. Removing the path deletes roughly 700 production lines and a dependency.

Proposed change (option A)

Change the default to file (clap default_value, the GlobalArgs::default(), CLI_CONTRACT.md line 56 and the env table, and the changelog as a MAJOR). diff stays accepted.
Next major: make diff an alias of file, and delete patch/diff.rs, patch/package.rs, the diff fetch and coverage code in blob_fetcher.rs/fetch_stage.rs/repair.rs, AppliedVia::Diff, PatchSources::diffs_path and qbsdiff. repair keeps sweeping orphaned .socket/diffs/* once, then the directory is ignored.

Size and scope

Step 1: ~20 lines plus docs.
Step 2: about −700 production lines and −1,000 test lines, and it lands as one PR.
Out of scope: blob retry (Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676) and streaming (Stream patch blob and diff downloads to disk (#571) #607).

Acceptance criteria

The owner picks an option.
(A, step 1) socket-patch apply on a cold cache issues blob GETs only; a test asserts that no /diffs/ request is made with the default.
(A, step 2) cargo tree -p socket-patch-cli lists no qbsdiff; the apply/repair suites stay green with diff fixtures removed.
(B) A cold-cache diff-mode test asserts that blobs are fetched only for the files no fetched archive covers.

Dependencies

Interacts with An invalid --download-mode fails apply and repair with exit 1 but is accepted by every other command #791 (typed --download-mode), Stream patch blob and diff downloads to disk (#571) #607 and Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676.
Blocks nothing.

Consolidated work — backlog review, 2026-10-08

The following standalone issues are now tracked here. Their closure consolidates scheduling; it does not mean their implementation is complete. Original reports and discussion remain linked below.

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

Track the remaining scope from An invalid --download-mode fails apply and repair with exit 1 but is accepted by every other command #791 in this issue. The accepted decision and open PR Remove --download-mode and the diff download path (#792) #1049 retire --download-mode. Preserve validation/removal expectations there rather than a separate issue for the soon-to-be-removed option.

The accepted removal in #792 / PR #1049 supersedes the old proposal to preserve and validate --download-mode. Finish that removal consistently across CLI arguments, environment configuration and core call sites, and test the resulting removal/unsupported-input behavior. Do not restore the option to satisfy its historical acceptance criteria.

Preserved scope and acceptance criteria from #791

Historical, superseded: 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().

Historical, superseded: 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.

Historical, superseded: 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.

audit-core: C25 decision #792, C10 tracking #793/#794, new C45 (#791)

I think this seems fine. We should try to clean up older and deprecated options we don't need any more.

[agent] Decision: option A, done fully in v5. This follows the maintainer's comment above and the cleanup direction in #966. v5 is not stable yet, so we skip the "keep diff as an alias for one major" step and remove the diff path and the flag in one PR.

What changes

Patch content is always fetched as per-file blobs. A cold-cache apply/get/scan --mode agent/repair makes no /diffs/ requests.
--download-mode and SOCKET_DOWNLOAD_MODE are removed. With diff gone, file would be the only value, so the flag would do nothing. Scripts that pass --download-mode … will get clap's unknown-argument error (exit 2). A leftover SOCKET_DOWNLOAD_MODE env var is ignored. This also makes An invalid --download-mode fails apply and repair with exit 1 but is accepted by every other command #791 moot, since there is no longer a value to validate.
.socket/diffs/ is no longer read. repair, scan GC, remove and rollback treat it like .socket/packages/: every archive in it is obsolete and gets swept.
--json appliedVia is always "blob". The "diff" value is removed from the contract.

Code removed (main @ 8cf19108)

core: patch/diff.rs and pub mod diff; resolve_from_diff, the diff-first branch of apply.rs (~L1011-1060) and AppliedVia::Diff; PatchSources::diffs_path; DownloadMode, DIFF_ARCHIVE, get_missing_archives, fetch_missing_diff_archives and the mode switch in fetch_missing_sources (api/blob_fetcher.rs); ApiClient::fetch_diff; package::read_archive_filtered; and qbsdiff in both crates' Cargo.toml and in the workspace Cargo.toml (including the [profile.dev.package.qbsdiff] block).
cli: the download_mode field in args.rs (L153-162, the Default impl and the empty-env list); the diff branches of fetch_stage.rs (patches_without_source, files_diffs_cannot_cover, the top-up at L377-398, deferred failures); the DownloadMode match and the created-files second pass in repair.rs; download_mode forwarding in get.rs/scan/mod.rs; and Diff in json_envelope.rs.
telemetry: track_patch_fetched loses its download_mode parameter. It sends "download_mode": "file" as a constant so the event shape stays stable.

One correction to the issue: patch/package.rs is not diff-only on current main. Its archive readers are used by vendor/*, hosted/npm_manifest.rs and redirect/vlt_heal.rs, so it stays. Only read_archive_filtered (used only by the diff path) is deleted. The saving is therefore smaller than the issue's ~700 lines, but the dependency and the extra round-trip per patch still go away.

Docs

CLI_CONTRACT.md: remove the --download-mode row (L56) and the SOCKET_DOWNLOAD_MODE env row (L1026); reword L652 to say vendored runs stage no blobs; change appliedVia (L1176) to "blob"; extend the GC note (L822) to say .socket/diffs archives are obsolete and swept; drop --download-mode from the defaults example in L1679.
docs/migrating-to-v5.md: add --download-mode / SOCKET_DOWNLOAD_MODE to "Retired spellings" (replacement: none, blobs are always used), and change the closing note to say .socket/diffs/ and .socket/packages/ are no longer read and are removed by cleanup.
No CHANGELOG edit (release agent).

Tests

Delete core/tests/diff_e2e.rs, cli/tests/diff_created_file_e2e.rs, the diff tests in apply.rs/fetch_stage.rs/repair.rs/blob_fetcher.rs, the fetch_diff cases in api_timeout_e2e.rs, and the --download-mode parse tests.
Rewrite the ~50 test files that set download_mode or mock /diffs/ so they use blobs.
Add a cold-cache apply test that asserts no /diffs/ request is made, and a parse test that asserts --download-mode is rejected with exit 2.
Acceptance: cargo tree -p socket-patch-cli shows no qbsdiff, and cargo test --workspace is green.

Coordination: #966 removes other get/scan flags and touches the same files (args.rs, get.rs, scan/mod.rs, CLI_CONTRACT.md, migrating-to-v5.md, cli_parse_*). Whichever PR lands second rebases, and both add rows to the same "Retired spellings" table. Blob retry (#676) gets simpler because it now covers one artifact kind.

A PR implementing this will follow.

Generated by Claude Code

maintainer-triage: record 2026-10-07 decisions

[agent] Implemented in #1049.

Generated by Claude Code

Stop logging the archive uuid in a remove test assert (#792)

Retrigger CI after a CodeQL SARIF upload infra failure (#792)

v5 release blocker (P1). Finish the already accepted v5 removal in PR #1049 before publishing the interface. Include the removed flag/env spelling and legacy diff-only-state migration in the v5 migration contract.

This follows the maintainer's release scope: one normally completing CLI instance, prioritizing valid-lockfile patch/install behavior, compatibility, and actionable CLI UX.

Navigation Menu

Repository navigation

Decide: make --download-mode file the default and retire the diff download path #792

Question

Problem (verified at `045d7ec`)

Proposed change (option A)

Size and scope

Acceptance criteria

Dependencies

Consolidated work — backlog review, 2026-10-08

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

Historical, superseded: Proposed change

Historical, superseded: Size and scope

Historical, superseded: Acceptance criteria

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Decide: make --download-mode file the default and retire the diff download path #792

Description

Question

Problem (verified at 045d7ec)

Proposed change (option A)

Size and scope

Acceptance criteria

Dependencies

Consolidated work — backlog review, 2026-10-08

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

Historical, superseded: Proposed change

Historical, superseded: Size and scope

Historical, superseded: Acceptance criteria

Activity

mikolalysenko commented on Oct 7, 2026

mikolalysenko commented on Oct 7, 2026

mikolalysenko commented on Oct 7, 2026

mikolalysenko commented on Oct 9, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Problem (verified at `045d7ec`)