Conversation
… buffer resolve_entry_point's callers rely on it logging the error before returning it. The arm that skips the directory-cache bust for an entry point whose path cannot fit a PathBuffer returned without logging, so the entry point was silently dropped: Bun.build succeeded without it when another entry point was given and hit the linker's no-chunks assertion when it was the only one, and a Worker given such a path reported "BuildMessage: undefined". Skip only the cache bust for those paths and fall through to the logging.
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
There was a problem hiding this comment.
LGTM — the length guard now falls through to the existing error-logging path instead of returning ahead of it, so an over-long entry point is reported like any other unresolved one.
What was reviewed:
resolve_entry_pointcontrol flow:break 'name falseskips only the cache-bust/retry (which needs thePathBuffer), then reacheslog_mut().add_error_fmt(...); entry points that fit take the identical path as before.- Confirmed callers (
enqueue_entry_points_normal, the Worker resolve path) rely on the log entry rather than theErr— the doc comment on this function says as much, and the fix is at the right layer. - Both new tests spawn a child / await the event, assert exact messages, cover the alone + alongside-valid-entry cases and the Worker face, and follow harness conventions (
Buffer.alloc,tempDir, concurrent pipe drain, exit-code last).
Extended reasoning...
Overview
The PR relocates an existing length guard in Transpiler::resolve_entry_point (src/bundler/transpiler.rs). Previously the guard was a separate Err(err) if <too long> => Err(err) match arm ahead of the arm that busts the directory cache, retries, and — crucially — logs the error. That arm returned Err without logging, so callers that consume the log (enqueue_entry_points_normal just continues on Err; the Worker turns the log into the error event's message) saw nothing: a build with zero entry points panicked in the linker, a build with a valid sibling silently succeeded, and a Worker fired BuildMessage: undefined.
The fix moves the same predicate inside the 'name labelled block as break 'name false, so the cache-bust step (the only part that needs the fixed-size PathBuffer) is skipped and control falls through to self.log_mut().add_error_fmt(...). The error is now logged with the same format as every other unresolved entry point. No behavior change for entry points that fit the buffer.
Two tests are added: one in bun-build-api.test.ts that spawns a child bun and asserts a 100 000-byte entry point produces exactly one ModuleNotFound BuildMessage and zero outputs, both alone and alongside a valid entry point; and one in worker.test.ts that asserts new Worker(longSpecifier) fires an error event whose message names the specifier. Both follow the repo's harness conventions (tempDir, bunEnv, Buffer.alloc(n, fill), concurrent stdout/stderr/exit drain, exit code asserted last, child process so a crash surfaces as a failed assertion).
Security risks
None. This is error-message plumbing on an already-failing resolve path; no new parsing, no allocation sized by the input, no privilege or filesystem effects. The over-long path never reaches join_abs_string_buf_z (unchanged from before — the guard predicate is the same), so the PathBuffer unchecked-write concern is unaffected.
Level of scrutiny
Low. The Rust change is an 8-line relocation of an existing condition within one function, changing which arm falls through to already-existing logging code. The function's own doc comment states it reports the error to the log; the fix makes the too-long branch honor that contract. The tests are additive and placed in the existing files for their modules.
Other factors
The PR description documents USE_SYSTEM_BUN=1 failing both new tests with the exact symptoms described (panic / success: true / BuildMessage: undefined) and bun bd test passing, plus that the full bun-build-api.test.ts and plugins.test.ts still pass. It explicitly scopes out the adjacent within-a-few-bytes resolver write (#35857). No prior human reviews or outstanding comments on the timeline (only a CodeRabbit rate-limit notice).
|
Status: nothing outstanding from review; the change is the 8-line relocation of the length guard described above plus the two tests, both verified failing on the released build and passing here. Sibling PRs from the same census: #38379 (shell mkdir/touch) and #38392 (_nodeModulePaths, onResolve paths); all three are independent. Waiting on CI. |
|
Updated 4:05 AM PT - Aug 14th, 2026
❌ @robobun, your commit f78f612 has some failures in 🧪 To try this PR locally: bunx bun-pr 38391That installs a local version of the PR into your bun-38391 --bun |
|
Ready for a maintainer. The change is unchanged since it was opened (head f78f612): the length guard in resolve_entry_point now only skips the cache bust, so the error is logged like any other; the Bun.build test and the Worker test both fail on the released build and pass here, and both passed on every lane of build 95583. The lanes that build reports as failed are retry-passed flakes in unrelated files (fs read-stream pos, inspect-error-leak, bun-patch and filter-workspace on Windows aarch64, a napi threadsafe-function batch, cluster-shared-leak, malformed-integrity-base64), so I am not pushing a retrigger for them. Sibling PRs from the same census: #38379, #38392. |
…-no-bundle entries resolve_entry_point is back to what main has: #38391 already carries the same change with tests that also cover Windows and Bun.build. The browser field message gets the same "(entry point)" suffix #38778 uses, and the new test pins that --no-bundle now resolves "entry" to entry.ts the way a bundling build does.
…linking zero entry points (#39799) ### Problem - `bun build` and `Bun.build()` abort with `panic: index out of bounds: the len is 0 but the index is 0` in `generate_chunks_in_parallel` (`generateChunksInParallel.rs:64`, `chunks[0]`) when every entry point is dropped (Sentry BUN-3RAS). With a live entry point beside it, the build exits 0 and silently emits fewer outputs. - Three producers drop an entry point without a log entry, so the drivers link with `graph.entry_points` empty: (a) a result with every path disabled (`"browser": {"./a.ts": false}`, or `fs` / `node:*` under the browser target); (b) an onResolve plugin that returns `external: true` for it; (c) an over-long specifier, which `resolve_entry_point` returned before it logged. ### Fix - `resolve_entry_point` rejects a disabled result: `"./a.ts" is disabled due to "browser" field in package.json (entry point)` or `Cannot use Node.js builtin "fs" as an entry point`. This covers the CLI, `Bun.build()` and the plugin fallback. It makes two no-path arms unreachable, so they are deleted: the `Ok(None)` return in `enqueue_entry_item` (the old drop site) and the `Worker entry point is missing` arm in `web_worker.rs`. - `on_resolve` logs `The entry point "x" cannot be marked as external` (esbuild's error). The length guard now only skips the cache bust, so an over-long entry point logs `ModuleNotFound` like any missing one. - Backstop: both drivers fail with `None of the entry points could be bundled` when no entry point survives parsing. The linker's `debug_assert` stays. The CLI drivers check the log after `wait_for_parse()`, as the JS driver did, so no parse task is in flight at teardown. - Verified: `test/bundler/bundler_browser.test.ts`, `bundler_plugin.test.ts`, `bun-build-api.test.ts`, `test/js/web/workers/worker.test.ts` (new cases, all red on 1.4.0). Other suites: see notes. ### Background - A disabled module is how the resolver represents `"browser": false` and browser-stubbed builtins: `Result::path()` is `None` and an import of it becomes `{}`. An entry point has nothing to emit in that state. - `enqueue_entry_item` appends each resolved entry point to `graph.entry_points` (a plugin answer arrives in `on_resolve` instead). The drivers wait for parsing, fail if the log has errors, then link. The linker needs one entry point, so every drop has to log. Supersedes #38778 and #38391. Carries the entry point arm of #35053, whose import path rewrite is independent. <details><summary>Notes</summary> Repros on 1.4.0 (each exits 134, now exits 1 or returns `success: false` with one message): ```sh bun -e 'await Bun.build({entrypoints:["node:fs"]})' bun build node:fs bun build fs echo '{"browser":{"./a.ts":false}}' > package.json; bun build --target=browser ./a.ts bun -e 'await Bun.build({entrypoints:["./b.ts"], plugins:[{name:"x", setup(b){ b.onResolve({filter:/b\.ts$/}, a => ({path:a.path, external:true})) }}]})' bun -e 'await Bun.build({entrypoints:["a".repeat(5000)]})' ``` Local debug build only: three `terminate()` tests in `worker.test.ts` (message flood, preload with un-awaited `import()`, `fs.readFile` completions) fail in this container, and fail the same way with the unmodified main sources built here. `production > works with sourcemaps` in `test/bake/dev/production.test.ts` hits its 5 s budget here and passes in 5.07 s with a longer one, with the expected `oh no!` output. The release binary passes all four. None of them involve entry point resolution. Other suites run on the debug build: `bundler_edgecase`, `bundler_naming`, `bundler_html`, `cli`, `test/bake/dev-and-prod`, and the three bundler files above in full. 1.3.x had the same drops and returned `success: true, outputs: []`. The port added the bounds check, so the drop now aborts. Silent drop on 1.4.0: `bun build --target=browser ./a.ts ./b.ts --outdir=out` exits 0 and writes only `b.js`. Now it exits 1 with the `./a.ts` error and writes nothing. The plugin test and the browser tests pin this form too. Long directory form of (c): a cwd of 3835 bytes plus a 500 byte relative entry point (`top_level_dir + entry + 4 > MAX_PATH_BYTES`) aborts on 1.4.0 alone and is silently dropped next to a valid entry point. With this branch both report `ModuleNotFound resolving "./eee...js" (entry point)` from the CLI and from `Bun.build()`. The same entry point as a 4337 byte absolute path still aborts in `load_as_file` (`src/resolver/resolver.rs:5888`), the resolver overflow #39626 is for. A specifier longer than the buffer inside a package with a `browser` field aborts in `check_browser_map` (`resolver.rs:5108`), which #37532 is for. Neither is an entry point drop. Worker: the length guard also made `new Worker(longName)` fire its error event with `BuildMessage: undefined`. The worker test pins the message. Unchanged: `--external ./b.ts` or `external: ["*"]` on an entry point still bundles it (entry points are exempt from external patterns, #12734). An absolute entry point path is never looked up in the browser map, so the bake and dev server callers of `resolve_entry_point`, which pass absolute paths, cannot hit the new error. `node:path` under the browser target still bundles its polyfill. Backstop reachability: the only known route left is `bun build --target=bun bun:wrap` in a release build (the specifier collides with the runtime's `bun:wrap` map key, so `enqueue_entry_item` returns `Ok(None)`). A debug build trips `assert_file_path_is_absolute` on that input first, so the backstop has no debug-runnable test of its own. Builtin specifiers as entry points under `--target bun`/`node` are a separate, pre-existing problem and are reported separately. Teardown: `enqueue_entry_points_common` schedules the runtime parse task before any entry point is resolved. #38778 saw ASAN crashes in `Worker::deinit_soon` on the CLI error path while the drivers still returned before `wait_for_parse()`. With this branch under the ASAN debug build, 30/30 runs of `bun build --target=browser ./a.ts` exit 1 with the message, and 20/20 runs with two bad entry points report both errors. `USE_SYSTEM_BUN=1` (1.4.0): the two new bun-build-api tests fail (the child aborts), the plugin test fails (the child aborts), 4 of the 5 new bundler_browser cases fail (the `--target=bun` control passes both ways by design), the worker test fails with `BuildMessage: undefined`. #38752 (`--no-bundle`) keeps its own message in the transform path, which this change does not touch. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bun-build-api.test.ts test/bundler/bundler_plugin.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
### Problem - GitHub closes only the first reference after a keyword, so "Fixes #1, #2" leaves #2 open. "Supersedes #3" links nothing, and no reference closes a pull request. - The last 1000 merged PRs name 274 such references. PR #32292 is open although merged #36135 says "Supersedes #32292". ### Fix - `.github/workflows/close-linked-issues.yml` runs on `pull_request_target` `closed` (a merge into the default branch of `oven-sh/bun`) and on `workflow_dispatch` with a PR number and `dry_run`. Everything is inline in one `actions/github-script` step, with no checkout. - Each open target is closed as `completed` with the comment "Closed as completed by #N." or "Superseded by #N.". Closed or missing targets, the PR itself and other repositories are skipped. - The parser has no regex. A closing keyword (close, fix, resolve, supersede, replace, any tense) must lead the reference, alone or in a list. A negated, hedged or noun keyword, or one whose subject is another reference, does not count ("may fix", "the rm fix #1", "#100 supersedes #1"). - Verified: `test/internal/close-linked-issues.test.ts` (333 cases) runs the YAML's script against fake `github`, `context` and `core`. Also the 1000-PR parse (Notes). ### Background - GitHub's own keywords are close, fix and resolve (-s, -ed). Each links one reference, and only a merge into the default branch closes it. - `pull_request_target` runs in the base repository with a write token, also for fork PRs. That is safe only when no PR-controlled code runs. Here the description is the only PR input, parsed as text. <details><summary>Notes</summary> A close through the API does not create the "closed this in #N" timeline link that GitHub makes for its own closes. The comment carries the PR number instead. How the parser was calibrated. I pulled the descriptions of the last 1000 merged PRs and listed every line with a keyword next to a reference. The keyword families, list shapes and reference forms in the script are the ones that appear there. A reference is `#1`, `owner/repo#1`, an issue or pull URL (bare or in `<>`), or a markdown link. Four lines would have been wrong with a plain keyword-then-reference rule, and each led to a rule: - "the open `rm` fix #37521" (#38379): "fix" as a noun. Base forms (fix, close, resolve, supersede, replace) count only at the start of a sentence or line, or after will, should, does, and, and a few similar words. "to" is not one of them ("unable to fix #1", "how to fix #1"). - "May also fix #12318 / #10046, untested" (#38242): hedged. may, might, could, would, partially and the negations disqualify the keyword, looking past adverbs such as "also". - "Supersedes the closed #26040" (#36289) and "a comment on closed #35351" (#35365): "closed" as an adjective. A determiner or preposition before the keyword disqualifies it. - "supersedes #33130's optimisation" (#35843): a number that continues into a word is not a reference. Review added: a reference before the keyword is the subject ("#100 supersedes #1"), also through "which" or "that" ("reverts #100, which fixed #1") and across a removed span ("#100 ~~also~~ fixes #1"). A hedge two words before the keyword disqualifies it ("hopefully this fixes #1", "could this fix #1?"). A clause that starts with if, when, once, until or unless is not a statement. The tokenizer keeps a line break as a token so that "Fixes #1" on one line and "Fixes #2" on the next stay two statements. Code spans, fences, indented code, blockquotes, HTML comments and strikethrough are skipped. The block stripping follows CommonMark for fences (also inside a blockquote), indented code, blockquotes with lazy continuation, setext underlines and HTML comments, and GFM for `~~` flanking. Result over the 1000 descriptions: 274 distinct references in 135 PRs. I checked the current state of all of them through GraphQL. All but one are closed (202 issues completed, 5 duplicates, 66 pull requests). The one open target is PR #32292, superseded by merged #36135. No open target is a false positive. Every review change kept this result. Patterns that are deliberately not handled: a bulleted list under "Closes:" on its own line (not seen in the sample), references separated by whitespace only ("#1 #2"), "fix for #1", and GH-1 style references. A `?` after the list is not treated as a question. The block parser tracks no list containers, so a second paragraph of a list item indented by four spaces is read as an indented code block and skipped. A removed span or inline comment reads as one word, so "Fixes <!-- n --> #1" finds nothing. The test suite covers: the phrases above, stopping at the right place in real sentences, CRLF descriptions, URLs with fragments or a `/files` suffix, case-insensitive `Owner/Repo#1`, the fake API where a lookup, an update or a comment fails, the `dry_run` input, an invalid `pr_number` input, an unmerged PR, a PR merged into a non-default branch, the merge event body against a later edit, and a description with no closing statement. The first revision of this PR checked out the repository and ran `scripts/close-linked-issues.ts`. Jarred asked for no checkout and no script file, so the script moved inline into the workflow and the test now reads it out of the YAML. </details> <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 9 · 2 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/internal/close-linked-issues.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/close-linked-issues.test.ts bun test v1.4.1 (4448a2e) test/internal/close-linked-issues.test.ts: (pass) finds "Fixes #39852" [176.21ms] (pass) finds "Closes #31772. Fixes #31771." [22.28ms] (pass) finds "- Fixes #39930" [12.28ms] (pass) finds "Fixes: #30429" [10.46ms] (pass) finds "FIXES #1" [7.86ms] (pass) finds "(Fixes #1)" [8.97ms] (pass) finds "**Fixes #1**" [10.20ms] (pass) finds "__Fixes #1__" [9.83ms] (pass) finds "_Fixes #1_" [11.25ms] (pass) finds "Fixes **#1**" [9.72ms] (pass) finds "**Fixes** #1" [7.13ms] (pass) finds "**Fixes:** #1" [8.11ms] (pass) finds "Fixes #1 and **#2**" [11.47ms] (pass) finds "Fixes **#1**, **#2**" [9.13ms] (pass) finds "## Why (fixes #13771, closes #30543)" [16.08ms] (pass) finds "Closes #11418" [19.46ms] (pass) finds "Resolves #1. Resolved #2. Resolve #3." [12.09ms] (pass) finds "Fixes #34055, #30327, #24394, #20816, #32403, #11898, #10056." [17.11ms] (pass) finds "Fixes #18192 and #31675 as a consequence" [10.45ms] (pass) finds "Fixes #1, #2, and #3" [10.96ms] (pass) finds "Fixes #1 & #2" [7.63ms] (pass) finds "Closes #33280, Closes #32864 and Closes #29696 (the timer in #32949 is orthogonal)" [20.29ms] (pass) finds "Closes #33182 and #32947 on top of current main (which already has #36304 for catalogs)." [16.12ms] (pass) finds "Fixes #1,\n#2" [7.76ms] (pass) finds "Fixes #1, #2,\nand #3" [9.27ms] (pass) finds "Fixes #1\nand #2" [8.57ms] (pass) finds "Fixes #1\n& #2" [6.80ms] (pass) finds "Fixes #1 and\n#2" [7.31ms] (pass) finds "Supersedes #39908 (same change, moved from a fork branch)" [13.21ms] (pass) finds "Supersedes #38778 and #38391. Carries the entry point arm of #35053." [14.43ms] (pass) finds "Supersedes #39193 and keeps its three tests." [11.48ms] (pass) finds "This supersedes #33306 and #32803. Their tests are kept here." [13.73ms] (pass) finds "- This replaces #33793. Its ... (truncated) Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` .github/workflows/close-linked-issues.yml | 950 ++++++++++++++++++++++++++++++ test/internal/close-linked-issues.test.ts | 598 +++++++++++++++++++ 2 files changed, 1548 insertions(+) ``` </details> **gate history** · 29 passed · 0 rejected · iteration 9 <details><summary>evidence per changed file</summary> ``` file reads edits tests .github/workflows/close-linked-issues.yml 6 12 0 test/internal/close-linked-issues.test.ts 3 11 0 ``` </details> <!-- robobun:evidence:end -->
Problem
Bun.build({ entrypoints: [name] })wherecwd/namedoes not fit a path buffer (4096 bytes on Linux, 1024 on macOS) aborts the process instead of failing the build:panic: index out of bounds: the len is 0 but the index is 0(release) /assertion failed: chunks.len() > 0ingenerateChunksInParallel.rs(debug).Bun.build({ entrypoints: ["./valid.js", name] })instead succeeds: one output, no logs, the long entry point silently dropped.new Worker(name)with such a path fires its error event withBuildMessage: undefined(the "Worker prints error: undefined" face from the path-length census).Transpiler::resolve_entry_point(src/bundler/transpiler.rs) is documented as logging the resolve error before returning it, and all of its callers rely on that (enqueue_entry_points_normalinbundle_v2.rsjustcontinues onErr; the Worker code turns the log into the event's message). The guard added for paths too long to build the cache-buster name in aPathBufferreturnedErrfrom a match arm ahead of the logging, so for those paths nothing was logged: the bundle went on with zero entry points, and the Worker converted an empty log.Fix
ModuleNotFound resolving "<name>" (entry point). Entry points that fit take the same path as before.test/bundler/bun-build-api.test.ts, "an entry point too long for a path buffer is reported like any other missing one": a child bun builds a 100000-byte entry point (longer than the buffer on Windows too) alone and next to a valid one; both must fail with that one message and no outputs.test/js/web/workers/worker.test.ts, "names the entry point when its path is too long for a path buffer": the error event's message must name the entry point.USE_SYSTEM_BUN=1: the build test fails (child aborts with the panic above; the two-entry-point case on its own reportssuccess: true, 1 output), the worker test fails withReceived: "BuildMessage: undefined".bun bd teston this branch: both pass; the fullbun-build-api.test.tsandplugins.test.tsfiles pass too. Three unrelatedterminate()tests inworker.test.tsfail identically on an unmodified main debug build in this container (release passes them).cwd/nameis within a few bytes of the buffer still reaches the uncheckedload_as_filewrite in the resolver; that is resolver: bound load_as_file path before writing into its PathBuffer #35857's site and is not touched here.Background
Bun.buildresolves each entry point up front withresolve_entry_point; on failure it retries once after invalidating the resolver's cache of the entry point's directory (so a file created moments ago is found), then logs the error. The bundle runs to the linker regardless and fails at the end if the log has errors, which is why a failure that was not logged turns into a build with no entry points at all.PathBuffer([u8; MAX_PATH_BYTES]) with unchecked joins, hence the guard: a path that long cannot name a directory that exists, so there is nothing to invalidate.Worker's entry point is resolved by the same function; the worker turns whatever the resolver logged into theerrorevent's message.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bun-build-api.test.ts