Atomic writes: convert bun init and test a write that fails

[robobun]

Stacked on #39689, which adds File::write_file_atomically and converts the package.json and snapshot writers. This PR holds what that branch leaves out: one more writer, and tests in which the write itself fails.

Problem

• bun init on a project with a package.json writes the new text over the old bytes and truncates afterwards (init_command.rs, 'write_package_json). A write that stops early leaves the start of the new file in front of the rest of the old one: package.json failed to write due to error EFBIG, file changed.
• Make concurrent bun test and bun install processes safe on shared files #39689 tests writers that succeed. Nothing tests a write that fails, which is what the atomic write is for.

Fix

• bun init writes package.json with write_file_atomically, after it closes the handle it read the file with. The Windows-only save and restore of that handle's position around the read is deleted: it only served the write through the same handle.
• A failed .snap write at the end of a serial bun test run sets the exit code, like a failed inline snapshot write. It used to leave main as an internal error. The --parallel worker (test/parallel/runner.rs, worker_flush_aggregates) still drops both results and exits 0. That needs a worker to coordinator message and is left out here on purpose. It is reported separately.
• Verified: withFileSizeLimit(blocks, cmd) in test/harness.ts runs a command under ulimit -f. New tests in bun-pm-pkg, bun-pm-version, init and new-snapshot.test.ts check that the old file is intact and no .tmp file is left, after EFBIG and after a process.exit() under --update-snapshots. All fail on 1.4 canary. On Make concurrent bun test and bun install processes safe on shared files #39689 alone the init test and the exit code assertion fail.

Background

• Bun ignores SIGXFSZ, so a write past RLIMIT_FSIZE returns EFBIG. A limit of 0 fails every write at once, which exposes a truncate-then-write writer. A limit of one block makes Linux cut the write short, which exposes a write-then-truncate writer. The tests only check that the file is intact, so they also pass on macOS, where the whole write fails.
• write_file_atomically in Make concurrent bun test and bun install processes safe on shared files #39689 returns the write error without trying the in-place fallback, so these tests hold with its fallback in place.
• bun create still writes in place. It rewrites the package.json it has just copied.

Notes

The first version of this PR also made write_file_atomically keep the owner of the old file. #39689 took that change (35bb9d3), together with a fix for its two-writer test, so this PR was rebuilt on top of it as one commit without the file.rs part. The review notes on the earlier commits (drained pipes, one-line comments, the deleted Windows seek pair, the --parallel exclusion) are all folded into the current commit.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/init/init.test.ts test/cli/install/bun-pm-pkg.test.ts test/cli/install/bun-pm-version.test.ts test/js/bun/test/snapshot-tests/new-snapshot.test.ts

[robobun]

CI for 5ae73a4 (build 101451): 178 of 179 jobs pass. The failed job is Windows 2019 x64, and its only failure is test/bake/deinitialization.test.ts, which fails on main as well and is reported separately. The new tests in init.test.ts, bun-pm-pkg.test.ts, bun-pm-version.test.ts and new-snapshot.test.ts pass on every lane. The one retried failure in new-snapshot.test.ts is the two-writer test from the base branch, noted on #39689.

Ready for review. It is based on #39689 and should land after it, or be folded into it.

[robobun]

Pushed f93c1c3: the init and pm pkg write-failure tests drain stdout too, as the review asked. Test-only change. The review thread is resolved.

[robobun]

Pushed aaba97f: the two comments in write_file_atomically that the comment check flagged are one line each. No code change. All review threads are resolved.

[robobun]

The review asked for the --parallel exclusion to be stated: the worker in test/parallel/runner.rs still drops the snapshot write results and exits 0. That is pre-existing, needs a worker to coordinator message, and is filed separately. The body now says so. No code change, head is still aaba97f, no threads open.

[robobun]

Pushed 5805943: the Windows-only get_pos/seek_to pair around the package.json read in bun init is gone. It existed for the in-place write this PR removes. cargo check passes for the Linux and Windows targets, and the init tests pass with the debug build. The body names the deletion. No threads open.

[claude]

LGTM — all three prior review points (stdout drain, parallel-worker exclusion note, dead Windows seek pair) are addressed, and the remaining diff is the focused change described.

What was reviewed:

• write_file_atomically: fchown before fchmod under #[cfg(unix)], both best-effort; ordering is correct (chown clears setuid/setgid, chmod restores). Stat field types match the fchown(u32, u32) wrapper on Linux/macOS.
• bun init: fd is .take()-dropped before the atomic write; had_package_json_file preserves the old " + package.json" print semantics on both fresh-init and write-failure paths.
• test_command.rs: .is_ok() folds into the existing exit-code path; the --parallel sibling is now stated in the PR body as filed separately.
• withFileSizeLimit and the four write-failure tests: hermetic, drain both pipes, assert file intact + no .tmp left.

Extended reasoning...

Overview

Stacked on #39689. Touches src/sys/file.rs (adds best-effort fchown to write_file_atomically), src/runtime/cli/init_command.rs (converts the package.json write from write+truncate to write_file_atomically, drops the read fd first, deletes the now-dead Windows position save/restore), src/runtime/cli/test_command.rs (a failed .snap write sets exit code 1 instead of propagating ? out of main), test/harness.ts (withFileSizeLimit helper), and four test files with new write-failure tests.

Security risks

None identified. withFileSizeLimit interpolates a TypeScript number into a shell string, so no injection surface. fchown is best-effort and only restores an owner the caller already had write access to (the open-for-write above would have failed otherwise). No auth, crypto, or path-traversal surfaces touched.

Level of scrutiny

Medium. src/sys/file.rs is foundational, but the change there is six lines under #[cfg(unix)], both calls discarded with let _ =, and mirrors existing safe_libc::fchown(.., st.st_uid, st.st_gid) call sites at lib.rs:7790/9059. The init_command.rs change is a mechanical conversion to the helper #39689 introduced; I traced had_package_json_file against the old package_json_file.is_some() logic and both the fresh-init and write-failure paths match. The test_command.rs change turns an internal-error ? into a proper exit-code-1, which the new snapshot test asserts.

Other factors

This PR has been through three review rounds from me; each was a nit and each was addressed (f93c1c3 drains stdout, aaba97f shortens comments, 5805943 deletes the dead seek pair I flagged). CI on 5ae73a4 was green except a known-failing-on-main Windows bake test; the three subsequent commits are test-only, comment-only, and a 4-line deletion I requested. No CODEOWNERS cover the touched files. The author kept O::RDWR on the read handle with a stated reason (fail-fast on an unwritable file), which is reasonable and preserves existing behavior. All review threads are resolved.

[robobun]

Rebuilt on the new head of #39689 (35bb9d3), which took the owner-preserving change from here and fixed its two-writer test. This PR is now one commit, 08098ae: the bun init conversion, the exit code for a failed .snap write, the harness helper and the write-failure tests. The import conflicts in bun-pm-pkg.test.ts and new-snapshot.test.ts were the only conflicts. With the debug build on the new base: new-snapshot, bun-pm-pkg and bun-pm-version pass (112 tests), and the init tests pass. The init test now checks for leftover .tmp files by suffix, since the temporary name no longer contains package.json. Body and title updated.

[claude]

LGTM — all three prior review notes are addressed (stdout drained in the two spawn tests, the parallel-worker exclusion is stated in the body, and the dead Windows get_pos/seek_to pair is deleted).

What was reviewed:

• init_command.rs: .take() closes the read fd before the rename; had_package_json_file preserves the old package_json_file.is_some() semantics on both success and error paths.
• test_command.rs: confirmed !write_snapshots_success reaches the exit_code = 1 branch and write_snapshot_file prints its own error before returning Err.
• withFileSizeLimit: -- as $0 + exec "$@" is correct; every caller is skipIf(isWindows).
• write_file_atomically handles the no-existing-file case (ENOENT → existing = None), so bun init in an empty dir still creates package.json.

Extended reasoning...

Overview

Stacked on #39689. This PR's own diff (7 files) converts bun init's package.json writer from write-then-truncate on the read fd to write_file_atomically, closes the read fd via .take() first, and deletes the Windows-only position save/restore that only served the removed in-place write. In test_command.rs, a failed .snap write now folds into write_snapshots_success (→ exit 1) instead of propagating out of main via ?. A new withFileSizeLimit harness helper wraps a command in ulimit -f N && exec "$@", and four new POSIX-only tests use it to prove the old file survives an EFBIG write and no .tmp file is left behind. The fchown-to-preserve-owner change in src/sys/file.rs mentioned in the title lives in the base branch, not this diff.

Security risks

None. No parsing of untrusted input, no auth/crypto/permissions. The ulimit wrapper is test-only and takes a numeric blocks argument interpolated into a fixed shell string.

Level of scrutiny

Medium. bun init and bun test are user-facing, but the change is a strict narrowing of failure modes (atomic replace instead of in-place overwrite; exit 1 instead of internal-error on a failed snapshot write). The Rust delta is ~35 net lines in init_command.rs (mostly deletions) and ~10 in test_command.rs. I traced had_package_json_file against the old package_json_file.is_some() on all three paths (existing file / no file / write error) and it matches. I confirmed write_file_atomically handles ENOENT on the target so a fresh bun init still creates the file, and that write_snapshot_file reports its own failure before the .is_ok() swallows the Err.

Other factors

This is my fourth pass. The three prior nits (undrained stdout pipe in two tests; parallel-worker sibling not stated as excluded; dead #[cfg(windows)] seek pair) were each addressed in a follow-up commit and the threads are resolved. The bug hunter found nothing on this run. CI was green on 5ae73a4 (178/179, the one failure is test/bake/deinitialization.test.ts on Windows, also failing on main); the three fixup commits since then are test-only stdout draining, comment shortening, and a 4-line dead-code deletion that cargo check passes on Linux and Windows per the author. The PR is stacked on #39689 and the description states the intended land order.