Skip to content

fix(parser): derive the nullable-boolean spelling set (2.5.2) - #197

Merged
ViperJuice merged 4 commits into
mainfrom
fix/nullable-boolean-spellings
Aug 26, 2026
Merged

ViperJuice merged 4 commits into
mainfrom
fix/nullable-boolean-spellings

Conversation

@ViperJuice

@ViperJuice ViperJuice commented Aug 26, 2026 •

Copy link
Copy Markdown
Collaborator

The regression

#194 introduced _NPM_NULLABLE_BOOLEAN_FLAGS to decide whether a literal null after an npm boolean flag is the flag's value or the package name. null is a real published npm package, so getting it wrong mints a wrong package identity — and refresher.py's freshness gate treats a matching identity as positive confirmation that a cached tool description still describes the configured package. Two different servers can therefore be served each other's tool descriptions.

That table was written by hand, with 12 entries. npm has five nullable boolean definitions (yes, optional, production, workspaces, expect-results) but eighteen spellings for them: y, ws, n and no are shorthands — n and no both expand to --no-yes — and every short alias / shorthand key is legal in both its -x and --x form.

Six spellings were missing. Shipped in 2.5.1:

spelling 2.5.0 2.5.1 (shipped) 2.5.2 (this PR) nopt
-y zz zz zz zz
--yes zz zz zz zz
--y zz null zz zz
-ws zz null zz zz
-n zz null zz zz
--n zz null zz zz
-no zz null zz zz
--no zz null zz zz

(input npm exec <flag> null zz). 2.5.0 got all six right, so this is strictly a regression from #194.

The fix: derive it

Four consecutive defects in this parser trace back to hand-transcribing npm's behaviour, so the set is generated, not pasted. .consiliency/notes/derive_npm_flags.py now emits a fourth table using this rule:

a spelling in _NPM_BOOLEAN_FLAGS is nullable iff its resolution target — after shorthand expansion and after stripping a leading no- — is a definition whose raw declared type includes null.

The "raw" matters: classify() drops null from the type set (that drop is what makes --yes a boolean at all rather than conditional), so nullability is read before the drop.

The generated set, exactly:

--expect-results --n --no --no-expect-results --no-optional --no-production
--no-workspaces --no-yes --optional --production --workspaces --ws --y --yes
-n -no -ws -y

Diff against the committed tables: _NPM_NULLABLE_BOOLEAN_FLAGS +6 / −0; _NPM_VALUE_FLAGS (118), _NPM_BOOLEAN_FLAGS (198) and _NPM_SKIP_FLAGS (16) are byte-identical to what is already committed.

--verify now covers it

Before this PR --verify checked three tables and said nothing about the nullable one — zero drift protection. Added:

  1. A null probe per definition, with the prediction "a BOOLEAN consumes null iff its declared type carries null".
  2. A spelling-level probe: node runs npm's own parser over all 442 enumerable spellings as npm exec <flag> null zz. This is the one that matters — the defect was a spelling omission with a correct definition set, so a definition-level probe alone would have missed it.
  3. _NPM_NULLABLE_BOOLEAN_FLAGS ⊆ _NPM_BOOLEAN_FLAGS (it is exempt from the pairwise-disjointness check, being a legitimate subset, so it gets a containment check in its place).
  4. record() raises on conflicting nullability, not just conflicting class. -y/--y are written twice (from yes's short and from the y shorthand); they agree today, which is not a reason to let a future npm disagree silently.

Negative control — the new spelling probe run against the shipped 2.5.1 set returns exactly the six regressed spellings:

negative control (2.5.1 set, 12 entries): 6 mismatch(es)
   npm exec --n null zz: derived nullable=False, nopt consumes null=True
   npm exec --no null zz: derived nullable=False, nopt consumes null=True
   npm exec --y null zz: derived nullable=False, nopt consumes null=True
   npm exec -n null zz: derived nullable=False, nopt consumes null=True
   npm exec -no null zz: derived nullable=False, nopt consumes null=True
   npm exec -ws null zz: derived nullable=False, nopt consumes null=True
positive control (derived set): 0 mismatch(es)

Verification

nopt cross-check, every boolean spelling (ad-hoc script, version_checker.__file__ printed to prove it exercised the worktree copy):

198 boolean spellings compared against nopt
  agreements: 198
  disagreements: 0
  nopt says 18 of them consume the null: (identical to pmcp's table)

uv run python .consiliency/notes/derive_npm_flags.py --verify:

npm 11.19.0: 181 definitions, 40 shorthands
CONDITIONAL ARITY -> left unlisted, scanner refuses (2): browser, color
VALUE flags carrying a Boolean member (0):
UNINTERPRETABLE type (0):
nopt cross-check: 0 mismatch(es) over 181 definitions
nullable spellings: 18 derived, 0 mismatch(es) over 442 spellings probed with a literal `null`
OK: 118 value + 198 boolean (18 of them nullable) + 16 skip entries match live npm 11.19.0

Full suite: uv run pytest -q → 2835 passed, 3 skipped, 25 deselected in 5m33s, exit 0.

Reported honestly: the first full run came back 1 failed, 2834 passed, 3 skipped, 1 error, and I do not want that laundered out of the record. Two distinct causes, neither in the diff:

  • ERROR …::test_true_and_false_are_consumed_by_any_boolean — self-inflicted. I ran uv lock, ruff, mypy and a targeted pytest while that suite was in flight, and each uv run re-syncs .venv, swapping the installed pmcp from 2.5.1 to 2.5.2 underneath the running interpreter. tests/test_version_checker.py passes 247/247 in isolation.
  • FAILED tests/runtime/test_subscriptions_e2e.py::test_connect_disconnect_refresh_each_deliver_all_three_kinds — shared host state. That harness asserts the live gateway on :3344 gains no child process across the test window; a gateway is running on this host and picked up one new child (3679740) from unrelated activity. uv does not spawn children of that process, so this is not the resync — it is the shared gateway.

The re-run above ran with nothing else touching the venv and is green.

Named regression classes still green (62 passed): TestNpmValueFlagCollisions (#180), TestNpmFailsClosed (#180), TestNpmPinnedOrderingSurvives, TestNpmBakedValueShorthandDoesNotConsume (#194), TestNpxBooleanLiterals (#194 npx-literal refusal), TestOnlyNullableBooleansConsumeNull.

Manifest: all 98 manifest servers that declare a launch command still resolve — 79 npm + 19 pypi, 0 unknown. (manifest.yaml has 107 entries: 3 declare no command key at all, and 6 remote servers declare command: "".)

ruff check src/ tests/, ruff format --check src/ tests/, mypy src/ — all clean. (ruff format --check . reports 4 files, all pre-existing on main and untouched here: .consiliency/notes/mutate.py, .consiliency/notes/verify_tables.py, diagnostics/issue-79-1b/*.py. CI scopes to src/ tests/.)

All Python runs used PYTHONDONTWRITEBYTECODE=1 with src/**/__pycache__ purged first.

Not verified here: CI itself, and the install-smoke path — neither was run locally.

Tests

TestOnlyNullableBooleansConsumeNull gains a parametrised pin over all 18 spellings, a counter-pin that -g --global -f --force --no-global leave null as the package, the identity consequence stated directly (-n null server-a and -n null server-b stay distinct), and a subset check that holds on a host without npm.

Also moved test_baked_value_shorthand_still_skips_an_ordinary_token back to TestNpmBakedValueShorthandDoesNotConsume — it had been absorbed into the nullable class but tests baked-value shorthands.

Scope

This does not close #195. Attached values (--global=pkg), npx's own -p= rewriting, and npx's -n removal remain unhandled; #195 stays open.

Fixes a regression from #194. Relates to #195. Version bumped to 2.5.2 (pyproject.toml, src/pmcp/__init__.py, uv.lock) with a CHANGELOG entry. No tag created or pushed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3

ViperJuice and others added 4 commits August 26, 2026 22:11
2.5.1 hand-wrote _NPM_NULLABLE_BOOLEAN_FLAGS with 12 entries. npm's five
nullable boolean definitions have 18 spellings -- `y`, `ws`, `n` and `no`
are shorthands (`n` and `no` both expand to `--no-yes`) and each is emitted
in both `-x` and `--x` form -- so `--y -ws -n --n -no --no` each read a
literal `null` as the PACKAGE rather than as the flag's value. `null` is a
real published npm package, so that mints a wrong identity, which
refresher.py's freshness gate then treats as positive confirmation.

derive_npm_flags.py now derives the table: a spelling is nullable iff its
resolution target, after shorthand expansion and after stripping a leading
`no-`, is a definition whose declared type includes `null`. `--verify` gains
a `null` probe per definition AND a spelling-level probe over all 442
enumerable spellings, plus a nullable-subset-of-boolean check. Confirmed the
new probe rejects the shipped 2.5.1 set with exactly 6 mismatches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3
…alue test

Parametrised pins for every spelling that consumes a literal `null` and for
a sample that must NOT, plus the identity consequence (`-n null server-a`
and `-n null server-b` stayed distinct) and a nullable-subset-of-boolean
check that holds without npm present.

test_baked_value_shorthand_still_skips_an_ordinary_token was absorbed into
TestOnlyNullableBooleansConsumeNull; it tests baked-value shorthands, so it
goes back to TestNpmBakedValueShorthandDoesNotConsume.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3
Patch hotfix for the six npm flag spellings 2.5.1 regressed. Notes in
CHANGELOG.md that the wider parser gap in #195 stays open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3
… exact set

Board review, adversarial seat, on the 2.5.2 diff.

The CHANGELOG claimed 2.5.1 regressed six spellings that 2.5.0 handled
correctly. Verified false by running each released version's own
detect_package_type: v2.4.1, v2.5.0 and v2.5.1 all resolve
'npm exec -n null zz' to the package 'null'. The blanket rule that handled
them correctly lived on main between two unreleased commits only, so no
shipped version was ever right. 2.5.2 is the first.

Also pins the nullable table by VALUE. Every other check was one-directional
-- the parametrized spellings prove each of the 18 consumes null, the subset
check proves none is unreachable, and an EXTRA entry passed both. An extra
entry is a wrong identity in the other direction. Verified the new pin goes red
when --global is injected into the table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T61zMNiPF4K29186r3uqc3
@ViperJuice
ViperJuice merged commit 0edaaf4 into main Aug 26, 2026
11 checks passed
@ViperJuice
ViperJuice deleted the fix/nullable-boolean-spellings branch August 26, 2026 23:01
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.

npm identity: the same collision class survives in rarer flag spellings

1 participant