Remove --download-mode and the diff download path (#792)

[Mikola Lysenko (mikolalysenko)]

Removes the diff download path and the --download-mode flag. Patch content is now always fetched as per-file blobs.

Why

On #792 the maintainer said: "We should try to clean up older and deprecated options we don't need any more." The decision on the issue is option A, done fully in v5. v5 is not stable yet, so we skip the one-major diff alias and remove the diff path and the flag in one PR. With diff gone, file would be the only value, so the flag would do nothing.

What changed

Core

• Deleted patch/diff.rs, the diff branch of apply, AppliedVia::Diff and PatchSources::diffs_path.
• Deleted DownloadMode, fetch_missing_sources and get_missing_archives from api/blob_fetcher.rs. The download loop fetches only blobs, and repair calls fetch_missing_blobs directly.
• Deleted ApiClient::fetch_diff and package::read_archive_filtered. The rest of package.rs stays, because vendor, hosted npm and vlt heal use it.
• Removed qbsdiff and its profile blocks (qbsdiff, bzip2, libbz2-rs-sys, suffix_array) from every Cargo.toml and from Cargo.lock.
• .socket/diffs is swept whole, like .socket/packages. cleanup_unused_archives is gone; only its own tests used it.
• apply_package_patch and apply_go_redirect no longer take a uuid argument. It only fed the diff lookup (about 40 call sites).
• Removed apply's mismatch-blob top-up. Staging now fetches every afterHash blob before apply runs, and apply stops before writing if any download fails, so that code could never fetch anything.

CLI

• --download-mode and SOCKET_DOWNLOAD_MODE are removed with no alias.
• Staging in fetch_stage.rs fetches blobs only, and repair has no second download pass.
• appliedVia is always "blob".

User-visible changes

• Passing --download-mode to any command is now clap's unknown-argument error (exit 2). A leftover SOCKET_DOWNLOAD_MODE is ignored.
• A cold-cache apply/get/scan/repair makes no /diffs/ requests.
• .socket/diffs/ is no longer read. repair, scan --prune, remove and rollback sweep every archive in it. The cleanup output still says "N unused diff archives" when it removes old ones.
• --json appliedVia no longer has the "diff" value.
• Kept for shape stability: repair's Downloaded/Verified event keeps details.mode, and the patch_fetched telemetry event keeps download_mode. Both are now always "file". Dropping them is the alternative if reviewers prefer it.

Docs

• CLI_CONTRACT.md: removed the flag and env rows, added a "removed in v5.0" note, made appliedVia "blob" only, updated the GC, prune and repair-event text, and changed the defaults example to name --vendor-source.
• docs/migrating-to-v5.md: added a Retired spellings row and rewrote the closing note about .socket/diffs/.
• CHANGELOG.md is unchanged.

Tests

• Deleted diff_e2e.rs, diff_created_file_e2e.rs and every diff and download-mode case.
• Added parse tests that apply --download-mode file|diff and repair --download-mode file exit 2.
• Added cold-cache checks to apply_network, the fetch-stage covgap test and the repair lifecycle test: no request path contains /diff and no .socket/diffs is created. The repair test also checks that a stale .socket/diffs/<referenced-uuid>.tar.gz is swept.
• cargo test -p socket-patch-core --no-fail-fast: 6119 passed, 1 failed. The failure is utils::digest::tests::production_digests_go_through_the_helpers, which also fails on main and names files this PR doesn't touch.
• cargo test -p socket-patch-cli: the lib, the 35 integration targets this PR touches, rollback, vendor, spawn_env_hygiene, in_process_remove_repair_lifecycle and the coverage_fix_*_silent_mute_exit targets all pass. The lib, rollback, spawn_env_hygiene, in_process_python_envs and the core api::client tests were re-run after rebasing onto main.
• cargo tree shows no qbsdiff.
• Not run locally: docker e2e and a full cargo test --workspace. CI covers those.
• Clippy with -D warnings reports one warning, an unused unix_default in python_crawler.rs. It comes from main, not this PR.

Review findings fixed

• tests/spawn_env_hygiene.rs still listed the deleted diff_created_file_e2e.rs in PENDING_RAW_SPAWNS, which would have failed no_new_bare_binary_spawns. Removed it.
• Repair's download-message unit tests used the diff-archive noun for output repair can no longer produce. They now use the blob noun.
• Removed stale comments that pointed to deleted code in rollback.rs, rollback_multicopy_blob_gate.rs, in_process_remove_repair_lifecycle.rs, golang_local.rs and args.rs.
• rustfmt also reformatted a few unrelated lines in vendor.rs and telemetry.rs. They are harmless.

Coordination

#966 touches the same files (args.rs, get.rs, scan/mod.rs, repair.rs, both docs, the cli_parse_* tests). Whichever PR lands second rebases, and both add rows to the Retired spellings table. Blob retry (#676) now covers one artifact kind, because diffs are gone.

Closes #792
Closes #791 (there is no --download-mode value left to validate)

🤖 Generated with Claude Code

---

Note

Medium Risk
Major v5 behavior change to how patches are fetched and applied across apply, get, scan, and repair; incorrect blob staging would fail applies that previously could fall back to diffs, though tests and contract updates cover the new blob-only path.

Overview
v5 removes the diff download path entirely. Patch content is always staged and applied from per-file blobs under .socket/blobs/; legacy .socket/diffs/ and .socket/packages/ are never read and are swept by repair, prune, remove, and rollback.

The global --download-mode flag and SOCKET_DOWNLOAD_MODE are gone (usage error if passed; env ignored). Staging (fetch_stage), repair, and agent download flows no longer branch on diff vs file or call diff/package archive endpoints. The qbsdiff dependency and patch/diff module are deleted, along with apply’s mismatch-blob top-up and writable_blobs overlay logic—staging must fetch every required afterHash blob up front.

JSON and UX now report appliedVia: "blob" only (no "diff"). apply_package_patch / apply_go_redirect drop the unused uuid parameter. Docs (CLI_CONTRACT.md) document the v5.0 breaking removal and updated GC/prune behavior.

^{Reviewed by Cursor Bugbot for commit df96c49. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] CI status: all green on 1357bcf (552 pass, 6 skipped).

• Fixed: CodeQL rust/cleartext-logging alert in crates/socket-patch-cli/tests/remove/remove_duality_invariants.rs. The new assert message interpolated the archive uuid, so it now labels the entries A/B instead (3be03b0).
• Infra, not this PR: hosted-e2e failed when corepack couldn't fetch yarn from registry.npmjs.org. The Windows Gradle multi-project e2e failed on a Maven Central 403. The CodeQL Analyze (rust) SARIF upload also failed, and default-setup runs can't be re-run, so an empty commit (1357bcf) retriggered the checks. All of them passed on the new head.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Merged main, CI green; ready for review.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent).

• Head: 9fc3cd30571fa4e08089138f0272e167e966ff3d
• CI: 520/520 check runs green (success/skipped/neutral) on this head, mergeable, no conflicts.
• Bugbot: reviewed this head (Cursor Bugbot check: success), no unresolved review threads.
• Changelog: untouched.

Nothing specific flagged for the reviewer beyond the PR description.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[final reviewer] Tanmay Singla (@Tanmay182003) One non-merge commit landed after your approval on 9fc3cd30: 6773eaa Keep per-blob download failure reasons under --silent apply (a burn-down fix for Bugbot's finding that apply --silent stopped printing which blob failed and why). It isn't enqueued until you take another look at head 6773eaa and CI is green.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Ready for review at 6773eaa6ac

• CI: all 466 check runs on the current head green or skipped (ci-ok success). sbt 1.2.8 / jdk 8 / agent failed once in "Run the agent group" and passed on a single re-run (run 37852597217, attempt 2), so it was a flake.
• Mergeable against main, no conflicts; no CHANGELOG.md change.
• Cursor Bugbot reviewed 6773eaa6; every review thread is resolved (the fetch_stage.rs finding is addressed by 6773eaa6).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Merged main (03b9418) to clear conflicts with #1031; head is now 01e4cb7. Removed Ready for review until CI is green on the new head. Conflict notes: migrating-to-v5 table keeps both #1031's removals and this PR's --download-mode row; CLI_CONTRACT events row uses main's repair name with this PR's mode: "file" content; cli_parse_repair keeps repair_download_mode_flag_is_removed. Merge fallout: a main-added test in manifest/schema.rs dropped the uuid arg this PR removed from apply_package_patch; removed main's unused FileEdit import in vlt_heal tests. Local: workspace check clean; cli 4620 pass / core lib 5798 pass, remaining failures are root-only chmod tests and one network test in the sandbox.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Re-labeled Ready for review at 01e4cb70fe (main merge 03b9418, which cleared the #1031 conflict).

• CI: all 466 check runs on the head are success/skipped (ci-ok green).
• Bugbot: Cursor Bugbot passed on 01e4cb7; no unresolved review threads.
• Mergeable, no CHANGELOG.md change. It needs a fresh human approval: 6773eaa landed after the earlier one.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Re-labeled Ready for review at 207ad36 (main merge of #1008 since the last label at 01e4cb7).

• CI: 466/466 check runs on this head are success/skipped. gradle 9.8.0 / jdk 21 / vendor / configuration-cache failed once in its "Install Maven 3.9.16" setup step (curl exit 22, before any test ran); one re-run passed.
• Bugbot: reviewed 207ad36, no findings; no unresolved review threads.
• Mergeable, no conflicts. No code changes since the previous label other than the main merge.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[final reviewer] The merge queue evicted this at 07:38 and it was CONFLICTING with main. I merged origin/main at 4d8a5d2: conflicts were in commands/get.rs and commands/repair.rs, where #1041 moved telemetry onto TelemetryAuth. I kept main's TelemetryAuth and org-warning code and dropped the download_mode binding and PatchSources import that only served the diff path this PR removes. cargo check --all-targets on both crates and cargo test -p socket-patch-cli --lib (890 passed) are clean locally.

Not re-enqueued: 6773eaa (an own commit) still landed after Tanmay Singla (@Tanmay182003)'s approval on 9fc3cd30 and hasn't been re-approved (see the earlier comment). Once it is re-approved and CI is green on 4d8a5d2, the next run sends it to the queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[final reviewer] CI on 4d8a5d2 (my main merge) was red: test (windows-latest, 1), coverage and test-release all failed in -p socket-patch-cli --test repair. repair_json_reports_unresolved_org_fallback_in_warnings came in from main with #1041 and still passed --download-mode file, which this PR removes, so repair exited with unexpected argument '--download-mode'.

Fixed in df96c49: the test drops the two flag lines (repair always fetches per-file blobs now). Locally, --test repair passes 125/127. The other two (…lock_file_unremovable, …cleanup_failure_is_reported…) fail the same way on main here because this sandbox runs as root, so they aren't this PR's.

Tanmay Singla (@Tanmay182003) this is a second non-merge commit after your approval on 9fc3cd30 (2 test lines, no product code), together with 6773eaa from before. Please take another look when you can. I removed Ready for review. The burn-down re-labels it once CI is green.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit df96c49. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent).

• Head: df96c49
• CI: all green (16 skipped, 453 success, total 469)
• Bugbot: reviewed df96c49, no new findings; no unresolved review threads
• Mergeable, no CHANGELOG.md changes.

---

Generated by Claude Code