bundler: canonicalize the asset source directory for [dir] only when string relativization falls outside root

[robobun]

Problem

assetNaming: "[dir]/..." produces a path like ./_.._/_.._/.../AZUREU~1/AppData/Local/Temp/.../src/lib/second/test.file instead of ./lib/second/test.file on Windows whenever the working directory contains an 8.3 short path component (the default for %TEMP% on GitHub/Azure runners is C:\Users\RUNNER~1\...):

> bun build ./src/lib/first/file.js --root=.\src --asset-naming "[dir]/test.[ext]" --outdir out
  hello.js                                                                                                            219 bytes  (entry point)
  .\_.._\_.._\_.._\_.._\_.._\_.._\AZUREU~1\AppData\Local\Temp\asset-naming-dir\src\lib\second\test.file  15 bytes   (asset)

The same failure mode also appears on POSIX with Bun.build({ files }) when the map keys and root reference the same directory through a symlink.

Cause

options.root_dir is get_fd_path-canonical (GetFinalPathNameByHandle on Windows, /proc/self/fd on Linux), so it always carries the long/resolved spelling. source.path.text for an asset handled by process_files_to_copy may not be: it is the resolver-joined path derived from the process cwd (which on Windows still carries 8.3 short components), or a literal Bun.build({ files }) key. relative_platform finds no common prefix, emits a run of .. segments, and the [dir] sanitizer rewrites them to _.._.

Entry/chunk [dir] substitution in compute_chunks already compensated by opening each chunk's directory and reading back its canonical path before relativizing.

Fix

Extract a shared options::source_dir_relative_to_root helper and route both process_files_to_copy and compute_chunks through it. The helper first relativizes with plain string math; only when that result walks above root_dir (the spelling-mismatch case this PR targets) and the source is in the file namespace does it openat+get_fd_path the source directory and relativize again. In the common case where the resolver's path already sits under the canonical root there is no filesystem access at all, and compute_chunks no longer pays the unconditional per-chunk openat it carried on main.

[name]/[ext] are basename-only and now come straight from source.path.text without the relativization round-trip. A plugin asset in a non-file namespace keeps the pure-string result so its virtual dirname is never opened against an unrelated cwd directory.

Test

naming/AssetNamingDirCanonicalRoot creates real/src/lib/{first,second} on disk, a project-link symlink (a junction on Windows) pointing at real, and runs Bun.build with files keys and root spelled through the link. Before this change the asset path is ./_.._/_.._/project-link/src/lib/second/test.file; after, ./lib/second/test.file.

naming/AssetNamingDirVirtualNamespace covers the non-file namespace guard: a plugin resolves virt:assets/icon.bin with loader: "file" while an assets symlink exists in cwd, and the asset [dir] stays _.._/assets (unchanged from main) rather than following the symlink.

Verified on Linux and Windows; the original Windows short-name CLI repro, bundler/cli.test.ts log cases, and bundler_naming/bundler_loader/bundler_files/bundler_plugin/bundler_edgecase/esbuild/loader all pass.

---

no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_naming.test.ts

[coderabbitai]

Walkthrough

The bundler centralizes [dir] computation relative to the configured root, optionally canonicalizes on-disk paths, updates copied-asset and chunk naming, and adds symlink and virtual-namespace regression tests.

Changes

[dir] placeholder handling

Layer / File(s)	Summary
Root-relative directory resolution src/bundler/options.rs	Adds shared relative-directory computation with optional on-disk canonicalization for paths outside the configured root.
Asset and chunk placeholder integration src/bundler/bundle_v2.rs, src/bundler/linker_context/computeChunks.rs	Copied assets use parsed path components and conditional [dir] computation; chunks use the shared helper with on-disk resolution enabled.
Symlink and virtual namespace coverage test/bundler/bundler_naming.test.ts	Adds tests for canonicalized symlink paths and non-canonicalized virtual namespace paths.

Possibly related PRs

• oven-sh/bun#34552: Updates Windows [dir] naming expectations related to the same placeholder behavior.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	It clearly summarizes the asset [dir] canonicalization fix when string relativization escapes the root.
Description check	✅ Passed	It includes the problem, cause, fix, and test verification, covering the required PR intent and validation details.

---

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

[robobun]

^{Updated 5:36 PM PT - Jul 18th, 2026}

❌ @robobun, your commit 4f0eff5 has 1 failures in Build #75547 (All Failures):

• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34558

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

bun-34558 --bun

[github-actions]

Found 1 issue this PR may fix:

1. bun build will drop files *outside* outdir #19591 - Reports [dir] expanding to .. path segments causing assets to be written outside outdir, which is the same root cause this PR fixes by canonicalizing source paths before relativizing against root_dir

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

Fixes #19591

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@src/bundler/linker_context/computeChunks.rs`:
- Around line 636-640: Update the chunk directory computation in the surrounding
linker context to derive the source’s on-disk status from path.is_file() instead
of unconditionally enabling filesystem canonicalization, preserving virtual
namespaces when names collide with real directories. Extend the virtual chunk
naming coverage with a case that reaches the [dir] branch rather than relying on
the existing entry "hello.[ext]" test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5e6828b6-5ad9-4e0d-9267-f399a3d88cdd

📥 Commits

Reviewing files that changed from the base of the PR and between 0170259 and d98a7b9.

📒 Files selected for processing (4)

• src/bundler/bundle_v2.rs
• src/bundler/linker_context/computeChunks.rs
• src/bundler/options.rs
• test/bundler/bundler_naming.test.ts

[claude]

All four earlier threads are addressed in the current revision; nothing new to flag. Leaving this for a human because it changes bundler output-path computation and drops the get_path error branch in compute_chunks in favour of the silent normalize_buf fallback — the PR description states the rationale, but that's a policy call worth a maintainer's eyes.

Checked: bun_sys::File and the path_buffer_pool guard both release on Drop, so the helper leaks neither the fd nor the buffer; [name]/[ext] now derive from the raw source.path.text instead of the relativized path, which is basename-equivalent; placeholder.dir is now only set when the template actually contains [dir], which is fine since print only reads it under the same condition; the ? on relative_alloc composes with both callers' error types via the existing From<AllocError>.

Extended reasoning...

Overview

The PR extracts a shared options::source_dir_relative_to_root helper (open the source dir, get_fd_path it, fall back to string normalization, then relative_alloc against root_dir) and routes both process_files_to_copy (asset [dir]) and compute_chunks (entry/chunk [dir]) through it. Two new subprocess tests in bundler_naming.test.ts cover the symlinked-root case and the non-file-namespace guard.

Security risks

None identified. The helper opens directories O_PATH|O_DIRECTORY read-only and only for paths the bundler already resolved; the on_disk flag prevents a plugin's virtual namespace path from being interpreted as a cwd-relative filesystem path (covered by the second test).

Level of scrutiny

Medium-high. This is bundler output-path computation — a wrong [dir] writes assets to the wrong location — and it deliberately removes the "Failed to get full path for directory" error branch from compute_chunks in favour of the same silent fallback that openat failure already took. The PR description argues this is a consistency fix and that no test exercised the removed branch; that reasoning is sound, but silently degrading vs. failing the build is a policy choice a maintainer should confirm.

Other factors

I left four inline threads on earlier revisions (comment length, expect(stderr).toBe(""), extract-a-helper, dropped why-comments); all four were addressed and are resolved. The bug-hunting pass on the current revision found nothing. I additionally verified: RAII cleanup for the fd and pooled buffer in the helper; that deriving [name]/[ext] from source.path.text directly is equivalent to the old relativized-then-split path; that leaving placeholder.dir at its default when [dir] isn't in the template is unobservable through PathTemplate::print; and that the helper's AllocError return propagates through both callers' ?. Not simple/mechanical enough for auto-approval, but nothing blocking from my side.

[claude]

No issues found — all four rounds of prior inline feedback (helper extraction, restored regression comment, comment length, combined-object stderr assertion) are addressed and the CodeRabbit thread is resolved. Deferring for a maintainer glance because the shared helper deliberately changes compute_chunks' get_path failure from a build-abort to a silent normalize_buf fallback; that's argued as consistent with the existing openat fallback but is a policy change on the entry-chunk path.

What was reviewed:

• source_dir_relative_to_root: RAII on the pooled path buffer and opened fd, empty-dir → b"." guard preserved with its regression comment, relative_alloc error propagation matches both callers' Result types.
• process_files_to_copy: [name]/[ext] now derived from source.path.text directly — basename-only, so equivalent to the old relativized pathname; placeholder.dir only set under needs(Dir), unused otherwise.
• compute_chunks refactor is behavior-preserving apart from the noted get_path error path; on_disk=true matches the removed unconditional openat.
• Both new tests use tempDir + junction/symlink, drain pipes concurrently, and pin the virtual-namespace asset to the pre-PR _.._/assets string result.

Extended reasoning...

Overview

The PR fixes asset [dir] placeholder resolution when the asset's source path and the configured root_dir spell the same on-disk directory differently (Windows 8.3 short components in the cwd, or symlinked Bun.build({ files }) keys). It extracts options::source_dir_relative_to_root(source_dir, root_dir, on_disk) — open + get_fd_path when on_disk, else string-normalize, then relative_alloc — and routes both process_files_to_copy (asset naming) and compute_chunks (entry/chunk naming) through it. Two subprocess tests cover the symlinked-root case and the virtual-namespace guard.

Security risks

None identified. The helper opens a directory read-only (O::PATH | O::DIRECTORY) relative to cwd and reads back its canonical path; inputs are bundler-internal source paths, not user-controlled at runtime. The virtual-namespace guard (on_disk = source.path.is_file()) prevents a plugin's virtual path from being resolved against an unrelated real cwd directory.

Level of scrutiny

Medium-high. Bundler output-path computation is user-visible, cross-platform (the motivating case is Windows-specific), and the refactor unifies two previously divergent code paths. The one deliberate behavior change — compute_chunks now silently falls back to string normalization when get_path fails after a successful openat, instead of aborting the build with a logged error — is documented in the PR description as aligning with the existing openat-failure fallback, and no test exercised the removed error, but it is still a policy choice a maintainer should sign off on.

Other factors

This is the fifth review pass on the PR. Earlier inline nits from me (extract the duplicated block, restore the bare-filename regression comment, trim comment length, combined-object stderr assertion) were all applied and the threads are resolved; CodeRabbit's virtual-namespace concern on the compute_chunks side was withdrawn after confirming on_disk=true preserves pre-PR behavior. The bug-hunting system found nothing this run. CI (#75144) is still building. The change is well-scoped and well-tested; I'm deferring rather than approving only because it is not a mechanical change and touches cross-platform output-path logic with an intentional error-handling relaxation.

[robobun]

Build #75547 (commit 4f0eff5, the string-first fallback): test/bundler/cli.test.ts and test/bundler/bundler_naming.test.ts are green on every lane including all three Windows targets (the log-case-1 regression from the earlier revision is gone). Remaining reds are all [flaky]/[pre-existing] on unrelated test files (install security-scanner matrix, proxy-stress, install-registry hoisting, shell-hang, webview-chrome, fs-promises FileHandle GC, solc, no-orphans, next-build, in-process-cron, install-lifecycle-scripts, 20144, and a pre-existing worker-message-port SIGABRT on the ASAN lane). Ready for another look.

[Jarred-Sumner]

This is very expensive to do if there are lots of entry points or chunks. Do we need to resolve in the filesystem?

[robobun]

Reworked in 815c7c2 to avoid the filesystem entirely: options.root_dir is now absolutized against top_level_dir via FileSystem::abs_buf (the same join the resolver uses for every source path) instead of get_fd_path. With the stored root sharing its spelling with source paths by construction, [dir] for both assets and entry chunks is a plain relative_alloc call. That also drops the per-chunk openat+get_fd_path that compute_chunks already carried on main to compensate for the mismatch. The root directory is still opened once at setup to report ENOENT.

Net: zero per-file syscalls (down from one per chunk on main, and from the one per asset the earlier revision of this PR added). Both new tests and the 249-test bundler sweep pass on Linux; Windows verification is building.

[robobun]

Follow-up to the above: the pure abs_buf approach regressed on-disk builds where the resolver realpaths source paths (so they no longer prefix-match an uncanonical root_dir); cli.test.ts log case 1 caught it on the Windows lanes. Dropped that commit.

4f0eff5 keeps root_dir canonical (as on main) and makes the helper relativize with plain string math first, falling back to openat+get_fd_path only when that result walks above root and the source is in the file namespace. So a project whose sources sit under the canonical root (the normal case) touches the filesystem zero times for [dir], and compute_chunks no longer does the per-chunk openat it has on main. The fallback only fires on the spelling-mismatch cases this PR targets (Windows 8.3 short names in cwd, Bun.build({ files }) keys through a symlink), and for a source that is genuinely outside root it changes nothing.

Verified on Windows: the short-name CLI repro, both new bundler_naming tests, and cli.test.ts log cases all pass.

[claude]

My previous comment about build #75530 / commit 052050d is stale — that build was from the since-dropped 815c7c2 line, and the branch had already been force-pushed back to 4f0eff5 (which build #75547 shows green on all three Windows lanes for cli.test.ts) before I posted it. Sorry for the noise; that concern is resolved.

No new issues found on 4f0eff5. Deferring to a human for the final look given this is cross-platform bundler output-path logic that went through several revisions, and compute_chunks now degrades silently on a get_path failure where main failed the build.

What was reviewed:

• The shared source_dir_relative_to_root helper: string-first relativize, openat+get_fd_path fallback only when the result walks above root and on_disk — traced that skipping the openat when pathname.dir already prefix-matches the canonical root_dir yields the same [dir] as main's unconditional canonicalize.
• compute_chunks call site passes on_disk = true, preserving pre-PR behaviour for entry chunks; the bundle_v2 asset site gates on source.path.is_file() so virtual-namespace assets stay pure-string.
• Two new tests cover the symlinked-root and virtual-namespace-collision cases; the bare-filename is_empty() guard and its regression comment carried over.

Extended reasoning...

Overview

The PR fixes [dir] placeholder expansion in assetNaming when root_dir (which is get_fd_path-canonical) and an asset's source.path.text spell the same directory differently — Windows 8.3 short names in the process cwd, or Bun.build({ files }) keys through a symlink. It extracts a shared options::source_dir_relative_to_root helper used by both process_files_to_copy (bundle_v2.rs) and compute_chunks (computeChunks.rs), replacing ~40 lines of duplicated logic in the latter. Four files touched: three Rust source files in src/bundler/ and two new tests in test/bundler/bundler_naming.test.ts.

Timeline correction

My last inline comment (2026-07-19 00:20, on computeChunks.rs:640) flagged cli.test.ts failing on Windows in build #75530 for commit 052050d. That commit was on the 815c7c2 "pure string math / no canonicalization" branch, which robobun had already dropped 17 minutes earlier (00:03) in favour of reverting to 4f0eff5 — the current HEAD — which robobun's earlier comment confirms green on all three Windows lanes in build #75547. My comment was reviewing a superseded state and should be considered resolved.

Security risks

None identified. This is output-path computation for bundler artifacts; no untrusted input reaches a security-sensitive sink. The openat uses O::PATH | O::DIRECTORY on a path derived from resolver-produced source paths, same as main did unconditionally.

Level of scrutiny

Medium-high. Bundler output-path logic is user-visible on every build, cross-platform (Windows 8.3, junctions, POSIX symlinks), and this PR went through five revision cycles including one (815c7c2) that regressed on-disk symlinked-root builds and Windows cli.test.ts. The final approach is sound and CI-verified, but two behaviour changes vs main deserve a human sign-off: (1) compute_chunks no longer opens each chunk's directory unconditionally — it skips the syscall when the string relativize already lands under root; (2) a get_path failure after a successful openat now silently falls back to the string result instead of logging an error and failing the build (robobun's justification: it now matches what both sites already did on openat failure, and no test exercised the removed path).

Other factors

All earlier review threads (test hygiene, helper duplication, dropped why-comments, virtual-namespace gating, the 815c7c2 regression) are resolved. The two new tests use tempDir, bunEnv/bunExe, drain both pipes concurrently, and assert a combined {assetLine, stderr, exitCode} object per repo conventions. The bug-hunting system found nothing on this revision.