Runtime plugins: stop treating a single-letter namespace as a Windows drive

[robobun]

A runtime Bun.plugin onResolve result with a single-letter namespace silently loses the namespace: the registered onLoad never runs and the import fails. Two or more letters work, and the same plugin works under Bun.build.

const mk = ns => ({
  name: "p" + ns,
  setup(b) {
    b.onResolve({ filter: new RegExp(`^virt-${ns}\\.mod$`) }, () => ({ path: "pp", namespace: ns }));
    b.onLoad({ filter: /.*/, namespace: ns }, () => ({ contents: `export default ${JSON.stringify("NS-" + ns)}`, loader: "js" }));
  },
});
for (const ns of ["q", "z", "A", "qq"]) {
  Bun.plugin(mk(ns));
  try { console.log(`ns=${ns} ->`, (await import(`virt-${ns}.mod`)).default); }
  catch (e) { console.log(`ns=${ns} -> ERR`, e.message.split("\n")[0]); }
}

ns=q  -> ERR Cannot find package 'q:pp' from ''
ns=z  -> ERR Cannot find package 'z:pp' from ''
ns=A  -> ERR Cannot find package 'A:pp' from ''
ns=qq -> NS-qq

Cause

onResolve's { path, namespace } is serialized into the module key "ns:path", and the loader re-parses it. Since #29393 the C++ module loader calls resolve() a second time on keys that moduleLoaderImportModule already resolved, so moduleLoaderResolve short-circuits a key whose "ns:" prefix names a registered onLoad namespace and returns it unchanged.

That short-circuit carved out Windows drive letters with colon == 1 && isASCIIAlpha(key[0]), which is true for every single-letter namespace, on every platform. "q:pp" was read as drive q:, fell through to the filesystem resolver, and failed.

PluginRunner::extract_namespace, which decides the namespace everywhere past that point (Bun__runVirtualModule, Zig__GlobalObject__resolve), already had the correct rule, so the two disagreed about what "q:pp" means.

Fix

moduleLoaderResolve now only treats a prefix as a drive when it is an actual Windows drive root (C:\ or C:/) and the process is running on Windows, matching extract_namespace.

extract_namespace spelled the same carve-out with an exclusive comparison (specifier[0] > b'a' && specifier[0] < b'z'), which excluded the a, z, A and Z drive letters; it now uses bun_paths::resolve_path::is_drive_letter so both sides state the same rule.

No change on non-Windows for extract_namespace; on Windows, a:\x / z:\x / A:\x / Z:\x and the bare drive root C:\ are now drives rather than namespaces.

Behavior change worth a maintainer's eye

One user-visible semantic changes off Windows, and it is the only judgement call in this PR.

With a single-letter namespace registered for onLoad and no onResolve for it, a specifier shaped like C:\foo\x.js now reaches that plugin instead of the filesystem resolver. This is not a new reading of C:\..., it is the reading the rest of the runtime already used. On stock 1.4.0, unpatched, Linux, an onResolve registered for namespace C already claims it:

onResolve(ns=C) path="\\__definitely_missing__\\x.js"
onLoad(ns=C)    path="\\__definitely_missing__\\x.js"
result: captured

extract_namespace gates its drive-root carve-out on Windows, so off Windows C:\foo has always been the namespace C to Zig__GlobalObject__resolve and Bun__runVirtualModule. moduleLoaderResolve was the single site that disagreed. This PR makes it agree.

The platform gate is load-bearing rather than cosmetic. Keeping the carve-out on every platform and only requiring the separator would still drop { path: "/pp", namespace: "q" }, whose key is q:/pp. That case and the C:\... case are the same shape pulling in opposite directions, so one of them has to lose per platform: drive roots exist only on Windows, so the drive reading wins there and the namespace reading wins everywhere else.

If you would rather keep C:\... rejecting off Windows, say so and I will drop the platform gate, at the cost of leaving single-letter namespaces with absolute paths broken.

Verification

test/js/bun/plugin/plugins.test.ts gains onResolve namespaces of any length reach onLoad, covering one-letter (q, A) and two-letter namespaces across bare (q:pp), nested (q:sub/pp) and rooted (q:/pp) paths. q:/pp is the one shape a one-letter namespace cannot have on Windows (it is a drive root there), so that case is asserted per platform.

test/js/bun/plugin/plugin-namespace-drive-letter.test.ts keeps asserting that a one-letter namespace never captures C:\... on Windows, and now asserts the other half of the rule elsewhere: platforms without drive roots read the prefix as the namespace, like extract_namespace already did for onResolve.

Both fail on main and pass with the fix.

The Windows half of the rule is covered on real Windows, not just by inspection: on the previous CI run both files ran on the windows 2019 x64 lane and passed (plugins.test.ts in shard 0/8, 35 pass, 0 fail; plugin-namespace-drive-letter.test.ts in shard 7/8). Before pushing, the #if OS(WINDOWS) branch was also compiled and exercised on Linux by forcing the predicate on, which flipped both per-platform assertions to their Windows values as expected.

The red check is infrastructure, not this diff: the latest build expired waiting for agents on every platform and ran zero tests, and the one before it failed a single darwin shard on a 120s artifact-download timeout. Breakdown in the first comment.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 9 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: 5691e93f-0bf4-4783-9e41-6a028061cb3e

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 6682922.

📒 Files selected for processing (4)

• src/bundler/transpiler.rs
• src/jsc/bindings/ZigGlobalObject.cpp
• test/js/bun/plugin/plugin-namespace-drive-letter.test.ts
• test/js/bun/plugin/plugins.test.ts

---

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

[robobun]

^{Updated 5:53 AM PT - Jul 6th, 2026}

@robobun, your commit 6682922 is building: #68970

[robobun]

Status: the diff is green, CI is red on infrastructure, this needs a maintainer.

Reproduction

The snippet in the description, against main (release-asan): q, z and A all fail with Cannot find package '<ns>:pp', qq loads. All four load with the fix.

Fail-before / pass-after on both test files with bun bd test:

without src/

(fail) a single-letter namespace never captures a Windows drive root
(fail) onResolve namespaces of any length reach onLoad
-   "bareUpper": "A|pp",
-   "nested": "q|sub/pp",
+   "bareUpper": "threw: Cannot find package 'A:pp' from ''",
+   "nested": "threw: Cannot find module 'q:sub/pp' from ''",

with src/

 36 pass
 1 todo
 0 fail
Ran 37 tests across 2 files.

test/js/bun/plugin/, test/bundler/bundler_plugin.test.ts, test/js/bun/resolve/ and test/js/bun/test/mock/ are green locally.

CI

Three builds ran on this branch. None produced a test failure attributable to this diff.

build	sha	outcome
68833	25ff333a	cookie-map.test.ts failed on stale Expires assertions, fixed on main by #33425, which landed one commit after this branch's base. Rebasing cleared it.
68875	bf973cbf	281 jobs passed. One failure: darwin 26 aarch64 - test-bun shard 1 hit buildkite-agent artifact download timed out after 120s. Zero tests ran on it; the darwin aarch64 - build-bun step passed and shard 0 downloaded the same artifact and passed.
68970	66829226	Zero jobs failed, zero annotations, zero tests run. Build steps expired waiting for agents on every platform (linux aarch64 - build-cpp, darwin aarch64 - build-rust, windows x64 - build-rust, freebsd x64 - build-bun, ...), leaving 200 downstream jobs waiting_failed and 50 expired.

66829226 is an empty ci: retrigger commit, so its tree is identical to bf973cbf. The real test evidence is 68833 and 68875.

Both new tests did run on real Windows, in 68833, in separate shards of the 8-way :windows: 2019 x64 - test-bun lane:

file	shard	result
test/js/bun/plugin/plugins.test.ts	0/8	35 pass, 0 fail
test/js/bun/plugin/plugin-namespace-drive-letter.test.ts	7/8	passed

So the per-platform assertions hold on Windows hardware: q:/pp threw there as a drive root, and C:\... was not captured by the registered C namespace.

I'm not pushing another ci: retrigger; a re-roll already went into 68970 and produced no signal. A maintainer re-running the build, or a fresh push, should clear it.

Open question

One user-visible semantic changes off Windows. It's written up in the description under "Behavior change worth a maintainer's eye", with the evidence that stock 1.4.0 already reads C:\... as the C namespace through onResolve, so this aligns the last disagreeing site rather than inventing a reading. Happy to drop the platform gate if you'd rather keep the old non-Windows behavior; it just leaves single-letter namespaces with absolute paths broken.

[claude]

I didn't find any issues — the fix looks correct and the two sides now agree on the drive-root rule — but this touches moduleLoaderResolve and changes observable behavior on non-Windows (a single-letter namespace like C now captures C:\\... imports where it previously didn't), so a maintainer should confirm that's the intended semantics.

Extended reasoning...

Overview

This PR fixes runtime Bun.plugin so that single-letter namespace values returned from onResolve reach their onLoad handler. Two sites are touched:

• src/jsc/bindings/ZigGlobalObject.cpp — moduleLoaderResolve's plugin-namespace short-circuit previously excluded any X: prefix on every platform; it now only excludes actual drive roots (X:\ / X:/) and only under #if OS(WINDOWS), via a new isWindowsDriveRoot helper.
• src/bundler/transpiler.rs — PluginRunner::extract_namespace had an exclusive-bounds bug (> b'a' && < b'z', missing a/z/A/Z) and a len > 3 check that skipped the bare 3-byte root; both replaced with the canonical is_drive_letter / is_sep_any helpers and len > 2.

Two tests are updated/added: a new subprocess test in plugins.test.ts covering one- and two-letter namespaces across bare/nested/rooted paths, and plugin-namespace-drive-letter.test.ts re-tightened to assert per-platform behavior.

Security risks

None identified. This is namespace/path prefix parsing for plugin dispatch; no auth, crypto, filesystem writes, or untrusted-length arithmetic is involved. The change narrows a heuristic rather than loosening one.

Level of scrutiny

Moderate-to-high. moduleLoaderResolve sits on the critical module-import path for the entire runtime, and the change is intentionally a behavior change:

• On non-Windows, a registered single-letter onLoad namespace (e.g. C) will now capture a specifier like C:\\foo\\bar.js, whereas before it fell through to the filesystem resolver and failed. The updated plugin-namespace-drive-letter.test.ts inverts its non-Windows expectation accordingly.
• On Windows, extract_namespace now treats a:\, z:\, A:\, Z:\, and the bare 3-byte C:\ as drives where it previously (buggily) treated them as namespaces.

Both changes look correct and bring the C++ and Rust sides into agreement, but they do alter user-observable resolution results, which is the kind of thing a maintainer should sign off on.

Other factors

The reasoning in the PR description is thorough, the diff is small and mechanical, cross-references between the two implementations are added in comments, and test coverage is good (per-platform assertions, multiple path shapes). No bugs were found by the automated bug hunt. CI is still building at the time of review.

[robobun]

a single-letter namespace like C now captures C:\... imports where it previously didn't, so a maintainer should confirm that's the intended semantics

Worth being precise here, because the runtime already had these semantics everywhere except the one line this PR touches. On stock 1.4.0, no patch, Linux, a plugin registered for namespace C already claims C:\...:

Bun.plugin({
  name: "probe",
  setup(b) {
    b.onResolve({ filter: /.*/, namespace: "C" }, args => {
      console.log("onResolve(ns=C) path=" + JSON.stringify(args.path));
      return { path: args.path, namespace: "C" };
    });
    b.onLoad({ filter: /.*/, namespace: "C" }, () => ({ contents: `export default "captured"`, loader: "js" }));
  },
});
console.log("result:", (await import("C:\\__definitely_missing__\\x.js")).default);

onResolve(ns=C) path="\\__definitely_missing__\\x.js"
onLoad(ns=C) path="\\__definitely_missing__\\x.js"
result: captured

That is extract_namespace doing its job: off Windows there are no drive roots, so C:\foo is the namespace C. Zig__GlobalObject__resolve and Bun__runVirtualModule have always agreed on that. The only site that read it differently was the moduleLoaderResolve short-circuit, and only for plugins that register onLoad without an onResolve for the same namespace, which is precisely the shape the old drive-letter check was covering up. So this PR does not introduce a new reading of C:\..., it makes the last disagreeing site match the other three.

The platform gate is also load-bearing rather than cosmetic. Keeping the carve-out on all platforms and only requiring the separator would still drop { path: "/pp", namespace: "q" }, since the key becomes q:/pp. That case and the C:\... case are the same shape and pull in opposite directions, so one of them has to lose on each platform: drive roots only exist on Windows, so that is where the drive reading wins, and everywhere else the namespace reading wins. plugin-namespace-drive-letter.test.ts now pins both halves, and rooted in the new test pins the boundary (q:pp and q:sub/pp are namespaces everywhere, q:/pp is a drive root on Windows only).

The Windows expectations are not theoretical: plugin-namespace-drive-letter.test.ts ran on :windows: 2019 x64 - test-bun (file 3/595) and passed.

---

Unrelated CI note: the red Windows lanes on the previous push were test/js/bun/cookie/cookie-map.test.ts failing on stale Expires assertions, fixed on main by #33425, which landed one commit after this branch's base. Rebased onto main; that file passes locally now (33 pass, 0 fail) and the diff is byte-identical.

[claude]

I didn't find any bugs, but this touches module-resolution logic in moduleLoaderResolve with platform-divergent behavior (and a subtle non-Windows change where C:\\... now reaches a registered C namespace instead of being rejected), so it's worth a human sanity-check before merging.

Extended reasoning...

Overview

This PR fixes runtime Bun.plugin handling of single-letter namespaces. Two code changes:

• src/jsc/bindings/ZigGlobalObject.cpp: the moduleLoaderResolve short-circuit previously excluded any key matching colon == 1 && isASCIIAlpha(key[0]) on all platforms, which meant every single-letter plugin namespace (q:pp) was misread as a Windows drive and fell through to the filesystem resolver. It now only carves out actual drive roots (C:\ / C:/) and only under #if OS(WINDOWS).
• src/bundler/transpiler.rs: extract_namespace's Windows-only drive check used exclusive comparisons (> b'a' && < b'z'), missing a/z/A/Z; it now delegates to is_drive_letter. The length check drops from > 3 to > 2 so a bare C:\ counts as a drive root.

Two test files updated: a new comprehensive test in plugins.test.ts covering one/two-letter namespaces across bare/nested/rooted paths with per-platform assertions, and plugin-namespace-drive-letter.test.ts reworked to assert both halves of the rule per platform.

Security risks

None identified. This is namespace-prefix parsing for plugin routing; no auth, crypto, or untrusted-input allocation is involved.

Level of scrutiny

Moderate-to-high. moduleLoaderResolve sits on the hot path for every ES module import when any runtime plugin registers an onLoad namespace, and the fix intentionally changes behavior on non-Windows: a static import like C:\\foo with a registered single-letter C namespace now reaches the plugin's onLoad instead of being rejected by the filesystem resolver (the old test asserted rejected: on all platforms; the new test asserts loaded:from-plugin on non-Windows). That aligns the C++ side with what extract_namespace already did on the Rust side, so it's the more consistent behavior — but it is a user-visible semantic change worth a maintainer's eye.

Other factors

• The Rust change is Windows-only via cfg!(windows); the > 3 → > 2 and a/z boundary fix only affect Windows, and the PR notes the Windows branch was exercised by forcing the predicate on Linux rather than on a Windows machine.
• Tests are well-constructed (subprocess isolation, per-platform assertions, fail-before/pass-after verified per the robobun comment) and the bug hunter found nothing.
• The change is small and the reasoning in the description is thorough, but module resolution + cross-platform divergence puts this outside the "mechanical/obvious" bucket for auto-approval.

[robobun]

the PR notes the Windows branch was exercised by forcing the predicate on Linux rather than on a Windows machine

That caveat is now retired, and I've updated the description accordingly. Both new test files did run on real Windows in the previous CI build, in separate shards of :windows: 2019 x64 - test-bun (the lane shards 8 ways):

file	shard	result
test/js/bun/plugin/plugins.test.ts	0/8 ([3/724])	35 pass, 0 fail
test/js/bun/plugin/plugin-namespace-drive-letter.test.ts	7/8 ([3/595])	passed

So the per-platform assertions held on Windows hardware: rooted (q:/pp) threw there as a drive root, and C:\... was not captured by the registered C namespace. The forced-predicate run on Linux was just the pre-push check; it agreed with CI.

a subtle non-Windows change where C:\... now reaches a registered C namespace instead of being rejected

Answered in #33443 (comment), with a repro showing that stock 1.4.0 on Linux already routes C:\... to a registered C namespace through onResolve. moduleLoaderResolve was the one site that read it differently, and only for plugins that register onLoad without a matching onResolve. Short version: this aligns the last disagreeing site rather than introducing a new reading, and the platform gate is required, not cosmetic, because without it { path: "/pp", namespace: "q" } stays broken.

Still worth a maintainer's eye on that semantic, which is why I'm flagging rather than assuming.

[claude]

I didn't find any issues — the fix is small and well-reasoned, and the two namespace-parsing sites now agree — but it changes module-resolution behavior in moduleLoaderResolve and flips what C:\\... means on non-Windows when a single-letter namespace is registered, so a human should sanity-check the cross-platform semantics.

Extended reasoning...

Overview

This PR fixes runtime Bun.plugin losing single-letter namespaces. It touches two native sites plus tests:

• src/jsc/bindings/ZigGlobalObject.cpp: extracts the drive-letter carve-out in moduleLoaderResolve into isWindowsDriveRoot(), which now only matches on Windows and only when the character after the colon is a path separator. Previously colon == 1 && isASCIIAlpha(key[0]) matched every single-letter prefix on every platform.
• src/bundler/transpiler.rs: PluginRunner::extract_namespace replaces the hand-rolled exclusive range check (> 'a' && < 'z', which excluded a/z/A/Z) with is_drive_letter(), and relaxes len() > 3 to len() > 2 so a bare C:\\ counts as a drive root.
• Two test files: a new it.concurrent in plugins.test.ts covering one- and two-letter namespaces across bare/nested/rooted paths with per-platform assertions, and plugin-namespace-drive-letter.test.ts updated to assert that non-Windows now treats C:\\... as the C namespace (a behavioral change this PR introduces).

Security risks

None identified. This is specifier-classification logic; the change narrows a heuristic rather than widening trust. Bounds are respected (key.length() > 2 guards key[2]).

Level of scrutiny

Moderate-to-high. moduleLoaderResolve in ZigGlobalObject.cpp is on the hot path for every ES module import, and the change is intentionally cross-platform-divergent. Two behavioral shifts deserve a human eye:

1. Non-Windows: a specifier like C:\\foo with a single-letter C namespace registered now short-circuits to the plugin instead of falling through to the filesystem resolver. The existing drive-letter test's assertion was inverted for non-Windows to reflect this. It's consistent with what extract_namespace already did downstream, but it is a user-visible change.
2. Windows: a:\\x, z:\\x, A:\\x, Z:\\x, and bare C:\\ are now classified as drive roots in extract_namespace (previously namespaces due to the exclusive-range and > 3 bugs).

Other factors

The two implementations (isWindowsDriveRoot in C++ and extract_namespace in Rust) now agree, and each cross-references the other in comments. Test coverage is thorough with per-platform branching, and the PR description reports both files passing on the Windows CI lane. I verified is_drive_letter uses inclusive bounds. No prior reviews or comments on the PR. Deferring because module resolution is critical enough that the cross-platform semantics change should be confirmed by a maintainer.

[robobun]

Same flag third time, so I've stopped burying the answer in comments and moved it into the description under "Behavior change worth a maintainer's eye". The old description only mentioned the Windows-side change, which made it look like nothing moved off Windows. That was the real omission, and it's fixed.

The one-line version, for anyone skimming: off Windows this does change what C:\... means to a registered single-letter namespace, but only in moduleLoaderResolve, which was the lone site disagreeing with extract_namespace. Stock 1.4.0 on Linux already routes C:\... to a registered C namespace through onResolve (repro in the description), so this aligns the last holdout rather than inventing a reading. The platform gate is required, not cosmetic: drop it and { path: "/pp", namespace: "q" } stays broken, since q:/pp and C:\... are the same shape pulling opposite ways.

Happy to drop the gate if a maintainer prefers the old non-Windows behavior; it just leaves absolute paths with one-letter namespaces broken.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.