Skip to content

fix(linker): point bin shim NODE_PATH at the hidden modules dir#613

Merged
jdx merged 4 commits into
mainfrom
fix/bin-shim-node-path
May 11, 2026
Merged

fix(linker): point bin shim NODE_PATH at the hidden modules dir#613
jdx merged 4 commits into
mainfrom
fix/bin-shim-node-path

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented May 11, 2026

Summary

Two stacked bugs caused astro check (and any tool that loads a transitive via require('typescript') from a shimmed bin) to fail on an aube install while it worked on pnpm:

  1. preferSymlinkedExecutables defaulted to symlink under the isolated linker. A symlink can't export env vars, silently making extendNodePath=true a no-op on every default install. pnpm's actual default flips based on the linker (symlink for node-linker=hoisted, shim for isolated); aube now mirrors that.
  2. The shim's NODE_PATH only covered the project's top-level node_modules/. Auto-installed peers hoisted to .aube/node_modules/ were invisible to Node's resolver when launched from a shim. The shim now emits a two-entry NODE_PATH covering both directories, matching the load-bearing entries of pnpm's own shim.

Repro

Astro project with @astrojs/check as a direct devDep and typescript only auto-installed as the package's peer:

$ aube install
$ ./node_modules/.bin/astro check
To continue, Astro requires the following dependency to be installed: typescript.
  pnpm add typescript

typescript@5.9.3 was on disk at node_modules/.aube/typescript@5.9.3/node_modules/typescript, but the shim's NODE_PATH pointed at the root node_modules/ which doesn't contain transitives.

After the fix:

$ ./node_modules/.bin/astro check
[content] Synced content
[types] Generated 57ms
[check] Getting diagnostics for Astro files in /tmp/cmp-aube-with-pnpm-lock...
Result (0 files): - 0 errors - 0 warnings - 0 hints

The same shim now reads:

export NODE_PATH="$basedir/..:$basedir/../.aube/node_modules"

Test plan

  • cargo test -p aube-linker --lib — 77 pass (new create_bin_shim_appends_hidden_modules_to_node_path covers the two-entry NODE_PATH)
  • cargo test -p aube --bin aube — 463 pass
  • cargo test --workspace --lib — all green
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo fmt --all -- --check — clean
  • End-to-end on /tmp/cmp-aube-with-pnpm-lock (Astro + Vite + @astrojs/check with existing pnpm-lock.yaml): aube install + astro check succeeds, matching pnpm behavior

🤖 Generated with Claude Code


Note

Medium Risk
Changes default POSIX .bin entry type (symlink → shim) under the isolated linker and alters NODE_PATH construction across platforms, which may affect how executables resolve dependencies and how tools observe .bin files.

Overview
Fixes isolated installs where shimmed CLIs couldn’t resolve transitives/auto-installed peers by expanding shim NODE_PATH to include both the top-level node_modules and the hidden virtual-store modules dir (.aube/node_modules).

On POSIX, preferSymlinkedExecutables now defaults to shims under the isolated linker (to make extendNodePath effective) while keeping the symlink default under nodeLinker=hoisted; global installs explicitly pass no hidden-modules path. Windows shims are updated to build multi-entry NODE_PATH correctly (always using ; as the delimiter), and tests/docs are updated to reflect the new defaults and NODE_PATH contents.

Reviewed by Cursor Bugbot for commit 9362ecd. Bugbot is set up for automated code reviews on this repo. Configure here.

jdx and others added 3 commits May 11, 2026 17:42
Two stacked bugs caused `astro check` (and any tool that loads a
transitive via `require('typescript')` from a shimmed bin) to fail
on an aube install while it worked on pnpm:

1. `preferSymlinkedExecutables` resolved to `true` whenever unset,
   so POSIX `.bin/<name>` was always a bare symlink. A symlink can't
   export env vars, which silently made `extendNodePath=true` a
   no-op on every default install. pnpm's actual default flips
   based on the linker: symlink under `node-linker=hoisted`,
   shim under `isolated`. aube now mirrors that — unset defaults to
   `false` for the isolated linker.

2. Even with a shim, `NODE_PATH` was set to `$basedir/..` —
   `<project>/node_modules/` only — so Node's resolver couldn't see
   auto-installed peers hoisted to `.aube/node_modules/`. The shim
   now emits a two-entry NODE_PATH covering the project's top-level
   `node_modules/` and the virtual store's hidden modules dir,
   matching the load-bearing entries of pnpm's own shim.

Repro: an Astro project with `@astrojs/check` as a direct devDep
(and `typescript` only auto-installed as the package's peer) ran
`astro check` and hit `requires the following dependency to be
installed: typescript`, even though `typescript` was on disk at
`node_modules/.aube/typescript@.../node_modules/typescript`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 11, 2026

Greptile Summary

Fixes two compounding bugs that caused shimmed bins (e.g. astro check) to fail under the isolated linker because auto-installed peers were invisible to Node's resolver. The fix adds a hidden_modules_dir field to BinShimOptions and introduces shim_node_path to build a complete two-entry NODE_PATH string that covers both the top-level node_modules/ and the virtual store's hidden node_modules/.

  • shim_node_path refactor: shim generators now receive the fully-assembled NODE_PATH value (with $basedir/ / %~dp0 prefixes already embedded) rather than a bare relative path, which is the only way to express a multi-entry list without repeating prefix logic in each generator.
  • Default preferSymlinkedExecutables flip: isolated-linker installs now default to shim (not symlink) so extendNodePath is effective; hoisted installs keep the symlink default. Both install and rebuild guard hidden_modules_dir with isolated.then_some(...), so hoisted users don't get a spurious .aube/node_modules entry.
  • Tests and docs: bats integration tests are updated to match the new shim-first default; new unit tests cover the two-entry POSIX and Windows NODE_PATH shapes.

Confidence Score: 5/5

Safe to merge — shim and default changes are well-guarded and tested end-to-end.

The isolated.then_some(...) guard in both install/mod.rs and rebuild.rs correctly limits the hidden-modules NODE_PATH entry to isolated layouts, so hoisted users see no change in their shim output. The shim_node_path refactor consolidates prefix construction into one place, reducing per-platform divergence. New unit tests cover the two-entry POSIX and Windows (cmd/ps1/sh) shapes explicitly, and the bats integration tests are updated to match the new shim-first default.

No files require special attention.

Important Files Changed

Filename Overview
crates/aube-linker/src/sys.rs Core shim generation refactored: BinShimOptions gains hidden_modules_dir, shim_node_path builds the complete NODE_PATH value with correct platform separators, and generator functions updated to accept the pre-built value. New unit tests cover POSIX and Windows two-entry paths.
crates/aube/src/commands/install/mod.rs Adds isolated flag to gate hidden_modules_dir and the new preferSymlinkedExecutables default; correctly passes None for hoisted layouts so no spurious shim entry is emitted.
crates/aube/src/commands/rebuild.rs Mirrors the same isolated/prefer_symlinked_executables logic from install/mod.rs; hidden_modules_dir is correctly gated with isolated.then_some(...).
crates/aube/src/commands/add.rs Global install correctly passes hidden_modules_dir: None since the global layout is hoisted-shaped and needs no second NODE_PATH entry.
test/optimistic_and_bin_settings.bats Integration tests updated to reflect the new shim-first default: the old symlink assertion becomes a shim assertion, and the two-entry NODE_PATH grep pattern is updated accordingly.
test/workspace.bats Symlink-specific test -L / readlink -f checks replaced with shim-agnostic test -f + test -x + shim-marker grep, correctly handling both symlink and shim layouts.

Reviews (2): Last reviewed commit: "fix(linker): use `;` on Windows + skip h..." | Re-trigger Greptile

Comment thread crates/aube/src/commands/install/mod.rs
Comment thread crates/aube/src/commands/rebuild.rs
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d0a1f1a. Configure here.

Comment thread crates/aube-linker/src/sys.rs Outdated
Address PR review feedback and the bats-serial CI failures:

- Windows: NODE_PATH is parsed by Node.js, which always splits on `;`
  (`path.delimiter`) regardless of which shell launched the bin. The
  prior change used `:` for the ps1 and Git-Bash `.sh` shims because
  their paths use `/`, but that conflated the path separator with
  the list separator — Node would have read the multi-entry value
  as one invalid path and dropped the hidden-modules entry. All
  three Windows shim variants now join with `;`.
- `install` and `rebuild`: gate `hidden_modules_dir` on
  `node_linker != Hoisted`. Under the hoisted layout the directory
  doesn't exist; appending it to NODE_PATH would just stuff a
  spurious non-existent entry into every shim. `add.rs` (global
  install, hoisted-shaped) already did this; the same guard now
  applies to project installs and rebuilds.
- bats: update the suite to match the new defaults. The default
  `.bin/<name>` is now a shim under the isolated linker rather
  than a symlink, with a two-entry NODE_PATH. Replace the assertion
  shape in `workspace.bats` for the workspace-bin test so it works
  for both shim and symlink outputs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jdx
Copy link
Copy Markdown
Contributor Author

jdx commented May 11, 2026

Pushed 9362ecd addressing the review feedback and the bats-serial CI failures:

Greptile (both files): gated hidden_modules_dir on node_linker != Hoisted in install/mod.rs and rebuild.rs. Under the hoisted layout that path doesn't exist, so it would have shown up as a non-existent NODE_PATH entry on Windows. add.rs already had this guard for its hoisted-shaped global installs.

Cursor Bugbot (Windows separator): real bug — NODE_PATH is parsed by Node.js, which always splits on ; on Windows regardless of the launching shell. The prior helper used : for the ps1 / Git-Bash .sh shims because their paths use /, but I'd conflated path separator with list separator. All three Windows shim variants now join with ; — Node sees both entries correctly. Added create_bin_shim_appends_hidden_modules_on_windows_uses_semicolon to lock the cmd/ps1/sh assertions.

bats: updated optimistic_and_bin_settings.bats for the new default (shim under isolated, with a two-entry NODE_PATH) and reworked workspace.bats's workspace-bin test so it accepts both shim and symlink outputs by reading the v1 marker comment when the entry isn't a symlink.

Verified: mise run ci:bats:serial passes locally end-to-end (92/92 plus the new ones), cargo test --workspace --lib is green, clippy/fmt clean, and the end-to-end Astro+Vite repro still resolves typescript from the shim.

This comment was generated by Claude.

@jdx jdx merged commit a54b562 into main May 11, 2026
17 checks passed
@jdx jdx deleted the fix/bin-shim-node-path branch May 11, 2026 18:28
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.

1 participant