Stream patch blob and diff downloads to disk (#571)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #571

Summary

Patch blob and diff downloads now stream chunk by chunk into their .socket-dl-* stage file. They are no longer buffered whole in memory first. ApiClient::fetch_blob and fetch_diff return a BinaryBody stream, and the two near-identical download loops in api/blob_fetcher.rs are merged into one. As the maintainer asked on #571, there is no size cap.

Why

• Issue: #571, register row C37 in register/20-audit-core.md. The living document covers the HTTP client in Part 7 (doc/07-infra-agent.md).
• Leverage: B 1 (Patch blob and diff downloads buffer the whole response body with no size cap #571), U 0, D 1 (two download loops become one, part of C15's "two near-identical downloaders"), R M. Score 3 + 1 − 2 = 2.
• Ranking: the higher-ranked candidates (Vendored→hosted takeover drops the vendored ledger entry when the revert drift-keeps the artifact #568, Hosted NuGet rewrites packages.lock.json entries at other versions of the patched id #593, E37) all change files that open PRs Fix hosted scan from a workspace member pinning nothing or the wrong files (#590, #417) #598, Unify hosted NuGet routing and XML splice anchors #597 and Read NuGet and Cargo crawler project files through the FIFO-safe reader (#592) #602 also change. C07 (URL builders disagree on a missing org slug) needs an owner decision on the fallback route.

What changed

• api/client.rs:
  • fetch_binary returns the unread 200 response as a BinaryBody.
  • BinaryBody::chunk() yields the next chunk. A failed or stalled read maps to the same ApiError::Network("Error reading {kind} body for {id}: …") as before.
  • Status classification (404 → None, 401/403/429, other) is unchanged.
• api/blob_fetcher.rs:
  • stream_cache_entry_atomic replaces write_cache_entry_atomic. It copies the body into the stage file one chunk at a time. For blobs it then hashes the staged file with compute_git_sha256_from_reader, and finally renames it into place.
  • On any failure it removes the stage file and every cache directory the call created that is still empty. That includes ancestors that create_dir_all made before it failed. This keeps the old guarantee that a fetch that lands nothing leaves no .socket/blobs/ or .socket/diffs/ behind.
  • download_entries(…, Entry::Blob | Entry::Diff) replaces download_hashes and the inline diff loop in fetch_missing_diff_archives.
• CHANGELOG.md: one Fixed entry.

Deleted

• The second download loop.
• The buffered resp.bytes() read in fetch_binary.

From git diff --numstat origin/main, src lines split at mod tests:

• production: about +193 / −160;
• tests: about +304 / −12.

Behavior

None, apart from memory use. Error strings, downloaded/failed counts, the exit codes, the JSON and the no-litter guarantees are unchanged.

One corner case changes. If the cache directory can't be created and the server serves bytes that don't match the hash, the entry now reports "Failed to write blob to disk" instead of "Content hash mismatch". The mismatch can only be known after the bytes are on disk.

Test evidence

• New blob_fetcher_edges_e2e::blob_and_diff_bodies_reach_disk_before_the_response_completes: a raw TCP server sends the first 256 KiB of a 512 KiB body, then holds back the rest. The test asserts that those 256 KiB are already in the stage file, for both blob and diff mode.
  • Red on main (src restored to origin/main): FAILED … panicked at blob_fetcher_edges_e2e.rs:849.
  • Green on this branch, on Linux, macOS and Windows.
• New failed_streams_leave_no_stage_and_no_created_cache_dir: covers a body cut short mid-stream (blob and diff), and a streamed blob whose content doesn't match its hash. In each case there is no stage file and no created .socket/ or .socket/blobs/.
• New partly_created_cache_dirs_are_removed_on_failure, for the Bugbot finding: its blobs path is .socket/ plus a 300-byte leaf, so .socket/ is created and then the leaf fails. It fails before 3362893 and passes after.
• New api_timeout_e2e::stalled_binary_bodies_are_network_errors_on_both_clients: the idle read bound fires on a streamed chunk read, on both reqwest clients.
• Commands run:
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • The core suites blob_fetcher_edges_e2e, api_timeout_e2e, binary_fetch_error_classification_e2e and covgap_api_blob_fetcher: all pass.
  • cargo test -p socket-patch-core --lib: passes, apart from the 4 known root-sandbox permission tests, which fail on main too.
  • The CLI suites apply, remove, cli, get, repair, diff_created_file_e2e and remove_rollback_api_overrides: all pass, apart from 2 repair tests that chmod a directory read-only (root ignores the mode bits). Both pass in CI.
• CI on ae928ae: all 338 checks are green, and Bugbot is clean.

Risk

M. Every apply/get/repair/rollback download goes through this path. Mitigations:

• The stage, hash and rename discipline is unchanged.
• File handles are closed before the rename, which Windows requires.
• The full CI matrix, including Windows, is green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V8joXSoT8cmkgc1AVv1wrq

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

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 ae928ae. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at ae928ae91c2b1c57d9e5367d1d0c72bd66424651.

• CI: 338/338 check runs completed on this head — 332 success, 6 skipped, 0 failing.
• Bugbot: reviewed ae928ae — no new issues; 0 unresolved review threads (earlier finding about partly created cache dirs was fixed in 3362893 with a regression test).
• Branch is up to date with main (0 behind) and mergeable.
• Reviewer focus: api/blob_fetcher.rs stream_cache_entry_atomic cleanup-on-failure path (stage file + empty created cache dirs), and the one changed error string when both the cache dir can't be created and the hash mismatches.

Slack announcement: not sent (Slack send tool unavailable this run).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Reviewed ae928ae91c2b1c57d9e5367d1d0c72bd66424651. Ready to merge as-is; no actionable findings.

Checked streamed hashing and atomic publication, body-error/timeout cleanup, cache-directory ownership, and production callers. The lack of a size cap matches the maintainer’s direction in #571.

Verification: 206 tests passed across the focused API integration suites (41), blob/hash unit tests (45), repair (116), rollback API overrides (2), and two additional reviewer tests. The reviewer controls cover unknown-length chunked and empty bodies, truncated framing, idle timeouts, and preservation of existing cache content on both authenticated/public clients and blob/diff paths. Production files remained unchanged.

Merge check against current main is clean. CI on this commit is complete: 332 successful checks, 7 skipped, all eight workflows finished successfully or skipped, and Bugbot is clean with no unresolved threads. Local tests ran on macOS; platform coverage also comes from CI.