Skip to content

node: withhold --enable-source-maps across the whole Node 26.x assert regression band - #784

Open
colinhacks wants to merge 1 commit into
mainfrom
assert-sourcemaps-gate
Open

node: withhold --enable-source-maps across the whole Node 26.x assert regression band#784
colinhacks wants to merge 1 commit into
mainfrom
assert-sourcemaps-gate

Conversation

@colinhacks

Copy link
Copy Markdown
Contributor

source_maps_safe withheld the flag only on 26.2.x. The regression is nodejs/node#63169 and hits every released 26.x: a no-message assert(false) throws TypeError [ERR_INVALID_ARG_TYPE] instead of AssertionError. Reproduced on 26.0.0, 26.3.0, 26.5.1, 26.7.0; 25.x is clean.

Gate is now major == 26 && version < 26.8.0. Fixed upstream by b5d37cd4 (nodejs/node#63215), unreleased as of v26.7.0. Stopgap — re-verify when 26.8.0 ships.

On 26.7.0, test/parallel/test-assert.js goes 4 failures to 0. Docs and the research doc's superseded verdict updated.

Refs nodejs/node#63169

Copilot AI lite review requested due to automatic review settings August 22, 2026 07:14
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview Aug 22, 2026 7:16am

Request Review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The gate itself is correct for every Node that exists today, and the band widening is well-evidenced. Two things to settle before merge: the 26.8.0 upper bound is not confirmed upstream and fails in the unsafe direction, and the docs outside typescript.mdx still promise source maps unconditionally on what is now the recommended Node major.

Reviewed changes — the full diff at 735da37, plus the surrounding injection paths, the existing flag tests, and the docs/marketing surfaces that describe source-map behavior.

  • Widened source_maps_safecrates/nub-core/src/node/flags.rs:48-50 now withholds --enable-source-maps for major == 26 && v < 26.8.0 instead of only 26.2.x, with a rewritten doc comment recording the mechanism, the reproductions, and the upstream fix.
  • NodeVersion::minor() removedcrates/nub-core/src/node/version.rs; the predicate switched to full-version comparison. Verified zero remaining callers repo-wide, so the removal is clean.
  • Band test re-pinnedflags.rs:1138-1168 now asserts the new edges (safe on 24.0.0 / 25.9.0 / 26.8.0 / 27.0.0; withheld on 26.0.0 / 26.2.0 / 26.7.0). It does fail with the bug present: 26.0.0 was safe under the old gate.
  • Docs + decision recordsite/content/docs/runtime/typescript.mdx:133 and a formal reversal of the 2026-07-24 "accept, no fix" verdict in wiki/research/nub-v0.5-augmentation-regressions.md.

Confirmed clean: compute_inject_flags is the only place the flag is added — node_options_safe_inject_flags filters its output, spawn.rs calls it directly, and nub-launcher/src/main.rs:288 calls the same nub-core function — so the gate covers both the argv and NODE_OPTIONS channels. No existing test asserts a remapped .ts frame or a positive process.sourceMapsEnabled, so nothing goes stale there.

⚠️ Docs outside typescript.mdx still promise source maps unconditionally

The PR updates the TypeScript page but leaves two other surfaces claiming the flag is always on. Under the old 26.2-only gate that was a rounding error; now the exception covers every released Node 26, which AGENTS.md names as the version docs and examples should recommend — so these read as false to the readers most likely to hit them.

Technical details
# Source-map claims left unqualified by the widened band

## Affected sites
- `site/content/docs/runtime/debugging.mdx:43` — "Nub transpiles TypeScript with an inline source map and injects `--enable-source-maps`, so the debugger maps the running code back to your original file". The named mechanism is not applied on Node 26.0–26.7. (Note the *outcome* claim may still hold: `--inspect` breakpoint mapping comes from the inline source map DevTools reads, not from `--enable-source-maps`, which only remaps `Error.stack`. So the fix is likely to correct the mechanism sentence rather than caveat the whole paragraph.)
- `site/public/skill.md:39` — "inline source maps + `--enable-source-maps` on by default, so stack traces point at your `.ts` source." This is the agent-facing surface served at `nubjs.com/skill.md`; "on by default" is now wrong on the recommended major.

## Required outcome
- A reader on Node 26 is not told stack traces are remapped when they are not.
- Deliberately out of scope unless you disagree: the one-line feature bullets (`site/content/docs/runtime/index.mdx:23`, the homepage) and the dated blog post — bullets don't carry caveats and blog posts are not retro-edited.

## Open questions for the human
- Is `skill.md` worth qualifying at all, or does the band churn too fast to be worth a version caveat in agent-facing copy?

ℹ️ Nitpicks

  • No CI leg runs Node 26 (.github/workflows/ci.yml pins the current legs to "24"; tier legs are 22.15 / 22.13 / 20.11 / 18.19), so neither the old nor the new band boundary has ever had automated signal. Worth knowing when judging how much the unit test at flags.rs:1138 is actually buying — it pins the predicate's arithmetic, not the empirical claim about Node.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

/// which breaks `node:test` assertions outright.
fn source_maps_safe(node_version: &NodeVersion) -> bool {
!(node_version.major() == 26 && node_version.minor() == 2)
!(node_version.major() == 26 && *node_version < NodeVersion::new(26, 8, 0))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 26.8.0 upper bound is unconfirmed upstream, and it fails in the unsafe direction. b5d37cd4 merged to Node main on 2026-08-20, but v26.x-staging was at 2026-08-18 and nodejs/node#63215 carries no backport marker or milestone — so 26.8.0 is verifiably the earliest release that could carry the fix, not a confirmed one. If it ships without it, nub silently re-injects and the type corruption returns, with no test and no CI leg (CI pins Node 24) to notice.

Technical details
# The stopgap's upper bound assumes the fix rather than verifying it

## Affected sites
- `crates/nub-core/src/node/flags.rs:49``!(node_version.major() == 26 && *node_version < NodeVersion::new(26, 8, 0))` opens the band at 26.8.0 unconditionally.
- `crates/nub-core/src/node/flags.rs:40-43` — "RE-VERIFY when 26.8.0 ships, and move the boundary up if it does not carry the fix" is a note to a human, not a mechanism. Nothing fires when 26.8.0 lands.

## Required outcome
- If 26.8.0 ships without `b5d37cd4`, nub must not silently re-enable `--enable-source-maps` on it.

## Suggested approach (optional)
- The PR's own asymmetry argument points one way: withholding costs stack-trace remapping (cosmetic), injecting corrupts the TYPE of a thrown `AssertionError` (breaks `node:test`). Gating on the *verified-good* set rather than the *assumed-good* set means withholding across all `26.x` until a release is checked, then narrowing. Both shapes need a follow-up nub release, but only one of them fails safe.
- If you'd rather keep the optimistic bound, a tracking issue is the minimum forcing function, since neither the unit test nor CI will ever exercise a real Node 26.

## Open questions for the human
- Is 26.x the Current line? If so, commits on `main` typically sweep into the next minor by default and the optimistic bound is probably right in practice — this is a risk-posture call, not a proven defect.

The stack frame reports `app.ts:2:8` — the line in your source, not the transpiled output. To turn it off, pass `--no-enable-source-maps`.

Source maps are injected on every supported Node version except the **26.2.x** patch band, where a Node regression makes a no-message `assert(false)` rethrow as a `TypeError` instead of an `AssertionError` when source maps are on. On 26.2.x Nub withholds the flag, so stack traces there are not remapped. Every other version (18.19 through 26.1, and 26.3+) gets source maps.
Source maps are injected on every supported Node version except **26.0 through 26.7**, where a Node regression ([nodejs/node#63169](https://github.com/nodejs/node/issues/63169)) makes a no-message `assert(false)` throw a `TypeError` instead of an `AssertionError` when source maps are on. Nub withholds the flag across that band, so stack traces are not remapped there. Every other version gets source maps: 18.19 through 25.x, and 26.8 onward, where the upstream fix ships.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"26.8 onward, where the upstream fix ships" states as settled fact something the code comment two files over hedges as a stopgap pending re-verification. The fix is on Node main and not confirmed in any release line yet, so this reads as a promise the docs may have to walk back. Something closer to "26.8 onward, once the upstream fix reaches a release" matches the evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants