Skip to content

detect_package_type: value-carrying flags are not skipped, so uvx/pip/cargo collide the same way #180's forms do #182

Closed

Description

@ViperJuice

Found by the correctness seat of the advisor board reviewing the plan for #180, and confirmed empirically on main @ f5ea5ec (v2.4.0).

#180 records collisions in the docker and npm branches. The same root cause is present in uvx, pip and cargo, and the #180 plan's research summary wrongly claimed those branches were "not implicated". They are:

uvx   ['--python', '3.12', 'pkg-a']                   -> ('pypi', '3.12')
uvx   ['--python', '3.12', 'pkg-b']                   -> ('pypi', '3.12')
uvx   ['--with', 'requests', 'srv-a']                 -> ('pypi', 'requests')
uvx   ['--with', 'requests', 'srv-b']                 -> ('pypi', 'requests')
pip   ['install', '--index-url', 'https://x', 'pkg-a'] -> ('pypi', 'https://x')
pip   ['install', '--index-url', 'https://x', 'pkg-b'] -> ('pypi', 'https://x')
cargo ['run', '--features', 'full', 'srv-a']          -> ('cargo', 'full')
cargo ['run', '--features', 'full', 'srv-b']          -> ('cargo', 'full')

Root cause

Those branches skip flags (arg.startswith("-")) but not the values those flags carry (src/pmcp/manifest/version_checker.py:284-314). So the first non-flag token is the flag's value, not the package.

The docker branch already solves exactly this, with a _value_flags set enumerating flags whose next token must be skipped (version_checker.py:336-359). uvx, pip and cargo have no equivalent.

Note uvx --from thing other -> ('pypi','thing') is correct — but only by luck: --from's value happens to be the package. That single safe flag is what made the branch look fine.

Why it matters

Same consequence as #180. v2.4.0's identity gate compares these names to decide whether a cached description still describes the configured package, so two different servers launched with the same flag value read as the same package and _same_package confirms identity across a real swap.

uvx is the most consequential of all of them — it is a common MCP launcher and --with is a normal pattern, which makes this more realistic than the registry:host:port case #180 is built around.

Why not folded into #180

#180's plan is scoped to its two measured branches, which is defensible discipline. Expanding it mid-flight to five branches with a new shared _value_flags design would turn a bounded fix into a parser redesign and lose the review that scoping bought. Filed separately so it gets its own plan and board round.

Suggested scope

  • A _value_flags-style skip set per ecosystem, mirroring the docker one at version_checker.py:336-359.
  • Decide the unparseable-form contract: returning "unknown" is safe for the identity gate (unknown forces a refresh) but changes version-lookup behaviour — the two caller classes may want different handling, which is worth settling once rather than per branch.
  • Pin each fix with an inequality assertion (d(form_a) != d(form_b)), not a value assertion. A value assertion passes against a parser that collapses both forms to something else.
  • Prove each new test RED against the current parser before the fix. Two hollow tests shipped in this repo's last two phases; both were caught only by a board.

Activity

  1. added a commit that references this issue on Aug 25, 2026
  2. ViperJuice commented on Aug 25, 2026

    @ViperJuice
    CollaboratorAuthor

    Broadened: the same root cause is in docker and npm too, so #180's fix does not close #180

    The advisor board reviewing #180's plan found that this issue is wider than filed. The root cause — a flag's value being taken as the package name — is not confined to uvx/pip/cargo. It is also present in the two branches #180 is about, which means #180's planned fix (tag-splitting + npm subcommand skip) narrows those collisions without closing them.

    Confirmed on main @ f5ea5ec:

    docker run --env-file .env old-image              -> ('docker', '.env')
    docker run --env-file .env new-image              -> ('docker', '.env')
    docker run --mount type=bind,src=/a old-image     -> ('docker', 'type=bind,src=/a')
    docker run --mount type=bind,src=/a new-image     -> ('docker', 'type=bind,src=/a')
    npm exec --package=old-pkg -- shared-bin          -> ('npm', 'exec')
    npm exec --package=new-pkg -- shared-bin          -> ('npm', 'exec')
    

    --env-file and --mount are ordinary docker run options. _docker_image_arg's _value_flags table (src/pmcp/manifest/version_checker.py:336-359) enumerates flags whose next token must be skipped, and it is incomplete — so the "docker already solves this" framing in my original filing was too generous. Docker solves it for the flags it happens to list.

    npm exec --package=<pkg> is the documented way to name the package explicitly, and _npm_package_arg discards option tokens wholesale, so the actual identity is thrown away and the binary name is compared instead. Two different packages exposing the same binary confirm as one.

    What this means for #180

    #180's plan ships anyway — the maintainer's call, and I think the right one: tag-splitting and the leading-subcommand skip are real, measured, independently correct fixes, and they land with a board round behind them. But #180 should not be closed by that PR. The identity gate will still falsely confirm for the forms above, and saying otherwise would put a "fixed" label on a hole that is still open.

    Suggested scope, revised

    The per-ecosystem _value_flags idea in the original filing is not enough, because it needs the table to be exhaustive and the docker case shows what happens when it is not. Consider instead:

    • Fail closed on unrecognised shapes. If the parser cannot confidently recover identity, return unknown. That already forces a refresh under the identity gate, so it is safe by construction and does not depend on enumerating every flag any tool may ever add. The cost is version-lookup behaviour for forms that presently work by accident, which is the trade worth deciding explicitly.
    • Subcommand-aware npm parsing, honouring --package= / -p and the x alias, rather than a generic skip.
    • Whatever is chosen, pin each form with an inequality assertion proved RED against the current parser.

    Related: #183, filed separately — a misparsed npm form makes update_server install and execute a registry package derived from the misparse, which is a different and more serious class than a mislabelled cache entry.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions