Skip to content

Drop _compat.py: if mcp/python-sdk#2610 still silent on 2026-06-12, port the fix as our own upstream PR — due 2026-06-12 #127

Description

@mlorentedev

Due date: 2026-06-12 (+21 days from the HIVE-115 PR-4 ship)

Why this issue exists

src/hive/_compat.py is a self-gated monkey-patch on mcp.shared.session.RequestResponder that works around the cancel-race tracked upstream at modelcontextprotocol/python-sdk#2610.

As of 2026-05-22:

  • Issue #2610 has been open for ~weeks with no maintainer engagement.
  • Independent confirmation of the same bug + same fix pattern was posted by jshaofa-ui on 2026-05-16 (we are not the only ones).
  • We have a battle-tested fix sitting in _compat.py — the patch is small, self-gated, degrades safely.
  • PR chore(deps): pin mcp <2.0 to guard _compat.py shim #125 capped mcp >= 1.26, <2.0 so the shim cannot silently break against a major refactor.

That gives the upstream maintainer a reasonable window to engage. 21 days is the escalation point.

The decision on 2026-06-12

On that date, evaluate:

  • Is modelcontextprotocol/python-sdk#2610 still OPEN?
  • Is there any maintainer comment since 2026-05-22 (even "we're looking at it" counts)?
  • Has any related PR been opened upstream?
  • Does the current mcp release ship a working fix (check mcp/shared/session.py for any guard around _completed)?

Outcome routes

  • Maintainer engaged / fix in flight → comment on this issue with the link, leave open until upstream merges, then drop _compat.py and bump mcp requirement.
  • Still silent + still broken → escalate: port src/hive/_compat.py into a clean PR against modelcontextprotocol/python-sdk. The shim is already self-contained and reviewable. Include the cancel-race regression test from tests/test_compat_shim.py::test_classify_cancellation_race so the upstream test suite gains coverage. Comment here with the PR link. Set a new deadline (+30d) to check on the PR review.
  • Bug reproduces, but we don't have bandwidth → comment here, defer this issue 14 days. Acceptable but document the choice.

Belt-and-braces

This issue is one of three notification layers (per _meta/patterns/pattern-triple-reminder-belt-and-braces in the maintainer's vault):

  • This GitHub issue (persistent record)
  • Remote routine queued for 2026-06-12 09:07 local (push notification via GitHub comment)
  • Dated TODO in vault 10_projects/hive/11-tasks.md

Any one of them is sufficient to surface the deadline; the others are redundancy against single-channel failure modes.

cc @mlorentedev

Activity

  1. mlorentedev commented on Jun 12, 2026

    @mlorentedev
    OwnerAuthor

    +21d escalation checkpoint — upstream still silent

    Hey @mlorentedev — it has been 21 days since PR-4 shipped. Upstream modelcontextprotocol/python-sdk#2610 is still OPEN with no new maintainer engagement since 2026-05-22. The installed mcp 1.27.2 (latest within the pinned >=1.26,<2.0 range) still carries the raw assertion at mcp/shared/session.py:129:

    assert not self._completed, "Request already responded to"

    No if self._completed: return guard exists anywhere in RequestResponder.__exit__ or respond(). The upstream fix has not shipped.

    Recommended action: port the fix ourselves

    src/hive/_compat.py is small, self-gated, and battle-tested across the four HIVE-115 PRs. Porting it to upstream is mostly a translation exercise:

    1. Fork modelcontextprotocol/python-sdk.
    2. Apply the same patch shape to mcp/shared/session.py (the _patched_exit and _patched_respond logic from _compat.py becomes inline guards in RequestResponder.__exit__ and respond).
    3. Port the regression test from tests/test_compat_shim.py::test_classify_cancellation_race — adapt to upstream's test layout.
    4. Open the PR referencing modelcontextprotocol/python-sdk#2610 and mlorentedev/hive#127.

    Estimated effort

    • ~2 hours coding (the work is already done; just translation + test port)
    • ~30 min review prep (cite the two independent reports: ours + jshaofa-ui 2026-05-16)
    • Variable review wait at upstream

    Outcome routes from here

    • Port the fix — open the upstream PR, comment its URL here, leave this issue open. Set a new +30d deadline to check on review state.
    • Defer 14 more days — comment with reason, leave open, update title due-date to 2026-06-26.
    • Accept _compat.py as permanent — comment with rationale (shim is stable, pin caps risk, upstream effort budget needed elsewhere), close with wontfix label. Reopen only if the shim begins failing.

    Posted by the +21d remote routine (one-shot, scheduled 2026-05-22 alongside this issue + the vault TODO for belt-and-braces — _meta/patterns/pattern-triple-reminder-belt-and-braces). Do not reschedule; the next step is a human call.


    Generated by Claude Code

  2. mlorentedev commented on Jun 19, 2026

    @mlorentedev
    OwnerAuthor

    Decision gate resolved — re-pointed to two distinct upstream races

    Re-evaluated against installed mcp 1.26.0. The original gate tracked python-sdk#2610, but the _compat.py workaround actually covers two distinct races with two distinct upstream trackers — so the gate is now split accordingly.

    1. __exit__ CancelledError leak (issue #75) — REMOVED ✅

    Upstream: python-sdk#2610 (open) · fix proposed in PR #2624 (open).

    On mcp 1.26.0 the issue-#75 symptom no longer reproduces. Server._handle_request catches the in-flight handler cancellation before it can reach RequestResponder.__exit__:

    except anyio.get_cancelled_exc_class():
        logger.info("Request %s cancelled - duplicate response suppressed", message.request_id)
        return

    That guard masks the leak regardless of whether #2610/#2624 land, so the __exit__ monkey-patch is redundant and has been removed.

    Coverage was preserved, not dropped: the old reproduction was a Windows-only subprocess test, so a new deterministic, cross-platform regression test now drives a real lowlevel Server receive loop and sends an actual notifications/cancelled mid-handler — tests/test_transport_recovery.py::TestProtocolLevelCancellation. It fails on any platform if the masking behaviour regresses (e.g. a future mcp/anyio bump).

    2. respond() "Request already responded to" assertion — STILL PATCHED

    Upstream: python-sdk#2416 (open) — not #2624 (that PR fixes only __exit__).

    This is the narrow race where notifications/cancelled marks the responder _completed between handler completion and respond(), firing assert not self._completed at mcp/shared/session.py:129. hive is unusually exposed via _helpers.run_sync_tool (git on a worker thread that can call respond() late). The respond short-circuit in hive._compat stays, guarded by tests/test_concurrency_fixes.py::TestRespondPatchPreventsKill.

    New gate

    Watch python-sdk#2416. Drop the remaining respond patch once a guarded respond() ships within our pinned mcp >=1.26,<2.0 range. Re-check by 2026-07-18.


    Housekeeping: the previous checkpoint referenced tests/test_compat_shim.py::test_classify_cancellation_race, which no longer exists — the guards now live in tests/test_concurrency_fixes.py and tests/test_transport_recovery.py.

  3. mlorentedev commented on Jun 19, 2026

    @mlorentedev
    OwnerAuthor

    Correction — re-validated on the pinned mcp (1.27.2), now in PR #248

    My checkpoint above was drafted against a local env pinned to mcp 1.26.0 on a stale branch. The repo actually pins mcp>=1.27,<2.0, so I re-validated against mcp 1.27.2 before implementing. The conclusions hold:

    • The __exit__ patch is redundant on mcp >=1.27: a real notifications/cancelled mid-handler on a bare lowlevel Server with no hive patches survives on 1.27.2 (Server._handle_request catches the in-flight cancellation via except anyio.get_cancelled_exc_class(): ... return when message.cancelled, before RequestResponder.__exit__).
    • The respond() race (python-sdk#2416) is not covered by that guard and stays patched (with its ghost-response observability).

    Implemented in #248 — removes the __exit__ patch, keeps the respond() patch, and adds a deterministic cross-platform regression test (TestProtocolLevelCancellation) so the masking behaviour is guarded on every platform (not just the prior Windows-only subprocess test).

    Gate unchanged: watch python-sdk#2416; re-check 2026-07-18.

  4. mlorentedev commented on Aug 8, 2026

    @mlorentedev
    OwnerAuthor

    This ticket's premise no longer holds

    Re-checked today against the code and all three upstream trackers. The escalation condition here is written as "if modelcontextprotocol/python-sdk#2610 is still silent on 2026-06-12, port the fix as our own upstream PR". Both halves of that have moved.

    1. _compat.py no longer depends on #2610. The __exit__ patch this ticket was written about has already been removed. _compat.py's docstring records why: the issue-#75 symptom stopped reproducing on mcp >= 1.27, because Server._handle_request catches the in-flight handler cancellation (except anyio.get_cancelled_exc_class(): ... return when message.cancelled) before it can reach RequestResponder.__exit__ — masking the leak whether or not #2610 ever lands. tests/test_transport_recovery.py guards that masking behaviour cross-platform.

    What remains in _compat.py is a patch on RequestResponder.respond, for a different upstream bug.

    2. #2610 already has an upstream fix PR. #2624 ("Fix completed request cancellation cleanup") is open. Porting the __exit__ fix ourselves would duplicate it.

    What the ticket should be about

    The shim we actually still carry tracks python-sdk#2416 — "AssertionError: Request already responded to — cancellation race in v1.27.0". Current state:

    • Open, last activity 2026-07-11.
    • Maintainer bot confirmed the bug on both main and origin/v1.x; respond() asserts not self._completed at session.py:133.
    • Reproduced independently by another user on Windows via Claude Code's stdio transport.
    • We already posted our reproduction and the guard we use, on 2026-06-19.
    • On 2026-07-11 a contributor said they intend to fix it — "replace assert not self._completed with a silent early return in RequestResponder.respond()", which is materially the same fix as ours. No PR has appeared in the ~4 weeks since.

    So the escalation logic still applies, just aimed at #2416 instead. The etiquette wrinkle is that the issue is informally claimed: opening a competing PR without a ping would be rude. Suggested sequence — comment on #2416 offering to open the PR if the volunteer has stalled, wait a short window, then port our guard.

    Adjacent finding

    AGENTS.md carried all three stale facts (wrong patch target, wrong tracker, and a stale mcp pin quoted as >=1.26,<2.0 when it is now >=1.27,<3.0). Corrected in #336.

    That PR also flags a lost guard: the narrow <2.0 cap existed so a major RequestResponder refactor could not silently break the shim, and #316 widened it to <3.0, which admits mcp 2.x — exactly the boundary the cap excluded. Not a live bug (apply() degrades loudly), but worth a deliberate decision on whether to re-narrow.

    Proposed retitle / rescope

    Drop _compat.py: gated on python-sdk#2416 (respond-after-cancel), not #2610 — ping the volunteer, then port our guard if stalled

  5. self-assigned this
    on Aug 8, 2026
  6. mlorentedev commented on Aug 8, 2026

    @mlorentedev
    OwnerAuthor

    Upstream state verified 2026-08-07 — the premise in this issue and in the last handoff was wrong on two counts

    1. The tracker is #2416, not #2610. This issue is written entirely around #2610, but the __exit__ patch that #2610 covers was already removed — the issue-#75 symptom stopped reproducing on mcp >= 1.27, where Server._handle_request catches the cancellation first. What _compat.py actually patches today is RequestResponder.respond, which is #2416. (#2610 also already has an open upstream fix PR, #2624.) This was established in the 2026-08-07 review; recording it here so the issue body stops pointing at the wrong tracker.

    2. "Claimed 2026-07-11 with no PR since" was false. The handoff carried this, but the claim was a PR:

    Date Event
    2026-04-10 #2418 opened — still open, but untouched since the day it was created
    2026-05-18 #2417 closed by maxisbey, no comment
    2026-06-19 We commented on #2416 with hive's guard
    2026-06-23 #2949 closed by maxisbey, no comment
    2026-07-11 shivamsingh-007 claims it and opens #3088 the same day
    2026-07-19 shivamsingh-007 closes their own PR

    The reason the handoff missed this: PRs referencing an issue do not appear in that issue's comment thread once closed unmerged. They have to be searched for separately.

    What this changes

    The lane is clear — there is no active claimant to defer to, so the "ping the claimant first" protocol no longer applies. But a new signal replaces it: four community PRs, none merged, two closed by a core contributor without any stated reason. A fifth blind community PR is a poor bet.

    So the escalation was re-shaped rather than executed: a maintainer-directed comment now asks whether a fix would be accepted and against which base (main, v1.x, or both) before any PR is written — #2416 comment.

    Next step: wait for a maintainer answer. PR authorization is deferred until there is one, or until silence makes the answer moot.

  7. mlorentedev commented on Sep 25, 2026

    @mlorentedev
    OwnerAuthor

    Superseded by #443 (merged 2026-09-25). hive now requires mcp 2.x, whose dispatcher never answers a cancelled request, so the RequestResponder.respond patch was deleted instead of waiting for python-sdk#2416 (which stays open for the 1.x line). tests/test_cancel_race.py guards the behaviour: it fails on 1.x without the patch and passes on 2.2.0.

    src/hive/_compat.py still exists, but it holds only the GHOST_RESPONSES counter. Deleting the file, and deciding what feeds the counter's cancellation source on 2.x, is tracked in #442. Closing this as done by #443.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions