bundler: don't close a borrowed resolver-cache fd after parsing

[robobun]

Fixes #33099

What happens

Inside a bun test process, if the test file imports a package whose node_modules/<pkg> entry is a symlink, the first in-process Bun.build() of an entry that also imports that package succeeds, but a later build fails reading the file behind the symlink:

error: EBADF reading file: ".../vendor/pkg/index.js"

Minimal repro (all three builds should succeed, only the first does on an unpatched build):

mkdir -p repro/vendor/pkg repro/node_modules && cd repro
ln -s ../vendor/pkg node_modules/pkg
echo '{"name":"pkg","version":"1.0.0","type":"module","exports":"./index.js"}' > vendor/pkg/package.json
echo 'export const value = 1' > vendor/pkg/index.js
echo 'import { value } from "pkg"; console.log(value)' > entry.ts
cat > repro.test.ts <<'T'
import { it } from "bun:test"
import { value } from "pkg"
void value
it("repro", async () => {
  for (let i = 1; i <= 3; i++) {
    const r = await Bun.build({ entrypoints: ["./entry.ts"] })
    if (!r.success) throw new Error(`${i} failed`)
  }
})
T
bun test repro.test.ts

Cause

A file reached through a symlinked node_modules entry has its file descriptor cached in the resolver's entry cache (finalize_result in src/resolver/resolver.rs) and handed to the bundler as result.file_fd. ParseTask read the file through that descriptor and then closed it (will_close_file_descriptor), but the descriptor was still stored in the resolver cache. The next in-process Bun.build() pulled the now-closed descriptor out of the cache and seek/read against it failed with EBADF (or returned another file once the fd number was recycled, which is the source of the Unexpected reading file / No matching export variants in the report).

The module loader (src/jsc/RuntimeTranspilerStore.rs) already handles this correctly: it closes the input descriptor only when it opened the file itself (should_close_input_file_fd = fd.is_none()). The bundler did not make that distinction.

Fix

ParseTask now closes a descriptor only when the parse task opened it itself. When task.contents_or_fd.file is already valid, the descriptor was borrowed from the resolver cache and must not be closed. This leaves freshly opened descriptors (the non-symlink path) closed as before, and keeps watch-mode behavior unchanged (it already skipped the close when a watcher was present).

Verification

• New test in test/bundler/bun-build-api.test.ts spawns bun test over a fixture with a symlinked node_modules package and asserts three consecutive in-process builds succeed. Fails on the unpatched build with EBADF reading file, passes with the fix.
• Full bun-build-api.test.ts suite passes (48 pass), including the existing "can be called thousands of times in one process" build stress test.
• Resolver cache tests pass (test/bundler/resolver/cache-invalidation.test.ts, cache-runtime.test.ts).

[robobun]

^{Updated 5:17 PM PT - Jun 29th, 2026}

❌ @robobun, your commit 2b7859a has 3 failures in Build #66979 (All Failures):

• test/regression/issue/20965.test.ts - code 1 on 🍎 14 aarch64
• test/js/sql/sql-mysql.transactions.test.ts - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 33102

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

bun-33102 --bun

[github-actions]

Found 3 issues this PR may fix:

1. server.reload() with HMR breaks bundler on second request in monorepo workspaces #26075 - server.reload() with HMR breaks bundler on second request in monorepo workspaces — same pattern of symlinked workspace packages causing "Unexpected reading file" on the second build
2. Unexpected reading file error when working with Bun.build + --hot #11123 - Unexpected reading file error when working with Bun.build + --hot — first build works, hot-reload triggered rebuild fails with "Unexpected reading file" in a monorepo with workspace symlinks
3. Intermittent Errors with Bun.build and Bun.FileSystemRouter when importing absolute paths #9517 - Intermittent errors with Bun.build and Bun.FileSystemRouter when importing absolute paths — subsequent Bun.build calls fail with "Unexpected reading file" / NotOpenForReading on node_modules files after resolver cache is primed

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #26075
Fixes #11123
Fixes #9517

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: eb9741fe-1a0a-4569-abbc-809877982e6c

📥 Commits

Reviewing files that changed from the base of the PR and between 67d7f9c and 2b7859a.

📒 Files selected for processing (1)

• test/bundler/bun-build-api.test.ts

---

Walkthrough

ParseTask.rs narrows file-descriptor closure to task-owned descriptors, bun-build-api.test.ts adds a symlinked-package regression test for repeated Bun.build() calls, and two documentation pages are reformatted.

Changes

FD ownership fix and regression test

Layer / File(s)	Summary
ParseTask fd ownership check src/bundler/ParseTask.rs	will_close_file_descriptor now closes only descriptors whose cached file handle is invalid, using opened_own_fd to distinguish task-owned descriptors from borrowed ones.
Symlinked package build regression test test/bundler/bun-build-api.test.ts	Imports filesystem and platform helpers, then adds a Windows-skipped concurrent regression test that symlinks a vendored package into node_modules, runs repeated Bun.build() calls through bun test, and checks for ALL_BUILDS_OK with exit code 0.

Documentation formatting updates

Layer / File(s)	Summary
Base64 example code block docs/guides/util/base64.mdx	The btoa() and atob() example in the base64 guide is wrapped in an explicit TypeScript code fence.
Web APIs table formatting docs/runtime/web-apis.mdx	The Web APIs support table is reformatted for spacing, wrapping, and separator alignment while keeping the same entries.

Possibly related PRs

• oven-sh/bun#33040: Also changes docs/guides/util/base64.mdx around the btoa()/atob() guidance.

Suggested reviewers

• dylan-conway
• RiskyMH

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Docs-only formatting changes in two mdx files are unrelated to #33099 and appear out of scope.	Remove the unrelated docs formatting changes or split them into a separate PR.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the core fix: ParseTask no longer closes borrowed resolver-cache file descriptors.
Description check	✅ Passed	The PR description covers the bug, cause, fix, and verification, even though the headings don't exactly match the template.
Linked Issues check	✅ Passed	The code and new test address #33099 by preserving borrowed fds and verifying repeated Bun.build() calls succeed in bun test.

---

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

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
bun	🟢 Ready	View Preview	Jun 29, 2026, 10:08 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[robobun]

These look like the same bug class: a file descriptor the resolver caches for a symlink-resolved package gets reused by a later in-process build after an earlier build already closed it, surfacing as "Unexpected reading file" / EBADF on the second build.

#26075 matches most directly (a symlinked workspace package failing on the second request, and the bug vanishing when server.reload() HMR is turned off). #9517 and #11123 report the same symptom on a rebuild and are plausibly the same cause.

I've only verified this PR against the #33099 reproduction, so I have not added the Fixes # lines for the others. The --hot and dev-server reload paths reach the bundler with a watcher that normally adopts the descriptor (so it is not closed there), which means their trigger may differ. I'd rather confirm each reproduction than auto-close on a guess. Happy to verify them against this branch if that's useful.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/bundler/bun-build-api.test.ts (1)

457-477: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use a disposable temp dir here.

tempDirWithFiles() returns a plain string, so this fixture never gets cleaned up. This test creates extra node_modules state and a symlink under that temp root, so it should use tempDir(...) with using instead of tempDirWithFiles(...).

♻️ Proposed fix

-      const dir = tempDirWithFiles("build-symlink-fd-cache", {
+      using dir = tempDir("build-symlink-fd-cache", {
         "vendor/pkg/package.json": `{"name":"pkg","version":"1.0.0","type":"module","exports":"./index.js"}`,
         "vendor/pkg/index.js": `export const value = 1;\n`,
         "entry.ts": `import { value } from "pkg";\nconsole.log(value);\n`,
         "repro.test.ts": `

-      });
+      });
       mkdirSync(join(dir, "node_modules"), { recursive: true });
       symlinkSync("../vendor/pkg", join(dir, "node_modules", "pkg"));

Also swap the harness import from tempDirWithFiles to tempDir.

As per coding guidelines, tests must be hermetic and leave nothing behind. Based on learnings, tempDirWithFiles(...) is not disposable while tempDir(...) is, and tempDir should use plain using in Bun tests.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/bundler/bun-build-api.test.ts` around lines 457 - 477, The test fixture
currently uses tempDirWithFiles() for a setup that creates extra node_modules
state and a symlink, but that helper returns a plain string and won’t clean up.
Update the setup in bun-build-api.test.ts to use tempDir() with a using binding
instead, and switch the harness import from tempDirWithFiles to tempDir so the
temporary root is disposable and the test stays hermetic.

Sources: Coding guidelines, Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@test/bundler/bun-build-api.test.ts`:
- Around line 457-477: The test fixture currently uses tempDirWithFiles() for a
setup that creates extra node_modules state and a symlink, but that helper
returns a plain string and won’t clean up. Update the setup in
bun-build-api.test.ts to use tempDir() with a using binding instead, and switch
the harness import from tempDirWithFiles to tempDir so the temporary root is
disposable and the test stays hermetic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38eab4af-f1c9-4650-82d9-2d02f28e71e3

📥 Commits

Reviewing files that changed from the base of the PR and between 54ac7b3 and 67d7f9c.

📒 Files selected for processing (1)

• test/bundler/bun-build-api.test.ts

[robobun]

Switched the regression test to tempDir(...) with using in 30f1168 so the symlink and node_modules fixture are cleaned up after the test. Verified it still fails before the fix (EBADF) and passes after.

For the out-of-scope flag: the base64.mdx / web-apis.mdx edits are autofix.ci reformatting pre-existing drift on main (it runs prettier over the whole tree on each push), not part of this fix. Reverting them just makes autofix re-apply, so I've left them.

[claude]

No issues found — the fix is small and well-reasoned (mirrors RuntimeTranspilerStore's ownership check), but since it changes fd-close semantics on the bundler/resolver-cache boundary, it's worth a quick human sign-off on the ownership model.

Extended reasoning...

Overview

The functional change is ~5 lines in src/bundler/ParseTask.rs: will_close_file_descriptor is narrowed so the parse task only closes entry.fd when it opened the file itself (i.e. ContentsOrFd::Fd { file, .. } came in with an invalid file). When file is already valid it was borrowed from the resolver's entry cache (symlink-resolved paths), and closing it left a stale fd that the next in-process Bun.build() would reuse → EBADF / "Unexpected reading file". A regression test in bun-build-api.test.ts spawns a bun test subprocess with a symlinked node_modules package and asserts three consecutive builds succeed. The two .mdx diffs are pure autofix.ci whitespace reflow with no content change.

Security risks

None identified. This is internal fd lifecycle bookkeeping; no auth, crypto, network, or user-input handling is touched. The change strictly removes a close, so the failure mode would be holding an fd open longer — but that fd is owned by the process-lifetime resolver cache, which is the intended owner.

Level of scrutiny

Medium-high. The diff is tiny and the analysis is convincing (it mirrors the existing should_close_input_file_fd = fd.is_none() pattern in RuntimeTranspilerStore.rs), but fd ownership at the bundler↔resolver boundary is exactly the kind of invariant where a maintainer who knows the resolver cache's lifecycle should confirm there's no path where a valid incoming file fd is not cache-owned (which would now go unclosed). That's a domain-knowledge check I can't fully discharge from the diff alone.

Other factors

My earlier nit (use test.concurrent.skipIf) was addressed in 54ac7b3, and the test was further tightened to use a disposable tempDir in 30f1168. The bug hunter found nothing. The robobun CI comment shows build-rust failures on freebsd/android/musl aarch64 cross-targets for an earlier commit — those look like infra and are unrelated to this 5-line guard change. None of the touched paths are covered by CODEOWNERS.

[robobun]

On the ownership-model question raised in review (is a valid incoming file fd always cache-owned, or could one now go unclosed?): it is always owned by the resolver's process-lifetime entry cache, never by the parse task.

task.contents_or_fd.file is set only from resolve_result.file_fd at construction (ParseTask.rs:255), and result.file_fd is populated only from Entry.cache().fd (resolver.rs:1735, 1804, 3921). Those Entry values live in the process-lifetime EntryStore, and their cache().fd is set only on the resolver's symlink-resolution paths. So a valid incoming file is always cache-owned; the task never opened it.

The narrowed guard therefore closes exactly the descriptors the task opened itself (the read_file_with_allocator fresh-open path, reached only when file came in invalid) and never one the resolver cache still references. That mirrors the module loader, which borrows the same cached fd without closing it (should_close_input_file_fd = fd.is_none(), RuntimeTranspilerStore.rs).

[robobun]

The bundler and resolver tests this PR touches pass. The red CI is unrelated flaky tests that vary from run to run and don't exercise the bundler or resolver:

• test/cli/update_interactive_install.test.ts (Windows x64-baseline), bun update --interactive
• test/napi/napi.test.ts (Windows x64-baseline), N-API addons
• test/js/node/test/parallel/test-net-connect-memleak.js (x64-baseline), socket GC timing

This change is a small guard in ParseTask plus a regression test; none of those paths touch fd lifecycle in the bundler. I re-ran CI once and it surfaced a different flaky test, so I've stopped re-triggering. Ready for review.

[claude]

I didn't find any issues and the ownership analysis checks out against RuntimeTranspilerStore.rs, but since this changes fd lifecycle on the bundler's hot path against the process-lifetime resolver cache, it's worth a maintainer confirming the cache-ownership invariant before merge.

Extended reasoning...

Overview

The substantive change is a ~5-line narrowing of will_close_file_descriptor in src/bundler/ParseTask.rs: the parse task now closes entry.fd only when task.contents_or_fd arrived with an invalid file (i.e. the task opened the fd itself via read_file_with_allocator), and leaves it open when the fd was borrowed from the resolver's entry cache. A regression test in test/bundler/bun-build-api.test.ts reproduces the EBADF on repeated in-process builds over a symlinked node_modules package. The two .mdx diffs are autofix.ci prettier reflow with no content change.

Security risks

None identified. This is local fd-lifecycle bookkeeping; no auth, crypto, network, or untrusted-input parsing is touched. The change is strictly narrower than before (closes a subset of what it used to), so it cannot introduce new use-after-close or double-close — the only theoretical risk is an fd leak if a valid incoming file were ever task-owned rather than cache-owned. The author traced that contents_or_fd.file is populated solely from resolve_result.file_fd ← Entry.cache().fd, which lives in the process-lifetime EntryStore, and the same borrow-without-close pattern already ships in RuntimeTranspilerStore.rs:809 (should_close_input_file_fd = fd.is_none()).

Level of scrutiny

Medium-high. The diff is tiny and the reasoning is sound, but this is resource management on the bundler's per-file hot path interacting with a process-global cache. If the cache-ownership invariant were ever violated (now or by a future change), long-running processes (dev server, watch mode, repeated Bun.build()) would slowly leak fds. That's the kind of invariant a maintainer familiar with resolver.rs / EntryStore should sign off on rather than a bot.

Other factors

My earlier nit (test.concurrent) was addressed in 54ac7b3, and the test was further hardened with using tempDir(...) in 30f1168. The one CI failure (test-net-connect-memleak.js) is an unrelated Node-compat net test. No CODEOWNERS cover src/bundler/. The fix is well-tested (regression test + the existing 400-build stress test in the same file still passes), so this is a confidence-in-invariant question, not a found-a-problem one.