Repository navigation
fix(parser): scope null to nullable booleans; refuse npx boolean literals - #194
Merged
Merged
Conversation
…erals Correctness seat reported after #192 merged -- I had not frozen the branch, so these two defects are live on main. Both confirmed against nopt AND the real npm binary before acting. BLANKET null MINTED WRONG IDENTITIES ACROSS ~193 BOOLEAN ENTRIES. #192 added "null" to the boolean-literal set for the whole table. nopt consumes null after only the 5 nullable booleans (null|Boolean): expect-results, optional, production, workspaces, yes. For every other boolean, null is the POSITIONAL -- and `null` is a real published npm package. Verified: nopt: npm exec --global null zz -> remain ["null","zz"] package is null nopt: npm exec --yes null zz -> remain ["zz"] package is zz main: both returned zz A fourth set, _NPM_NULLABLE_BOOLEAN_FLAGS, is derived from the same null|Boolean test the generator already performs -- 12 spellings including --no- negations and the -y / --ws shorthands. NPX INVERTS LITERAL CONSUMPTION, AND #192 REGRESSED IT. bin/npx-cli.js pre-scans argv and inserts `--` before the first positional, so a plain-name boolean switch never consumes a literal under npx. Confirmed against the real binary, not just the parser: npx --offline --global true zz-not-a-real-pkg -> fetches registry.npmjs.org/true The package is `true`. #192 returned the trailing token, and its own test (test_real_boolean_still_consumes_the_literal, command="npx") PINNED that wrong behaviour. That test now uses npm, with the npx case covered separately. npx now REFUSES on a literal rather than modelling the rule, because the --no- family behaves differently again -- the pre-scan does not recognise no-X as a switch, so nopt still consumes there. A refusal cannot mint a wrong identity, and a config carrying a bare true/false/null after a boolean is vanishingly rare. MY FIRST CUT OF THAT BROKE `npx -y pkg` -- the most common MCP form -- by consuming a token in the non-literal path. Caught by re-running the regression set rather than by the new tests, which passed. The npx branch now consumes nothing. All 8 regression forms verified intact; all 5 of #180's original collisions still closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
The changelog CI check was FAILING on this PR and I had reported it green without reading the current status -- caught by the red-team seat, which is a verification failure of mine, not a finding about the code. Also records the known remaining gap the board found: the same class survives in rarer spellings (attached --global=pkg, nopt abbreviations -n/--y, npx's -p= rewriting and -n removal). Filed separately rather than iterated on here; no bundled manifest server uses any of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Merged
ViperJuice
added a commit
that referenced
this pull request
Aug 26, 2026
Ships #180: npm no longer reads a flag's value as the package name. npm was the last of the five ecosystems still failing OPEN on an unrecognised flag -- the scan skipped anything starting with "-" and took the next bare token, so `npm exec --loglevel silly server-a` and `... server-b` both resolved to `silly`. 2.4.0's identity gate reads a matching name as a POSITIVE confirmation, so two unrelated servers collapsed into one identity and one was served the other's cached tool descriptions. Flag arity is now generated from npm's own config schema (@npmcli/config's definitions and shorthands) rather than transcribed by hand, and an unlisted flag reports no identity instead of guessing. Also folds in #194, which repaired two defects the first cut introduced and which never shipped: a blanket "null means unset" rule that made `null` -- a real published npm package -- collapse distinct servers, and npx's inverted boolean handling (npx inserts `--` before the first positional, so a boolean switch consumes nothing there; `npx --global true pkg` runs the package `true`). Patch rather than minor: no bundled manifest server changes identity, all 98 launchable entries resolve exactly as before, and the only behaviour change a user can hit is a refusal to auto-update a server launched with a flag npm's own schema does not describe. Known remaining gap filed as #195: the same class survives in rarer spellings (attached values, nopt abbreviations, npx's own argv rewriting). No bundled server uses any of them. Claude-Session: https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 26, 2026
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.
Fixes two live defects on
main, both introduced by #192. The correctness seat reported them after that PR merged — I had not frozen the branch, which is my process error, not the seat's.Both confirmed against nopt and the real npm binary before acting.
1. A blanket
nullrule minted wrong identities across ~193 boolean entries#192 added
"null"to the boolean-literal set for the whole table. nopt consumesnullafter only the 5 nullable booleans (null|Boolean):expect-results,optional,production,workspaces,yes. For every other boolean,nullis the positional — andnullis a real published npm package.A fourth set,
_NPM_NULLABLE_BOOLEAN_FLAGS, is derived from the samenull|Booleantest the generator already performs — 12 spellings including--no-negations and the-y/--wsshorthands.Worth naming: my plan-round correction — "drop
null, it means unset" — was right about classification and I wrongly generalised it to parsing. The same word means different things in the two places.2. npx inverts literal consumption, and #192 regressed it
bin/npx-cli.jspre-scans argv and inserts--before the first positional, so a plain-name boolean switch never consumes a literal under npx. Confirmed against the real binary, not just the parser:The package is
true. #192 returned the trailing token — and its own test pinned that wrong behaviour (test_real_boolean_still_consumes_the_literal, withcommand="npx"). That test now usesnpm; the npx case is covered separately.npx now refuses on a literal rather than modelling the rule, because the
--no-family behaves differently again — the pre-scan doesn't recogniseno-Xas a switch, so nopt still consumes there. A refusal cannot mint a wrong identity, and a config carrying a baretrue/false/nullafter a boolean is vanishingly rare.My first cut broke
npx -y pkgThe most common MCP launch form. I consumed a token in the non-literal path. The new tests passed — only re-running the full regression set caught it. That is the fourth time this session a fix for a board finding introduced its own defect, and precisely why the regression set is maintained separately from the tests.
Verification
All 8 regression forms intact —
npm --silent exec pkg,npm exec pkg,npm install i,npm exec exec,npx -y exec,npx -y pkg,npm exec -- pkg,npm exec --global false a.All 5 of #180's original collisions still closed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.