Skip to content

resolver: skip auto-install for invalid npm package names #29255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 2 commits into
base: main
Choose a base branch
from
+115 −1

test: drop unused stderr bindings

6dbd60a
Select commit
Loading
Failed to load commit list.
Open

resolver: skip auto-install for invalid npm package names #29255

test: drop unused stderr bindings
6dbd60a
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 13, 2026 in 17m 13s

Code review found 1 potential issue

Found 5 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 0
🟣 Pre-existing 1
Severity File:Line Issue
🟣 Pre-existing src/resolver/resolver.zig:1907 isNPMPackageName accepts empty-scope @/pkg, hole in new auto-install gate

Annotations

Check notice on line 1907 in src/resolver/resolver.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

isNPMPackageName accepts empty-scope @/pkg, hole in new auto-install gate 

The new gate at resolver.zig:1907 relies on `strings.isNPMPackageName`, but that function has a pre-existing bug where empty-scope specifiers like `@/pkg` incorrectly pass the check. This means `@/pkg`-style specifiers still reach the auto-install path, potentially triggering the same reentrant `EventLoop.tick()` SIGSEGV this PR aims to prevent. The fix in `isNPMPackageNameIgnoreLength` should use `slash_index > 1` instead of `slash_index > 0` to require at least one character between `@` and `/
View more details on Claude