From 11fbbe9f1a41eb59bc39c74a76abcf9234b5b90a Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:47:22 -0700 Subject: [PATCH 1/4] transpile: report a file: URL as a stripped script's sourceURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node's type stripping emits pathToFileURL(filename).href as the `//# sourceURL` of a stripped script, and the inspector surfaces that value as Debugger.scriptParsed's `url` — the key editors and DevTools match breakpoints against. nub emitted the bare filesystem path, so every transpiled .ts/.cts/.mts reached them under a different script identity than the same file under node. The percent-encoding matches pathToFileURL byte for byte. Its set is wider than the WHATWG path percent-encode set — `[`, `]`, `^`, `|` and `~` are escaped too — so it is derived from a sweep against v26.7.0 rather than from the URL spec. CACHE_SCHEMA goes to "7": the sourceURL is baked into the cached body, so a "6"-era entry would keep serving the old identity. Upstream's test/parallel/test-inspector-strip-types.js now passes under nub on 4 of its 5 scenarios, matching plain node on the same checkout (the fifth fails identically under both). test-debugger-probe-typescript reports the right `url` and still differs on line/column, which is transpiled-position fidelity rather than script identity. --- crates/nub-cache-key/src/lib.rs | 2 +- crates/nub-cli/tests/ts_source_url.rs | 132 ++++++++++++++++++++++++++ crates/nub-native/src/cache.rs | 111 ++++++++++++++++++++-- 3 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 crates/nub-cli/tests/ts_source_url.rs diff --git a/crates/nub-cache-key/src/lib.rs b/crates/nub-cache-key/src/lib.rs index 6501b77cf..811c4b00a 100644 --- a/crates/nub-cache-key/src/lib.rs +++ b/crates/nub-cache-key/src/lib.rs @@ -11,7 +11,7 @@ //! its `NUB_VERSION` / `CACHE_SCHEMA` / `BUILD_ID` consts. //! //! `filename` is in the preimage because the cached body carries a per-file -//! `//# sourceURL=` magic comment: two byte-identical sources in +//! `//# sourceURL=` magic comment: two byte-identical sources in //! the same directory would otherwise collide on one entry and the second file //! would be served the first's `sourceURL`, misattributing V8 stack frames and //! debugger source mapping (issue #171). diff --git a/crates/nub-cli/tests/ts_source_url.rs b/crates/nub-cli/tests/ts_source_url.rs new file mode 100644 index 000000000..fed9e36a7 --- /dev/null +++ b/crates/nub-cli/tests/ts_source_url.rs @@ -0,0 +1,132 @@ +//! A transpiled TypeScript file's script identity is the `file:` URL Node's own +//! type-stripping reports, not a bare filesystem path. +//! +//! nub appends a `//# sourceURL` to every transpiled body, and that value is what +//! the inspector reports as `Debugger.scriptParsed`'s `url` — the key editors and +//! DevTools match breakpoints against. Node reports the module's URL there +//! (`pathToFileURL(filename).href`, via `convertCJSFilenameToURL`); emitting the +//! path instead gave every `.ts` file a different identity under nub than under +//! `node`. Upstream pins the same contract in +//! `test/parallel/test-inspector-strip-types.js`, including `hasSourceURL`. +//! +//! Deliberately NOT asserted here: the rendered stack frame. nub runs with +//! `--enable-source-maps`, and Node's source-map frame formatter strips the +//! `file://` scheme off the mapped source by design +//! (`prepare_stack_trace.js`, `originalSourceNoScheme`), so a mapped frame shows a +//! bare path whatever the sourceURL says. The inspector URL is the surface this +//! bug is actually about. +//! +//! The fixture supplies its OWN expectation — `pathToFileURL(__filename).href` +//! plus URLs resolved against it — so the assertion is a differential against +//! Node's URL for the same files rather than path arithmetic repeated in the +//! test. Each fixture directory is run TWICE against a private cache directory: +//! the sourceURL is baked into the cached body, so a cold transform and a warm +//! cache hit are separate claims. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn nub_binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_nub")) +} + +fn host_node_usable() -> bool { + Command::new("node") + .arg("--version") + .output() + .is_ok_and(|out| out.status.success()) +} + +/// A `.ts` entry that pulls in one dependency per module format, so all three +/// load paths (entry, `require` of `.cts`, `import` of `.mts`) are covered by one +/// process. `Debugger.enable` is posted before either dependency loads, which is +/// what makes their `scriptParsed` notifications observable. +const PROBE: &str = r#"const inspector = require("node:inspector"); +const { pathToFileURL } = require("node:url"); + +const session = new inspector.Session(); +session.connect(); +const seen: Record = {}; +session.on("Debugger.scriptParsed", ({ params }: any) => { + if (/\.[cm]?ts$/.test(params.url)) seen[params.url] = params.hasSourceURL; +}); +session.post("Debugger.enable", async () => { + require("./dep.cts"); + await import("./dep.mts"); + const self: string = pathToFileURL(__filename).href; + const expected: string[] = [self, new URL("dep.cts", self).href, new URL("dep.mts", self).href]; + console.log(JSON.stringify({ expected, seen })); +}); +"#; + +/// Write the fixture into `dir`, run it, and assert every TypeScript file in the +/// graph reported Node's own URL for itself as its script identity. +fn assert_script_urls(dir: &Path, cache: &Path) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(dir.join("package.json"), r#"{ "type": "commonjs" }"#).unwrap(); + std::fs::write(dir.join("probe.ts"), PROBE).unwrap(); + std::fs::write( + dir.join("dep.cts"), + "const y: number = 2;\nmodule.exports = y;\n", + ) + .unwrap(); + std::fs::write(dir.join("dep.mts"), "export const z: number = 3;\n").unwrap(); + + for pass in ["cold", "warm"] { + let output = Command::new(nub_binary()) + .arg("probe.ts") + .current_dir(dir) + .env("XDG_CACHE_HOME", cache) + .output() + .expect("failed to spawn nub"); + assert!( + output.status.success(), + "nub exited {:?} ({pass}) in {}\nstderr: {}", + output.status, + dir.display(), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("fixture must emit JSON ({pass}), got {stdout:?}: {e}")); + + for expected in parsed["expected"].as_array().unwrap() { + let href = expected.as_str().unwrap(); + assert!( + href.starts_with("file:///"), + "({pass}) the fixture's own oracle must be a file: URL, got {href:?}" + ); + assert_eq!( + parsed["seen"][href], + serde_json::json!(true), + "({pass}) {href} must reach the inspector under Node's own URL for it, \ + with hasSourceURL set.\n reported: {}", + parsed["seen"] + ); + } + } +} + +/// The baseline contract, on a path with nothing to escape. +#[test] +fn transpiled_typescript_reports_its_file_url_to_the_inspector() { + if !host_node_usable() { + return; + } + let temp = tempfile::tempdir().unwrap(); + assert_script_urls(&temp.path().join("plain"), &temp.path().join("cache")); +} + +/// The percent-encoding has to match Node's `pathToFileURL` too, not merely carry +/// a `file://` prefix: the fixture's expectations come from Node, so a naive +/// `format!("file://{path}")` passes the test above and fails here. Unix-only — +/// `%`, `#` and `?` are not usable in a Windows filename. +#[cfg(unix)] +#[test] +fn percent_encoded_path_matches_node() { + if !host_node_usable() { + return; + } + let temp = tempfile::tempdir().unwrap(); + assert_script_urls(&temp.path().join("a b%c#d?e"), &temp.path().join("cache")); +} diff --git a/crates/nub-native/src/cache.rs b/crates/nub-native/src/cache.rs index 68b18a8b1..faa74752f 100644 --- a/crates/nub-native/src/cache.rs +++ b/crates/nub-native/src/cache.rs @@ -6,9 +6,10 @@ //! `NUB_VERSION \0 CACHE_SCHEMA \0 build_id \0 source \0 ext \0 tsconfig_hash \0 (pkg_type||"") \0 filename` //! → blake3 → 64-hex lowercase → cache FILENAME. `filename` is in the key //! because the cached body carries a per-file `//# sourceURL` comment (issue -//! #171). (SCHEMA "6" added `filename`; SCHEMA "5" = compile-time build-id era; -//! SCHEMA "4" folded a runtime exe-hash here, SCHEMA "3" / the old JS path used -//! SHA-256 and a key without that component.) +//! #171). (SCHEMA "7" = the `//# sourceURL` file-URL switch; SCHEMA "6" added +//! `filename`; SCHEMA "5" = compile-time build-id era; SCHEMA "4" folded a +//! runtime exe-hash here, SCHEMA "3" / the old JS path used SHA-256 and a key +//! without that component.) //! On-disk entry: `[16-hex integrity = blake3(body)[..16]][body]`, where //! `body = format_byte('c'|'m') + post_processed_code`. //! Atomic write via a `*.tmp` sibling + rename (the `*.tmp` suffix is what @@ -17,7 +18,7 @@ //! The post-processing that the old JS did in `loadTranspile` after `transform` //! moves in here so the cached bytes are the FINAL bytes: drop oxc's empty //! `export {};` marker for CJS, append the inline base64 sourceMap, append the -//! `//# sourceURL=` magic comment. +//! `//# sourceURL=` magic comment. use std::sync::atomic::{AtomicU64, Ordering}; @@ -27,7 +28,10 @@ use oxc_napi::OxcError; use crate::transform::{TransformOptions, transform}; -/// On-disk entry format version. Bumped to "6" when `filename` was folded into +/// On-disk entry format version. Bumped to "7" when the `//# sourceURL` switched +/// from a bare filesystem path to the `file:` URL Node emits (see +/// [`source_url`]) — the sourceURL is baked into the stored body, so a "6"-era +/// entry would keep serving the old script identity. "6" folded `filename` into /// the key (issue #171): a "5"-era entry was named without `filename` in the /// preimage, so two byte-identical sources in a directory collided on one entry /// and the second was served the first's `//# sourceURL`. "5" was the move from a @@ -35,7 +39,7 @@ use crate::transform::{TransformOptions, transform}; /// constant is hashed INTO the key, so the filenames are disjoint across schemas /// — old entries are silently ignored (a miss), never mis-read. (SCHEMA "4" was /// the blake3 + exe-hash era; "3" / the old JS path used SHA-256.) -const CACHE_SCHEMA: &str = "6"; +const CACHE_SCHEMA: &str = "7"; const INTEGRITY_LEN: usize = 16; /// Lockstep with `runtime/version.mjs` via `make version`; the sole version /// component of the key (a new nub release ships any emit change + a rebuilt addon). @@ -131,8 +135,11 @@ pub fn transform_cached( "\n//# sourceMappingURL=data:application/json;base64,{b64}\n" )); } - // sourceURL magic comment — absolute file path, as Node's strip-types does. - code.push_str(&format!("\n//# sourceURL={filename}\n")); + // sourceURL magic comment — the `file:` URL, as Node's strip-types does. + code.push_str(&format!( + "\n//# sourceURL={}\n", + source_url(&filename, cfg!(windows)) + )); // Store: body = format_byte + code. if let Some(dir) = cache_dir.as_deref() { @@ -199,6 +206,94 @@ fn integrity(body: &[u8]) -> String { blake3::hash(body).to_hex()[..INTEGRITY_LEN].to_string() } +/// Whether a byte must be percent-encoded inside the path of a `file:` URL, as +/// Node's `pathToFileURL` spells it (Node 20+, where the conversion moved into +/// ada's `href_from_file`). Swept over the whole byte range against v26.7.0: the +/// set is WIDER than the WHATWG path percent-encode set — `[`, `]`, `^`, `|` and +/// `~` are escaped too — so deriving it from the URL spec instead of from Node +/// produces a string that differs on exactly the paths nobody tests. +fn must_percent_encode(b: u8) -> bool { + !b.is_ascii() + || matches!( + b, + 0x00..=0x20 + | b'"' + | b'#' + | b'%' + | b'<' + | b'>' + | b'?' + | b'[' + | b'\\' + | b']' + | b'^' + | b'`' + | b'{' + | b'|' + | b'}' + | b'~' + | 0x7f + ) +} + +fn push_encoded(out: &mut String, s: &str) { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + for b in s.bytes() { + if must_percent_encode(b) { + out.push('%'); + out.push(HEX[(b >> 4) as usize] as char); + out.push(HEX[(b & 0x0f) as usize] as char); + } else { + out.push(b as char); + } + } +} + +/// The `//# sourceURL` a transpiled file carries: the `file:` URL spelling of +/// `path`, byte-identical to Node's `pathToFileURL(path).href`. +/// +/// This is a compatibility contract, not a spelling preference. Node's own +/// type-stripping emits the module's URL (`convertCJSFilenameToURL`, i.e. +/// `pathToFileURL(...).href`) as the sourceURL, and the inspector reports it +/// verbatim as `Debugger.scriptParsed`'s `url` — the key editors and DevTools +/// match breakpoints against. A bare path silently gives a `.ts` file a different +/// script identity under nub than under `node`. (A RENDERED stack frame is a +/// weaker witness: nub runs with `--enable-source-maps`, and Node's frame +/// formatter strips the `file://` scheme off the mapped source, so a mapped frame +/// shows a bare path whatever this returns.) `path` always arrives absolute here +/// (the loader derives it with `fileURLToPath`); anything else is passed through +/// unconverted, matching Node's `convertCJSFilenameToURL` fallback. +fn source_url(path: &str, windows: bool) -> String { + let mut url = String::with_capacity(path.len() + 16); + if !windows { + if !path.starts_with('/') { + return path.to_string(); + } + url.push_str("file://"); + push_encoded(&mut url, path); + return url; + } + let forward = path.replace('\\', "/"); + if let Some(rest) = forward.strip_prefix("//") { + // UNC `\\server\share\…` → `file://server/share/…`. The hostname is a URL + // authority rather than a path segment, so it is not percent-encoded. + let host_end = rest.find('/').unwrap_or(rest.len()); + url.push_str("file://"); + url.push_str(&rest[..host_end]); + push_encoded(&mut url, &rest[host_end..]); + return url; + } + let bytes = forward.as_bytes(); + let drive_absolute = + bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/'; + if !drive_absolute { + return path.to_string(); + } + url.push_str("file:///"); + push_encoded(&mut url, &forward); + url +} + /// Read a cache entry and return `(format_byte, code)` on a verified hit. /// /// The integrity check is byte-identical to the on-disk format: the stored From f66d3cb229d422385e18bf0e1a35b31c8cfb9153 Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Thu, 27 Aug 2026 03:25:11 -0700 Subject: [PATCH 2/4] transpile: spell the sourceURL for the host Node's escape band MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sourceURL is percent-encoded to match `pathToFileURL`, but that escape set is not fixed across nub's supported range. nodejs/node#54545 widened it to cover `[`, `]`, `^`, `|` and `~`, and it reached the release lines out of order, so it takes bands rather than one floor — measured on real binaries at each boundary tag: 18.x (incl. 18.20.8) narrow 20.18.3+ wide 20.0.0 - 20.18.2 narrow 22.12.0+ wide 21.x (incl. 21.7.3) narrow 23.1.0+ wide 22.0.0 - 22.11.x narrow 24.0.0+ wide 23.0.x narrow Emitting one fixed set gave one file two identities inside a single process on the narrow band: Node computes a module's own URL with its own `pathToFileURL`, so on v22.11 the loader carried `w[x]y^z~q/probe.ts` while `Debugger.scriptParsed` reported `w%5Bx%5Dy%5Ez%7Eq/probe.ts`, and a breakpoint set against the module URL matched no script. `transformCached` now takes `process.version` and derives the band from it. The URL FORM stays unconditional. Node's own type stripping only began reporting a `file:` URL in v26.4.0, backported to v24.19.0, and v25.x never got it; following the host there would flip nub's script identity on a patch bump. `url_band` joins the cache-key preimage, because the band changes the bytes that get stored: without it an `nvm use` across a boundary serves a body carrying the other band's spelling. The schema stays at "7" — both halves land together, and neither shipped. The URL helpers move to `nub-cache-key`, which exists so the cache's pure logic can be unit-tested off the `test = false` cdylib. Both `windows` branches are now covered against captured `pathToFileURL(p, { windows: true })` output. The integration test asserts the cold pass actually populated the cache directory, so a disabled cache fails instead of quietly running cold twice. --- crates/nub-cache-key/src/lib.rs | 56 ++++- crates/nub-cache-key/src/source_url.rs | 281 +++++++++++++++++++++++++ crates/nub-cli/tests/ts_source_url.rs | 15 ++ crates/nub-native/src/cache.rs | 148 ++++--------- runtime/transform-core.mjs | 6 +- 5 files changed, 392 insertions(+), 114 deletions(-) create mode 100644 crates/nub-cache-key/src/source_url.rs diff --git a/crates/nub-cache-key/src/lib.rs b/crates/nub-cache-key/src/lib.rs index 811c4b00a..65e0340ce 100644 --- a/crates/nub-cache-key/src/lib.rs +++ b/crates/nub-cache-key/src/lib.rs @@ -1,9 +1,10 @@ -//! Transpile-cache key derivation, factored out of `nub-native` so the -//! invalidation contract is unit-testable (see this crate's Cargo.toml for why a -//! test can't live in the cdylib `nub-native`). +//! The transpile cache's pure core, factored out of `nub-native` so it is +//! unit-testable (see this crate's Cargo.toml for why a test can't live in the +//! cdylib `nub-native`): the key derivation here, and in [`source_url`] the +//! `//# sourceURL` spelling the key has to cover. //! //! The key preimage is NUL-separated and order-fixed (no trailing separator): -//! `nub_version \0 schema \0 build_id \0 source \0 ext \0 tsconfig_hash \0 pkg_type \0 filename` +//! `nub_version \0 schema \0 build_id \0 source \0 ext \0 tsconfig_hash \0 pkg_type \0 filename \0 url_band` //! → blake3 → 64-hex lowercase → the cache FILENAME. Every component is hashed //! IN, so a change to ANY of them (a nub release, a CACHE_SCHEMA bump, a rebuild //! at a new build-id) yields a disjoint filename — old on-disk entries are @@ -15,6 +16,14 @@ //! the same directory would otherwise collide on one entry and the second file //! would be served the first's `sourceURL`, misattributing V8 stack frames and //! debugger source mapping (issue #171). +//! +//! `url_band` is in the preimage for the same reason one step further out: that +//! sourceURL is spelled to match the HOST Node's `pathToFileURL`, whose escape set +//! changed mid-release-line ([`source_url::host_widens_url_path`]). Without it, a +//! project built under one Node and then run under another — an `nvm use`, or +//! `nub node pin` — would be served a body carrying the other band's spelling. + +pub mod source_url; /// blake3 of the NUL-separated key preimage → 64-hex lowercase. /// @@ -22,7 +31,9 @@ /// components (compile-time consts on the production path); `source` / `ext` / /// `tsconfig_hash` / `pkg_type` / `filename` are the per-file inputs. `filename` /// is keyed in because it is baked into the cached body as the `//# sourceURL` -/// comment, so distinct files must not share an entry (issue #171). +/// comment, so distinct files must not share an entry (issue #171); `url_band` +/// ([`source_url::band_tag`]) is keyed in because that same comment is spelled to +/// match the host Node, so one file has two valid bodies across hosts. #[allow(clippy::too_many_arguments)] pub fn cache_key( nub_version: &str, @@ -33,6 +44,7 @@ pub fn cache_key( tsconfig_hash: &str, pkg_type: &str, filename: &str, + url_band: &str, ) -> String { let mut h = blake3::Hasher::new(); h.update(nub_version.as_bytes()); @@ -50,6 +62,8 @@ pub fn cache_key( h.update(pkg_type.as_bytes()); h.update(b"\0"); h.update(filename.as_bytes()); + h.update(b"\0"); + h.update(url_band.as_bytes()); h.finalize().to_hex().to_string() } @@ -63,9 +77,12 @@ mod tests { const TSCONFIG: &str = "tsconfig-hash"; const PKG: &str = "module"; const FILENAME: &str = "/proj/src/x.ts"; + const BAND: &str = "url-wide"; fn key(version: &str, schema: &str, build_id: &str) -> String { - cache_key(version, schema, build_id, SRC, EXT, TSCONFIG, PKG, FILENAME) + cache_key( + version, schema, build_id, SRC, EXT, TSCONFIG, PKG, FILENAME, BAND, + ) } /// A rebuilt binary (new build-id) must not serve a prior build's entries: @@ -115,6 +132,7 @@ mod tests { TSCONFIG, PKG, "/proj/alpha.ts", + BAND, ); let beta = cache_key( "0.0.1", @@ -125,10 +143,36 @@ mod tests { TSCONFIG, PKG, "/proj/beta.ts", + BAND, ); assert_ne!( alpha, beta, "identical content at different paths must yield distinct cache keys" ); } + + /// The cached body carries a sourceURL spelled for the host Node's escape + /// band, so the same file under two hosts must not share one entry — this is + /// what stops `nvm use` from serving the other band's spelling. + #[test] + fn cache_key_distinguishes_the_url_bands() { + let wide = cache_key( + "0.0.1", "6", "abc1234", SRC, EXT, TSCONFIG, PKG, FILENAME, "url-wide", + ); + let narrow = cache_key( + "0.0.1", + "6", + "abc1234", + SRC, + EXT, + TSCONFIG, + PKG, + FILENAME, + "url-narrow", + ); + assert_ne!( + wide, narrow, + "the two sourceURL escape bands must yield distinct cache keys" + ); + } } diff --git a/crates/nub-cache-key/src/source_url.rs b/crates/nub-cache-key/src/source_url.rs new file mode 100644 index 000000000..4ad2433b3 --- /dev/null +++ b/crates/nub-cache-key/src/source_url.rs @@ -0,0 +1,281 @@ +//! The `//# sourceURL` a transpiled body carries, spelled exactly as the HOST +//! Node's `pathToFileURL(path).href` would spell it. +//! +//! Node's loader computes a module's own URL with `pathToFileURL`, so a sourceURL +//! that disagrees with it gives one file two identities inside a single process: +//! `Debugger.scriptParsed` reports nub's spelling while a breakpoint set against +//! the module URL carries Node's, and the two never match. That is why the +//! encoding is pinned to the host rather than to the URL spec. +//! +//! The URL FORM is not host-gated. Node's own type stripping only started +//! reporting a `file:` URL in v26.4.0 (backported to v24.19.0); before that it +//! reported the bare path, and v25.x never got the change. Reproducing that would +//! flip nub's script identity on a patch bump, so nub always emits the URL — the +//! form upstream is migrating toward. Only the ENCODING, which decides whether +//! two URLs for one file are equal, follows the host. + +/// Whether the host's `pathToFileURL` percent-encodes the widened characters +/// `[`, `]`, `^`, `|` and `~`. +/// +/// nodejs/node#54545 widened the escape set, and it reached the release lines out +/// of order, so this cannot be reduced to a single floor. All five characters move +/// together in one step — verified by sweeping every printable byte through real +/// binaries at each boundary tag: +/// +/// | Host band | Widened | +/// | --- | --- | +/// | 18.x (through 18.20.8) | no | +/// | 20.0.0 – 20.18.2 | no | +/// | 20.18.3+ | yes | +/// | 21.x (through 21.7.3) | no | +/// | 22.0.0 – 22.11.x | no | +/// | 22.12.0+ | yes | +/// | 23.0.x | no | +/// | 23.1.0+ | yes | +/// | 24.0.0+ | yes | +/// +/// v20.18.3 and v22.12.0 are the same behavior by two different mechanisms — +/// v20/v22 got widened regexes in the pure-JS `pathToFileURL`, and the move to +/// ada's native `href_from_file` came later — so the band is about the escape +/// set, not about which implementation produces it. +/// +/// An unparseable or absent version reads as WIDENED: every release line still +/// receiving patches is above its boundary, so that is the answer that is right +/// for anything nub will actually meet. +pub fn host_widens_url_path(node_version: &str) -> bool { + let Some((major, minor, patch)) = parse_version(node_version) else { + return true; + }; + match major { + 0..=19 => false, + 20 => (minor, patch) >= (18, 3), + 21 => false, + 22 => minor >= 12, + 23 => minor >= 1, + _ => true, + } +} + +/// `v22.12.0` / `22.12.0` → `(22, 12, 0)`. Trailing prerelease or build metadata +/// is ignored; anything that does not start with three numeric components is +/// rejected so the caller can apply its own default. +fn parse_version(raw: &str) -> Option<(u32, u32, u32)> { + let trimmed = raw.strip_prefix('v').unwrap_or(raw); + let mut parts = trimmed.split('.'); + let major = parts.next()?.parse().ok()?; + let minor = parts.next()?.parse().ok()?; + let tail = parts.next()?; + let end = tail + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(tail.len()); + let patch = tail[..end].parse().ok()?; + Some((major, minor, patch)) +} + +/// The cache-key component naming the encoding band, so a body emitted for one +/// band is never served to a host on the other. +pub fn band_tag(widened: bool) -> &'static str { + if widened { "url-wide" } else { "url-narrow" } +} + +/// Whether a byte must be percent-encoded inside a `file:` URL path. +/// +/// The always-escaped set is Node's `encodePathChars` plus the WHATWG path +/// percent-encode set, which between them cover the C0 controls, space, `"`, `#`, +/// `%`, `<`, `>`, `?`, `\`, backtick, `{`, `}`, DEL and every non-ASCII byte. The +/// `widened` five ride on [`host_widens_url_path`]. Both halves were read off real +/// binaries rather than off the URL spec, because the spec set matches neither +/// band exactly. +fn must_percent_encode(b: u8, widened: bool) -> bool { + if !b.is_ascii() { + return true; + } + if widened && matches!(b, b'[' | b']' | b'^' | b'|' | b'~') { + return true; + } + matches!( + b, + 0x00..=0x20 | b'"' | b'#' | b'%' | b'<' | b'>' | b'?' | b'\\' | b'`' | b'{' | b'}' | 0x7f + ) +} + +fn push_encoded(out: &mut String, s: &str, widened: bool) { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + for b in s.bytes() { + if must_percent_encode(b, widened) { + out.push('%'); + out.push(HEX[(b >> 4) as usize] as char); + out.push(HEX[(b & 0x0f) as usize] as char); + } else { + out.push(b as char); + } + } +} + +/// The `file:` URL spelling of `path`, byte-identical to the host's +/// `pathToFileURL(path).href`. +/// +/// `path` always arrives absolute in production (the loader derives it with +/// `fileURLToPath`); anything else is passed through unconverted, matching Node's +/// `convertCJSFilenameToURL` fallback. `windows` is a parameter rather than a +/// `cfg!` so both branches are exercisable from either host. +pub fn file_url(path: &str, windows: bool, widened: bool) -> String { + let mut url = String::with_capacity(path.len() + 16); + if !windows { + if !path.starts_with('/') { + return path.to_string(); + } + url.push_str("file://"); + push_encoded(&mut url, path, widened); + return url; + } + let forward = path.replace('\\', "/"); + if let Some(rest) = forward.strip_prefix("//") { + // UNC `\\server\share\…` → `file://server/share/…`. The hostname is a URL + // authority rather than a path segment, so it is not percent-encoded. + let host_end = rest.find('/').unwrap_or(rest.len()); + url.push_str("file://"); + url.push_str(&rest[..host_end]); + push_encoded(&mut url, &rest[host_end..], widened); + return url; + } + let bytes = forward.as_bytes(); + let drive_absolute = + bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/'; + if !drive_absolute { + return path.to_string(); + } + url.push_str("file:///"); + push_encoded(&mut url, &forward, widened); + url +} + +#[cfg(test)] +mod tests { + use super::{file_url, host_widens_url_path}; + + /// The band boundaries, taken from running `pathToFileURL('/xy')` over + /// every printable ASCII byte on real binaries at each tag — including the + /// three that pin the v20 boundary (20.18.2 narrow, 20.18.3 wide) and the last + /// release of each line that never widened (18.20.8, 21.7.3). + #[test] + fn band_boundaries_match_the_measured_releases() { + for narrow in [ + "v18.19.0", "v18.20.8", "v20.10.0", "v20.18.0", "v20.18.1", "v20.18.2", "v21.1.0", + "v21.7.3", "v22.6.0", "v22.11.0", "v23.0.0", + ] { + assert!( + !host_widens_url_path(narrow), + "{narrow} does not escape [ ] ^ | ~" + ); + } + for wide in [ + "v20.18.3", "v20.19.0", "v22.12.0", "v22.15.0", "v23.1.0", "v23.3.0", "v24.0.0", + "v26.7.0", + ] { + assert!(host_widens_url_path(wide), "{wide} escapes [ ] ^ | ~"); + } + } + + /// A version nub cannot parse must not silently pick the legacy spelling: a + /// nightly, a `-pre` build, or a future line is far likelier to be widened. + #[test] + fn unparseable_version_defaults_to_widened() { + for odd in ["", "not-a-version", "v99", "v27.0.0-pre"] { + assert!(host_widens_url_path(odd), "{odd:?} must read as widened"); + } + } + + /// The characters that move with the band, and the ones that must not. + #[test] + fn widened_characters_follow_the_band() { + assert_eq!( + file_url("/w[x]y^z~q|r", false, true), + "file:///w%5Bx%5Dy%5Ez%7Eq%7Cr" + ); + assert_eq!( + file_url("/w[x]y^z~q|r", false, false), + "file:///w[x]y^z~q|r" + ); + // Escaped in BOTH bands — captured from v18.19.0 and v26.7.0 alike. + for band in [true, false] { + assert_eq!( + file_url("/a b%c#d?e", false, band), + "file:///a%20b%25c%23d%3Fe" + ); + // On POSIX a backslash is an ordinary filename byte, never a separator. + assert_eq!(file_url("/x\\y", false, band), "file:///x%5Cy"); + } + } + + /// Non-ASCII goes out as percent-encoded UTF-8, uppercase hex, in both bands. + #[test] + fn non_ascii_is_percent_encoded_utf8() { + for band in [true, false] { + assert_eq!(file_url("/xé€y", false, band), "file:///x%C3%A9%E2%82%ACy"); + } + } + + /// Windows drive paths, against `pathToFileURL(p, { windows: true }).href` + /// captured on v26.7.0: backslashes become separators (never `%5C`, unlike + /// POSIX), and the drive letter sits in the path, not the authority. + #[test] + fn windows_drive_paths_match_captured_output() { + assert_eq!( + file_url(r"C:\a b\x~y.ts", true, true), + "file:///C:/a%20b/x%7Ey.ts" + ); + assert_eq!( + file_url(r"C:\a b\x~y.ts", true, false), + "file:///C:/a%20b/x~y.ts" + ); + assert_eq!( + file_url(r"C:\a%b\x.ts", true, true), + "file:///C:/a%25b/x.ts" + ); + assert_eq!( + file_url(r"C:\a\x#y?z.ts", true, true), + "file:///C:/a/x%23y%3Fz.ts" + ); + assert_eq!( + file_url(r"C:\a\x[y]^z|w.ts", true, true), + "file:///C:/a/x%5By%5D%5Ez%7Cw.ts" + ); + assert_eq!( + file_url(r"C:\a\x[y]^z|w.ts", true, false), + "file:///C:/a/x[y]^z|w.ts" + ); + // An already-forward-slashed drive path is accepted unchanged. + assert_eq!(file_url("C:/a/b.ts", true, true), "file:///C:/a/b.ts"); + } + + /// UNC paths, same capture: the share host is the URL authority and stays + /// unencoded, while the path after it is encoded normally. + #[test] + fn windows_unc_paths_keep_the_host_unencoded() { + assert_eq!( + file_url(r"\\server\share\a b\x.ts", true, true), + "file://server/share/a%20b/x.ts" + ); + assert_eq!( + file_url(r"\\server\share\w[x]~y.ts", true, true), + "file://server/share/w%5Bx%5D%7Ey.ts" + ); + assert_eq!( + file_url(r"\\server\share\w[x]~y.ts", true, false), + "file://server/share/w[x]~y.ts" + ); + } + + /// A non-absolute input is passed through untouched on either platform, + /// matching Node's `convertCJSFilenameToURL`, which only converts when + /// `isAbsolute(filename)`. + #[test] + fn non_absolute_input_is_left_alone() { + assert_eq!(file_url("[eval]", false, true), "[eval]"); + assert_eq!(file_url("relative/x.ts", false, true), "relative/x.ts"); + assert_eq!(file_url(r"relative\x.ts", true, true), r"relative\x.ts"); + // Rooted but not drive-qualified is not absolute on Windows. + assert_eq!(file_url("/rooted/x.ts", true, true), "/rooted/x.ts"); + } +} diff --git a/crates/nub-cli/tests/ts_source_url.rs b/crates/nub-cli/tests/ts_source_url.rs index fed9e36a7..c69ae28d6 100644 --- a/crates/nub-cli/tests/ts_source_url.rs +++ b/crates/nub-cli/tests/ts_source_url.rs @@ -104,6 +104,21 @@ fn assert_script_urls(dir: &Path, cache: &Path) { parsed["seen"] ); } + + // Without this, a cache directory that never got written — the mkdirSync + // in transform-core.mjs swallows its error and disables the cache — would + // silently turn the warm pass into a second cold one, and the entry whose + // sourceURL is baked in would go untested. + if pass == "cold" { + let entries = std::fs::read_dir(cache.join("nub/transpile")) + .map(|d| d.count()) + .unwrap_or(0); + assert!( + entries > 0, + "the cold pass must leave entries in {}, or the warm pass tests nothing", + cache.join("nub/transpile").display() + ); + } } } diff --git a/crates/nub-native/src/cache.rs b/crates/nub-native/src/cache.rs index faa74752f..aab66eebb 100644 --- a/crates/nub-native/src/cache.rs +++ b/crates/nub-native/src/cache.rs @@ -3,11 +3,12 @@ //! the old JS cache, so warm caches survive the JS→Rust move (no global miss). //! //! Cache key preimage (no trailing separator): -//! `NUB_VERSION \0 CACHE_SCHEMA \0 build_id \0 source \0 ext \0 tsconfig_hash \0 (pkg_type||"") \0 filename` -//! → blake3 → 64-hex lowercase → cache FILENAME. `filename` is in the key -//! because the cached body carries a per-file `//# sourceURL` comment (issue -//! #171). (SCHEMA "7" = the `//# sourceURL` file-URL switch; SCHEMA "6" added -//! `filename`; SCHEMA "5" = compile-time build-id era; SCHEMA "4" folded a +//! `NUB_VERSION \0 CACHE_SCHEMA \0 build_id \0 source \0 ext \0 tsconfig_hash \0 (pkg_type||"") \0 filename \0 url_band` +//! → blake3 → 64-hex lowercase → cache FILENAME. `filename` and `url_band` are +//! in the key because the cached body carries a per-file `//# sourceURL` +//! comment (issue #171) spelled for the host Node's escape band. (SCHEMA "7" = +//! the `//# sourceURL` file-URL switch, which also added `url_band`; SCHEMA "6" +//! added `filename`; SCHEMA "5" = compile-time build-id era; SCHEMA "4" folded a //! runtime exe-hash here, SCHEMA "3" / the old JS path used SHA-256 and a key //! without that component.) //! On-disk entry: `[16-hex integrity = blake3(body)[..16]][body]`, where @@ -24,14 +25,17 @@ use std::sync::atomic::{AtomicU64, Ordering}; use napi::Result; use napi_derive::napi; +use nub_cache_key::source_url; use oxc_napi::OxcError; use crate::transform::{TransformOptions, transform}; /// On-disk entry format version. Bumped to "7" when the `//# sourceURL` switched -/// from a bare filesystem path to the `file:` URL Node emits (see -/// [`source_url`]) — the sourceURL is baked into the stored body, so a "6"-era -/// entry would keep serving the old script identity. "6" folded `filename` into +/// from a bare filesystem path to the `file:` URL Node reports (see +/// [`source_url::file_url`]) — the sourceURL is baked into the stored body, so a +/// "6"-era entry would keep serving the old script identity. That same change +/// added `url_band` to the preimage, which is what keeps two hosts' spellings +/// apart; the schema stays at "7" because both landed together. "6" folded `filename` into /// the key (issue #171): a "5"-era entry was named without `filename` in the /// preimage, so two byte-identical sources in a directory collided on one entry /// and the second was served the first's `//# sourceURL`. "5" was the move from a @@ -73,6 +77,11 @@ pub struct CachedTransformResult { /// the cached body; `cache_dir` is the JS enable/disable signal (`None` ⇒ skip all /// I/O, just transform). The remaining key components are JS-supplied so the key /// stays byte-identical to the old pipeline. +/// +/// `node_version` is `process.version` — the host whose `pathToFileURL` spelling +/// the emitted `//# sourceURL` has to match. It is passed rather than read here +/// because every other key component is already JS-supplied, and because it is an +/// INPUT to the cached bytes: the derived band goes into the key alongside them. #[allow(clippy::too_many_arguments, clippy::needless_pass_by_value)] #[napi] pub fn transform_cached( @@ -84,6 +93,7 @@ pub fn transform_cached( pkg_type: String, format_byte: String, cache_dir: Option, + node_version: String, ) -> Result { let format = if format_byte == "c" { "commonjs" @@ -91,7 +101,17 @@ pub fn transform_cached( "module" }; - let key = cache_key(&source, &ext, &tsconfig_hash, &pkg_type, &filename); + // The band, not the raw version: a patch bump inside one band emits identical + // bytes, so keying the version itself would drop a warm cache for nothing. + let widened = source_url::host_widens_url_path(&node_version); + let key = cache_key( + &source, + &ext, + &tsconfig_hash, + &pkg_type, + &filename, + source_url::band_tag(widened), + ); // Cache hit path. if let Some(dir) = cache_dir.as_deref() { @@ -135,10 +155,10 @@ pub fn transform_cached( "\n//# sourceMappingURL=data:application/json;base64,{b64}\n" )); } - // sourceURL magic comment — the `file:` URL, as Node's strip-types does. + // sourceURL magic comment — the `file:` URL, spelled as the host would. code.push_str(&format!( "\n//# sourceURL={}\n", - source_url(&filename, cfg!(windows)) + source_url::file_url(&filename, cfg!(windows), widened) )); // Store: body = format_byte + code. @@ -175,20 +195,21 @@ pub fn transform_cached( const BUILD_ID: &str = env!("NUB_NATIVE_BUILD_ID"); /// blake3(NUB_VERSION \0 SCHEMA \0 BUILD_ID \0 source \0 ext \0 tsconfig_hash \0 -/// pkg_type \0 filename) → 64-hex lowercase. blake3 (SIMD) replaces SHA-256 on the -/// hot path; the compile-time `BUILD_ID` is folded in so a rebuilt binary -/// auto-invalidates the cache without any runtime I/O. `filename` is keyed in -/// because the cached body bakes in a per-file `//# sourceURL` (issue #171), so -/// two byte-identical sources at different paths must not share an entry. The -/// derivation lives in the napi-free `nub-cache-key` crate so its invalidation -/// contract is unit-testable (this cdylib has `test = false` — a test harness -/// can't link the napi symbols). +/// pkg_type \0 filename \0 url_band) → 64-hex lowercase. blake3 (SIMD) replaces +/// SHA-256 on the hot path; the compile-time `BUILD_ID` is folded in so a rebuilt +/// binary auto-invalidates the cache without any runtime I/O. `filename` and +/// `url_band` are keyed in because the cached body bakes in a per-file +/// `//# sourceURL` (issue #171) whose spelling follows the host Node, so neither +/// two paths nor two hosts may share an entry. The derivation lives in the +/// napi-free `nub-cache-key` crate so its invalidation contract is unit-testable +/// (this cdylib has `test = false` — a test harness can't link the napi symbols). fn cache_key( source: &str, ext: &str, tsconfig_hash: &str, pkg_type: &str, filename: &str, + url_band: &str, ) -> String { nub_cache_key::cache_key( NUB_VERSION, @@ -199,6 +220,7 @@ fn cache_key( tsconfig_hash, pkg_type, filename, + url_band, ) } @@ -206,94 +228,6 @@ fn integrity(body: &[u8]) -> String { blake3::hash(body).to_hex()[..INTEGRITY_LEN].to_string() } -/// Whether a byte must be percent-encoded inside the path of a `file:` URL, as -/// Node's `pathToFileURL` spells it (Node 20+, where the conversion moved into -/// ada's `href_from_file`). Swept over the whole byte range against v26.7.0: the -/// set is WIDER than the WHATWG path percent-encode set — `[`, `]`, `^`, `|` and -/// `~` are escaped too — so deriving it from the URL spec instead of from Node -/// produces a string that differs on exactly the paths nobody tests. -fn must_percent_encode(b: u8) -> bool { - !b.is_ascii() - || matches!( - b, - 0x00..=0x20 - | b'"' - | b'#' - | b'%' - | b'<' - | b'>' - | b'?' - | b'[' - | b'\\' - | b']' - | b'^' - | b'`' - | b'{' - | b'|' - | b'}' - | b'~' - | 0x7f - ) -} - -fn push_encoded(out: &mut String, s: &str) { - const HEX: &[u8; 16] = b"0123456789ABCDEF"; - for b in s.bytes() { - if must_percent_encode(b) { - out.push('%'); - out.push(HEX[(b >> 4) as usize] as char); - out.push(HEX[(b & 0x0f) as usize] as char); - } else { - out.push(b as char); - } - } -} - -/// The `//# sourceURL` a transpiled file carries: the `file:` URL spelling of -/// `path`, byte-identical to Node's `pathToFileURL(path).href`. -/// -/// This is a compatibility contract, not a spelling preference. Node's own -/// type-stripping emits the module's URL (`convertCJSFilenameToURL`, i.e. -/// `pathToFileURL(...).href`) as the sourceURL, and the inspector reports it -/// verbatim as `Debugger.scriptParsed`'s `url` — the key editors and DevTools -/// match breakpoints against. A bare path silently gives a `.ts` file a different -/// script identity under nub than under `node`. (A RENDERED stack frame is a -/// weaker witness: nub runs with `--enable-source-maps`, and Node's frame -/// formatter strips the `file://` scheme off the mapped source, so a mapped frame -/// shows a bare path whatever this returns.) `path` always arrives absolute here -/// (the loader derives it with `fileURLToPath`); anything else is passed through -/// unconverted, matching Node's `convertCJSFilenameToURL` fallback. -fn source_url(path: &str, windows: bool) -> String { - let mut url = String::with_capacity(path.len() + 16); - if !windows { - if !path.starts_with('/') { - return path.to_string(); - } - url.push_str("file://"); - push_encoded(&mut url, path); - return url; - } - let forward = path.replace('\\', "/"); - if let Some(rest) = forward.strip_prefix("//") { - // UNC `\\server\share\…` → `file://server/share/…`. The hostname is a URL - // authority rather than a path segment, so it is not percent-encoded. - let host_end = rest.find('/').unwrap_or(rest.len()); - url.push_str("file://"); - url.push_str(&rest[..host_end]); - push_encoded(&mut url, &rest[host_end..]); - return url; - } - let bytes = forward.as_bytes(); - let drive_absolute = - bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/'; - if !drive_absolute { - return path.to_string(); - } - url.push_str("file:///"); - push_encoded(&mut url, &forward); - url -} - /// Read a cache entry and return `(format_byte, code)` on a verified hit. /// /// The integrity check is byte-identical to the on-disk format: the stored diff --git a/runtime/transform-core.mjs b/runtime/transform-core.mjs index 7cf8c7b96..488b60a62 100644 --- a/runtime/transform-core.mjs +++ b/runtime/transform-core.mjs @@ -846,8 +846,12 @@ export function loadTranspile(url, ext) { tsconfig: RUNTIME_TSCONFIG || null, compilerOptions: RUNTIME_COMPILER_OPTIONS, }); + // process.version decides how the appended `//# sourceURL` is percent-encoded: + // it is spelled to match THIS host's pathToFileURL, whose escape set widened + // mid-release-line, so the same file has two valid spellings across hosts. + // Native derives the band from it and folds that band into the cache key. const result = nubNative.transformCached( - filePath, source, opts, ext, `${tsconfigHash || ""}\0${runtimeHash}`, pkgType || "", formatByte, getCacheDir() ?? undefined, + filePath, source, opts, ext, `${tsconfigHash || ""}\0${runtimeHash}`, pkgType || "", formatByte, getCacheDir() ?? undefined, process.version, ); if (result.errors.length > 0) { const details = result.errors.map((e) => e.codeframe || e.message).join("\n\n"); From a36e3e6e2c2ab8ed84dcbb6be6c50cddab19be58 Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Thu, 27 Aug 2026 03:41:51 -0700 Subject: [PATCH 3/4] transpile: cover the host-band plumbing end-to-end, record Cause 7 fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration fixture directories held only characters escaped the same way in both bands, so nothing exercised `process.version` → `transformCached`'s 9th argument → `host_widens_url_path` → the emitted bytes. A wrong value there fell through to widened and still passed on any modern host. The all-platform fixture directory is now `w[x]y^z~q`. Those four characters are band-sensitive and legal in a Windows filename, so the Windows leg exercises the same path; `|` is band-sensitive but illegal there, so it joins the unix-only fixture as `a b%c#d?e|f`. The fixture already derives its expectation from the host's own `pathToFileURL`, so this is a differential: forcing the 9th argument to a narrow-band version fails both tests on a v26.7.0 host. wiki/research/nub-v0.5-augmentation-regressions.md records Cause 7 as fixed, drops the stale emitter line reference for the current symbols, and keeps the original analysis. Two additions: the inspector script-identity symptom that shares the same string, and the two facts the original recommendation did not anticipate — the host-dependent escape set, and why the URL form is deliberately not host-gated. Both symptoms re-verified against the built binary. Coverage: with the bare path `nub --test --experimental-test-coverage` drops every TypeScript row and reports a fake 100%; with the fix `lib.ts | 100.00 | 100.00 | 50.00` matches plain node. --- crates/nub-cache-key/src/source_url.rs | 6 +++++ crates/nub-cli/tests/ts_source_url.rs | 25 +++++++++++++------ .../nub-v0.5-augmentation-regressions.md | 9 ++++--- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/crates/nub-cache-key/src/source_url.rs b/crates/nub-cache-key/src/source_url.rs index 4ad2433b3..ec7dae739 100644 --- a/crates/nub-cache-key/src/source_url.rs +++ b/crates/nub-cache-key/src/source_url.rs @@ -13,6 +13,12 @@ //! flip nub's script identity on a patch bump, so nub always emits the URL — the //! form upstream is migrating toward. Only the ENCODING, which decides whether //! two URLs for one file are equal, follows the host. +//! +//! The unconditional form is not merely forward-looking. On a v22.15 or v24.10 +//! host an ordinary `.js` CommonJS module ALREADY reports a `file:` URL, because +//! nub does not intercept no-op plain JS and Node names it itself. The bare path +//! therefore made a `.ts` file the odd one out among its own siblings in one +//! process, not just different from a future Node. /// Whether the host's `pathToFileURL` percent-encodes the widened characters /// `[`, `]`, `^`, `|` and `~`. diff --git a/crates/nub-cli/tests/ts_source_url.rs b/crates/nub-cli/tests/ts_source_url.rs index c69ae28d6..945d75de8 100644 --- a/crates/nub-cli/tests/ts_source_url.rs +++ b/crates/nub-cli/tests/ts_source_url.rs @@ -122,20 +122,31 @@ fn assert_script_urls(dir: &Path, cache: &Path) { } } -/// The baseline contract, on a path with nothing to escape. +/// The baseline contract, plus the host-band plumbing. +/// +/// The directory name carries four of the five characters whose escaping depends +/// on the host's Node version, so this is the end-to-end witness that +/// `process.version` reaches the encoder: hand `transformCached` the wrong value +/// and nub spells them the other way while the fixture's oracle — the host's own +/// `pathToFileURL` — does not, and the assertion fails. `|` is left out because +/// Windows forbids it in a filename; `[`, `]`, `^` and `~` are all legal there, so +/// the Windows leg exercises the same plumbing. +/// +/// The surrounding temp path has nothing to escape, so it covers the ordinary +/// no-op case in the same run. #[test] fn transpiled_typescript_reports_its_file_url_to_the_inspector() { if !host_node_usable() { return; } let temp = tempfile::tempdir().unwrap(); - assert_script_urls(&temp.path().join("plain"), &temp.path().join("cache")); + assert_script_urls(&temp.path().join("w[x]y^z~q"), &temp.path().join("cache")); } -/// The percent-encoding has to match Node's `pathToFileURL` too, not merely carry -/// a `file://` prefix: the fixture's expectations come from Node, so a naive -/// `format!("file://{path}")` passes the test above and fails here. Unix-only — -/// `%`, `#` and `?` are not usable in a Windows filename. +/// The characters Windows cannot put in a filename, so the test above can never +/// reach them: `%`, `#` and `?` from the always-escaped set, and `|`, the fifth +/// band-sensitive character. A naive `format!("file://{path}")` passes the test +/// above and fails here. #[cfg(unix)] #[test] fn percent_encoded_path_matches_node() { @@ -143,5 +154,5 @@ fn percent_encoded_path_matches_node() { return; } let temp = tempfile::tempdir().unwrap(); - assert_script_urls(&temp.path().join("a b%c#d?e"), &temp.path().join("cache")); + assert_script_urls(&temp.path().join("a b%c#d?e|f"), &temp.path().join("cache")); } diff --git a/wiki/research/nub-v0.5-augmentation-regressions.md b/wiki/research/nub-v0.5-augmentation-regressions.md index 7bc7b5ff0..cf6ab4558 100644 --- a/wiki/research/nub-v0.5-augmentation-regressions.md +++ b/wiki/research/nub-v0.5-augmentation-regressions.md @@ -86,15 +86,17 @@ Muting nub's own experimental warnings with a blunt flag also mutes the warnings - **Severity:** Med — and a judgment call: the same flag legitimately mutes nub's own noise. - **Recommendation:** replace the blunt flag with a selective preload-side `warning` listener that drops only warnings for features **nub itself unflagged**, delegating the rest. Two config ignore-entries (`test-process-warnings`, `test-domain-multi`) currently blame "webstorage ExperimentalWarning"; both reasons are factually wrong and should be corrected. -### Cause 7 — Transpiled TypeScript gets a bare-path `sourceURL` +### Cause 7 — Transpiled TypeScript gets a bare-path `sourceURL` (FIXED) A `sourceURL` written as a filesystem path rather than a `file://` URL makes Node's coverage collector skip the file, so TypeScript coverage reports empty. - **Tests:** ~2 (typescript-coverage; contributes to test-runner-coverage). -- **What breaks:** the emitter at `crates/nub-native/src/cache.rs:135` writes `//# sourceURL=` (a filesystem path) instead of a `file://` URL. Node's coverage collector skips any URL not starting with `file:`, so **`nub --test --experimental-test-coverage` on any TypeScript project reports an empty coverage table and a fake 100%** (verified end-to-end: node shows `lib.ts | 100 | 100 | 50.00`, nub drops the row entirely). +- **What broke:** the transpile-cache emitter wrote `//# sourceURL=` (a filesystem path) instead of a `file://` URL. Node's coverage collector skips any URL not starting with `file:`, so **`nub --test --experimental-test-coverage` on any TypeScript project reported an empty coverage table and a fake 100%** (verified end-to-end: node shows `lib.ts | 100 | 100 | 50.00`, nub dropped the row entirely). +- **Second symptom, same string:** the inspector reports that `sourceURL` verbatim as `Debugger.scriptParsed`'s `url`, which is the key editors and DevTools match breakpoints against. A bare path gave every `.ts` file an identity no other module in the same process used — an ordinary `.js` file already reported a `file:` URL on the same host — so a breakpoint resolved against the module URL matched no script. - **Landed:** 2026-06-26 (#181). Between benchmarks. - **Severity:** High — silent wrong coverage on the exact workflow nub targets (TS). -- **Recommendation:** emit the module's `file://` URL as `sourceURL` (the URL is already at the load boundary). Needs a transpile-cache version bump — the bad URL is baked into on-disk entries. +- **FIXED 2026-08-27 by [#800](https://github.com/nubjs/nub/pull/800).** The emitter writes the `file:` URL, spelled to match the host Node's own `pathToFileURL`: `nub-cache-key::source_url` derives the spelling and the post-transform step in `crates/nub-native/src/cache.rs` calls it. Both symptoms are covered end-to-end — the coverage row now matches node's exactly, and an inspector test asserts the reported script URL against the host's own `pathToFileURL` for the same file. +- **Two things the original recommendation did not anticipate.** First, `pathToFileURL`'s escape set is not one set: [nodejs/node#54545](https://github.com/nodejs/node/pull/54545) widened it to cover `[`, `]`, `^`, `|` and `~`, and it reached the release lines out of order (20.18.3, 22.12.0, 23.1.0, and every 24+), so the spelling has to follow the host rather than a single fixed table. Second, the URL *form* deliberately does **not** follow the host: Node's own type stripping only began reporting a `file:` URL in 26.4.0, backported to 24.19.0, and 25.x never got it, so matching the host there would flip a file's script identity on a patch bump. The transpile-cache schema went to "7" as recommended, and the host's escape band joined `filename` as a key component — the URL is baked into the stored body, so without it an `nvm use` across a boundary serves the other band's spelling. ### Cause 8 — Explicit `--env-file` values get `$`-expanded (RECLASSIFIED: intentional, bun-parity) @@ -130,4 +132,5 @@ That left zero automated compat signal for the whole June→July window in which Every revision to this document, with the date and what changed. +- 2026-08-27 — Cause 7 fixed by [#800](https://github.com/nubjs/nub/pull/800): banded `file:` URL sourceURL + `url_band` cache-key component. Recorded the second symptom (inspector script identity) alongside the coverage one, and the two findings the original recommendation did not anticipate — the host-dependent escape set, and why the URL form is NOT host-gated. - 2026-07-24 — Initial write-up. 70 regressions → 9 root causes with git provenance. Key finding: none introduced by/since the benchmark; all landed 2026-06-03…07-09 during v0.1→v0.5 development; causes 2/3/4/9b predate the June benchmark, the rest are the 53→70 growth. Disambiguated the 07-23 fix "revert" (an editor discard) from code regressions. From 29fa6d8528759c0a6bdf4438e04411a54b4dd5cd Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Thu, 27 Aug 2026 04:05:08 -0700 Subject: [PATCH 4/4] wiki: state which Cause 7 symptom carries a committed test --- wiki/research/nub-v0.5-augmentation-regressions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/research/nub-v0.5-augmentation-regressions.md b/wiki/research/nub-v0.5-augmentation-regressions.md index cf6ab4558..407f8b89d 100644 --- a/wiki/research/nub-v0.5-augmentation-regressions.md +++ b/wiki/research/nub-v0.5-augmentation-regressions.md @@ -95,7 +95,7 @@ A `sourceURL` written as a filesystem path rather than a `file://` URL makes Nod - **Second symptom, same string:** the inspector reports that `sourceURL` verbatim as `Debugger.scriptParsed`'s `url`, which is the key editors and DevTools match breakpoints against. A bare path gave every `.ts` file an identity no other module in the same process used — an ordinary `.js` file already reported a `file:` URL on the same host — so a breakpoint resolved against the module URL matched no script. - **Landed:** 2026-06-26 (#181). Between benchmarks. - **Severity:** High — silent wrong coverage on the exact workflow nub targets (TS). -- **FIXED 2026-08-27 by [#800](https://github.com/nubjs/nub/pull/800).** The emitter writes the `file:` URL, spelled to match the host Node's own `pathToFileURL`: `nub-cache-key::source_url` derives the spelling and the post-transform step in `crates/nub-native/src/cache.rs` calls it. Both symptoms are covered end-to-end — the coverage row now matches node's exactly, and an inspector test asserts the reported script URL against the host's own `pathToFileURL` for the same file. +- **FIXED 2026-08-27 by [#800](https://github.com/nubjs/nub/pull/800).** The emitter writes the `file:` URL, spelled to match the host Node's own `pathToFileURL`: `nub-cache-key::source_url` derives the spelling and the post-transform step in `crates/nub-native/src/cache.rs` calls it. The inspector symptom is pinned by a regression test that asserts the reported script URL against the host's own `pathToFileURL` for the same file; the coverage symptom was re-verified by hand against the fixed build (the table matches node's exactly) but carries no committed test. - **Two things the original recommendation did not anticipate.** First, `pathToFileURL`'s escape set is not one set: [nodejs/node#54545](https://github.com/nodejs/node/pull/54545) widened it to cover `[`, `]`, `^`, `|` and `~`, and it reached the release lines out of order (20.18.3, 22.12.0, 23.1.0, and every 24+), so the spelling has to follow the host rather than a single fixed table. Second, the URL *form* deliberately does **not** follow the host: Node's own type stripping only began reporting a `file:` URL in 26.4.0, backported to 24.19.0, and 25.x never got it, so matching the host there would flip a file's script identity on a patch bump. The transpile-cache schema went to "7" as recommended, and the host's escape band joined `filename` as a key component — the URL is baked into the stored body, so without it an `nvm use` across a boundary serves the other band's spelling. ### Cause 8 — Explicit `--env-file` values get `$`-expanded (RECLASSIFIED: intentional, bun-parity)