[RyoKusnadi]

Summary

npm and npx were not found after installing nodejs/node through tools. The shims in the tools bin dir were dangling symlinks.

Changes

• Skip the hardlink in createShimFile when the target is itself a symlink, and point the shim at the target's absolute path instead. link(2) does not follow symlinks on Linux, so the old code copied node's relative npm -> ../lib/node_modules/npm/bin/npm-cli.js link into the bin dir, where it dangled.
• Dangling shims from earlier installs are replaced on the next install.
• Add a test for a symlinked target that also replaces a dangling shim.

Related Issues

Closes #3229

Checklist

• Code follows the project style guidelines
• Self-review of the code has been performed
• Tests have been added or updated as needed
• Documentation has been updated as needed
• Changes have been tested locally

Tested with go test ./internal/tools/....

---

Summary by cubic

Fixes npm and npx not being found after installing nodejs/node through tools, caused by dangling shims in the bin dir.

Hard-linking a symlinked target copied the relative link (e.g. npm -> ../lib/node_modules/npm/bin/npm-cli.js) into the bin dir, where it dangled. Shims now point at the target's absolute path when the target is a symlink, and dangling shims from earlier installs are replaced on the next install. Added a test covering both behaviors. Closes #3229.

^{Written for commit 136f4db. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Fixed command shims for symlinked executable targets. When an executable is reached through a symlink, the shim now uses an appropriate fallback instead of creating a hard link to the symlink itself. This helps prevent dangling shims and ensures the resulting command remains executable and points to the intended target.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: ad515f7a-3ccf-4886-99a6-f9e8ffa02575

📥 Commits

Reviewing files that changed from the base of the PR and between eac382e and 136f4db.

📒 Files selected for processing: 2

• internal/tools/aqua/shim.go
• internal/tools/aqua/shim_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The aqua shim creation path now skips hard-link attempts for symlink targets. A new isSymlink helper uses Lstat to detect symlinks. A non-Windows test checks that a shim for a symlinked npm target resolves and is executable.

Changes: Aqua shim creation

Layer / File(s)	Summary
Symlink target handling internal/tools/aqua/shim.go, internal/tools/aqua/shim_test.go	Shim creation checks whether the target is a symlink before attempting a hard link. The new test verifies that a shim for a symlinked npm target resolves and is executable.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 136f4

The npm shim fix appears ready to merge after normal checks; no unresolved issue was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: preserving npm and npx shims for symlinked tool binaries.
Description check		The description includes all required sections, explains the root cause and fix, references issue #3229, completes the checklist, and records the test command.
Linked Issues check		Issue #3229 requires npm to work after installing nodejs/node through tools; its report also identifies npx as affected by the same dangling-symlink behavior. The PR skips hard-link creation f…
Out of Scope Changes check		The changes are limited to internal/tools/aqua/shim.go and its focused test. The implementation and test directly address issue #3229 by fixing symlinked tool targets and stale shims. No unrelated b…

Full details: Linked Issues check

Explanation

Issue #3229 requires npm to work after installing nodejs/node through tools; its report also identifies npx as affected by the same dangling-symlink behavior. The PR skips hard-link creation for symlink targets, creates the shim with the target path, and replaces an existing dangling shim. The added non-Windows test verifies that a symlinked npm target produces a non-dangling executable shim and replaces the old dangling shim. The generic shim code applies the fix to both npm and npx.

Full details: Out of Scope Changes check

Explanation

The changes are limited to internal/tools/aqua/shim.go and its focused test. The implementation and test directly address issue #3229 by fixing symlinked tool targets and stale shims. No unrelated behavior or files are changed.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[yohamta0]

Thanks @RyoKusnadi, nice catch on the hardlink following (or not following) the symlink. This fixes corepack too. LGTM!