Repository navigation
docs(roadmap): amend v12 on the fable seat's findings - #168
Merged
Merged
Conversation
The fable seat reported after v12 merged. Three roadmap-changing findings, each verified against source before applying. 1. INLINE RECONCILIATION SELF-DEADLOCKS -- and this is the fix I had just added. _index_capabilities awaits _send_request (manager.py:1292), and those futures are resolved by the very read loop that received the notification (pending.future.set_result at :1791 and :2010, the two dispatch functions). Calling reconciliation inline makes the loop await a future only it can resolve. EC-FANOUT-6 now mandates a spawned, per-server-coalesced task rather than merely naming the hazard. 2. A CATALOG-FRESHNESS CRITERION WAS MISSING. Every notification-shaped criterion can pass while the gateway still serves the old catalog -- get_tool reads self._tools (manager.py:2698), populated only by _index_tools. New EC-FANOUT-9 requires that after a real downstream emission, a client fetching THROUGH the gateway sees the new tool and can invoke it. It is also the only criterion that catches the deadlock above, since a deadlocked re-index never refreshes anything. 3. THE WRAPPER DECISION IS REVERSED: is_version_newer and are_versions_comparable are now DELETED, not kept and policed. The seat overturned the first draft on this roadmap's own principle, and the argument is decisive -- my replacement test was ITSELF syntactic, missing `f = is_version_newer; not f(...)`, `== False`, and the if/else truthiness inversion that reproduces the collapse with no negation at all. That is the #163 lesson applied to the lint's own replacement. Deletion collapses EC-TRISTATE-3 and -4 into absence; the verification is now an import-time assertion rather than a grep. Also: Assumption 2 now cites its evidence instead of asserting it (mcp 2.x opens the standalone GET stream on notifications/initialized, SDK streamable_http.py:566, which pmcp sends at manager.py:2185), and notes that fake_remote drives only the remote loop so stdio needs its own test. EC-UPDPATH-4's doc framing becomes "update_server follows the same live-environment contract as every other spawn path", making the uniformity the invariant rather than a probe-window special case. The seat independently reproduced the other three seats' findings and upheld EC-UPDPATH-4's document-don't-freeze decision, with better reasoning than the roadmap had: a frozen update_server would be the only snapshot-semantics spawn path, and a crash-reconnect would silently un-freeze it anyway.
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.
Amends
specs/phase-plans-v12.md. The fable seat — the fourth panel seat, filled natively rather than through the CLI — reported after #167 merged. Docs only.Three roadmap-changing findings, each verified against source before applying.
1. Inline reconciliation self-deadlocks — and that was the fix I had just applied
_index_capabilitiesawaits_send_request(manager.py:1292). Those futures are resolved by the very read loop that received the notification —pending.future.set_resultat:1791and:2010, the two dispatch functions. Calling reconciliation inline makes the loop await a future only it can resolve. Instant deadlock, not a hypothetical.#167 named the hazard in EC-FANOUT-6 but didn't mandate a mechanism. It now requires a spawned, per-server-coalesced background task — coalescing also bounds a downstream that spams
list_changed.2. A catalog-freshness criterion was missing
Every notification-shaped criterion in #167 can pass while the gateway still serves the old catalog:
get_toolreadsself._tools(manager.py:2698), populated only by_index_tools.New EC-FANOUT-9: after a real downstream emission, a client fetching through the gateway sees the new tool and can invoke it. It is also the only criterion that catches the deadlock above — a deadlocked re-index never refreshes anything, so a slow-subscriber test cannot see it.
3. The wrapper decision is reversed — now deletion
#167 kept
is_version_neweras a wrapper and policed it with a test asserting nosrc/call site negates it. The seat overturned that on this roadmap's own principle, and the argument is decisive:My replacement test was itself syntactic. It misses
f = is_version_newer; not f(...),x == False, andif is_version_newer(...): ... else: <treat as up to date>— the last of which reproduces the collapse with no negation at all. That is precisely the #163 lesson ("a syntactic check cannot prove a dataflow property") applied to the lint's own replacement. A smaller lint inherits the same unsoundness with fewer lines.Grep-verified: after EC-TRISTATE-2 migrates
refresher.py:235and:435, both wrappers have zerosrc/callers. pmcp's public surface is its MCP tool schemas, not this internal module, so there is no compatibility constituency.is_version_orderablestays — genuinely unary.Deletion collapses EC-TRISTATE-3 and -4 into absence, and the verification becomes an import-time assertion rather than a grep.
Also
notifications/initialized(SDKstreamable_http.py:566), which pmcp sends atmanager.py:2185— so the remote path is reachable. Also notesfake_remotedrives only the remote loop, so stdio needs its own test.update_serverfollows the same live-environment contract as every other spawn path", making the uniformity the documented invariant rather than a probe-window special case.What the seat confirmed
It independently reproduced the other three seats' findings — the index-backed catalog chain, the absence of any per-server dimension in the SDK's
SubscriptionFilter(verified:tools_list_changed,prompts_list_changed,resources_list_changed,resource_subscriptionsonly), andcheck_stalenessas a third identity-blind site.It also upheld EC-UPDPATH-4's document-don't-freeze decision, with better reasoning than the roadmap had: a frozen
update_serverwould be the only snapshot-semantics spawn path, and a crash-reconnect minutes later would silently un-freeze it — so the guarantee could not be kept even in principle.Panel tally across both rounds: 4 seats, 4 sets of findings, every one valid. Two of them landed on code I had written in response to an earlier finding.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.