fix(env): strip trailing slash from node: builtin specifiers (#555)

[tsushanth]

Closes #555.

When an alias chain feeds _resolve a value like punycode/, pathe's resolveAlias rewrites it through the punycode -> node:punycode mapping and tacks the trailing slash back on, producing node:punycode/. That string then hits the id.startsWith("node:") short-circuit and was returned verbatim. Node rejects the resulting specifier with ERR_UNKNOWN_BUILTIN_MODULE because trailing slashes are not legal on node: builtins, even though the equivalent package form punycode/ is fine.

The fix normalises only inside the node: branch and only when the trimmed name is a real builtin. So:

• node:punycode/ -> node:punycode (the bug)
• node:punycode -> unchanged
• node:not-a-real-builtin/ -> unchanged (no surprise rewrites of unknown values)
• non-node: strings -> unchanged path

This keeps the regular package-path behaviour the issue calls out (my-module/ stays my-module/).

Tests

Three new cases under describe("resolvePath") in test/env.test.ts:

• the reporter's repro: an alias chain that ends at punycode/ resolves to node:punycode
• a literal node:punycode alias stays unchanged
• a node:not-a-real-builtin/ alias is left alone

All three pass with the patch and fail (or never run) against main. The pre-existing resolves all nodeCompat paths failure on main (unenv/node/inspector/promises missing) is unrelated; confirmed by running the suite on main without these changes.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Fixed resolution of Node builtin module aliases with trailing slashes to prevent loading invalid or unknown builtin identifiers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9864257b-f1e6-4c38-a9a3-bea0847c7ab6

📥 Commits

Reviewing files that changed from the base of the PR and between f89b7cc and 73ccb4a.

📒 Files selected for processing (2)

• src/env.ts
• test/env.test.ts

---

📝 Walkthrough

Walkthrough

The PR adds trailing-slash normalization to the resolvePaths function's internal _resolve logic. When a node: specifier ends with /, the code strips it and returns the trimmed identifier only if it corresponds to a real Node builtin module; otherwise the original specifier is preserved. Three test cases validate this behavior for real builtins, literal node: specifiers, and non-builtin node: values.

Changes

Node Builtin Trailing Slash Normalization

Layer / File(s)	Summary
Trailing slash handling for node: builtins src/env.ts, test/env.test.ts	The _resolve function inside resolvePaths detects node: specifiers ending with /, strips the slash, and returns the trimmed value only when it matches an actual Node builtin module; otherwise the original specifier is kept unchanged. Three test cases validate stripping for real builtins, preservation of literal node: specifiers, and retention of trailing slashes for non-builtin node: values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• resolvePath should strip trailing slashes from node: builtin specifiers #555: Targets the same code-level fix in resolvePaths to strip trailing slashes from Node builtin specifiers only when the trimmed identifier matches a real builtin module.

Poem

🐰 A trailing slash walks into a builtin resolver, hoping for a shortcut.
But our code says: "Only real builtins get trimmed, the rest stay as-is!"
Now paths resolve true, with logic both fuzzy and precise. 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically describes the main change: fixing trailing slash handling in node: builtin specifiers, which aligns perfectly with the changeset's core purpose.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[tsushanth]

Apologies @ashtonjurgens — your #556 was open about 6 hours before this and touches the exact same files (src/env.ts + test/env.test.ts). The Fixes: keyword wasn't picked up by the issue sidebar so I missed the parallel. Closing in favor of yours.