Skip to content

fix(rewrite): route npm lint scripts through rtk npm (biome-safe)#1491

Open
LukaPrebil wants to merge 1 commit intortk-ai:developfrom
LukaPrebil:fix/1489-npm-run-lint-biome
Open

fix(rewrite): route npm lint scripts through rtk npm (biome-safe)#1491
LukaPrebil wants to merge 1 commit intortk-ai:developfrom
LukaPrebil:fix/1489-npm-run-lint-biome

Conversation

@LukaPrebil
Copy link
Copy Markdown

Summary

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk <command> output inspected

Important: All PRs must target the develop branch (not master).
See CONTRIBUTING.md for details.

`npm run lint`, `npm lint`, `npm [rum|urn|x] lint`, and `npx lint`
previously rewrote to `rtk lint`, which defaults to running ESLint via
`npx eslint`. On Biome projects the JSON parse fails silently with exit
0, hiding real lint failures from agents.

Route these through `rtk npm` / `rtk npx` instead - the generic npm
filter is script-agnostic and preserves biome's real output and exit
code. Mirrors rtk-ai#678's pattern for the pnpm side.

Scope:
- new early-return block in rewrite_segment_inner handles the npm/npx
  `lint` variants with env prefix and redirect suffix preserved
- `split_npm_dispatch` + `match_lint_script` helpers keep the block
  compact
- npm rule pattern extended with `|lint` + `(\s|$)` suffix so bare
  `npm lint` still classifies as `rtk npm`
- `npm lint` / `npm run lint` / `npm run-script lint` / `npm rum|urn lint`
  / `npx lint` removed from lint rule's rewrite_prefixes
- 12 new unit tests (incl. env-prefix, redirect, word-boundary
  regression against `npm lint-staged`, and asserts that bare biome /
  eslint still route to rtk lint)
- `test_classify_lint` intentionally unchanged: classify still picks
  the lint rule (last-match wins), the rewrite now short-circuits to
  rtk npm/npx. Documented classify/rewrite drift.

Out of scope (filed separately):
- biome check routing through the biome TOML filter
- rtk lint exit-0 on auto-defaulted ESLint JSON parse failure
- package.json-aware filter dispatch for npm run scripts
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 24, 2026

CLA assistant check
All committers have signed the CLA.

@LukaPrebil
Copy link
Copy Markdown
Author

Smoke test (buggy 0.37.2 vs this PR)

Scratch package.json with "lint": "<mock>" that emits diagnostics and exits 1:

Scenario Installed rtk 0.37.2 This PR
Biome-shaped diagnostics, exit 1 ESLint output (JSON parse failed: EOF ...), exit 0 (silent) Real biome diagnostics, exit 1 (correct)
ESLint-shaped diagnostics, exit 1 n/a (same class of parse failure) Real ESLint diagnostics, exit 1 (correct)
Clean pass with message, exit 0 unchanged "All files pass linting." preserved, exit 0
Silent success, exit 0 no output unchanged No false warning, exit 0

And rewrite coverage (rtk rewrite "<cmd>"):

Command Before After
npm run lint rtk lint rtk npm run lint
npm run lint --fix rtk lint rtk npm run lint --fix
npm run-script lint rtk lint rtk npm run-script lint
npm lint rtk lint rtk npm lint
npm rum/urn/x lint rtk lint (npm x lint was not rewriteable) rtk npm <alias> lint
npx lint rtk lint rtk npx lint
FOO=1 npm run lint FOO=1 rtk lint FOO=1 rtk npm run lint
npm run lint 2>&1 n/a rtk npm run lint 2>&1
npm lint-staged (regression) not rewritten not rewritten (word-boundary protected)
biome check (out of scope) rtk lint check unchanged
eslint src/ (out of scope) rtk lint src/ unchanged
pnpm lint (#678's scope) rtk lint unchanged

Accepted tradeoff: classify/rewrite drift

classify_command("npm run lint") still returns rtk lint because the lint rule's regex pattern was intentionally left alone (last-match wins, and the lint rule comes after the npm rule in RULES[]). Only the rewrite short-circuits via the new early-return. This affects rtk discover analytics output, not any real command execution. Tightening classify would require removing the p?np(m|x) alternation from the lint rule's pattern, which widens scope to the biome / eslint bare-PM variants too — left for a follow-up if maintainers want classify/rewrite parity.

The same drift already exists for pnpm lint after PR #678 ships, so the behaviour is consistent across the two sides.

Token-savings tradeoff

For projects that really do use ESLint, npm run lint previously ran through rtk lint which added -f json and applied the structured filter_eslint_json (high compression). After this PR, rtk npm run lint runs the user's literal script (no -f json injection) and applies only the generic filter_npm_output (boilerplate strip). That is a savings regression for pure-ESLint projects in exchange for correctness on Biome projects. Aligns with the "Correctness VS Token Savings" principle in CONTRIBUTING.md, and is recoverable via package.json-aware filter dispatch (see follow-ups).

Follow-up issues (will file separately)

  1. rtk lint silently exits 0 when auto-defaulted ESLint produces non-JSON output — adapter-level hardening. The reproducer from bug(rewrite): npm run lint forced into rtk lint on Biome projects #1489 (rtk lint ./no-op.sh → "ESLint output (JSON parse failed: EOF ...)" with exit 0) still reproduces after this PR, because this PR only fixes the rewrite path.
  2. biome check currently routes rtk lint biome checkfilter_generic_lint. It should route through the existing biome TOML filter added in Support Biome for lint command #316, either by a new rewrite rule direct to rtk biome or by teaching rtk lint to consult the TOML registry when it detects biome as the explicit linter.
  3. rtk npm run <script>: extend script-body detection in package.json to eslint/prettier/tsc. My next PR (stacked on this one) seeds the detector + wires the biome filter only; the other tools are additive follow-ups.

@pszymkowiak pszymkowiak added bug Something isn't working effort-medium 1-2 jours, quelques fichiers filter-quality Filter produces incorrect/truncated signal labels Apr 24, 2026
@pszymkowiak
Copy link
Copy Markdown
Collaborator

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🟡 Risk medium

Summary

Fixes a bug where npm [run|run-script|rum|urn|x] lint, npm lint, and npx lint commands were incorrectly routed through rtk lint (the ESLint JSON adapter), which silently swallowed parse failures on Biome projects. Now these commands route through rtk npm / rtk npx instead, with an early-return in rewrite_segment_inner and updated rule patterns.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Linked issues: #1489


Analyzed automatically by wshm · This is an automated analysis, not a human review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort-medium 1-2 jours, quelques fichiers filter-quality Filter produces incorrect/truncated signal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants