From f15333c472dcddf1d36fe8c859cc8fbf945d2b64 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Thu, 11 Jun 2026 04:07:38 +0900 Subject: [PATCH] feat(pi-shell): add npx unknown-tool fallback def defs/npx.toml strips the first-run install prompt, npm warn/notice lines, and blanks for 'npx ' invocations of tools without a dedicated Rust filter, capping output. Routed tools are claimed first by dispatch, so the def only fires for unknown tools. Op: extend --- crates/pi-shell/src/minimizer/defs/npx.toml | 87 +++++++++++++++++++++ crates/pi-shell/src/minimizer/engine.rs | 65 +++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 crates/pi-shell/src/minimizer/defs/npx.toml diff --git a/crates/pi-shell/src/minimizer/defs/npx.toml b/crates/pi-shell/src/minimizer/defs/npx.toml new file mode 100644 index 000000000..fdf0fce16 --- /dev/null +++ b/crates/pi-shell/src/minimizer/defs/npx.toml @@ -0,0 +1,87 @@ +# Ported from snip/filters/npx.yaml (declarative DSL → local TOML schema). +# match_command adapted for the local (program, subcommand) dispatch model. +# +# This def is the catch-all for UNKNOWN `npx ` invocations only. It is NOT +# safe by construction on `match_command` alone: engine.rs reaches a pipeline +# along TWO paths, and a bare `^npx$` with no subcommand gate leaks on both. +# 1. Standalone (apply_identity -> resolve_pipeline): fires when +# filters::supports("npx", sub) is FALSE. `npx nx` lands here, and a bare +# npx def sorts before `nx-wrapped` (npx.toml < nx.toml in build.rs file +# order), so find() would return THIS def first and shadow nx-wrapped's +# NX-banner strip. +# 2. Overlay (apply_pipeline_overlay): fires AFTER the Rust filter when +# filters::supports("npx", sub) is TRUE. `npx eslint`/`npx jest`/... run +# their Rust lint/test filter, then resolve_pipeline re-selects this def +# and re-applies max_lines/strip on top, truncating diagnostics the Rust +# filter intentionally kept. +# match_subcommand below excludes every subcommand owned elsewhere so neither +# path leaks. The excluded set is the SOURCE-OF-TRUTH union of: +# - filters::supports("npx", _) == true (mod.rs npx arm: tsc, eslint, biome, +# jest, vitest, playwright; js_tools NPX_ROUTABLE_TOOLS: tsc, eslint, prisma, +# prettier, next), and +# - nx (claimed by nx-wrapped in nx.toml). +# The pattern is the lookahead-free anchored COMPLEMENT of that word set: the +# `regex` crate is RE2-style with no look-around, so a `(?!...)` negative cannot +# be used and a positive allowlist would match the wrong (excluded) tokens. It +# was machine-generated as the trie-complement and verified against the full +# known set plus prefix/suffix edge cases (e.g. `eslint-config`, `nextfoo`, +# `playwrigh` must still be claimed; `eslint`, `next`, `playwright` must not). +# If a tool is ever added to the npx routing in filters::supports, add it here +# too — the engine regression tests in engine.rs (npx_def_does_not_shadow_*/ +# npx_def_does_not_overlay_*/npx_def_fires_for_unknown_tool) fail loudly on +# desync. +# +# snip's `^Need to install` / `^Ok to proceed` patterns are widened here to the +# full literal npm install-prompt lines so the strip is unambiguous against +# default human output (the JSON-reporter tiers never apply to npx preambles). + +[filters.npx] +description = "Condensed npx output for unknown tools — strip install prompts, npm warn/notice noise, and blank lines" +match_command = "^npx$" +# Anchored complement of {nx, tsc, eslint, biome, jest, vitest, playwright, +# prisma, prettier, next}: matches any npx subcommand EXCEPT those routed/owned +# elsewhere (see header). Lookahead-free for the RE2-style `regex` crate. +match_subcommand = "^(?:|[^bejnptv].*|b(?:|[^i].*|i(?:|[^o].*|o(?:|[^m].*|m(?:|[^e].*|e(?:.+)))))|e(?:|[^s].*|s(?:|[^l].*|l(?:|[^i].*|i(?:|[^n].*|n(?:|[^t].*|t(?:.+))))))|j(?:|[^e].*|e(?:|[^s].*|s(?:|[^t].*|t(?:.+))))|n(?:|[^ex].*|e(?:|[^x].*|x(?:|[^t].*|t(?:.+)))|x(?:.+))|p(?:|[^lr].*|l(?:|[^a].*|a(?:|[^y].*|y(?:|[^w].*|w(?:|[^r].*|r(?:|[^i].*|i(?:|[^g].*|g(?:|[^h].*|h(?:|[^t].*|t(?:.+)))))))))|r(?:|[^ei].*|e(?:|[^t].*|t(?:|[^t].*|t(?:|[^i].*|i(?:|[^e].*|e(?:|[^r].*|r(?:.+))))))|i(?:|[^s].*|s(?:|[^m].*|m(?:|[^a].*|a(?:.+))))))|t(?:|[^s].*|s(?:|[^c].*|c(?:.+)))|v(?:|[^i].*|i(?:|[^t].*|t(?:|[^e].*|e(?:|[^s].*|s(?:|[^t].*|t(?:.+)))))))$" +strip_ansi = true +strip_lines_matching = [ + "^Need to install the following packages", + "^Ok to proceed\\? \\(y\\)", + "^npm warn", + "^npm notice", + "^\\s*$", +] +truncate_lines_at = 120 +max_lines = 50 + +[[tests.npx]] +name = "first-run strips install preamble + npm warn/notice, keeps tool output" +# Exercises every content-strip pattern: the install-prompt header, the +# `Ok to proceed?` line, npm warn/notice noise, and blank lines. snip's port +# does not strip the bare `name@version` package line, so it survives — that +# residue is faithful to the donor and documented here. +input = """ +Need to install the following packages: +cowsay@1.6.0 +Ok to proceed? (y) + +npm warn deprecated foo@1.0.0: use bar instead +npm notice New major version of npm available! + _______ +< Hello > + ------- + \\ ^__^ + \\ (oo)\\_______ + (__)\\ )\\/\\ + ||----w | + || || +""" +expected = """cowsay@1.6.0 + _______ +< Hello > + ------- + \\ ^__^ + \\ (oo)\\_______ + (__)\\ )\\/\\ + ||----w | + || || +""" diff --git a/crates/pi-shell/src/minimizer/engine.rs b/crates/pi-shell/src/minimizer/engine.rs index be2ce7f47..30ee497a0 100644 --- a/crates/pi-shell/src/minimizer/engine.rs +++ b/crates/pi-shell/src/minimizer/engine.rs @@ -595,6 +595,71 @@ only_on_exit = [0] assert_ne!(failed.filter, "pipeline+builtin"); assert!(failed.text.contains("file changed")); } + + // Regression guards for the builtin npx catch-all def (defs/npx.toml). Its + // `match_subcommand` must exclude every subcommand owned/routed elsewhere so + // it cannot shadow `nx-wrapped` (standalone path) or overlay the Rust + // lint/test filters (overlay path). These tests fail loudly if that + // exclusion set ever desyncs from filters::supports's npx routing. + #[test] + fn npx_def_does_not_shadow_nx_wrapped() { + // `npx nx` is NOT routed by filters::supports, so apply_identity falls to + // the standalone resolve_pipeline branch. nx-wrapped (nx.toml) must win + // and strip the NX banner; the bare npx def must not claim subcommand + // `nx`. + let cfg = MinimizerConfig { enabled: true, ..Default::default() }; + let input = "\n > NX Running target build for 3 projects\n\n> nx run app:build\nError: \ + build failed\n\n > NX Ran target build for 3 projects (2s)\n"; + let out = apply("npx nx build", input, 0, &cfg); + assert!( + !out.text.contains("NX Running target"), + "npx def shadowed nx-wrapped; NX banner survived: {:?}", + out.text + ); + assert!(out.text.contains("Error: build failed")); + } + #[test] + fn npx_def_does_not_overlay_routed_eslint() { + // `npx eslint` IS routed by filters::supports, so the Rust lint filter + // runs and apply_pipeline_overlay re-selects a matching pipeline. The npx + // def must NOT match subcommand `eslint`, so no overlay re-applies its + // max_lines/strip on top of the lint diagnostics. Output must equal the + // direct `eslint` invocation (same filter label, not "pipeline+builtin"). + let cfg = MinimizerConfig { enabled: true, ..Default::default() }; + let mut input = String::new(); + for i in 0..120 { + input.push_str(&format!("/src/file{i}.ts:{i}:1 error Something is wrong rule/name\n")); + } + let npx = apply("npx eslint src/", &input, 1, &cfg); + let direct = apply("eslint src/", &input, 1, &cfg); + // Label differs by program token (npx -> "builtin", eslint -> "eslint"), + // but it must NOT be the overlay label and the TEXT must be the un-mutated + // lint output — same lines, no max_lines truncation. + assert_ne!( + npx.filter, "pipeline+builtin", + "npx def overlaid the routed eslint filter output" + ); + assert_eq!( + npx.text, direct.text, + "npx eslint output diverged from direct eslint (overlay mutated it)" + ); + } + #[test] + fn npx_def_fires_for_unknown_tool() { + // The def must still strip the install preamble / npm warn/notice noise + // for genuinely UNKNOWN tools (its whole purpose). cowsay is not routed + // or owned by any other def, so the npx pipeline claims it. + let cfg = MinimizerConfig { enabled: true, ..Default::default() }; + let input = "Need to install the following packages:\ncowsay@1.6.0\nOk to proceed? \ + (y)\n\nnpm warn deprecated foo@1.0.0: use bar instead\n< Hello >\n"; + let out = apply("npx cowsay hello", input, 0, &cfg); + assert!(out.changed); + assert!(!out.text.contains("Need to install")); + assert!(!out.text.contains("Ok to proceed")); + assert!(!out.text.contains("npm warn")); + assert!(out.text.contains("< Hello >")); + } + #[test] fn enabled_known_filter_minimizes() { let cfg = MinimizerConfig { enabled: true, ..Default::default() };