Skip to content

Commit efa9c85

Browse files
committed
Address by-construction review: ServerSession reads standalone channel from Connection; hygiene
- ServerSession: drop standalone_outbound kwarg; read connection.outbound at the channel-select sites (one source of truth for the standalone channel; the documented two-arg form is now correct rather than mis-wiring) - serve_loop: add to __all__; list in module docstring; soften the recipe-ownership claim - _write: drop dead extra_headers kwarg - _SingleExchangeDispatchContext.can_send_request: field(init=False) so True is unconstructible - inbound: use UnsupportedProtocolVersionErrorData typed model for -32022 data; document why INTERNAL_ERROR is unmapped; anchor the header-validation TODO - Sharpen the notification-POST rejection comment to cite the spec's cannot-accept branch - Docstring corrections: session.py module docstring (per-request, not per-connection); Server.run() drops the internal tracking ref - TODO anchors aligned to current tracking entries - Tests: use public client_params property; complete ERROR_CODE_HTTP_STATUS parametrize; assert .error.code over message-text regex
1 parent d3b0977 commit efa9c85

12 files changed

Lines changed: 52 additions & 63 deletions

src/mcp/server/_streamable_http_modern.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class _SingleExchangeDispatchContext:
6767
request_id: RequestId
6868
message_metadata: MessageMetadata
6969
cancel_requested: anyio.Event = field(default_factory=anyio.Event)
70-
can_send_request: bool = False
70+
can_send_request: bool = field(default=False, init=False)
7171

7272
async def send_raw_request(
7373
self,
@@ -104,8 +104,6 @@ async def _write(
104104
scope: Scope,
105105
receive: Receive,
106106
send: Send,
107-
*,
108-
extra_headers: Mapping[str, str] | None = None,
109107
) -> None:
110108
"""Serialise a JSON-RPC reply with the table-mapped HTTP status."""
111109
status = ERROR_CODE_HTTP_STATUS.get(msg.error.code, _OK_STATUS) if isinstance(msg, JSONRPCError) else _OK_STATUS
@@ -118,7 +116,6 @@ async def _write(
118116
json.dumps(body, separators=(",", ":")),
119117
status_code=status,
120118
media_type="application/json",
121-
headers=dict(extra_headers) if extra_headers else None,
122119
)(scope, receive, send)
123120

124121

@@ -163,10 +160,13 @@ async def handle_modern_request(
163160
try:
164161
req = JSONRPCRequest.model_validate(decoded)
165162
except ValidationError:
166-
# Well-formed JSON that isn't a single request object (a notification,
167-
# a response, a batch). TODO(L57): the 2026 HTTP path carries no
168-
# client→server notifications (cancellation is via SSE close), so a
169-
# plain rejection is currently sufficient.
163+
# Well-formed JSON that isn't a single request object. The transport
164+
# spec permits notification POSTs and gives the server two responses
165+
# (202 accept / 4xx cannot-accept; streamable-http §Sending Messages
166+
# item 5). The core protocol defines no client→server notifications
167+
# over HTTP at 2026-07-28 (cancellation is SSE-stream close), so this
168+
# entry takes the cannot-accept branch. TODO(L57): S4 owns the
169+
# strict-vs-lenient choice.
170170
rej = JSONRPCError(
171171
jsonrpc="2.0",
172172
id=None,

src/mcp/server/lowlevel/server.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ def __init__(
226226
# Context-tier middleware: wraps every inbound request (including
227227
# `initialize`, lookup, validation, handler) with
228228
# `(ctx, method, params, call_next)`. Applied in `ServerRunner._on_request`.
229-
# TODO(maxisbey): provisional - signature and semantics change with the
229+
# TODO(L54): provisional - signature and semantics change with the
230230
# Context/middleware rework (covariant `Context[L]`, outbound seam) before
231231
# v2 final.
232232
self.middleware: list[ServerMiddleware[LifespanResultT]] = []
@@ -439,7 +439,7 @@ async def run(
439439
) -> None:
440440
"""Serve a single connection over the given streams until the read side closes.
441441
442-
Thin wrapper over `serve_loop` (L28): enters the server lifespan,
442+
Thin wrapper over `serve_loop`: enters the server lifespan,
443443
then drives the loop. Transports with their own lifespan owner
444444
(the streamable-HTTP manager) call `serve_loop` directly instead.
445445
"""

src/mcp/server/runner.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
pure kernel: it holds a pre-populated `Connection` and reads
66
`connection.protocol_version` / `connection.outbound` as facts. Driving a
77
dispatcher loop and tearing down the connection live in the free-function
8-
drivers (`serve_connection`, `serve_one`); the entry constructs the
9-
`Connection`, the driver tears it down.
8+
drivers (`serve_connection`, `serve_loop`, `serve_one`); the entry constructs
9+
the `Connection`, the driver tears it down.
1010
1111
`ServerRunner` holds a `Server` directly - `Server` is the registry.
1212
"""
@@ -65,6 +65,7 @@
6565
"aclose_shielded",
6666
"otel_middleware",
6767
"serve_connection",
68+
"serve_loop",
6869
"serve_one",
6970
"to_jsonrpc_response",
7071
]
@@ -362,7 +363,7 @@ def _compose_server_middleware(
362363
def _make_context(
363364
self, dctx: DispatchContext[TransportContext], meta: RequestParamsMeta | None, protocol_version: str
364365
) -> ServerRequestContext[LifespanT, Any]:
365-
# TODO(maxisbey): remove for Context rework. Reads the SHTTP per-request
366+
# TODO(L54): remove for Context rework. Reads the SHTTP per-request
366367
# data off the raw `dctx.message_metadata` carrier; replace with the
367368
# per-transport context once that lands.
368369
md = dctx.message_metadata
@@ -373,9 +374,9 @@ def _make_context(
373374
else:
374375
request = close_sse_stream = close_standalone_sse_stream = None
375376
# Per-request session: `dctx` is the request-scoped channel (auto-threads
376-
# its own request_id on streamable HTTP); `connection.outbound` is the
377-
# standalone channel. `related_request_id` on the public API selects.
378-
session = ServerSession(dctx, self.connection, standalone_outbound=self.connection.outbound)
377+
# its own request_id on streamable HTTP); the standalone channel is read
378+
# off `connection.outbound`. `related_request_id` on the public API selects.
379+
session = ServerSession(dctx, self.connection)
379380
return ServerRequestContext(
380381
session=session,
381382
lifespan_context=self.lifespan_state,
@@ -450,8 +451,8 @@ async def serve_loop(
450451
"""Drive ``server`` in loop mode over a stream pair until the channel closes.
451452
452453
Builds the loop-mode `JSONRPCDispatcher` + `Connection` and hands them to
453-
`serve_connection`, so the dispatcher-construction recipe (notably the
454-
`inline_methods={"initialize"}` rule) lives in one place. Callers that own
454+
`serve_connection`, so loop-mode callers share one dispatcher-construction
455+
recipe (notably the `inline_methods={"initialize"}` rule). Callers that own
455456
a lifespan (the streamable-HTTP manager) pass it in; callers that don't
456457
(`Server.run` for stdio/memory) enter the lifespan and then call this.
457458
"""

src/mcp/server/session.py

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
"""`ServerSession`: server-to-client requests and notifications.
22
3-
A thin proxy over `JSONRPCDispatcher` and `Connection`. One instance per
4-
client connection (built by `ServerRunner`). Handlers reach it as
5-
`ctx.session` and use the typed helpers (`create_message`, `elicit_form`,
3+
A per-request proxy built by the kernel for each inbound request. Exposes the
4+
request-scoped outbound channel and the connection's standalone channel.
5+
Handlers reach it as `ctx.session` and use the typed helpers (`elicit_form`,
66
`send_log_message`, ...) to call back to the client.
7-
8-
The receive-loop, initialize handling, and per-request task isolation that
9-
used to live here are now owned by `JSONRPCDispatcher` and `ServerRunner`.
107
"""
118

129
from typing import Any, TypeVar, overload
@@ -39,15 +36,8 @@ class ServerSession:
3936
never crosses the `Outbound` Protocol.
4037
"""
4138

42-
def __init__(
43-
self,
44-
request_outbound: Outbound,
45-
connection: Connection,
46-
*,
47-
standalone_outbound: Outbound | None = None,
48-
) -> None:
39+
def __init__(self, request_outbound: Outbound, connection: Connection) -> None:
4940
self._request_outbound = request_outbound
50-
self._standalone_outbound = standalone_outbound if standalone_outbound is not None else request_outbound
5141
self._connection = connection
5242

5343
@property
@@ -81,7 +71,7 @@ async def send_request(
8171
pydantic.ValidationError: The peer's result does not match `result_type`.
8272
"""
8373
related = metadata.related_request_id if metadata is not None else None
84-
channel = self._request_outbound if related is not None else self._standalone_outbound
74+
channel = self._request_outbound if related is not None else self._connection.outbound
8575
data = request.model_dump(by_alias=True, mode="json", exclude_none=True)
8676
opts: CallOptions = {}
8777
if request_read_timeout_seconds is not None:
@@ -101,7 +91,7 @@ async def send_notification(
10191
related_request_id: types.RequestId | None = None,
10292
) -> None:
10393
"""Send a typed server-to-client notification."""
104-
channel = self._request_outbound if related_request_id is not None else self._standalone_outbound
94+
channel = self._request_outbound if related_request_id is not None else self._connection.outbound
10595
data = notification.model_dump(by_alias=True, mode="json", exclude_none=True)
10696
await channel.notify(data["method"], data.get("params"))
10797

src/mcp/server/streamable_http_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
346346
await http_transport.handle_request(scope, receive, send)
347347
else:
348348
# Unknown or expired session ID - return 404 per MCP spec
349-
# TODO: Align error code once spec clarifies
349+
# TODO(L62): Align error code once spec clarifies
350350
# See: https://github.com/modelcontextprotocol/python-sdk/issues/1821
351351
logger.info(f"Rejected request with unknown or expired session ID: {request_mcp_session_id[:64]}")
352352
body = JSONRPCError(

src/mcp/shared/inbound.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
CLIENT_CAPABILITIES_META_KEY,
1919
CLIENT_INFO_META_KEY,
2020
PROTOCOL_VERSION_META_KEY,
21+
UnsupportedProtocolVersionErrorData,
2122
)
2223
from mcp.types.jsonrpc import (
2324
HEADER_MISMATCH,
@@ -40,6 +41,8 @@
4041
MCP_PROTOCOL_VERSION_HEADER: Final = "mcp-protocol-version"
4142
"""Canonical lowercase name of the HTTP header carrying the MCP protocol version."""
4243

44+
# INTERNAL_ERROR is deliberately unmapped (→ HTTP 200): the spec assigns no status to
45+
# -32603, and whether handler-origin errors get 5xx is an open S4 question — see TODO(L66).
4346
ERROR_CODE_HTTP_STATUS: Final[Mapping[int, int]] = MappingProxyType(
4447
{
4548
PARSE_ERROR: 400,
@@ -127,6 +130,7 @@ def classify_inbound_request(
127130
"client-capabilities envelope keys",
128131
)
129132

133+
# TODO(L59): also validate Mcp-Method / Mcp-Name per SEP-2243 §Server Validation
130134
if headers is not None and headers.get(MCP_PROTOCOL_VERSION_HEADER) != protocol_version:
131135
return InboundLadderRejection(
132136
code=HEADER_MISMATCH,
@@ -137,7 +141,9 @@ def classify_inbound_request(
137141
return InboundLadderRejection(
138142
code=UNSUPPORTED_PROTOCOL_VERSION,
139143
message="Unsupported protocol version",
140-
data={"supported": list(supported_modern_versions), "requested": protocol_version},
144+
data=UnsupportedProtocolVersionErrorData(
145+
supported=list(supported_modern_versions), requested=protocol_version
146+
).model_dump(mode="json"),
141147
)
142148

143149
return InboundModernRoute(

src/mcp/shared/jsonrpc_dispatcher.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ async def _handle_request(
684684
# anyio absorbs the scope's own cancel at __exit__, and
685685
# `cancelled_caught` (unlike `cancel_called`) guarantees the
686686
# result write above did not happen - no double response.
687-
# TODO(maxisbey): spec says SHOULD NOT respond after cancel;
687+
# TODO(L38): spec says SHOULD NOT respond after cancel;
688688
# the existing server always has, so match that for now.
689689
answer_write_started = True
690690
await self._write_error(req.id, ErrorData(code=0, message="Request cancelled"))
@@ -707,7 +707,7 @@ async def _handle_request(
707707
await self._write_error(req.id, error)
708708
else:
709709
logger.exception("handler for %r raised", req.method)
710-
# TODO(L19): code=0 pins existing-server compat; JSON-RPC says
710+
# TODO(L58): code=0 pins existing-server compat; JSON-RPC says
711711
# INTERNAL_ERROR. Revisit per the suite's divergence entry.
712712
await self._write_error(req.id, ErrorData(code=0, message=str(e)))
713713
if self._raise_handler_exceptions:

tests/server/test_session.py

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ def _make_session(
5757
"""Single-channel session: the stub is both request and standalone outbound."""
5858
client_info = Implementation(name="c", version="0") if capabilities is not None else None
5959
conn = Connection.from_envelope(protocol_version, client_info, capabilities, outbound=outbound)
60-
return ServerSession(outbound, conn, standalone_outbound=outbound)
60+
return ServerSession(outbound, conn)
6161

6262

6363
def _two_channel_session(request_ch: Outbound, standalone_ch: Outbound) -> ServerSession:
6464
"""Distinct request/standalone outbounds so routing assertions can tell the channels apart."""
6565
conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None, outbound=standalone_ch)
66-
return ServerSession(request_ch, conn, standalone_outbound=standalone_ch)
66+
return ServerSession(request_ch, conn)
6767

6868

6969
@pytest.mark.anyio
@@ -145,23 +145,6 @@ async def test_send_notification_routes_by_related_request_id():
145145
assert [m for m, _ in request_ch.notifications] == ["notifications/progress"]
146146

147147

148-
@pytest.mark.anyio
149-
async def test_standalone_outbound_defaults_to_request_outbound():
150-
"""SDK-defined: omitting `standalone_outbound` collapses both channels onto the
151-
request one — the duplex case (stdio) where a single stream serves both.
152-
153-
`conn.outbound` is the no-channel sentinel here, so traffic landing on `only`
154-
proves the default is the request outbound (the alternate impl — defaulting to
155-
`conn.outbound` — would raise `NoBackChannelError`)."""
156-
only = StubOutbound(result={"roots": []})
157-
conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
158-
session = ServerSession(only, conn)
159-
await session.send_request(types.ListRootsRequest(), types.ListRootsResult)
160-
await session.send_notification(types.ToolListChangedNotification())
161-
assert only.requests[0][0] == "roots/list"
162-
assert only.notifications[0][0] == "notifications/tools/list_changed"
163-
164-
165148
@pytest.mark.anyio
166149
async def test_send_request_validates_the_client_result_against_the_surface_schema():
167150
"""A spec-method result that fails the per-version surface schema raises
@@ -228,6 +211,6 @@ def test_protocol_version_proxies_connection():
228211
"""SDK-defined: `session.protocol_version` reads through to the held `Connection`."""
229212
_ARBITRARY_VERSION = "sentinel-version" # identity-only: any string the connection holds
230213
conn = Connection.from_envelope(_ARBITRARY_VERSION, None, None)
231-
session = ServerSession(StubOutbound(), conn, standalone_outbound=conn.outbound)
214+
session = ServerSession(StubOutbound(), conn)
232215
assert session.protocol_version == _ARBITRARY_VERSION
233216
assert session.client_params is None

tests/server/test_stateless_mode.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def _no_channel_session(request_ch: StubOutbound | None = None) -> tuple[ServerS
4747
conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
4848
assert conn.has_standalone_channel is False
4949
request = request_ch if request_ch is not None else StubOutbound()
50-
return ServerSession(request, conn, standalone_outbound=conn.outbound), request
50+
return ServerSession(request, conn), request
5151

5252

5353
@pytest.fixture
@@ -154,7 +154,7 @@ async def test_loop_connection_outbound_does_not_raise_no_back_channel():
154154
standalone = StubOutbound(result={"roots": []})
155155
conn = Connection.for_loop(standalone)
156156
assert conn.has_standalone_channel is True
157-
session = ServerSession(StubOutbound(), conn, standalone_outbound=conn.outbound)
157+
session = ServerSession(StubOutbound(), conn)
158158
result = await session.list_roots() # pyright: ignore[reportDeprecated]
159159
assert isinstance(result, types.ListRootsResult)
160160
assert standalone.requests[0][0] == "roots/list"

tests/server/test_streamable_http_modern.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ async def test_handle_modern_request_routes_with_mis_shaped_envelope_client_info
129129
seen: list[object] = []
130130

131131
async def greet(ctx: ServerRequestContext, params: PaginatedRequestParams) -> dict[str, Any]:
132-
seen.append(ctx.session._connection.client_params)
132+
seen.append(ctx.session.client_params)
133133
return {"ok": True}
134134

135135
server: Server[Any] = Server("test")

0 commit comments

Comments
 (0)