Repository navigation
fix(parser): classify flag values so a flag's argument is never the package name (#182) - #190
Conversation
Closes-plan-for: #182 Planning artifact only. Under board review at time of commit. All seven collisions reproduced on main @ 75eb9c7 before planning: uvx --python/--with, pip --index-url, cargo --features, docker --env-file/--mount, npm exec --package=. In each case the first non-flag token is a FLAG'S VALUE, not the package, so two different servers confirm as one identity under the 2.4.0 gate. The design decision is the one #183 already settled in this same function. Docker enumerates value-carrying flags in _value_flags and that enumeration is ALREADY incomplete -- --env-file and --mount are missing, which is two of the seven. uvx/pip/cargo do not enumerate at all. Extending the table would fix the two flags the issue names and leave the class open, which is the fix-the-instance error the last three rounds caught. So: --flag=value is self-delimiting and safe; a bare --flag before the candidate token is ambiguous without a per-tool table, and that table is the thing that keeps being incomplete. Ambiguous -> ("unknown", None), which the identity gate reads as cannot-confirm -> refresh and update_server refuses on before building a probe. The cost is stated rather than buried: a server launched as `uvx --python 3.12 pkg` becomes unverifiable and can no longer be auto-updated. Accepted because the alternative is what ships today -- silent wrong descriptions with no signal. Flagged for resolution BEFORE implementing: README documents pinning as args ["--python","3.12","--from",...], so the plan must reconcile that form rather than discover it broken. And #180's status must be settled explicitly at the end -- it was narrowed once already. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Resolved the README question the plan flagged, before implementing rather
than by discovery, and the answer changes the design.
README:1133-1137 documents a first-party pin form:
uvx --python 3.12 --from index-it-mcp==1.2.0 index-it-mcp
Measured on main: that resolves to ('pypi', '3.12') -- the PYTHON VERSION as
the package name. #182 is therefore not hypothetical for uvx; it already
mis-identifies a config this repo recommends in its own README.
Two design consequences. --from must be honoured wherever it appears in argv,
not only as the leading flag: a naive left-to-right ambiguity rule hits the
bare --python first and returns unknown, BREAKING the documented form instead
of fixing it. With a global --from scan the form resolves to index-it-mcp and
no doc change is needed -- the fix repairs it.
Acceptance criterion updated from "reconcile the README" to the concrete RED
assertion: that exact argv must yield index-it-mcp, where today it yields
3.12.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…review
All three CLI seats returned DISAGREE. Every finding verified before acting.
MY ACCEPTANCE CRITERIA WERE LOGICALLY IMPOSSIBLE. The design makes six
ambiguous pairs return ("unknown", None), and the criteria simultaneously
demanded all seven pairs compare UNEQUAL. Two unknowns are equal, so no
implementation could ever satisfy it -- verified. The real safety property is
that unknown never CONFIRMS identity, which _same_package already enforces at
refresher.py:213. Criteria now assert the exact unknown tuple for the six
refusals, and exact-name-plus-inequality only for the one genuinely
recoverable case.
THE CHANGE LIST OMITTED THE NPM SCANNER. It covered uvx, pip, cargo and
docker; _npm_package_arg was never mentioned, so the seventh collision would
have shipped unfixed. Verified: it skips --package=old and -- as flags and
returns "bin". Treating --package=old as self-delimiting is not enough -- the
scanner must read the value OUT of it. This is the only one of the seven where
identity is recoverable rather than refused.
THE GLOBAL --from SCAN WAS UNBOUNDED, AND HAS A PIN HAZARD. uv passes
everything after the tool name to the tool, so an unbounded scan can mistake a
SERVER'S OWN later --from for uvx's package identity; the scan must stop at the
tool-name/-- boundary. Separately and worse: _detect_effective_version_pin
skips every "-"-prefixed token, so --from=pkg==1.2.0 reports NO PIN while the
spaced form reports 1.2.0 -- verified both ways. If identity starts honouring
the = spelling without pin detection learning it, update_server classifies the
package, sees no pin, and probes for LATEST, updating a server the operator
explicitly pinned. Identity and pin must share one boundary-aware scan, the
way _npm_package_arg already is.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…sification Correctness seat found a regression the other three seats missed entirely, and it is decisive. Verified before acting. MY RULE WOULD HAVE BROKEN THE CANONICAL DOCKER CONFIG. "Any bare flag before the candidate yields unknown" turns all of these -- correct today -- into unknown: uvx --quiet my-package --arg -> my-package docker run -i --rm mcp/server:latest -> mcp/server docker run -e KEY=val --rm ghcr.io/org/mcp -> ghcr.io/org/mcp docker run -it --rm img -> img `docker run -i --rm <image>` is THE canonical docker MCP shape -- this repo's own README uses `docker run -it --rm` at :1528, and -it is a combined short boolean no value-table matches. Essentially every real docker server would have become permanently unverifiable and un-auto-updatable, and three currently-green tests would have turned red, contradicting this plan's own "suite stays green" criterion. The regression dwarfs the collisions it closes. Replaced with per-ecosystem three-way classification: known-value flags consume the next token, known-boolean flags skip, known-positive flags yield the package, and any UNLISTED bare flag defaults to unknown. This is not the denylist #183 rejected -- docker's table failed open because the default for an unlisted flag was "take the next token as the image"; inverting the default means an omission costs auto-update for one odd config instead of a silent collision. Same table, opposite failure direction. Honest residual recorded: a WRONG entry still fails open, so entries must be verified per-tool and pinned by tests, never bulk-imported. It also dissolves the --from hazard. With --python classified as known-value, a plain left-to-right scan resolves the README form deterministically and no whole-argv scan is needed -- removing the fail-open misidentification the board found, where a global scan turns `uvx mypkg --from other` from mypkg into other and re-collides through the new path. Acceptance criteria reconciled: the class property is now stated satisfiably (unequal OR unknown, plus exact expected values), the no-probe test requires all four of ok/package_type/message/probes per the #183 model rather than a bare probes==[] that passes on any unrelated refusal, and the four canonical forms above are now explicit criteria. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…sign
The previous commit rewrote the design section but its criteria edits failed
mid-script on a text mismatch, so the criteria still described the REJECTED
fail-closed rule while the design described the replacement. Fixed here; the
plan is now internally consistent.
Criteria now:
- State the class property satisfiably: for every pair, d(a) != d(b) OR
d(a) == ("unknown", None), plus each pair's exact expected value. The old
form demanded pairs be simultaneously unknown AND unequal, which no
implementation can satisfy.
- Require _npm_package_arg to read the value OUT of --package=<pkg>. Treating
it as merely self-delimiting still returns "bin" -- verified.
- Pin the four canonical forms the rejected design would have broken, including
`docker run -it --rm img`, the shape this repo's own README uses at :1528.
- Pin `uvx mypkg --from other` -> mypkg, the fail-open misidentification a
whole-argv --from scan would have introduced.
- Require the no-probe test to assert all four of ok/package_type/message/
probes. A bare probes==[] passes when update_server refuses for any unrelated
earlier reason, so it does not pin what it claims.
- Force an explicit decision on --from value normalization, since the manifest
carries `--from browser-use[cli]` and extras handling changes that identity.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…defect Maintainer decision on the two questions the board required be settled before implementation. PEP 508 base name: browser-use[cli] -> browser-use, index-it-mcp==1.2.0 -> index-it-mcp. This is not cosmetic. manifest.yaml:352-357 ships `--from browser-use[cli]`, and measured against PyPI, browser-use[cli] returns None while browser-use returns 0.13.8 -- a first-party manifest entry whose version checks are SILENTLY FAILING today. Stripping extras repairs it. Cost is one refresh for that server's cached identity, the same one-time migration 2.4.0 and 2.4.1 made. git+https://... values keep the URL as identity rather than returning unknown. Distinct URLs are distinct identities so the gate stays correct, and the PyPI lookup fails closed to None, which the gate already reads as cannot-confirm -> refresh. No new code path. Also replaced the last stale "add the same ambiguity rule" text in the docker change item -- that described the REJECTED design. Docker now reads: add --env-file and --mount to _value_flags, classify -i -t -it -d --rm --init as known-boolean, and default unlisted bare flags to unknown. The remaining "ambiguity rule" mention is inside the rejected-design explanation, where it belongs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
`detect_package_type` skipped flags but not the tokens those flags CARRY, so the first "non-flag" argument was routinely a flag's value. Two different servers then produced one identity, and 2.4.0's identity gate reads an equal name as a POSITIVE confirmation -- serving the wrong package's cached tool descriptions indefinitely. Seven forms collided on main @ 75eb9c7, each returning a flag's value: uvx --python 3.12 pkg-a / pkg-b -> both ('pypi', '3.12') uvx --with requests srv-a / srv-b -> both ('pypi', 'requests') pip install --index-url URL a / b -> both ('pypi', 'https://x') cargo run --features full srv-a / srv-b -> both ('cargo', 'full') docker run --env-file .env img-a / img-b -> both ('docker','.env') docker run --mount type=bind,... a / b -> both ('docker','type=bind,...') npm exec --package=old/new -- bin -> both ('npm', 'bin') Each flag is now classified per ecosystem as value-taking, boolean, or POSITIVE (`uvx --from`, `cargo -p`, `npm --package` -- whose value IS the package), and anything unlisted returns ("unknown", None). This is not the denylist #183 rejected. Docker's `_value_flags` table had the same shape and failed OPEN: an unlisted flag fell through to "take the next token as the image", which is how `--env-file` and `--mount` became two of these seven. Inverting the default means an omission costs auto-update for one odd config -- safe, loud, fixable by adding an entry -- instead of a silent collision. The residual is that a WRONG entry still fails open, so the tables were transcribed from each tool's own `--help` and verified entry by entry; memory got docker `--pull`, uv's repeatable `--quiet`/`--verbose` and pip's `--platform` wrong in drafting. The scan is plain left-to-right and stops at `--`. A whole-argv `--from` hunt was rejected: it turns `uvx mypkg --from other` from `mypkg` into `other`, a fail-open misidentification introduced by the fix. With `--python` classified as value-taking, the README's documented pin form resolves without one. Also repaired, all previously silent: * `--from` values are PEP 508 requirements. `browser-use[cli]` -> the manifest ships this, and PyPI returns None for the literal string, so that entry's version checks were failing. A `git+https://` value keeps the whole URL as identity. * uvx pin detection shares `_uvx_package_arg` with identity instead of scanning independently. `--from=pkg==1.2.0` reported NO pin (update_server would have moved a pinned server to latest); `--with requests==2.0 pkg` reported a dependency's version as the server's pin. * The README form `uvx --python 3.12 --from index-it-mcp==1.2.0 index-it-mcp` identified the package as `3.12`. Known residual, deliberately left open: `_npm_package_arg` does NOT fail closed on an unrecognised flag, so `npm exec --loglevel silly <pkg>` still reads `silly`. Inverting it there would break the pinned ordering where a leading global flag (`npm --silent exec <pkg>`) must be skipped, not refused. Closes #182 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbUnAuFarzK5kRz92t8MW8
Mutation testing each new test against its OWN branch's rule (never against the class) surfaced five surviving mutants. Three were real weaknesses and are fixed; two were tests claiming more than they pin and now say so. Fixed: * `_takes_a_value_ambiguously`'s `"=" not in arg` clause was DEAD. The scanner decided the self-delimiting case itself, before ever consulting the predicate, so removing the clause changed nothing -- the rule had two homes and only one was live. The scanner now asks the predicate, which is what "one home for the rule" was supposed to mean. * Repeated `--package` had no test. npm allows the flag more than once, and returning the first of several DIFFERENT packages is exactly the guess this change exists to stop. * The `uvx` positional-normalization mutation never applied -- its anchor did not match the formatted source, so it had been silently reported as unproven rather than passing. Re-anchored; it now kills. Disclosed rather than papered over: * The explicit `--` terminator is REDUNDANT, not load-bearing: `--` also trips the fail-closed default, so deleting the branch changes no observable result and no test can distinguish the two. Kept for documentation, and both the code and the test docstring now say this instead of implying a proof that does not exist. * `npm exec --package <pkg>` (spaced) does not discriminate the fix -- skipping `--package` as an ordinary flag leaves its value as the first bare token anyway. Only the `=` spelling and the repeated-flag refusal actually pin the extraction. The test is kept as a regression guard and relabelled as one. * The README pin form pins `--python`'s classification, not `--from`'s: that form ends with a positional equal to the `--from` base name, so ignoring `--from` still yields the right answer. All 20 mutants now killed, each by its own single node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbUnAuFarzK5kRz92t8MW8
…verified The source comment and CHANGELOG claimed the flag tables were verified against each tool's own `--help`. That was only true of the entries taken from the help dumps; a handful had been hand-added from memory, which is precisely the practice the plan forbids, because a boolean entry that actually takes a value fails OPEN -- its value becomes the package name. Checked all 296 entries mechanically, one at a time, against live help (`.consiliency/notes/verify_tables.py`). Findings: * TEN entries were absent from their tool's help and could not be verified: uvx `--python-preference`, `--native-tls`, `--version`, `-V`; pip `--python`, `--report`, `--dry-run`, `--break-system-packages`, `--check-build-dependencies`; docker `--disable-content-trust`. All REMOVED rather than kept. Removal is fail-closed, so it costs auto-update for an unusual config and never correctness. pip's `--break-system-packages` and `--dry-run` are real modern-pip booleans simply absent from the pip 22.0.2 available here; re-add them against a newer pip rather than from memory. * THREE entries are exempt with a stated reason, not silently: `-it`/`-ti` are combined short booleans docker only documents separately, and npm prints a bare `--package` in its option list with the metavar only in its usage block. * The verifier's own first two "misclassified" reports were ITS bugs, not the table's: pip's `--extra-index-url` description wraps onto a line reading just `--index-url.`, which parsed as a definition with no metavar and clobbered the real entry; and `...` (uv's repeatable-counter marker) is not sentence punctuation. Both fixed, and both are recorded because each cost a real investigation. `_docker_image_arg`'s value table is promoted to the module-level `_DOCKER_VALUE_FLAGS` alongside every other table. It was a function local, so it was rebuilt on every call and could not be inspected by the verifier at all. Working scripts archived under `.consiliency/notes/` rather than added to the suite: help text is version-specific and CI has none of these CLIs installed. Behaviour stays pinned by the pure unit tests. Verification: 2764 passed / 3 skipped (baseline 2737/3), ruff check, ruff format --check, mypy all clean, and all 20 mutants still killed per node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbUnAuFarzK5kRz92t8MW8
…pointer
Correctness seat found the 21st mutant -- two, in fact -- and both were real.
Verified end-to-end with an import-provenance guard before acting.
M2, version_checker.py:585. Deleting the `following.startswith("-")` arm left
all 364 tests green, after which `uvx --from --offline pkg-a` and `... pkg-b`
BOTH resolved to ('pypi','--offline'). Confirmed the collision survives the
whole pipeline: _same_package('--offline','pypi','--offline','pypi') returns
True, so it passes the identity gate. Not reachable from a launchable config
-- real uv rejects the form -- but an unpinned guard is the shape this module
keeps being corrected for, so it is pinned rather than argued away.
M1, :570. Deleting `or None` from the attached-value return left `--from= a`
and `--from= b` both yielding ('pypi',''). Blunted downstream by
_same_package's falsy guard, so criterion-level rather than gate-level, but
unpinned all the same.
MY OWN FIX HAD A HOLLOW SPOT, caught by mutation-proving it rather than
trusting it. The end-of-argv test used ["--from"], which returns before ever
reaching the guard -- so the third mutant (dropping the `following is None`
arm) passed all four new tests. Probed five inputs looking for one that
distinguishes it: none does. That arm is UNREACHABLE, because a trailing
positive flag simply ends the loop and falls through to unknown anyway. The
test is now labelled non-discriminating and says why, rather than implying a
proof it cannot give.
Also retargeted the npm-residual docstring from #182 to #180. This PR closes
#182 on merge, so the code's only tracking pointer would have dangled the
moment it landed.
Kills verified per node, on a copy, provenance-checked:
drop startswith arm -> 2 tests fail
drop `or None` -> 1 test fails
drop is-None arm -> 0 fail (unreachable; now documented as such)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…stay distinct
Red-team seat found a collision this change ITSELF introduced. Verified before
acting.
_pep508_base_name listed @ among the name terminators, so a direct reference
truncated to its bare name:
pkg @ git+https://x/y -> ('pypi', 'pkg')
pkg @ git+https://x/z -> ('pypi', 'pkg') COLLIDES
Two different repositories collapsing to one identity is the exact defect
class #182 exists to close, newly created by the fix for it -- and it violated
this plan's own acceptance criterion that distinct URLs keep distinct
identities. The existing test covered only the BARE url form and missed the
standard named one, which is why nothing caught it.
@ is no longer a terminator. Verified after: the two repos resolve distinctly,
and extras/version normalization is unaffected -- browser-use[cli] ->
browser-use, index-it-mcp==1.2.0 -> index-it-mcp, pkg>=1.0 -> pkg. Pinned by
TestDirectReferencesStayDistinct, whose second test exists specifically to
catch a carve-out that disables the rest of the rule.
Also recorded the round's two FALSE alarms, both landing on spots already
labelled in-code as non-discriminating: the README test does have an assertion
body (its docstring was read as the whole test), and the `--` branch is
already documented as measured-redundant-but-kept. Disclosure did not prevent
the flags, but made them checkable in seconds rather than needing a fresh
investigation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Implementation board — 1 AGREE, 1 DISAGREE, 1 PARTIALLY AGREE; two real defects, both mine
Both dissents were right. Fixed in 1. A collision this fix introduced (red-team)
Two different repositories, one identity — the exact defect class #182 exists to close, newly created by the fix for it. It also violated this PR's own acceptance criterion that distinct URLs keep distinct identities. The existing test covered only the bare URL form and missed the standard named one, which is why nothing caught it.
2. The 21st mutant — and my fix for it had a hollow spot (correctness)Deleting the Then, mutation-proving my own patch rather than trusting it: a third mutant (dropping the Kills verified per node, on a copy, provenance-checked: 3. Two findings were false — and how they failed is the interesting part
Both landed exactly where the implementer had already written "this does not pin what it looks like it pins." The disclosure didn't prevent the flag, but it made each checkable in seconds instead of requiring a fresh investigation — a concrete argument for writing non-discrimination into the code rather than into a report. Verification at
|
Closes #182. Narrows #180 further, and recommends it stay open — see #180 status.
The defect
detect_package_typeskipped flags but not the values those flags carry, so the first non-flag token was often a flag's argument. Two different servers then resolved to one identity — and v2.4.0's gate compares exactly this name to decide whether a cached description still describes the configured package, so it read that as a positive confirmation and served the wrong package's tool descriptions.All seven reproduced on
main; all seven now resolve distinctly:Design: three-way classification, fail-closed default
Per ecosystem: known-value flags consume the next token, known-boolean flags skip, known-positive flags yield the package, and any unlisted bare flag →
("unknown", None).This is not the denylist #183 rejected. Docker's table failed open because unlisted meant "take the next token as the image" — an omission silently produced a wrong identity. Inverting the default means an omission costs auto-update for one odd config (safe, loud, fixable) instead of a silent collision (unsafe, invisible).
Two designs the board rejected before this one
A first draft — "any bare flag before the candidate yields unknown" — would have broken the canonical configs, all correct today:
docker run -it --rmis the shape this repo's own README uses (:1528). Essentially every real docker server would have become permanently unverifiable. A second draft added a whole-argv--fromscan, which turnsuvx mypkg --from otherfrommypkgintoother— a fail-open misidentification introduced by the fix. All five forms are now pinned as regression tests.Two live defects repaired along the way
The README's own pin form was mis-identified.
README.md:1133documentsuvx --python 3.12 --from index-it-mcp==1.2.0 index-it-mcp, which resolved to('pypi','3.12')— the Python version as the package name.A pinned server could have been moved to latest.
--from=pkg==1.2.0reported no pin while the spaced form reported1.2.0. Both now report1.2.0.And
browser-use's version checks were silently failing.manifest.yaml:352ships--from browser-use[cli]; PyPI returnsNonefor that name and0.13.8forbrowser-use. PEP 508 base-name normalization repairs it. Confirmed live, not assumed.The flag tables were verified mechanically, and ten entries were dropped
The claim "verified entry by entry against
--help" was not true when first written — several entries were added from memory, the practice the plan forbids and the direction that fails open. All 296 entries were then checked against live help output:--pullwrong (it takes a value — the plan's own named hazard), plus uv's--quiet/--verboseand pip's--platform.-it/-ticombined shorts; npm's bare--package).Mutation proof: 20/20 killed, each by its own single node
The harness carries an import-provenance guard — without it every mutant would have falsely survived while the unmutated worktree served the imports. Three tests claimed more than they pinned and were fixed or relabelled:
_takes_a_value_ambiguously's"=" not in argclause was dead code — the rule had two homes and only one was live.--packagehad no test; returning the first of several different packages is the exact guess this change exists to stop. Now fails closed.Two tests are non-discriminating and now say so in their docstrings instead of implying a proof.
#180 status — recommend it stays OPEN
All three forms from its narrowing comment are closed, and its two original forms stay fixed. But by #180's own closing sentence — "until the identity gate actually holds for ordinary commands" — the npm branch still fails open on unlisted flags:
Converting npm needs an enumeration of its boolean globals so the pinned ordering (
npm --silent exec pkg→pkg) keeps passing — a new design surface deserving its own board round. The middle ground, a fail-open npm value table, is precisely the shape this plan rejected, so it was not introduced into a fifth ecosystem.Verification
2764 passed, 3 skipped(baseline 2737, +27); ruff and mypy clean; all seven collisions closed; all five non-negotiable forms unchanged; 98/98 manifest servers resolve.🤖 Generated with Claude Code
https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.