Title: fix: never mint a request id already used by a completed caller-supplied id by ayaangazali · Pull Request #3127 · modelcontextprotocol/python-sdk · GitHub
Open Graph Title: fix: never mint a request id already used by a completed caller-supplied id by ayaangazali · Pull Request #3127 · modelcontextprotocol/python-sdk
X Title: fix: never mint a request id already used by a completed caller-supplied id by ayaangazali · Pull Request #3127 · modelcontextprotocol/python-sdk
Description: Summary Fixes #3126. When a caller supplies its own request id via CallOptions["request_id"], the dispatcher's minted-id sequence could later land on that same id once the supplied request completed, sending two different requests under one id in the same session. The spec says the requestor must never reuse a request id within a session, and the SDK pins the same rule as protocol:request-id:unique in tests/interaction/_requirements.py. Root cause Both mint loops only skipped ids still in flight: request_id = self._allocate_id() while request_id in self._pending: # entries are popped on completion request_id = self._allocate_id() The comment right above ("Mint past any key a supplied id occupies") shows the intent, but the guard stopped holding the moment the supplied request completed. Since coerce_request_id folds "2" and 2 into one key, numeric-string ids hit this too. DirectDispatcher had the same pattern with _in_flight_ids (discarded in a finally). Fix Accepting a supplied id now advances the mint counter past its coerced integer key: if isinstance(pending_key, int): self._next_id = max(self._next_id, pending_key) That makes the old skip loops provably unreachable (the counter is always past every supplied integer key and every previously minted id), so they are removed rather than left as dead branches the coverage gate would flag. Two deliberate scope notes: A caller re-supplying its own completed id stays allowed; that behavior is asserted by test_send_raw_request_with_in_flight_request_id_raises_and_frees_id_on_completion and this PR does not touch it. Only the dispatcher's own minting is constrained. Minted id values shift when a caller supplies integer ids (e.g. the pinned sequence in test_minted_ids_skip_a_caller_supplied_id_still_in_flight went from [1, 2, 4] to [4, 5, 6]). Ids are opaque per spec, so I did not treat this as a breaking change needing a migration entry; happy to add one if you see it differently. Tests New test_minted_ids_never_reuse_a_completed_supplied_id, parametrized over both dispatchers and both id spellings (2, "2"). Fails on current main with wire ids [2, 1, 2]; passes with the fix ([2, 3, 4]). Updated the pinned sequence in test_minted_ids_skip_a_caller_supplied_id_still_in_flight. CallOptions.request_id docstring extended with the new guarantee. Ran locally: ./scripts/test # 5448 passed, coverage 100.00%, strict-no-cover clean uv run --frozen ruff format . && uv run --frozen ruff check . uv run --frozen pyright # 0 errors AI disclosure, per CONTRIBUTING: Claude Code helped me find this and sanity-check the invariant reasoning; I wrote up the repro, ran everything locally, and I can defend each line here in my own words. First PR to this repo and I'm a freshman still learning the ropes, so if the counter-advance approach clashes with something I haven't seen, please say so and I'll rework it.
Open Graph Description: Summary Fixes #3126. When a caller supplies its own request id via CallOptions["request_id"], the dispatcher's minted-id sequence could later land on that same id once the supplied re...
X Description: Summary Fixes #3126. When a caller supplies its own request id via CallOptions["request_id"], the dispatcher's minted-id sequence could later land on that same id once the...
Opengraph URL: https://github.com/modelcontextprotocol/python-sdk/pull/3127
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:8044798d-ee1d-db49-9ab8-da1851219edc |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 8CF0:3C6F7B:627EBCB:84003F2:6A5DF82B |
| html-safe-nonce | 2dcb2e39a4af44f73aad23d1b9472ef0ff7c50b928a1472cec538bc76e0d144b |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4Q0YwOjNDNkY3Qjo2MjdFQkNCOjg0MDAzRjI6NkE1REY4MkIiLCJ2aXNpdG9yX2lkIjoiNzIzMzk3MTk0MTkzODE2NTgwMyIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | 041efb39e05a47e88bf933a85859ce8e24ed9adfbb2e278872b57d24c7e81dc8 |
| hovercard-subject-tag | pull_request:4082098681 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/modelcontextprotocol/python-sdk/pull/3127/files |
| twitter:image | https://avatars.githubusercontent.com/u/231403453?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/231403453?s=400&v=4 |
| og:image:alt | Summary Fixes #3126. When a caller supplies its own request id via CallOptions["request_id"], the dispatcher's minted-id sequence could later land on that same id once the supplied re... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 7f8603048a7a8270fc5b845c28ec71e80a9a899268387bcf65814ee1b51c0cc4 |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/modelcontextprotocol/python-sdk git https://github.com/modelcontextprotocol/python-sdk.git |
| octolytics-dimension-user_id | 182288589 |
| octolytics-dimension-user_login | modelcontextprotocol |
| octolytics-dimension-repository_id | 862584018 |
| octolytics-dimension-repository_nwo | modelcontextprotocol/python-sdk |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 862584018 |
| octolytics-dimension-repository_network_root_nwo | modelcontextprotocol/python-sdk |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | e2a5561aeb049d0632a92906af784af5d414c603 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width