| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟡 The new json_response=True bullet's parenthetical "(ctx.elicit(), sampling)" over-claims for sampling: the default sampling API (ctx.session.create_message()) sends with related_request_id=None, so it rides the connection's standalone channel — which never consults the can_send_request flag this PR wires up — and still routes to the GET stream (or silently hangs if none is attached, the out-of-scope case the PR description acknowledges) rather than raising NoBackChannelError. Only request-scoped calls (ctx.elicit(), Resolve-driven sampling, or an explicit related_request_id) get the new fail-fast; consider dropping "sampling" or qualifying it as request-scoped sampling.
Extended reasoning...What the doc claims vs. what the code does
The new bullet in docs/run/index.md says that under json_response=True "a tool that calls back into the client mid-request (ctx.elicit(), sampling) raises NoBackChannelError on this leg". That is accurate for ctx.elicit() / ctx.elicit_url() and for Resolve-driven sampling, because those stamp related_request_id=ctx.request_id and therefore travel the request-scoped DispatchContext, whose can_send_request this PR sets to False in JSON-response mode. It is not accurate for the default sampling API.
The code path
ServerSession.create_message() defaults related_request_id=None, and ServerSession.send_request() selects the channel with channel = self._request_outbound if related is not None else self._connection.outbound (src/mcp/server/session.py:88-89). With no related id, sampling rides the connection's standalone channel. For a stateful loop session, Connection.for_loop installs the JSONRPCDispatcher itself as that outbound, and its send_raw_request never consults can_send_request — only the per-message DispatchContext does (src/mcp/shared/jsonrpc_dispatcher.py:167). The mcpserver Context has no sampling helper that stamps a related id (only elicit/elicit_url do, src/mcp/server/mcpserver/context.py:185,218), so the ordinary way a tool samples — ctx.session.create_message(...) — goes standalone by default.
Why the new guards don't fire on this path
Both new mechanisms in this PR are keyed to the request-scoped leg. The transport_context_for fail-fast is only checked in DispatchContext.send_raw_request, which the standalone path bypasses. And the new message_router drop branch in src/mcp/server/streamable_http.py requires related_request_id is not None before it fires; a sampling request with no related id falls through to GET_STREAM_KEY as before.
Step-by-step proof
(If the client has a GET stream attached, the request is delivered there instead — also not the documented NoBackChannelError.)
Why this is a nit
Nothing in the code is wrong — the PR's actual behavior change is correct, tested, and matches its description. The inaccuracy is one word in one new doc bullet; the neighboring legacy-clients.md note and troubleshooting.md entries are correctly scoped to the request-scoped channel. All verifiers who traced this agreed.
Suggested fix
Drop "sampling" from the parenthetical, or qualify it, e.g.: "a tool that calls back into the client on the request-scoped channel (ctx.elicit(), sampling with a related request id)".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.