Skip to content

fix: remove the automatic update notices (#150) - #159

Merged
ViperJuice merged 11 commits into
mainfrom
fix/150-remove-update-notices
Aug 20, 2026
Merged

ViperJuice merged 11 commits into
mainfrom
fix/150-remove-update-notices

Conversation

@ViperJuice

@ViperJuice ViperJuice commented Aug 20, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #150.

Removes the automatic "update available" notices. gateway.update_server is unchanged — it still probes, reports, restarts and refuses pinned servers. What goes away is the gateway volunteering a claim it cannot substantiate.

Why removal rather than a ninth fix

Eight attempts failed at one joint: pmcp cannot observe which package artifact a running server is executing. Not from descriptions_cache.version (an upstream snapshot taken at describe time), not from serverInfo.version (an implementation version — FastMCP 1.x reports the SDK's, mcp 2.x defaults to ""), not from the npx/uv runner caches (no server→entry binding; one package was observed cached at 1.2.0, 1.2.1 and 1.2.2 concurrently), and not from spawn argv (npm resolves after create_subprocess_exec returns).

The only remaining method — running each server's own --version — was rejected 3/3 by the review board as disqualifying: it is a full launch of the server's argv with its real credentials, on a schedule. For a manifest entry using @latest it could execute newly published code with those credentials before the operator chose to update. Measured: probing one flag-ignoring server spawned 7 processes and opened 5 sockets. Coverage would have been ~50% even then.

The full analysis for all eight attempts is committed under .consiliency/plans/.

What this is: a breaking change to tool output

update_warning and stale_updates are removed from the schemas, not left permanently empty. server.py serialises with model_dump() and no exclude_none, so those keys are emitted today as explicit nulls — verified — which means their absence is a real wire change for a client doing key-presence checks.

The board recommended outright removal over keeping always-None fields or deprecating across a release: a field that can never be populated advertises a capability pmcp no longer has.

Scope

update_warning= attachment sites 26 → 0 (describe 1, invoke 5, provision 20)
_get_update_warning calls 3 → 0
Background indexer + lifecycle calls removed (server.py ×2)
Public schema fields 4 removed
Tests 11 notice tests removed, 3 guards added

I miscounted the attachment sites twice before the board caught it — first 6, then 16, when the real number is 26. provision alone holds 20. The plan was corrected to mandate driving grep -c to zero rather than working from any hand-written list, and that is how the edit was performed.

Two other findings while implementing: _effective_notice_target and _notice_package_matches_cache never existed on main (dead residue of the abandoned #154 branch), and the README had two obsolete lines, not the one the plan identified — the second promised "background stale-version indexing is active".

Verification

  • pytest -q → 2547 passed, 3 skipped, 0 failed (2555 baseline − 11 removed + 3 added)
  • ruff check ., ruff format --check src/ tests/, mypy src — clean
  • Completeness: grep -rn 'update_warning|stale_updates|_stale_check_cache|_stale_index|_get_update_warning' src/ tests/ returns nothing
  • Guard proof: partially re-introducing the feature (one schema field + one attribute) fails all three new tests. They catch a re-introduction rather than passing vacuously — the failure mode that has bitten this issue repeatedly.

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

Maintainer decision after the board rejected the eighth attempt 3/3 and
explicitly recommended removal. gateway.update_server is unchanged; what goes
is the gateway VOLUNTEERING a claim it cannot substantiate.

Surface enumerated from source rather than assumed: 3 emission sites
(catalog_search stale_updates, describe and invoke update_warning across six
invoke return paths), the background indexer plus its two server.py lifecycle
calls, and FOUR PUBLIC SCHEMA FIELDS across CatalogSearchOutput, SchemaCard,
InvokeOutput and ProvisionOutput. Also README:480, which promises the field.

Flagged honestly: unlike every prior plan in this series, this one DOES change
the protocol surface, and ProvisionOutput.update_warning has a writer outside
the three known sites that must be traced rather than assumed dead.

Open question left for the board: remove the fields outright, retain them as
permanently-None, or deprecate for one release.
…th site

Traced before boarding, as the plan itself flagged. provision does not merely
carry the field: it calls _get_update_warning at handlers.py:4188 and attaches
update_warning at TEN further return paths. Total attachment points are 16
(describe 1, invoke 5, provision 10), not the six originally documented.

A removal stopping at describe/invoke would leave provision calling a deleted
method -- the half-removal failure the plan warns about.
…ast radius

_effective_notice_target and _notice_package_matches_cache have ZERO references
in src/ or tests/. The plan claimed they were 'used only by the above'; in fact
they were never wired in and are already-dead residue from the abandoned #154
attempts.

Also recorded: all deleted machinery is confined to handlers.py, but 15 TEST
references depend on _stale_check_cache -- so the test deletions are the larger
part of this change, and each needs classifying as 'asserts the removed feature'
versus 'asserts update_server'.
The board caught a second undercount. Authoritative count from source:
grep -c 'update_warning=' handlers.py = 26, plus 3 _get_update_warning calls.
describe 1, invoke 5, provision TWENTY (4200 through 4630).

My first draft said 6. My own pre-board correction said 16, because I stopped
reading the grep output at 4411 when it continues to 4630.

The miscount pattern is itself the finding: provision holds 20 of 26 sites, so
the edit is concentrated exactly where I kept failing to look. Implementation
must be driven by grep -c reaching zero, not by any hand-enumerated list.
Board review of this PR. gateway.update_server's description still read 'Use
this when invoke/describe/provision warn that a newer version is available' --
shipped to every client through tools/list, instructing agents to wait for
notices this PR removes.

My completeness check could not catch it: the grep searched IDENTIFIERS, and
this is PROSE. Added a guard over get_gateway_tool_definitions() so the class
of gap is closed, not just this instance. Verified it goes RED against the old
wording.
@ViperJuice

Copy link
Copy Markdown
Collaborator Author

Board review: 1 AGREE / 1 PARTIALLY AGREE / 1 DISAGREE — one real defect, fixed

codex found a blocking defect that my own completeness check structurally could not catch, and it was a good catch.

The defect

gateway.update_server's tool description still read:

"Use this when invoke/describe/provision warn that a newer version is available."

That string ships to every client through tools/list (server.py:263-269), so the public interface was instructing agents to wait for notices this PR removes.

Why my verification missed it: the completeness grep searched identifiers — update_warning, stale_updates, _stale_check_cache and friends — and returned nothing, correctly. This is prose. A symbol-based check cannot see it, and neither could the three guard tests, which assert on model fields and attributes.

The fix

Reworded to say the gateway does not volunteer notices, so nothing will prompt you.

More importantly, added test_exported_tool_descriptions_do_not_promise_notices, which scans every exported tool description for the offending phrases. That closes the class of gap rather than the single instance. Verified RED against the old wording:

AssertionError: gateway.update_server description still promises update notices: 'warn that a newer version'

I swept the rest of src/ for equivalent prose and this was the only occurrence.

Answers to the other review questions, verified

  • gateway.update_server survived intact — probe, restart-gating, pin refusal, descriptions-cache persist and config resolution are all still present and referenced. Only its writes to the deleted cache were removed.
  • No unrelated coverage lost. All 11 removed tests are notice-specific by name. test_catalog_search_stale_updates_coexist_with_cli_hints also touched cli_hints, so I checked: cli_hints retains 5 dedicated tests.
  • A separate staleness feature was correctly left alone. pmcp refresh --check-versions → refresher.check_staleness() is user-initiated and untouched, which is exactly the on-request model this change preserves.

Verification

  • pytest -q → 2548 passed, 3 skipped, 0 failed
  • ruff check, ruff format --check src/ tests/, mypy src — clean
  • All CI checks green on the previous head, first pass

@ViperJuice
ViperJuice merged commit 7318f7e into main Aug 20, 2026
11 checks passed
ViperJuice added a commit that referenced this pull request Aug 20, 2026
Minor rather than patch: #159 removes four fields from public tool output
(update_warning from describe/invoke/provision, stale_updates from
catalog_search), which is a breaking change for a client reading them.

Also merges the two duplicate '### Fixed' headings that accumulated under
[Unreleased] from three separate PRs, and syncs __init__.__version__, which
pyproject alone would have left at 2.1.1.
@ViperJuice
ViperJuice deleted the fix/150-remove-update-notices branch August 22, 2026 19:46
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.

update_server and the stale-notice paths resolve server config differently

1 participant