dev server: re-bundle a failed route once per file change and close the descriptors the watcher declines

[robobun]

Problem

• While a bun ./index.html app (or Bun.serve({development: true, routes})) sits on a build error, every request for the page re-bundles everything the route imports: check_route_failures (src/runtime/bake/DevServer.rs) marks all reachable files stale and returns Rebuild whenever failures are reachable, and nothing records that this was already done, so the failed bundle puts the route straight back into the same state for the next request.
• Each of those re-bundles leaks one file descriptor per module, permanently. With a watcher present, ParseTask keeps the descriptor it read the file through so the bundle thread can hand it to the watcher, but on_parse_task_complete (src/bundler/bundle_v2.rs) discarded the FdOwnership result of Watcher::add_file. For a file that is already watched (every re-parse under the dev server, including a plain hot reload of an already watched file) add_file declines the descriptor and nobody closed it.
• Two pipelined requests in that state got no response at all. That is a consequence of the first point: uWS closes a connection when a second pipelined request arrives while the first one is still deferred, and the re-bundle deferred every request.
• Release behaviour only: assume_perfect_incremental_bundling defaults to true in debug builds, which is why the bake test harness sets BUN_ASSUME_PERFECT_INCREMENTAL=0 for every test. The doc comment explaining the field was lost in the port; this restores it.

Fix

• RouteBundle::rebundled_for_failures: check_route_failures re-bundles a failing route once and sets the flag; later requests serve the stored failures directly (the same path BUN_ASSUME_PERFECT_INCREMENTAL=1 takes, so the page is served synchronously and pipelined requests are answered). start_async_bundle clears the flag on every route when a hot-reload bundle starts, so each file change allows exactly one more re-bundle per route. This keeps the escape hatch the field documents (a refresh after a save still gets a full re-bundle) without making it fire on every request.
• check_route_failures now clears incremental_result.failures_added before tracing. It still held the previous bundle's list, and the trace appends to it; without this the first served page after the re-bundle listed every failure twice (verified: 368 byte payload vs 184 with the clear).
• ContentsOrFd::Fd / WatcherData carry owns_file / owns_fd, set only when the parse opened the descriptor itself (not when it was borrowed from the resolver's entry cache, which is why the earlier fix in watcher: stop serving the watchlist's stored fd to --hot reload reads #37050 left this site alone). on_parse_task_complete closes an owned descriptor the watcher did not adopt. Behaviour without a watcher is unchanged (the descriptor is still closed right after the read, as before).
• Tests in test/bake/dev/bundle.test.ts, both fail against the current release and pass with this change:
  • "requests for a route with build errors do not re-bundle it every time": one bundle after each broken write, then 5 requests with 0 bundles (was 5), twice to cover the flag being cleared by the second write, then recovery. Uses two small harness helpers (dev.waitForBundle, dev.countBundles) built on the existing watch-synchronization events.
  • "re-bundling a watched file does not leak a file descriptor" (Linux, reads /proc/<pid>/fd): 5 hot reloads leave the descriptor count for the file unchanged (was +1 per reload).
• The report's reproduction (100 modules, 200 requests on a broken build) on a debug build with BUN_ASSUME_PERFECT_INCREMENTAL=0: fds 117 -> 3207 and 0 pipelined responses before; fds 116 -> 116, 2 bundles total, pipelined pair answered 500 + 500 after.
• test/bake/dev/* (bundle, css, html, hot, esm, react-spa, plugins, sourcemap, server-sourcemap), test/bake/framework-router.test.ts, test/bake/deinitialization.test.ts, test/bake/dev-and-prod.test.ts, test/bundler/{bundler_files,bundler_plugin,bundler_defer,bun-build-api}.test.ts and test/cli/watch/watch.test.ts pass on the debug build; cargo clippy is clean for bun_bundler and bun_runtime.

Background

• The dev server keeps two incremental graphs (client and server) of every bundled file. A route is served from the graphs; RouteBundle::server_state tracks whether the route still needs bundling, and PossibleBundlingFailures means "some file somewhere failed, trace this route's imports to find out whether one of them is reachable" (check_route_failures).
• Bundling failures are stored per file in bundling_failures; incremental_result.failures_added is scratch space that a bundle fills while indexing failures and that the FindErrors trace also appends to. send_serialized_failures renders whatever list it is given into the error page.
• assume_perfect_incremental_bundling is the dev server's distrust of its own incremental tracking: when false (release builds), a route with failures is fully re-bundled before its errors are shown, so a bug in incremental invalidation cannot leave a stale error page. The flag added here bounds that to once per file change.
• With a file watcher attached to the bundler, each parse keeps the descriptor it read from and on_parse_task_complete hands it to Watcher::add_file on the bundle thread (kqueue needs one descriptor per watched file; on Linux the watcher just stores and later closes it). add_file returns FdOwnership::Watcher when it stored the descriptor and FdOwnership::Caller when the file was already in the watchlist, in which case the caller still owns it. The resolver can also cache a descriptor for symlinked files and lend it to the parse, which is the case owns_file distinguishes: that descriptor must never be closed here.
• A related note, not changed here: start_next_bundle_if_present starts no bundle when the queued routes produce no entry points, but leaves the requests that were deferred to that bundle unanswered and the routes in the Bundling state. The synchronous path in ensure_route_is_bundled handles that case ("possible with layouts"); the deferred path does not. It is independent of this change, since the re-bundle issued by check_route_failures always produces entry points (it marks the route's own files stale).

Reproduction used (from the report, Linux)

import fs from "node:fs"; import net from "node:net"; import os from "node:os";
const N = 100, d = fs.mkdtempSync(os.tmpdir() + "/e-"); fs.mkdirSync(d + "/m");
fs.writeFileSync(d + "/index.html", `<script type=module src=./app.ts></script>`);
fs.writeFileSync(d + "/app.ts", [...Array(N)].map((_, i) => `import {v${i}} from "./m/m${i}.ts";`).join("\n") + `\nimport {g} from "./lib.ts"; console.log(g,${[...Array(N)].map((_, i) => "v" + i).join("+")});`);
for (let i = 0; i < N; i++) fs.writeFileSync(`${d}/m/m${i}.ts`, `export const v${i}=${i};`);
fs.writeFileSync(d + "/lib.ts", "export const g=1;\n");
// Debug builds need BUN_ASSUME_PERFECT_INCREMENTAL=0 in the child's env to behave like a release build.
const p = Bun.spawn([process.execPath, "./index.html", "--host=127.0.0.1", "--port=0"], { cwd: d, stdout: "pipe" });
let port, acc = ""; for await (const c of p.stdout) { acc += new TextDecoder().decode(c); const m = /127\.0\.0\.1:(\d+)/.exec(acc); if (m) { port = m[1]; break; } }
const fds = () => fs.readdirSync(`/proc/${p.pid}/fd`).length, get = async () => (await fetch(`http://127.0.0.1:${port}/`)).status, sleep = ms => new Promise(r => setTimeout(r, ms));
const pipe2 = () => new Promise(r => { let b = ""; const s = net.connect(+port, "127.0.0.1", () => s.write("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n".repeat(2))); s.on("data", x => b += x); s.on("close", () => r((b.match(/HTTP\/1.1 \d+/g) || []).length + " responses")); setTimeout(() => s.destroy(), 2000); });
await get(); for (let i = 0; i < 200; i++) await get(); console.log("healthy 200 req: fds", fds(), "| pipelined:", await pipe2());
fs.writeFileSync(d + "/lib.ts", "export const g=1;\n@@@\n"); await sleep(800);
for (let i = 0; i < 200; i++) await get(); console.log("error-state 200 req: fds", fds(), "| pipelined:", await pipe2());
fs.writeFileSync(d + "/lib.ts", "export const g=1;\n"); await sleep(800); console.log("fixed:", await get(), "fds", fds()); p.kill(9);

Bun 1.4.0 release: healthy 200 req: fds 116 | pipelined: 2 responses, error-state 200 req: fds 20716 | pipelined: 0 responses, fixed: 200 fds 20820.

This branch (debug build, BUN_ASSUME_PERFECT_INCREMENTAL=0, 50 requests): healthy: fds 116 | 2 responses, error-state: fds 116 -> 116 | 2 responses, fixed: 200 fds 116; the build error is printed twice in total (the reload and the single re-bundle) instead of once per request.

---

[review] gate passed · iteration 0 · 6 files touched

fails on main (without fix)

ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/bundle.test.ts
bun test v1.4.0 (8326d1bd3)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-YUue3k
�[0;30mdev|�[0m Started development server: http://localhost:42491
�[0;30mdev|�[0m �[32mBundled page in 192ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 62ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 45ms�[
... (truncated)

release without fix: 2 FAILED
bun test v1.4.0-canary.1 (8326d1bd3)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-Kq2jLQ
�[0;30mdev|�[0m Started development server: http://localhost:34581
�[0;30mdev|�[0m �[32mBundled page in 3ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 2ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 1ms�[0m�[2m:�[0m routes/index.ts
(pass)  DEV:bundle-1: import identifier doesnt get renamed [214.08ms]
�[0;30mdev|�[0m Started development server: http://localhos
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/bundle.test.ts
bun test v1.4.0 (8326d1bd3)

test/bake/dev/bundle.test.ts:
Dev server testing directory: /tmp/bun-dev-test-CrJ94B
�[0;30mdev|�[0m Started development server: http://localhost:36235
�[0;30mdev|�[0m �[32mBundled page in 200ms�[0m�[2m:�[0m routes/index.ts �[2m+ 1 more�[0m
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "db.ts" is not accepted by routes/index.ts,
�[0;30mdev|�[0m �[32mReloaded in 70ms�[0m�[2m:�[0m db.ts
�[0;30mdev|�[0m [Bun] Hot update was not accepted because it or its importers do not call `import.meta.hot.accept`. To prevent full page reloads, call `import.meta.hot.accept` in one of the following files to handle the update:
�[0;30mdev|�[0m 
�[0;30mdev|�[0m Module "routes/index.ts" is a root module that does not self-accept.
�[0;30mdev|�[0m �[36m[x2]�[0m �[32mReloaded in 70ms�[
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     773bb14b96
  features     baseline

22 deps, 120 codegen, 1174 objects in 843ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1235] install /workspace/bun
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 107 installs across 153 packages (no changes) [27.00ms]
[2/1235] gen ErrorCode+*.h
[3/1235] gen bindgenv2
[4/1235] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 1 install across 2 packages (no changes) [4.00ms]
[5/1235] fetch tinycc
[tinycc] up to date
[6/1234] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 111 installs across 104 packages (no changes) [8.00ms]
[7/1234] fetch zlib
[zlib] up to date
[8/1234] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[9/1234] gen .bind.ts → GeneratedBindings.cpp
[10/1234] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp

... (truncated)

diff hotspot

src/bundler/ParseTask.rs                    | 34 ++++++++---
 src/bundler/bundle_v2.rs                    | 37 +++++++-----
 src/runtime/bake/DevServer.rs               | 27 ++++++++-
 src/runtime/bake/dev_server/route_bundle.rs |  6 ++
 test/bake/bake-harness.ts                   | 43 ++++++++++++++
 test/bake/dev/bundle.test.ts                | 91 +++++++++++++++++++++++++++++
 6 files changed, 213 insertions(+), 25 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                         reads  edits  tests
src/bundler/ParseTask.rs                        11      8      0
src/bundler/bundle_v2.rs                         7      5      0
src/runtime/bake/DevServer.rs                   12      6      0
src/runtime/bake/dev_server/route_bundle.rs      1      1      0
test/bake/bake-harness.ts                        4      1      0
test/bake/dev/bundle.test.ts                     2      4      0

[robobun]

Status: reproduced with Bun 1.4.0 (error-state requests: fds 116 -> 20716 over 200 requests, pipelined pair unanswered) and with a debug build using BUN_ASSUME_PERFECT_INCREMENTAL=0. With this branch the same run stays at 116 descriptors, bundles once after the failing save, and answers the pipelined pair.

Both new tests in test/bake/dev/bundle.test.ts fail against the release binary (Received: 5 bundles, Received: 6 descriptors) and pass with the change. Waiting on CI.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes fd ownership hand-off across the parse-worker → bundle-thread → watcher boundary and adds state to the dev server's request/bundle machine, a human look would still be worthwhile.

What was reviewed:

• owns_file propagation through every ContentsOrFd::Fd construction site and both run exit paths (success and the entry.deinit() error return at ParseTask.rs:2683 — deinit frees contents only, not the fd, so the owned fd still reaches on_parse_task_complete).
• The adopted short-circuit in bundle_v2.rs against Watcher::add_file's three outcomes and should_add_watcher() == false; matches the established FdOwnership pattern at bundle_v2.rs:4548, jsc_hooks.rs:3611, RuntimeTranspilerStore.rs:947. Resolver-cached fds (owns_fd = false) are never closed here.
• rebundled_for_failures lifecycle: single RouteBundle construction site initializes it, cleared only under had_reload_event; failures_added.clear() pairs with the existing scope-exit clear.

Extended reasoning...

Overview

Two coupled fixes plus tests. (1) RouteBundle::rebundled_for_failures bounds the release-build "distrust incremental tracking" re-bundle in check_route_failures to once per route per file change; start_async_bundle clears it for every route when had_reload_event. A stale incremental_result.failures_added is now cleared before the trace. (2) ContentsOrFd::Fd and WatcherData gain owns_file/owns_fd so on_parse_task_complete can close a task-opened descriptor when Watcher::add_file returns FdOwnership::Caller (or the file is excluded from watching / add_file errored). Two new dev-server tests and two harness helpers.

Security risks

None. No user-controlled input reaches new parsing or allocation; the change tightens resource release and gates a rebuild.

Level of scrutiny

High. This is fd lifecycle across a thread boundary with three ownership sources (resolver cache, task-opened, stdio) and three sinks (immediate close, watcher adoption, bundle-thread close). I traced each combination and did not find a double-close or a new leak, and the pattern matches four existing FdOwnership::Watcher call sites — but someone who owns the dev server / bundler should confirm the had_reload_event gate is the right reset point for the flag (a request-triggered bundle with had_reload_event = false intentionally does not clear it, which appears to be the desired behaviour since the escape hatch is for file changes).

Other factors

Tests are well-constructed: both are verified to fail against the release binary (Received: 5 bundles / Received: 6 fds) per the robobun comment; the fd test counts only descriptors pointing at the specific file via /proc/<pid>/fd readlink, so it is not sensitive to unrelated fds; the re-bundle test loops two distinct broken writes to cover the flag being cleared. The PR description also flags an unrelated latent issue in start_next_bundle_if_present (deferred requests when queued routes yield no entry points) that is out of scope here.

[robobun]

Thanks for the pass. To confirm the one point raised: clearing the flag only on had_reload_event is intentional. A request-triggered bundle (a first request for another route) only adds files that were not bundled yet, so it cannot change what an already re-bundled route's stored failures say; a hot-reload bundle can, which is why it resets every route. If a file change lands while a forced re-bundle is still in flight, the reload that follows clears the flag again and the next request re-bundles one more time, which is the same convergence as before, just bounded.

The read error path works the same way as the parse error path: the parse task only sets owns_file after the read has produced a descriptor, so a failed read leaves the resolver-provided (or invalid) descriptor in place with owns_file = false and nothing is closed.

CI is green so far (178 of 179 jobs passed, one still running).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 11 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

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: 4463c669-5f8e-470e-9147-5b186e44d54c

📥 Commits

Reviewing files that changed from the base of the PR and between 7aad387 and 773bb14.

📒 Files selected for processing (6)

• src/bundler/ParseTask.rs
• src/bundler/bundle_v2.rs
• src/runtime/bake/DevServer.rs
• src/runtime/bake/dev_server/route_bundle.rs
• test/bake/bake-harness.ts
• test/bake/dev/bundle.test.ts

---

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

[claude]

I reviewed this PR and didn't find any bugs. Because it threads fd-ownership state through the parse pipeline and adds state to the dev server's route-bundle lifecycle — both areas where a subtle mistake is a double-close or a stuck route — a human look is still worthwhile.

Checked: owns_file is only set when opened_own_fd && entry.fd.is_valid() and the resolver-cache-borrowed fd path stays owns_file: false, so the new close in on_parse_task_complete cannot hit a borrowed fd. The !adopted branch matches the pattern already used at jsc_hooks.rs:3602 and bundle_v2.rs:4492. rebundled_for_failures is initialized at the sole RouteBundle construction site and cleared only on had_reload_event, which the author's follow-up comment justifies. The failures_added.clear() before the trace is correct — the existing scopeguard clears on exit but not on entry.

Extended reasoning...

Overview

Two coupled fixes in the dev server + bundler:

1. src/runtime/bake/DevServer.rs / route_bundle.rs: adds RouteBundle::rebundled_for_failures so check_route_failures re-bundles a failing route once per file change instead of on every request. Also clears incremental_result.failures_added before the FindErrors trace (previously carried the last bundle's list, doubling the rendered failures on the first served page). Restores the lost doc comment on assume_perfect_incremental_bundling.
2. src/bundler/ParseTask.rs / bundle_v2.rs: threads owns_file/owns_fd through ContentsOrFd::Fd → WatcherData so on_parse_task_complete can close a parse-opened descriptor when Watcher::add_file returns FdOwnership::Caller (already-watched file). Previously the return value was discarded and every re-parse under the dev server leaked one fd per module.

Tests: two new devTests in test/bake/dev/bundle.test.ts (bundle-count assertion via new waitForBundle/countBundles harness helpers; Linux-only /proc/<pid>/fd count for the leak). Both are documented as failing on the release binary.

Security risks

None. No user-controlled input handling changes; the fd close is gated on owns_fd which is only set for descriptors this parse opened itself, so there's no double-close of the resolver's cached fd.

Level of scrutiny

High. Per the repo's review guidance, fd/resource ownership is the most-blocked category — "pair every acquisition with its release at the acquisition site" and "every allocation has exactly one named owner, released exactly once". This PR changes who closes a descriptor and when, across a thread boundary (parse worker → bundle thread) and across two code paths (watcher present vs. absent). Getting owns_fd wrong in either direction is a double-close or a leak. The rebundled_for_failures flag also adds state to a state machine that already has five State variants and interacts with had_reload_event / current_bundle / deferred requests.

Other factors

• The fd-ownership change follows the exact pattern already used at three other add_file call sites (jsc_hooks.rs:3602, RuntimeTranspilerStore.rs:946/973, bundle_v2.rs:4492), which reduces risk.
• All ContentsOrFd::Fd construction sites are updated (Rust would not compile otherwise); the resolver-provided fd (ParseTask.rs:274) and the entry-point fd (bundle_v2.rs:4750) both correctly get owns_file: false.
• The author's follow-up comment addresses the "why only clear on had_reload_event" question and the read-error path; both explanations check out against the code.
• CI reported 178/179 green; the tests are stated to fail on the unfixed binary and pass with the change.
• The PR notes but does not fix a related issue in start_next_bundle_if_present (deferred requests left unanswered when queued routes produce no entry points) — flagged as independent, worth a maintainer glance to confirm scope.

[robobun]

Nothing new to address from the second pass. For anyone comparing the two revisions: the rebase only resolved a conflict with #39448, which removed the watcher's loader argument from the add_file call that this PR restructures; the diff is otherwise unchanged. The start_next_bundle_if_present note in the description is deliberately left out of scope, as it is unreachable from the re-bundle this PR bounds (that re-bundle always has entry points).

CI for the rebased revision is at 175 of 179 jobs passed with the rest still running, none failed.

[robobun]

Superseded by #39488, which consolidates the open Bake PRs from this period into one branch. It carries this PR's change and its test, reworked where needed (see the commit in #39488 that names this PR). Closing this one in its favor.