Write every utils::fs atomic file through one stage-and-rename core (#728)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #728

Summary

Every atomic file write in utils::fs now goes through one blocking core, stage_and_rename_blocking. The core owns the stage name, the stage's creation mode, the fsync, the order of setting the mode and then renaming, the unlink on error, and the directory fsync. Before this, the async writer (stage_and_rename + create_stage + commit_stage) and the blocking atomic_write_sync copied that code line for line. Three places built stage names. Now one stage_path(path, prefix) builds them, and the blob cache's .socket-dl- stage uses it too.

Why

• Issue #728, register row C21 (register). With two copies, every hardening fix had to land twice. The .npmrc stage-mode fix only exists in both copies because someone remembered to apply it to each.
• Leverage: B 0 · U 1 (#726's get blob writer reuses this) · D ≈3 (2 writer copies → 1, 3 stage-name builders → 1, 4 permission prologues → 1) · R L. This is the top eligible item in register/90-refactor.md's Queue. The higher-scoring rows touch files that open PRs are changing.

What changed

• utils/fs.rs:
  • A private WriteOpts { capture, durable, preserve_mode, record } struct, with three named policies: COMMIT_POINT, ARTIFACT and UNSYNCED.
  • One async write_atomic, which handles group-commit capture and the durability barrier, then runs the core on run_blocking and records the path for the next barrier.
  • One blocking stage_and_rename_blocking.
  • The six public writers are now 1–10 line wrappers, so none of their ~100 call sites change. atomic_write_sync is the core with durable: true.
  • stage_open_options replaces create_stage.
• api/blob_fetcher.rs: the streaming cache writer builds its stage name with utils::fs::stage_path(dest, ".socket-dl-"). Since Stream patch blob and diff downloads to disk (#571) #607 that writer streams the body and checks the hash between stage and rename, so it keeps its own body (the reason is noted on the issue).

Deleted

create_stage, atomic_write_bytes_as, the async stage_and_rename / commit_stage pair, the copy inside atomic_write_sync, four repeated "read the destination's permissions" prologues, and the stage-name code in blob_fetcher.

• git diff --stat: 2 files, +256 / −180.
• Production code is +171 / −168: the deleted copies are offset by the WriteOpts table and its docs.
• Tests are ≈ +85 / −12.
• Grep: .socket-stage- and .socket-dl- are now formatted in exactly one production function (stage_path).

Behavior

None. The bytes, modes, fsync points, stage-name shape and error cleanup are all the same:

• Off Unix every stage is still fsynced.
• The directory fsync is still Unix-only and only runs for durable writes.
• The barrier still runs only before a commit point.

Two internal differences, neither observable in the output:

• Copies and pool hops. The async writers copy content once into the blocking closure. They now make one hop to the blocking pool instead of one per tokio fs op.
• Cancellation. A dropped write future now finishes its stage + rename instead of possibly leaving a stage behind. The destination is still either all old or all new.

The blob stage's fallback stem for a path with no file name is now file instead of blob. That path can't occur, because blob destinations are always <dir>/<hash>.

Tests

• New every_writer_shares_one_stage_and_rename_core runs all six writers (with both preserve_mode arms for unsynced and sync) on a 0600 destination. Each writer writes the exact bytes and leaves no stage behind. A writer keeps the 0600 bits exactly when its policy preserves mode, so the blocking and async writers agree.
• New stage_path_is_a_unique_hidden_sibling.
• The existing stage-mode test now goes through stage_open_options. The RLIMIT_FSIZE torn-write child test, group-commit replay and capture tests, and durability tests still pass.

Commands:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --lib: 5027 passed. 4 failed, the known root-only tests that fail on main too (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files).
• cargo test -p socket-patch-core --test blob_fetcher_edges_e2e --test covgap_api_blob_fetcher: 30 passed.
• cargo test -p socket-patch-cli --all-features --test vendor_group_commit_e2e --test in_process_agent_reapply --test cli_apply_silent: 21 passed.
• cargo test --workspace --all-features: not completed locally, because the sandbox ran out of disk linking the 145+ CLI test binaries. CI covers it:
  • pass: test (ubuntu-latest), test (windows-latest) and every e2e job;
  • fail only in -p socket-patch-cli --lib: test (macos-latest), test-release and coverage. The failures are the two vex_consumed tests that fail on main too, which Fix vex alias tests broken by store-copy merge #851 fixes (see the comment below).

Risk

L. This is the crash-safety write path, so the risk is in the details. The fsync, mode and rename order is copied from the existing blocking writer, which the group-commit replay already used, and the tests above pin each policy.

---

Note

Medium Risk
Touches the crash-safe patch and cache write path (fsync, rename, permissions), though behavior is intended to stay the same and new tests lock each writer policy.

Overview
Fixes #728 by routing every utils::fs atomic writer through a single blocking stage_and_rename_blocking path, with async entry points delegating via write_atomic and a WriteOpts policy table (COMMIT_POINT, ARTIFACT, UNSYNCED) for capture, durability barrier, fsync, mode preservation, and artifact recording.

The duplicated async stack (create_stage, commit_stage, stage_and_rename, atomic_write_bytes_as) and the extra copy inside atomic_write_sync are removed; the six public writers become thin wrappers. Stage file names are built in one place: stage_path(path, prefix), and the blob cache downloader in blob_fetcher now uses it with the .socket-dl- prefix instead of local formatting.

Tests add coverage that all writers share the same core (bytes, modes, no stage litter) and that stage_path produces unique hidden siblings for both .socket-stage- and .socket-dl- prefixes.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] The coverage check fails in -p socket-patch-cli --lib, on two tests: commands::vex_consumed::tests::hosted_expands_alias_only_copies and hosted_reuses_expanded_npm_copies_and_merges_alias_variants. That failure doesn't come from this PR:

• coverage is red on main @ 4646693 too (run).
• The same two tests fail locally on this branch.
• This diff touches only utils/fs.rs and api/blob_fetcher.rs in core.

The cause is #605's store-copy merge. #851 fixes these tests, and #850 adjusts the same file. I'm not porting that fix here, because this routine never edits a file that open PRs are already changing. CI will go green here once #851 merges and main is merged in.

---

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

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review.

• Head: c5118ba2185375ec29a851481bad85800ecc909e
• CI: 338/338 check runs green (success/skipped) on this head; mergeable clean
• Bugbot: reviewed c5118ba, no new issues; no open threads
• Reviewer focus: refactor only: single stage_and_rename_blocking core in utils::fs; path-filtered workflows did not run (338 checks)

---

Generated by Claude Code