pack: build the tarball in memory and write it out only once it is complete

[robobun]

Problem

• bun pm pack streams the archive straight into <name>-<version>.tgz (write_open_filename in pack(), src/runtime/cli/pack_command.rs). Every failure after that point, in the entry loops, in archive_package_json / add_archive_entry, or at write_close, exits 1 and leaves that truncated file at the destination (tar -tzf reports gzip: unexpected end of file), where a following bun publish ./*.tgz or a re-run picks it up.
• bun publish from a directory packs the same way, reads the file back into memory, and unlinks it after publishing (publish_command.rs, the // TODO: read this into memory unlink). So a failure between packing and that unlink, a failing postpack for example, leaves the tarball in the package directory, and bun publish --dry-run, which packs nothing, unlinks whatever file of that name the user had there.
• The result of archive_write_data was used as usize::try_from(..).expect("int cast"). When the write itself fails (disk full; libarchive buffers about 60 KiB, so any package bigger than that) the result is panic: int cast: TryFromIntError(NegOverflow) and a crash report instead of an error.

Fix

• The archive is written into a GrowingBuffer through archive_write_open2, the sink Bun.Archive already uses (src/runtime/api/Archive.rs). This is what the // TODO: experiment with archive.writeOpenMemory() above the old write_open_filename call asked for, and the shape npm uses (pacote builds the tarball in memory, npm pack writes it out afterwards).
• bun pm pack hashes the buffer and writes it to the destination in one step (write_tarball). That write is now the only thing that can leave a partial file, and it unlinks the file when it fails. The unlink is gated on lstat reporting a regular file, the only thing File::create can have created or truncated; a --filename that is a symlink or a device is left alone, and a destination that could not be opened at all is not touched.
• bun publish takes the bytes straight from the buffer into Publish::Context.tarball_bytes and never writes a file. This deletes the read-back of the tarball in pack(), the unlink in publish_command.rs, and Publish::Context.abs_tarball_path, whose only remaining use was a capacity hint. It fixes the failing-postpack leftover and the --dry-run unlink as well. bun pm pack with a failing postpack still keeps the tarball, as npm does (it also writes the file before running postpack).
• Why the output is unchanged: archive_write_open_filename set bytes_in_last_block = 1 for a regular file; with a custom sink libarchive would otherwise pad the last block to 10 KiB, so pack() sets it explicitly (new write_set_bytes_in_last_block binding). Tarballs for a fixture with a scoped name, a bin, a 150 KB binary file and a README, at gzip levels 9, 1 and 0, are byte for byte identical to the ones the build before this change produced, with the same file mode (File::create uses the same flags and 0666 as libarchive did) and the same Shasum / Integrity / sizes output.
• Memory: bun publish already read the whole tarball into memory; bun pm pack now holds the compressed bytes until it writes them instead of streaming them, which is what npm does too.
• write_entry_data checks the archive_write_data result and reports failed to write "<file>" to tarball: <reason> instead of panicking. The only way the in-memory sink fails is allocation, and libarchive leaves describing a failed client write to the client, so GrowingBuffer::write_callback now records the reason with archive_set_error, the way libarchive's own file writer does. archive_package_json / add_archive_entry had Result return types with no error path; they return the entry directly now. The *mut Archive is replaced by the crate's WriteArchive owner, which makes the write half of the local ArchivePtrExt shim and the crate's write_open_filename / write_free wrappers dead, so they are removed.
• Verified:
  • test/cli/install/bun-pack.test.ts, "the archive is not padded to a full tar record": the gunzipped tarball of a two-file package is exactly 6 tar blocks (3072 bytes). This pins the bytes_in_last_block setting: the same sink without it, as Bun.Archive uses it, produces 10240 bytes for the same input (measured). Passes on the release too, as it should.
  • test/cli/install/bun-pack.test.ts, new describe: bun pm pack under ulimit -f 0, so the one write of the tarball fails with EFBIG, for the default destination, --destination and --filename (no file is left); a file that cannot be opened (no file is created; skipped as root); a symlink --filename survives; an existing read-only destination keeps its contents. All fail on the current release and pass with this change, also when run as a non-root user; whole file 83 pass, 2 skipped as root.
  • test/cli/install/bun-publish.test.ts: a failing postpack leaves no tarball in the package directory (release: exit 3 and the file is left); --dry-run leaves a tarball that bun pm pack had put there untouched (release: deletes it). Whole file 41 pass.
  • test/js/bun/archive.test.ts (shares GrowingBuffer): 106 pass.
  • The original report's repro ("bin" naming a directory, EISDIR) now leaves nothing behind; pack: skip bins reached through symlinks; publish: do not read the readme through a symlink #38707 separately makes that input stop erroring.
  • cargo check -p bun_runtime -p bun_libarchive for x86_64-pc-windows-msvc and aarch64-apple-darwin; cargo clippy on both crates; cargo fmt --check.

Background

• libarchive write side: the format writer (pax) and filters (gzip) produce bytes that are handed to a client sink. archive_write_open_filename installs a sink that opens the path with O_CREAT | O_TRUNC and writes as it goes; archive_write_open2 installs caller-provided callbacks, here GrowingBuffer, which appends to a Vec. Output is blocked in 10 KiB units; bytes_in_last_block says how the final, partial block is padded (1 = not at all).
• archive_write_close writes the gzip trailer and the tar end-of-archive blocks; the bytes are not a readable .tgz before that.
• archive_write_data returns the number of bytes taken or a negative status code. When a client write callback fails, libarchive sets no error text itself; the callback is expected to call archive_set_error (libarchive's own file sink reports "Write error" that way).
• Global::crash() is exit(1) and does not unwind, so in pack() a destructor cannot be used for cleanup; the fix works by not having anything on disk to clean up until the single final write.
• RLIMIT_FSIZE (ulimit -f) caps the size of files a process may write; bun ignores SIGXFSZ at startup, so exceeding it makes write fail with EFBIG. The tests use it to make the tarball unwritable regardless of the user they run as.

Repro on the current release

mkdir fsz && cd fsz
printf '{"name":"fsz","version":"1.0.0"}' > package.json; echo x > index.js
(ulimit -f 0; bun pm pack); echo "exit=$?"; ls *.tgz
# error: failed to close archive: Write error
# exit=1
# fsz-1.0.0.tgz            <- 0 bytes, left behind

head -c 131072 /dev/urandom > big.bin
(ulimit -f 0; bun pm pack)
# panic: int cast: TryFromIntError(NegOverflow)

printf '{"name":"pp","version":"1.0.0","scripts":{"postpack":"exit 3"}}' > package.json
bun publish; ls *.tgz
# error: script "postpack" exited with code 3
# pp-1.0.0.tgz             <- left behind by publish

With this change: EFBIG: File too large: failed to write tarball: "/.../fsz-1.0.0.tgz" (write), exit 1, no file, in both of the first two cases; the publish case exits 3 with no file.

Earlier shape of this PR

The first version kept streaming to disk and added an OpenTarball handle whose crash() closed the archive and unlinked the destination, called from each of the twelve failure sites between open and close (plus an archive_write_fail binding so that close did not pad out the unfinished entry). Review pointed out that the two TODOs quoted above describe the in-memory shape, which needs none of that, deletes the read-back code, and also covers the publish leftovers; this version implements that instead. The tests carried over with the error text adjusted, except for the large-package case that drove the archive_write_data check into a failing disk: with the in-memory sink that check is only reachable through allocation failure, so it has no test.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-pack.test.ts test/cli/install/bun-publish.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c3cdb617-6713-49d1-a102-7e0483b2790c

📥 Commits

Reviewing files that changed from the base of the PR and between 6324a58 and 3511edd.

📒 Files selected for processing (5)

• src/libarchive/lib.rs
• src/runtime/cli/pack_command.rs
• src/runtime/cli/publish_command.rs
• test/cli/install/bun-pack.test.ts
• test/cli/install/bun-publish.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 4:38 AM PT - Aug 15th, 2026}

❌ @robobun, your commit 3511edd has some failures in Build #97498 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38721

That installs a local version of the PR into your bun-38721 executable, so you can run:

bun-38721 --bun

[robobun]

Reproduced on the current release (1.4.0-canary): the handoff's "bin"-names-a-directory input and, independently, ulimit -f 0 both exit 1 and leave a 0 byte <name>-<version>.tgz behind; a package over ~60 KiB compressed under the same limit panics (int cast); bun publish with a failing postpack leaves the tarball in the package directory and bun publish --dry-run deletes an existing one. With this PR (the archive is built in memory and bun pm pack writes it once; bun publish never writes a file) none of these leave or remove a file, and the tarballs produced are byte-identical to before. Tests: the describe block and the unpadded-record test in test/cli/install/bun-pack.test.ts and the two new tests in bun-publish.test.ts, all failing on the release build (except the two that pin unchanged behaviour) and passing with the change.

CI for 3511edd (build 97498): every test lane passed, including the darwin lane, which ran on its retry after a first attempt expired waiting for an agent; that expired attempt is the only reason the build shows as failed. Ready for review.

[claude]

LGTM — focused fix that cleans up the truncated tarball on every pack failure path, and replaces the archive_write_data int-cast panic with a proper error.

What was reviewed

• Traced every Global::crash() between write_open_filename and write_close in pack() — all converted to tarball.crash(); failures after a successful close intentionally still leave the (now valid) tarball.
• OpenTarball::crash: write_fail → write_close → lstat-gated unlink is safe to call even from the write_close failure path (libarchive treats a second close on a FATAL archive as a no-op); the lstat regular-file gate prevents deleting a symlink/device passed as --filename.
• bun_sys::lstat/unlink exist on both POSIX and Windows (via sys_uv); the close-before-unlink ordering handles Windows' open-file restriction.
• Tests: ulimit -f 0 is a clean way to force EFBIG regardless of uid; the new test.concurrent cases use their own tempDir so they don't race the file-level beforeEach packageDir.

Extended reasoning...

Overview

The PR fixes bun pm pack (and bun publish from a directory) leaving a truncated .tgz behind when packing fails after the output file has been created. It touches:

• src/libarchive/lib.rs: adds the archive_write_fail FFI binding and a write_fail() wrapper.
• src/runtime/cli/pack_command.rs: introduces OpenTarball { archive, path } and routes every failure between write_open_filename and a successful write_close through OpenTarball::crash(), which marks the archive fatal, closes it, unlinks the destination if it is a regular file, and exits 1. Also replaces the usize::try_from(archive.write_data(..)).expect("int cast") panic with a real error via write_entry_data(), and drops the now-vacuous Result<_, AllocError> returns from archive_package_json/add_archive_entry.
• Tests in bun-pack.test.ts and bun-publish.test.ts covering the default destination, --destination, --filename, mid-entry write failure (the former panic), an unreadable input file, and a symlink --filename that must survive.

Security risks

None identified. The only filesystem mutation added is an unlink of the exact path pack itself just opened with O_CREAT|O_TRUNC, gated on lstat reporting a regular file so a symlink or device passed as --filename is never removed. No user input flows into the unlink path beyond what already flowed into write_open_filename.

Level of scrutiny

Moderate. This is native Rust touching a CLI command's error paths and adding one libarchive FFI symbol, not a hot path or security boundary. The change is contained to pack() and its two helpers; archive_write_fail is a documented libarchive abort primitive. Calling write_close a second time from the write_close failure branch is safe because write_fail moves the archive to ARCHIVE_STATE_FATAL and libarchive's close short-circuits in that state. bun_sys::lstat / bun_sys::unlink both exist for POSIX and Windows (via libuv), and the fd is closed before the unlink so Windows can delete the file.

Other factors

• The PR description is unusually thorough — every design choice (why not a Drop guard, why close before unlink, why gate on lstat, why ulimit -f) is justified, and cross-target cargo check plus clippy/fmt were run.
• Tests are hermetic, drain both pipes concurrently, use test.concurrent with per-case tempDir (so no shared-state races with the file-level packageDir), and skip appropriately on Windows and when running as root. The ulimit -f 0 approach is robust across uid.
• Removing the dead Result<_, AllocError> return type is the right call per the repo's "delete dead code in the same PR" rule; both call sites are updated.
• The multi-agent bug hunt found nothing.

[robobun]

Reworked since the first round (the LGTM above was for the earlier OpenTarball shape, summarised at the bottom of the description): instead of streaming to disk and unlinking on each failure path, pack() now builds the archive in memory through the GrowingBuffer sink Bun.Archive uses, bun pm pack writes the finished bytes once, and bun publish uploads them without touching the disk. This implements the two existing TODOs in pack_command.rs / publish_command.rs, removes the read-back and unlink code, and also fixes bun publish leaving the tarball behind when postpack fails and --dry-run deleting an existing tarball. Output verified byte-identical at gzip levels 0, 1 and 9; details in the description.

[claude]

I reviewed this PR and the bug hunting system found no issues. My earlier note about the description/title mismatch has been addressed. Because this is a substantial refactor of the pack/publish write path — switching from streaming-to-file to build-in-memory, with a subtle bytes_in_last_block requirement for byte-identical output and a changed memory profile for bun pm pack — a human look would still be worthwhile.

What was reviewed:

• GrowingBuffer/WriteArchive lifetime ordering (tarball_buffer declared before archive, explicit drop(archive) before consuming the buffer) — correct.
• pack_destination() simplification vs. the old NUL-byte dance — make_path takes &[u8], so the old dance was unnecessary; equivalent.
• File::create flags/mode (O_WRONLY|O_CREAT|O_TRUNC, 0666) match what archive_write_open_filename used.
• write_tarball's lstat-gated unlink and the removed --dry-run unlink in publish_command.rs — both covered by new tests.

Extended reasoning...

Overview

This PR changes bun pm pack and bun publish (from a directory) to build the tarball entirely in memory via libarchive's archive_write_open2 + the existing GrowingBuffer sink, instead of streaming to disk with archive_write_open_filename. bun pm pack then writes the finished bytes once via a new write_tarball(); bun publish uploads the bytes directly and never touches disk. This fixes three bugs: truncated .tgz files left behind on any mid-pack failure, bun publish --dry-run deleting a pre-existing tarball, and a panic on negative archive_write_data return. Files touched: src/libarchive/lib.rs (adds write_set_bytes_in_last_block and archive_set_error bindings, removes write_open_filename/write_free wrappers, makes GrowingBuffer::write_callback set an error string on OOM), src/runtime/cli/pack_command.rs (~200 lines net removed: the read-back-and-hash block, the write half of ArchivePtrExt, the destination-mkdir dance; adds pack_destination/write_tarball/write_entry_data), src/runtime/cli/publish_command.rs (removes abs_tarball_path field and its unlink), plus new tests in bun-pack.test.ts and bun-publish.test.ts.

Security risks

None identified. The change reduces filesystem interaction (publish no longer writes/unlinks a file) and the new write_tarball unlink on failure is gated by lstat → regular-file, so a --filename pointing at a symlink or device is not deleted. No new untrusted-input parsing.

Level of scrutiny

High. This is the code path that produces the bytes uploaded to package registries. The write_set_bytes_in_last_block(1) call is load-bearing for byte-identical output (without it, the last gzip block would be padded to 10 KiB and every tarball's shasum would change); the PR description says this was verified byte-for-byte at gzip levels 0/1/9, but that check is not automated. The memory-profile change (holding the full compressed tarball in RAM for bun pm pack) matches npm's behavior and bun publish already did this, but is a behavioral shift worth a maintainer's sign-off.

Other factors

The bug hunting system found no issues. Test coverage is strong: ulimit -f 0 for EFBIG on the final write across default/--destination/--filename, an unreadable input file, a symlink --filename that must survive, a read-only destination that must keep its contents, plus bun publish postpack-failure and --dry-run cases. I checked File::create (src/sys/file.rs:117) uses the same O_WRONLY|O_CREAT|O_TRUNC + 0666 as libarchive's file writer, and Dir::make_path (src/sys/dir.rs:99) takes &[u8] so the simplified pack_destination() is equivalent to the old NUL-byte dance. The comment-cop feedback (long comments) was addressed in 4f324ba, and my earlier note about the stale PR description was resolved. This is a well-executed change that implements two existing TODOs, but its scope and the criticality of the pack output put it past the threshold for auto-approval.

[robobun]

The one open point from the review above, that the byte-identical claim was only checked by hand, is now covered by a test (3511edd): the gunzipped tarball of a two-file package must be exactly 6 tar blocks. Without the bytes_in_last_block setting the same sink pads it to 10240 bytes (that is what Bun.Archive, which uses the sink without the setting, produces for the same input), so dropping the setting fails the test. The description's Verified section lists it.

[claude]

I reviewed this PR and didn't find any bugs; my earlier note about the title/description being out of date has been resolved. Because this reworks how bun pm pack and bun publish do I/O (streaming → in-memory via GrowingBuffer/archive_write_open2, with bytes_in_last_block(1) needed for byte-identical output and a WriteArchive-Drop / tarball_buffer ordering invariant), a human look would still be worthwhile.

What was reviewed:

• GrowingBuffer outlives WriteArchive (declared first; explicit drop(archive) before to_owned_slice()); archive_write_free in Drop is a no-op after the explicit write_close.
• pack_destination passing &buf[..dir_end] to make_path — that helper takes &[u8], so the old NUL-swap dance was unnecessary.
• write_tarball uses File::create(.., true) = O_WRONLY|O_CREAT|O_TRUNC, mode 0666, matching what archive_write_open_filename set; lstat-gated unlink leaves symlinks/devices alone.
• Removed write_open_filename/write_free wrappers have no remaining callers; the padding invariant is pinned by the new gunzipSync byte-count test.

Extended reasoning...

Overview

This PR changes bun pm pack from streaming the archive to disk via archive_write_open_filename to building it in a GrowingBuffer via archive_write_open2 (the same sink Bun.Archive uses) and writing it once complete via a new write_tarball(). bun publish from a directory now takes the bytes straight from the buffer and never writes a file, deleting the read-back, the abs_tarball_path field, and the post-publish unlink. A new archive_write_set_bytes_in_last_block binding is added and set to 1 to preserve byte-identical output (libarchive would otherwise pad the last 10 KiB record for a custom sink). The archive_write_data result is now checked (write_entry_data) instead of .expect("int cast")-panicking on a negative return, and GrowingBuffer::write_callback records ENOMEM via archive_set_error so the message reaches the user. archive_package_json/add_archive_entry drop their vestigial Result return, the raw *mut Archive becomes the crate's RAII WriteArchive, and the write half of the local ArchivePtrExt shim plus the crate's write_open_filename/write_free wrappers are removed as dead.

Security risks

None identified. No new untrusted-input parsing; the new file write goes to the same computed destination as before; the unlink on write failure is gated on lstat reporting a regular file so a --filename symlink/device is not deleted. Removing the unconditional unlink from bun publish is strictly safer (previously --dry-run would delete a user's existing tarball).

Level of scrutiny

Medium-high. This is an architectural change to the I/O path of two user-facing CLI commands, not a mechanical fix. The correctness hinges on (a) bytes_in_last_block(1) keeping output byte-identical (pinned by the new test and the description's manual verification at gzip 0/1/9), (b) the tarball_buffer/WriteArchive drop ordering (buffer declared first, archive dropped explicitly before to_owned_slice()), and (c) File::create matching the flags/mode libarchive used. All three check out on inspection, but the surface area and the fact that this PR was already reworked once from a different design suggest a maintainer should confirm the shape.

Other factors

• My earlier inline comment (title/description described the superseded OpenTarball design) has been addressed — the current title and description accurately describe the in-memory approach, and the earlier shape is summarised in a <details> block.
• The comment-cop bot's "paragraph-long comment" flags were addressed in 4f324ba (comments shortened to one line each); one flag on the pre-existing ArchivePtrExt doc comment was correctly noted as not introduced by this PR.
• Test coverage is thorough: EFBIG under ulimit -f 0 for the default/--destination/--filename cases, an unreadable input file, a symlink --filename surviving, a read-only pre-existing destination kept intact, bun publish with a failing postpack leaving no tarball, --dry-run leaving an existing tarball untouched, and the new no-padding assertion. Root-only skips are documented.
• The one memory trade-off (holding the compressed tarball in memory for bun pm pack) matches npm's behaviour and bun publish already did this; the description calls it out explicitly.