Skip to content

feat(updpath): gate description freshness on package identity (v12 UPDPATH) - #179

Merged
ViperJuice merged 12 commits into
mainfrom
phase/v12-updpath
Aug 24, 2026
Merged

ViperJuice merged 12 commits into
mainfrom
phase/v12-updpath

Conversation

@ViperJuice

@ViperJuice ViperJuice commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

The last v12 phase. Cached server descriptions were paired with a config by name and freshness decided on version alone, at three sites — so swapping the configured package at an equal version served the wrong package's tool descriptions indefinitely.

Executed as five lanes in isolated worktrees, reviewed by a four-seat board before merge.

What changed

  • Package-identity gate at all three sites (refresh_server, refresh_all, check_staleness). An unknown side means "cannot confirm identity", which resolves to refresh — never to "cannot compare, so skip the check", the fail-open collapse that shipped three times (fix: version comparison fails closed instead of fabricating update notices #155, Follow-ups from #155: hand-written semver, negation hazard, digest ambiguity, CI guard fragility #156, fix: close the four #155 follow-ups (#156) #163).
  • package_type on the cached entry. A bare name cannot tell npm foo from pypi foo, and both produce orderable release versions so compare_versions never returns incomparable. Pre-existing caches have no type, read as unknown, and refresh once. No format bump.
  • --check-versions honours --cache-dir — it computed the path and then dropped it.
  • --check-versions splits its report. A server whose package cannot be classified is reported as unconfirmable, not stale, and without the --force remedy that would never settle it.
  • Probe-window environment contract documented and pinned, both halves: a config-driven change (including a rotated manifest credential) is refused; a genuinely ambient variable is live at spawn.

Three defects the pre-merge board caught

Board: grok PARTIALLY AGREE, codex DISAGREE, fable PARTIALLY AGREE, gemini ERROR. Every finding was verified against source and proved by mutation before being acted on.

1. The phase hollowed out two pre-existing regression guards. Adding _same_package as an earlier conjunct meant TestUpToDateShortCircuit's fixtures — which carry no package_type — now fail identity and never reach compare_versions. Both tests still passed, for the identity reason. Rewriting the short-circuit to != "newer", so an incomparable pair reads as up to date — precisely the #156/#164 fail-open — left the entire suite green. A phase whose purpose is closing a fail-open had silently removed the only coverage of a different one.

2. update_server was a third write site for package identity, not the second. It set version, tools and generated_at but never package/package_type. So an entry could carry the new package's version and live tools under the old package's label — after which _same_package would confirm identity against the stale label and the short-circuit would serve the wrong package's descriptions. That is this phase's headline defect, reachable in three operator steps, in code the phase itself touched. The lane audited two sites and the plan repeated the count.

3. The CLI split's group boundary was unpinned. The swap test asserted only that the server name appeared — and it appears in both groups. Routing a classifiable equal-version mismatch into the "could not confirm" group, withholding the remedy that actually settles it, left the suite green.

Two operator-visible consequences, documented not hidden

  • refresh_all now drops an unclassifiable server's cached descriptions when regeneration fails, where it previously kept them. For a node/python server there was never evidence of a mismatch, only an inability to confirm one. Deliberate: the alternative — "only drop when positively different" — recreates the two-meanings-for-one-boolean shape behind the three bugs above. Both boards ruled the trade correct.
  • A cache written before this change refreshes once, because it has no recorded package type.

Verification

2699 passed, 3 skipped, 0 failed; ruff clean; mypy clean (42 files); validate-roadmap OK; validate_plan_doc 0 errors.

roadmap_sha256 was re-pinned at closeout — the board split on this and the maintainer chose a clean-validating artifact over preserving the planning-time binding. Reasoning is recorded in the plan; UPDPATH is the first plan to refresh at closeout, and that is the convention going forward.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

ViperJuice and others added 10 commits August 24, 2026 11:58
`update_server` probes for a new package version, then re-resolves the
config before restarting. Two DIFFERENT things happen to the child
environment across that window, and an earlier single-sentence reading
of this area conflated them:

- A CONFIG-DRIVEN change is refused. Every manifest credential is
  config-driven, because `_manifest_server_to_config` resolves it into
  `LocalMcpServerConfig.env` at load time -- so rotating it mid-probe
  makes the recheck's child env differ from the probe's and the
  #151 TOCTOU guard returns ok=False without
  restarting. That is the guard working as designed.
- A GENUINELY AMBIENT variable is live at spawn. Both sides of the
  guard derive from one `stripped_base`, so an ambient change cancels
  out and can never refuse, and `sanitized_subprocess_env` calls
  `os.environ.copy()` at spawn time.

`TestUpdateServerAmbientEnvironment` pins both. The ambient test uses a
real `ClientManager._connect_stdio` with `asyncio.create_subprocess_exec`
patched at the spawn boundary, and asserts against the env dict actually
handed to the spawn -- a fully mocked ClientManager has no spawn to
capture, and asserting on `os.environ` would pass without the contract
holding. It rotates a non-managed key on purpose: a managed key would
exercise `sanitized_subprocess_env`'s `own` arm, which is the refusal
path, not the live-spawn one.

Behaviour is unchanged; `update_server`'s docstring now states both
halves instead of neither.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
All three refresh sites paired a cached description with a configured
server by NAME and then decided freshness by comparing VERSIONS only.
None asked whether the cache still described the same PACKAGE, so a swap
at an equal version served the wrong package's tool descriptions
indefinitely:

- same ecosystem -- `old-pkg@1.0.0` cached, `new-pkg@1.0.0` configured,
  both npm, both orderable, equal;
- cross ecosystem -- `package` was a bare name with no ecosystem, and
  npm/pypi/cargo all produce orderable release versions, so pypi
  `foo@1.0.0` read as the same package as npm `foo@1.0.0`.

The docker case was already covered: a version against a digest is
`incomparable`, which is not `not_newer`, so the short-circuit never
fired for it.

- `GeneratedServerDescriptions` gains `package_type: str | None = None`,
  populated where the entry is written as well as where it is read.
  Absent defaults to None, not "", so a pre-existing cache reads as
  unknown and is refreshed once.
- New `_same_package` predicate returns False when any of cached name,
  cached type, configured name or configured type is empty, None or
  "unknown". An unknown side means "cannot confirm identity, refresh" --
  never "cannot compare, skip the check".
- `detect_package_type` / `pkg_name` resolution MOVES above the
  short-circuit in `refresh_server` (exactly one call remains) so the
  comparison can consult identity at all.
- `refresh_all` applies the gate to the pair it assembles itself, and
  neither its None/raise fallback nor its final merge loop can put a
  mismatched entry back into the saved cache.
- `check_staleness` ADDS a mismatch to the stale dict -- its polarity is
  inverted relative to the short-circuit, so the guard is not a
  mechanical port.
- `pmcp refresh --check-versions --cache-dir X` now actually inspects X:
  `run_refresh` computed `cache_path` and then dropped it.

Refs #178 (EC-UPDPATH-1..3, 6, 7; IF-0-UPDPATH-1, 2)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…v contract

SL-3 (SL-docs), the terminal lane of UPDPATH. No source or test file is
touched; `git status --porcelain -- src tests` is empty.

CHANGELOG (`[Unreleased]`)
- Fixed: the package-identity gate across all three refresh sites, the
  "cannot confirm identity -> refresh, never cannot compare -> skip the
  check" semantics, the new `package_type` on the cached entry and its
  refresh-once migration, and the cosmetic `srv: 1.0.0 -> 1.0.0` a stale
  `(cached_version, latest_version)` pair prints at an equal version.
- Fixed: `pmcp refresh --check-versions` honours `--cache-dir`, which
  `run_refresh` computed and then dropped.
- Changed: `--check-versions` now reports an unclassifiable local server
  (`node /opt/srv.js`) as stale on EVERY run, permanently -- stated as a
  known rough edge with its reason rather than buried, since a refresh
  does not settle it.
- Changed: `refresh_all` now drops such a server's cached entry when
  regeneration fails, where it previously retained it. There is no
  evidence of a mismatch for a node/python server, only an inability to
  confirm one, so a transient startup failure now costs descriptions.

README
- "Subordinate MCP Updates" states both halves of the probe-window
  environment contract, matching `update_server`'s docstring: a
  config-driven change (including a rotated manifest credential) is
  REFUSED with the guarantee "the config restarted onto is the config
  that was probed" and applies on the next update; a genuinely ambient
  variable is LIVE at spawn, the same as connect, refresh and
  auto-reconnect. `docs/` was not created -- README is where a user
  meets this.
- One line for `pmcp refresh --check-versions` in CLI Commands.

Roadmap and plan
- `specs/phase-plans-v12.md` gains "Post-execution amendments -- UPDPATH
  (2026-08-24)" inside the UPDPATH section: EC-7 fell out of
  `_same_package` for free with no code path of its own; the gate landed
  INSIDE the short-circuit conjunction after the version fetch, because
  gating before it broke `TestUpToDateShortCircuit`'s call-count
  assertion; both unanticipated operator-visible consequences; EC-3
  needing a `refresh_all`-local `mismatched` set rather than just a
  gated pair; and the fixture work that `--numstat` hides.
- `plans/phase-plan-v12-UPDPATH.md` records only where the plan itself
  was imprecise, and notes its now-stale `roadmap_sha256` was left alone
  deliberately, as its v12 predecessors' pins were.

docs-catalog rescan helper unavailable; manual catalog audit.
`_shared/scaffold_docs_catalog.py` does not exist on this host. Audited
by hand: added `plans/phase-plan-v12-UPDPATH.md` (83 -> 84 entries),
added the `UPDPATH` alias to CHANGELOG.md, README.md,
specs/phase-plans-v12.md and the new plan entry, bumped `last_updated`.
Root-level entries verified to survive -- SPEC_COMPLIANCE.md in
particular, which a rescan WOULD have dropped (#171:
it is root-level and absent from the helper's KNOWN_FILES).

Intentionally skipped, all outside the catalog's scan roots and
consistent with its existing contents: `.pr_details.md`,
`team_announcement.md`, `.consiliency/**`, `.dev-skills/**`,
`.opencode/**`, `diagnostics/**`, `scripts/**`, and
`src/pmcp/resources/code_execution_guide.md`.

Verification: `uv run ruff format --check src/ tests/` -> 115 files
already formatted. `uv run pytest tests/ -q` -> 2694 passed, 3 skipped.
No markdown linter is configured in this repo (no markdownlint/mdformat/
remark config, and none in pyproject.toml or .github/workflows), so
SL-3.4's markdown check is a recorded no-op.

Refs #178

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
The identity gate makes `check_staleness` report a server whose package it
cannot look up, because "cannot confirm identity" has to resolve to refresh
rather than to "skip the check". That rule is right, but the CLI printed the
whole dict under one heading: a server launched as `node /opt/srv.js` appeared
under "servers with newer versions" on every run, permanently, beneath a
`Run 'pmcp refresh --force' to update.` footer that could not settle it --
the next check still cannot classify the package.

The semantics were never the problem, the display was. Split the report by
whether the latest version is known. Genuinely stale servers keep the existing
output and the force footer; servers whose current version could not be looked
up get their own heading saying so, noting that this is not the same as being
out of date, and pointing at the next plain `pmcp refresh`, which regenerates
them because the freshness short-circuit does not fire for an unconfirmed
identity. The footer prints only when something is genuinely stale, and an
empty result still prints "All cached descriptions are up to date."

The unresolved-version literal is read as a cli-local constant rather than
imported from `refresher._UNIDENTIFIED`: that one describes the identity
fields, a different axis from the latest-version slot.

`check_staleness` and refresher.py are untouched -- its return value is the
contract and it is correct.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Board on phase/v12-updpath @ 40426cf: grok PARTIALLY AGREE, codex DISAGREE,
fable PARTIALLY AGREE, gemini ERROR (the known headless bug). Every finding
was verified against source and proved by mutation before being acted on.

The phase hollowed out two pre-existing regression guards. Adding
_same_package as an earlier conjunct meant TestUpToDateShortCircuit's
fixtures -- which carry no package_type -- now fail identity and never reach
compare_versions. Both tests still passed, for the identity reason, pinning
nothing about the version comparison they exist to guard. Rewriting the
short-circuit to != "newer", so an incomparable pair reads as up to date --
exactly the #156/#164 fail-open -- left the entire suite green. Fixed by
making both fixtures confirm identity: package_type="npm" for the unorderable
case, and the incomparable case rebuilt as a same-ecosystem docker pair
(cached tag vs fetched digest), since the original npm-cache-vs-docker-config
pairing is now refused on ecosystem alone by this phase's own gate. Both now
fail under the mutant and pass without it.

update_server was a third write site for package identity, not the second.
It set version, tools and generated_at but never package or package_type. A
legacy entry passing through it stayed permanently unverifiable despite
update_server having just classified the package; worse, the entry could
carry the NEW package's version and live tools under the OLD package's label,
after which _same_package would confirm identity against the stale label and
the short-circuit would serve the wrong package's descriptions -- the exact
defect this phase closes, reachable in three operator steps. The lane audited
two sites and the plan repeated the count.

The CLI split's group boundary was unpinned. The swap test asserted only that
the server name appeared, and the name appears in both groups. Routing a
classifiable equal-version mismatch into the "could not confirm" group --
withholding the --force remedy that actually settles it -- left the suite
green. Now pinned on the rendered line and the remedy.

Amendment corrected: "TestUpToDateShortCircuit needed nothing" was true for
staying green and false for staying load-bearing. Recorded with the general
lesson -- inserting a conjunct ahead of an existing check silently retires
every test downstream of it, and they all keep passing, so only mutation
detects it.

Suite 2699 passed / 3 skipped; ruff, mypy, validate-roadmap clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
This phase's own docs lane amended the roadmap, so the planning-time pin no
longer matched and validate_plan_doc reported (FM) roadmap_sha256 does not
match the referenced roadmap as an error.

The pre-merge board split. The red-team seat called it blocking: the
validator is authoritative and prior stale pins do not override the contract.
The adversarial and correctness seats ruled the opposite -- the field is
provenance, and re-pinning falsifies it, since this plan was not planned
against a roadmap whose amendment block contains this plan's own closeout.
The correctness seat noted the phase-loop runtime itself refuses to gate live
behaviour on this hash, treating drift as not-corroborated rather than an
error.

Maintainer chose re-pin, so a shipped artifact validates clean rather than
normalising a validator error across three plans. The planning-time binding
stays recoverable from this file's history. TRISTATE and FANOUT keep their
stale pins; UPDPATH is the first to refresh at closeout and that is the
convention going forward.

The re-pin is deliberately the LAST step before merge -- computing it earlier
goes stale the moment a docs lane or board fix touches the roadmap again,
which happened twice during this closeout. The value the board saw (060fe0)
was already obsolete by the time the board fixes landed; the shipped pin is
e2511bc.

validate_plan_doc: 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
…DPATH)

Cached descriptions were paired with a server config BY NAME and freshness
decided on VERSION alone, at three sites. Swapping the configured package at
an equal version served the wrong package's tool descriptions indefinitely.

Five lanes, four boards, three defects caught pre-merge. See the phase's
post-execution amendments in specs/phase-plans-v12.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
ViperJuice and others added 2 commits August 24, 2026 22:28
Second board round on the delta. Both CLI seats returned DISAGREE and found,
independently, that the fix for the FIRST board's finding was itself defective.

The F2 test was a hollow pin. It asserted entry.package == "@playwright/mcp"
against a fixture already labelled "@playwright/mcp" -- the value
detect_package_type returns there -- so the assertion held whether or not the
assignment ran. Deleting desc_entry.package = package_name left it green. That
is the same failure mode the previous commit documented, reproduced one commit
later. The entry now starts under a different package and each assignment is
pinned by its own mutant.

Relabelling was the wrong repair. update_server deliberately preserves
capability_summary on a version bump, on the reasoning that it only feeds
startup MCP-instructions text and one refresh cycle behind is a small gap.
That reasoning does not survive a relabel: once package AND version both name
the new package, the freshness short-circuit CONFIRMS identity and never
regenerates the entry, so the old package's summary would be served under the
new package's label permanently. A genuine package change now deletes the
entry and lets the next refresh rebuild it wholesale -- this phase's own rule
applied to itself. A same-package version bump is unchanged and still
backfills package_type.

The drop exposed a real regression in an unrelated test:
test_update_server_uses_configured_override_not_manifest used cache-entry
survival as a proxy for "the override drove the probe", while its override
names a different package than the fixture's label. Fixed the fixture rather
than weakening the guard, and recorded which test now owns that behaviour.

detect_package_type's identity collisions (registry:5000/old-image and
registry:5000/new-image both -> ("docker","registry"); npm exec old-pkg /
new-pkg both -> ("npm","exec")) are filed as #180 and recorded
as a known limitation. Pre-existing, and detect_package_type backs every
version lookup in the codebase, so re-parsing it belongs in its own plan.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
Third re-pin this closeout, and the reason is the ordering trap the plan
itself documents: the pin must be computed AFTER the last roadmap edit, and
recording the second board round's findings amended the roadmap again.

Board round 1 saw 060fe0. Round 2 shipped e2511bc. Neither was final; the
roadmap is now 2015eca. validate_plan_doc: 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbREMUsxgJ8TDBLpJ56PWb
@ViperJuice

Copy link
Copy Markdown
Collaborator Author

Second board round on the delta — both CLI seats DISAGREE, three findings, all fixed

The first board reviewed 40426cf. The fixes made in response landed after it and were unreviewed, so they went back to the board as their own delta. That round found that the fix for the first board's finding was itself defective. Fixed in be01820.

1. My F2 test was a hollow pin — the exact failure mode the previous commit documented

test_update_server_relabels_the_entry_... asserted entry.package == "@playwright/mcp" against a fixture already labelled @playwright/mcp — the value detect_package_type returns there. So the assertion held whether or not the assignment ran:

deleted: desc_entry.package = package_name
1 passed

Two seats found this independently. It was written one commit after the amendment explaining that green is not evidence. The entry now starts under a different package, and each assignment is pinned by its own mutant.

2. Relabelling was the wrong repair — it made a mixed entry permanent

update_server deliberately preserves capability_summary on a version bump, on the reasoning that it only feeds startup MCP-instructions text and being one refresh cycle behind is a small, non-functional gap.

That reasoning does not survive a relabel. Once package and version both name the new package, the freshness short-circuit CONFIRMS identity and never regenerates the entry — so the old package's summary would be served under the new package's label permanently, not one cycle behind. My fix satisfied the identity check that would otherwise have healed it.

A genuine package change now drops the entry and lets the next refresh rebuild it wholesale — this phase's own rule applied to itself: cannot confirm the cache describes this package → refresh, never patch around it. A same-package version bump is unchanged and still backfills package_type.

The drop exposed a real regression in an unrelated test: test_update_server_uses_configured_override_not_manifest used cache-entry survival as a proxy for "the override drove the probe", while its override names a different package than the fixture's label. Fixed the fixture rather than weakening the guard, and recorded which test now owns that behaviour.

3. Known limitation, filed not fixed — #180

detect_package_type collapses distinct packages for some legal command forms:

docker ['run', 'registry:5000/old-image'] -> ('docker', 'registry')
docker ['run', 'registry:5000/new-image'] -> ('docker', 'registry')
npm   ['exec', 'old-pkg']                 -> ('npm', 'exec')

So _same_package confirms identity across a real swap for those forms, and EC-UPDPATH-1/3/6 do not hold there. Pre-existing — nothing compared package names before this phase — and detect_package_type backs every version lookup in the codebase, so re-parsing it has blast radius well beyond this closeout. Filed as #180 and recorded in the phase's amendments so the next phase inherits the boundary rather than assuming the gate is total.

Verification

2700 passed, 3 skipped, 0 failed; ruff clean; mypy clean; validate-roadmap OK; validate_plan_doc 0 errors.

roadmap_sha256 re-pinned a third time. That is the documented ordering trap working as described: the pin must be computed after the last roadmap edit, and recording each board round amends the roadmap again. Round 1 saw 060fe0, round 2 shipped e2511bc, final is 2015eca.

Board tally across both rounds

Round grok codex fable gemini
Phase @ 40426cf PARTIALLY AGREE DISAGREE PARTIALLY AGREE ERROR
Delta DISAGREE DISAGREE no report EMPTY

Six findings total, six fixed or filed. The two that mattered most were both in code I had already verified and signed off.

@ViperJuice
ViperJuice merged commit e7907df into main Aug 24, 2026
11 checks passed
@ViperJuice
ViperJuice deleted the phase/v12-updpath branch August 24, 2026 22:38
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.

1 participant