Repository navigation
docs(roadmap): phase-plans-v12 - #167
Merged
Merged
Conversation
Closes what v11 named as deferred (downstream notification fan-out, typed JSON-RPC errors) and replaces the fail-closed version boolean whose failure mode has now been fixed three separate times. Three phases: TRISTATE -> UPDPATH, with FANOUT as a parallel root. Panel: grok-4.6, gpt-5.6-sol, gemini-3.7-flash-high, plus a natively-filled fable seat. Two seats blocked the first draft. Every finding below was verified against source before the roadmap changed. - FANOUT published list_changed straight to CatalogEventSink WITHOUT re-indexing ClientManager's catalog. A client acting on that notification would refetch the OLD catalog, and a removed tool would stay invocable, because _index_* only adds or overwrites. That is a notification asserting something the gateway has not done -- the silent-misreport class that ended in removing update notices entirely (#150). Now: reconcile, then publish, and only if something actually moved. - EC-FANOUT-3 required filtering by originating server while this roadmap's own Non-Goals exclude per-origin subscriptions, and CatalogEventSink carries only an event kind. Self-contradictory; deleted. - UPDPATH missed check_staleness (refresher.py:426), a third site pairing cache and config by name and comparing versions only, reachable from `pmcp refresh --check-versions`. Added, with a CLI-level regression test. - Two dispatch paths exist, not one: _handle_stdout_line and the SSE loop drop notifications identically. Wiring only stdio would leave every remote server silent. Found by reading the source rather than trusting the assumption. - Assumption 2 ("the read loop sees notification frames") is verified and demoted to a day-1 RED test across both transports. - Re-indexing sends tools/list back to the connection whose reader is mid- dispatch. That self-deadlock is now its own exit criterion. - The lane partition was wrong: three lanes write manager.py, two of them in the same if/else. Serialized rather than partitioned by branch. Two verification commands in the first draft were false greens -- one checked a file that has never existed, and both greps matched comments and docstrings. Replaced with the acceptance test, which parses instead of guessing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
specs/phase-plans-v12.md. Docs only — nosrc/change, so no CHANGELOG entry.Thesis
Close the two items v11 explicitly deferred (downstream notification fan-out; typed JSON-RPC errors), and replace the fail-closed version boolean whose failure mode has now been fixed three separate times with a representation that makes it unrepresentable.
Three phases, 20 exit criteria, passes
phase-loop validate-roadmap.Panel
Four seats: grok-4.6, gpt-5.6-sol, gemini-3.7-flash-high, and the fable seat filled natively rather than through the CLI. Two seats blocked the first draft, and they were right. Every finding was verified against source before the roadmap changed.
The blocking one
FANOUT routed a downstream notification straight to
CatalogEventSinkwithout re-indexingClientManager's catalog. Those indexes backcatalog_search,describeandinvoke— so a client told "the catalog changed" would refetch and get the old catalog, and a tool the downstream server removed would stay invocable, because_index_*only adds or overwrites.That is a notification asserting something the gateway has not done: the same silent-misreport class that ended in removing update notices outright (#150), which this roadmap's own Context cites as the lesson. It was on track to repeat it.
Now: reconcile, then publish — and only if something actually moved.
The rest
CatalogEventSinkcarries an event kind, not a server identity. Not implementable as written; deleted.check_staleness(refresher.py:426) pairs cache and config by name and compares versions only, backingpmcp refresh --check-versions. A cache forold-package@2.0.0againstnew-package@1.0.0reports "up to date" at the CLI. Added with a CLI-level regression test, plus a criterion for pre-2.2.1 caches with nopackagefield._handle_stdout_lineand the SSE loop drop notifications identically and carry the same error-truncation TODO. Wiring only stdio would leave every remote server silent.id, so it falls through with noelse. Demoted to a day-1 RED test across both transports, since the SSE path is the one that could be swallowed by the SDK.tools/listback to the connection whose reader is mid-dispatch.manager.py, two of them in the sameif/else. Serialized rather than partitioned by branch — better found here than at merge.Two false greens in my own verification recipe
One checked
tests/test_version_checker_lint.py, which has never existed — permanently green, proving nothing. My first correction was also wrong: narrowing the grep to^[^#]*still matched docstring lines atversion_checker.py:560and:614. Distinguishing a call from prose needs a parse, so the recipe now runs the acceptance test instead of reimplementing it badly.That this roadmap's Cross-Cutting Principles opens with "a test must fail against the bug it names" — and its verification block shipped two checks that could not fail — is the finding worth keeping.
Decision the roadmap makes
EC-UPDPATH-4 documents rather than freezes the ambient environment across
update_server's probe window (#162), reasoning that freezing pins a credential the operator deliberately rotated. It is the one criterion encoding a judgement rather than a defect, flagged as such.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.