cli: dispatch -r/--filter before info verbs to the PM engine#40
Merged
Conversation
…ist/why/…) `nub -r <verb>` and `nub --filter <x> <verb>` for the read-only info family (list/ls/la/ll, why, outdated, licenses, audit, peers, query, check, sbom, bin, root, view, search) fell through to the Node file-runner instead of dispatching to the PM engine, because the leading-flag reorder only recognized the install family. The verb token reached Node as a module/option, producing `Cannot find module 'list'` / `node: bad option: --filter`. Replace the hand-curated NORMALIZABLE_LEADING_FLAG_VERBS list with is_normalizable_leading_flag_verb, which recognizes any PM verb nub can dispatch — the same authority the bareword arm consults (SUBCOMMANDS + the engine verb registry). Tying the two together keeps the reorder set and the dispatch set from drifting, so every current and future PM verb that accepts a leading workspace flag reorders correctly. Add a routing regression test that dispatches `nub -r list` / `--filter x why` / etc. for real (no `--help` short-circuit) and asserts the verb reached the engine, i.e. the file-runner crash markers are absent. Refs the pnpm conformance work (D1). Claude-Session: https://claude.ai/code/session_01YRvztkcr4fzfg9rD5edUwa
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes a pnpm-compat CLI routing gap where leading workspace flags (-r/--recursive, --filter/-F, …) placed before read-only/info PM verbs were not normalized, causing those invocations to fall through to the Node file-runner instead of dispatching to the embedded PM engine.
Changes:
- Replace the hand-maintained
NORMALIZABLE_LEADING_FLAG_VERBSlist with a predicate (is_normalizable_leading_flag_verb) that keys off nub’s dispatchable verb sets. - Update
normalize_leading_run_flagsto use the predicate sonub -r <info-verb>/nub --filter <x> <info-verb>normalize into verb-first order. - Add an integration-style test ensuring “leading workspace flags before info verbs” do not hit Node file-runner crash markers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/nub-cli/src/cli.rs | Broadens leading-flag normalization to any dispatchable PM verb so info verbs route to the PM engine instead of Node. |
| crates/nub-cli/tests/cli_grammar_parity.rs | Adds a regression test that executes nub and asserts info-verb forms don’t fall through to the Node file-runner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1030
to
+1032
| fn is_normalizable_leading_flag_verb(verb: &str) -> bool { | ||
| SUBCOMMANDS.contains(&verb) || crate::pm_engine::lookup_verb(verb).is_some() | ||
| } |
Comment on lines
+1016
to
+1019
| /// into canonical subcommand-first order; otherwise the verb token falls | ||
| /// through to the Node-passthrough/file-runner path and Node fails on, e.g., | ||
| /// `--require list` (`Cannot find module 'list'`) or `node: bad option: | ||
| /// --filter`, falsely reporting success. |
Contributor
Author
|
Shipped in v0.1.10: https://github.com/nubjs/nub/releases/tag/v0.1.10 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
nub -r <verb>andnub --filter <x> <verb>for the read-only info verb family (list/ls/la/ll,why,outdated,licenses,audit,peers,query,check,sbom,bin,root,view,search) fell through to the Node file-runner instead of dispatching to the PM engine:nub -r list→Error: Cannot find module 'list'nub --filter a list→node: bad option: --filterpnpm -r <cmd>/pnpm --filter <x> <cmd>are the canonical workspace invocations, so this was a high-traffic compat break.Root cause
normalize_leading_run_flagsreorders a leading run-flag (-r/--recursive/--filter/-F/…) to sit after the verb, but only when the verb was in the hand-curatedNORMALIZABLE_LEADING_FLAG_VERBSlist — which covered the install family only. The info family was absent, so the verb token was never reordered and reached Node's file-runner as a--requiremodule / bad option.Fix
Replace the hand-curated list with
is_normalizable_leading_flag_verb, which recognizes any PM verb nub can dispatch — the same authority the bareword dispatch arm consults (SUBCOMMANDS+ the engine verb registrypm_engine::lookup_verb). Tying the reorder set to the dispatch set keeps them from drifting: every verb nub dispatches in first position is now also reorderable behind a leading workspace flag. Verbs that don't accept-r/--filterremain safe to reorder — clap/the engine rejects the unsupported flag downstream with a proper non-zero error, matching pnpm.Exit-code note
The conformance catalog flagged a secondary "exit 0 on crash" defect. Reproduced against the pre-fix code:
nub -r listreturned raw exit 1 andnub --filter a listreturned exit 9 (Node's bad-option code) — both non-zero. The reported "exit 0" was a measurement artifact of a| headpipe (PIPESTATUS of the tail). The file-runner faithfully propagates Node's exit code viaexit_code_from_status; no exit-code change was needed.Test
Adds
leading_global_flags_before_info_verb_reach_enginetocli_grammar_parity.rs. The existing grammar table can't guard this case (appending--helpshort-circuits before dispatch), so the new test dispatches for real and asserts the file-runner crash markers (Cannot find module,node: bad option) are absent.Verification
cargo fmt --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test -p nub-cli --test cli_grammar_parity— 8 passed (1 pre-existing ignore)cargo test -p nub-cli --test pm_verbsand the cli unit tests — greennub -r list/nub --filter a why anow reach the engine; file-run,--importpassthrough, andnub -r <file>(no-verb) paths unchanged.Refs the pnpm conformance work (D1).
https://claude.ai/code/session_01YRvztkcr4fzfg9rD5edUwa