Skip to content

feat(run): fall back to node_modules/.bin on a script miss#500

Open
afonsojramos wants to merge 5 commits into
nubjs:mainfrom
afonsojramos:feat/run-bin-fallback
Open

feat(run): fall back to node_modules/.bin on a script miss#500
afonsojramos wants to merge 5 commits into
nubjs:mainfrom
afonsojramos:feat/run-bin-fallback

Conversation

@afonsojramos

Copy link
Copy Markdown
Contributor

Summary

nub run <name> now falls back to node_modules/.bin/<name> when the name matches no script, so locally installed CLIs run without a pass-through script. nub run astro dev works in a project with no "astro": "astro" boilerplate — matching bun run and aube's own documented contract ("If no script matches, aube falls back to node_modules/.bin/<name>"), which nub's Rust run path had not implemented.

nub run astro dev        # no script needed; args forward as with a script
nub run tsc --noEmit

Behavior, in short:

  • A script always wins over a same-named bin.
  • Resolution walks up to the workspace root, so hoisted installs work; Yarn PnP trees resolve through the PnP bin-runner nub exec already uses.
  • Local only — a bin miss never reaches the registry (that stays nubx/dlx territory).
  • A bin is not a lifecycle: no pre/post hooks, no npm_lifecycle_* env. --if-present treats a resolvable bin as present. /regex/ selectors never fall back.
  • Double miss keeps exit 1 and the existing missing script error, extended to note the .bin lookup.

Design decisions

  • Hook at the two script-miss sites (single-package selector and -w root lookup), trying the bin before --if-present, matching aube's order. Resolution and launch reuse the nub exec machinery (find_bin walk-up, shebang-aware launch_bin) — no new resolution logic.
  • PnP probe mode: the shared pnp-bin-run.cjs runner gained a quiet --probe (exit 0/1, no output) so a PnP bin miss doesn't shadow the missing-script error with the runner's own 127 message.
  • Plain names only: the fallback refuses path-shaped names, so nub run ../foo can't traverse out of .bin via the walk-up (found in review; regression test included).
  • Recursive fan-out unchanged: -r/--filter keep script-only resolution; per-member bin fallback is a deliberate follow-up.

Session-settled decisions carried from planning: fallback scope limited to single-package + -w (user-approved, over including -r/--filter now); no lifecycle hooks for bins (user-approved, over honoring pre<name>/post<name>); PnP parity included via the existing runner (user-approved, over node_modules-only); double miss keeps exit 1 (user-approved, over exec's 127 shape).

Test plan

  • 11 new integration tests (written red-first): fallback + arg forwarding, script precedence, non-node (#!/bin/sh) bins, walk-up to the workspace root, --silent preamble suppression, --if-present semantics, no lifecycle hooks, double-miss error shape, regex selectors never falling back, -w root bin + root-script precedence, path-traversal guard.
  • cargo test -p nub-core -p nub-cli: full suite green.
  • PnP matrix: 18/18, including 3 new checks — run-bin (PnP fallback), run-bin-miss (probe-silent double miss keeps the missing-script error), ws-run-bin (from a member).
  • Conformance run-missing-script cell still pins exit 1; expectation note updated to record the new requirement (bin lookup must also miss).
  • Windows is exercised by the .cmd-shaped fixtures on the windows CI leg; find_bin's extension handling is already unit-tested.

Post-Deploy Monitoring & Validation

No additional operational monitoring required — this is a local CLI behavior change with no service, telemetry, or data-path impact; the conformance suite and PnP matrix pin the behavior on every CI run.

Documentation

runner/run.mdx gains a "Local binaries" section (precedence, no-registry boundary, PnP, --if-present, the deliberate pnpm run/npm run deviation), and runner/exec.mdx notes when exec remains the explicit form.

@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview, Comment Jul 21, 2026 10:11pm

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changesnub run <name> now falls back to node_modules/.bin/<name> when the name matches no package.json script, giving locally installed CLIs a script-free entry point (bun/aube parity).

  • Two script-miss hooks in cli.rs — the single-package ScriptSelection::None arm in run_selected_scripts and the -w/--workspace-root None arm in run_script both try run_bin_fallback_for_plain_name before --if-present/error. Recursive (-r/--filter) correctly stays script-only (it returns via run_workspace_target before the fallback site).
  • Path-shaped names refusedis_plain_bin_name rejects / and \, so ../foo can't traverse out of .bin via the walk-up and a /regex/ selector never falls back.
  • Resolution reuses nub exec machineryfind_bin walk-up + shebang-aware launch_bin; no lifecycle hooks, no npm_lifecycle_* env for a bin.
  • PnP --probe mode in pnp-bin-run.cjs — a resolve-only pass (exit 0/1, no output) keeps a PnP bin miss silent so the missing script error owns the double-miss instead of the runner's 127.
  • Error/docs/tests — the missing script error gains a (and no node_modules/.bin/<name>) note; run.mdx/exec.mdx document the fallback and the deliberate pnpm run/npm run deviation; 11 integration tests, +3 PnP-matrix cells, and a conformance note pin the behavior.

The implementation is clean and reuses existing resolution/launch code rather than duplicating it. I traced the interactions that looked risky and they hold: the dep-freshness gate is once-per-process-latched and fires in run_script before the fallback, so the PnP probe's run_file_in_dir re-entry returns early rather than double-gating or reporting a spurious miss; a Policy::Error staleness abort short-circuits before the fallback is reachable; and script-always-wins is enforced by resolving the script first at both sites. The scope cuts (no -r/--filter fan-out, no lifecycle hooks) are documented and user-approved.

One non-actionable note for awareness: the fallback preamble prints $ <name> <args> whereas a script run prints $ <resolved command>. That difference is inherent (a bin has no command string) and the tests pin the current shape, so nothing to change.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — one new commit since the prior review restructures the PnP bin-run probe path to avoid process.exit(), fixing a Windows-only crash where an aborted exit made a bin HIT read as a miss.

  • process.exit()-free probe path in pnp-bin-run.cjs — on Windows, process.exit() with piped stdio races libuv's handle teardown and can abort on the UV_HANDLE_CLOSING assert, surfacing a crash code instead of the intended 0/1. The probe now sets process.exitCode and lets the event loop drain: if (!api && !probe) skips the 127 exit for probes, a miss calls probeMiss() (exitCode = 1), a hit sets exitCode = 0, and the ESM-fallback process.exit(1) is confined to the non-probe run block. The bin-resolution loop was wrapped in if (api) so the fall-through works.

I traced all four probe branches (no-api / miss / hit / run) and each exits with the correct code with no reachable process.exit() on the probe path. The Rust caller (try_run_bin_fallback, cli.rs:4940) still keys off run_file_in_dir(...) != 0, so keying on an exitCode-driven natural exit is behaviorally equivalent to the old explicit exits — the fix is transparent to the caller. The non-PnP probe case (!api && probe) correctly falls through to probeMiss() and reports a miss.

Pullfrog  | View workflow run | Using Claude Opus𝕏

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