Skip to content

Fix npx wrapper PATH leak from node@latest bootstrap#5341

Merged
dhh merged 37 commits intobasecamp:devfrom
timohubois:pin-npx-wrapper-to-mise-node-runtime
Apr 27, 2026
Merged

Fix npx wrapper PATH leak from node@latest bootstrap#5341
dhh merged 37 commits intobasecamp:devfrom
timohubois:pin-npx-wrapper-to-mise-node-runtime

Conversation

@timohubois
Copy link
Copy Markdown
Contributor

@timohubois timohubois commented Apr 17, 2026

Follow-up to #5247. That earlier fix intentionally moved Omarchy's generated npx wrappers to mise exec node@latest -- npx ... so npm-distributed CLIs would keep working inside projects pinned to old Node versions.

This PR preserves that intent: wrappers remain system-wide lazy-loaded npm launchers, and the CLI itself is still bootstrapped with Omarchy's selected node@latest runtime.

The bug fixed here is the side effect reported in #5451: mise exec node@latest prepends node@latest/bin to PATH, and long-running agent CLIs like Codex can pass that modified PATH on to workspace commands. That makes commands run inside the project resolve node@latest instead of the project's own Node.

Changes

  • Resolve Omarchy's node@latest install with mise where node@latest, installing it globally on first use if needed.
  • Run npx through the matching absolute node binary so the bootstrap runtime stays consistent.
  • Use npx --package ... -- which ... only to resolve the package bin path.
  • Execute the resolved CLI entrypoint with the selected node@latest binary without exporting or prepending node@latest/bin into PATH.
  • Fail clearly with exit 127 if no npm bin can be resolved.
  • Regenerate existing npx wrappers via migration.

Testing

  • codex --version
  • opencode --version
  • playwright-cli --version
  • Verified from a project pinned to Node 10 that codex sandbox linux node -v resolves the project Node instead of node@latest.
  • Verified the wrapper still bootstraps Codex, Opencode, and Playwright with the selected mise node@latest runtime.

Copilot AI review requested due to automatic review settings April 17, 2026 09:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Pins Omarchy’s generated npx wrappers so both node and npx are executed from the same mise-managed node@latest install (avoiding PATH drift where npx could come from a different runtime).

Changes:

  • Update omarchy-npx-install to resolve the node@latest install root via mise where and execute node + npx from that same root.
  • Add a migration to regenerate existing shipped wrappers using the updated wrapper template.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
migrations/1776348723.sh Regenerates the default set of npx wrappers to pick up the new pinned-runtime wrapper logic.
bin/omarchy-npx-install Changes generated wrapper to run node and npx from the same mise where node@latest install path.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings April 17, 2026 14:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Pins Omarchy-generated npx wrappers so both node and npx are executed from the same mise-managed node@latest installation (avoiding drift between the selected Node runtime and whatever npx happens to be on PATH).

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

  • Update omarchy-npx-install to resolve the mise node@latest install root and execute node + npx from that install explicitly.
  • Add a migration to regenerate existing wrappers using the updated wrapper template.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
migrations/1776348723.sh Adds a migration to rerun wrapper generation via install/packaging/npx.sh.
bin/omarchy-npx-install Updates generated wrapper contents to invoke node/npx directly from the mise node@latest install root.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dhh
Copy link
Copy Markdown
Member

dhh commented Apr 22, 2026

Could you share a concrete reproduction where mise exec node@latest -- npx picks up a foreign npx? My understanding is that mise exec prepends the runtime's bin directory to PATH for the duration of the child process, so npx should resolve to the same install as node.

@timohubois
Copy link
Copy Markdown
Contributor Author

@dhh Thanks for the feedback! I dug into this more and was able to reproduce it in a mixed shell setup (direnv + Nix + mise).
In that environment, running:
mise exec node@latest -- npx ...

can fail to keep npx coupled to the selected node runtime rather than the node@latest selected by mise, leading to errors such as:
Error: Cannot find module 'node:path'

A key part of the issue is that npx is a #!/usr/bin/env node script. Even when invoked via an absolute path, it can still pick up a different node from the ambient PATH, so $node_root/bin/npx alone isn’t sufficient to guarantee consistency.

I tried a few alternatives (mise exec, --fresh-env, shims, direct paths), but they weren’t reliable in this setup.

The fix I’m proposing now is to ensure the wrapper resolves both node and npx from the same runtime:

export PATH="$node_root/bin:$PATH"
exec "$node_root/bin/npx" --yes ...

This keeps the fix wrapper-local and ensures that npx resolves the matching node, even in mixed environments.

From my perspective, this fix makes sense if the wrapper is intended to keep npx launchers independent of the shell and tied to the selected mise Node runtime. I really appreciate your perspective on this. If I’ve misunderstood the intended scope, I’m happy to close the PR.

Copilot AI review requested due to automatic review settings April 27, 2026 09:23
@timohubois
Copy link
Copy Markdown
Contributor Author

Did some rework, related to the reported issue #5451.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Omarchy’s generated npx wrappers so both node and npx are executed from the same mise-managed node@latest installation, and adds a migration to regenerate existing wrappers accordingly.

Changes:

  • Add a migration to re-run the npx wrapper installation script.
  • Update omarchy-npx-install to generate wrappers that locate node@latest’s install root and run node + npx from that same location.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
migrations/1776348723.sh Runs the existing install/packaging/npx.sh to regenerate wrappers with the new behavior.
bin/omarchy-npx-install Generates wrappers that resolve node@latest’s install path and pin both node and npx to that runtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@timohubois timohubois changed the title Pin npx wrapper to mise node runtime Isolate npx wrapper runtime from project PATH Apr 27, 2026
@dhh dhh merged commit 11e549a into basecamp:dev Apr 27, 2026
@timohubois timohubois changed the title Isolate npx wrapper runtime from project PATH Fix npx wrapper PATH leak from node@latest bootstrap Apr 27, 2026
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.

3 participants