Skip to content

Commit c0462d2

Browse files
committed
Pin initialize-abandon suppression and tidy the interaction suite
- Add an interaction test proving a timed-out initialize sends no notifications/cancelled, and drop the stale deferred rationale for the initialize-not-cancellable requirement - Record null-id error-response drops in the requirements manifest and describe the cancelled-request error response as applying to both seats - Record wire messages only after the inner send succeeds, so a failed or cancelled write is not logged as sent - Bound the remaining indefinite waits, refresh comments stale since the courtesy-cancel change, and assert the issue-88 timeout by error code - Rewrite the suite README's concurrency section for the dispatcher model
1 parent 20aff7a commit c0462d2

6 files changed

Lines changed: 112 additions & 24 deletions

File tree

tests/interaction/README.md

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,13 @@ many requirements at once; if the assertions would be separate, write separate t
193193

194194
### Notifications and concurrency
195195

196-
The client's receive loop dispatches each incoming message to completion before reading the next,
197-
and the in-memory transport delivers everything on one ordered stream. Together these guarantee
198-
that every notification a server handler emits before its response reaches the client callback
199-
before the originating request returns — so tests collect notifications into a plain list and
200-
assert after the call, with no synchronisation. The exceptions:
196+
The client's dispatcher starts a task per incoming notification in arrival order but does not
197+
await it before reading the next message, so completion order is not structural. What still
198+
holds: the in-memory transport delivers everything on one ordered stream, and a callback that
199+
records synchronously (no `await` before the append) finishes its scheduling slice before the
200+
awaited request's waiter — woken strictly later — resumes. So tests whose callbacks are plain
201+
appends may still collect into a list and assert after the call. A callback that awaits before
202+
recording loses that ordering and must synchronise. The other exceptions:
201203

202204
- a notification not triggered by a request the test is awaiting needs an `anyio.Event` set in
203205
the receiving handler and awaited under `anyio.fail_after(5)`;
@@ -220,9 +222,8 @@ but still inside an outer `async with`, and no restructure can avoid it.
220222

221223
A handful of `# pragma: lax no cover` markers in `src/` cover teardown exception handlers whose
222224
execution is timing-dependent under the in-process HTTP bridge — the POST-stream and
223-
stateless-session `except Exception` handlers in `server/streamable_http*.py`, the `_terminated`
224-
check in `message_router`, and the response-stream double-close guard in
225-
`BaseSession._receive_loop`. `strict-no-cover` does not check `lax` lines; do not promote them to
226-
strict `no cover` without first making the teardown ordering deterministic. The suite also relies
227-
on a one-line `src/mcp/server/sse.py` fix (`sse_stream_reader.aclose()`) that closes a stream the
228-
SSE leg would otherwise leak.
225+
stateless-session `except Exception` handlers in `server/streamable_http*.py` and the
226+
`_terminated` check in `message_router`. `strict-no-cover` does not check `lax` lines; do not
227+
promote them to strict `no cover` without first making the teardown ordering deterministic. The
228+
suite also relies on a one-line `src/mcp/server/sse.py` fix (`sse_stream_reader.aclose()`) that
229+
closes a stream the SSE leg would otherwise leak.

tests/interaction/_helpers.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,11 @@ def __init__(self, inner: WriteStream[SessionMessage], log: list[SessionMessage]
6767
self._log = log
6868

6969
async def send(self, item: SessionMessage, /) -> None:
70-
self._log.append(item)
70+
# Record only after the inner send returns: a send that raises (or is cancelled
71+
# mid-write) never reached the transport, so logging it first would fabricate a
72+
# "sent" message in wire-level assertions.
7173
await self._inner.send(item)
74+
self._log.append(item)
7275

7376
async def aclose(self) -> None:
7477
await self._inner.aclose()

tests/interaction/_requirements.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,18 +268,15 @@ def __post_init__(self) -> None:
268268
divergence=Divergence(
269269
note=(
270270
"The spec says receivers of a cancellation SHOULD NOT send a response for the cancelled "
271-
"request; the server sends an error response (code 0, 'Request cancelled'), which is what "
272-
"unblocks the SDK client's pending call."
271+
"request; both seats send an error response (code 0, 'Request cancelled') instead — the "
272+
"server for cancelled client requests, and the client for cancelled server-initiated "
273+
"requests — which is what unblocks the sender's pending call."
273274
),
274275
),
275276
),
276277
"protocol:cancel:initialize-not-cancellable": Requirement(
277278
source=f"{SPEC_BASE_URL}/basic/utilities/cancellation#behavior-requirements",
278279
behavior="The client never sends notifications/cancelled for the initialize request.",
279-
deferred=(
280-
"Not implemented in the SDK: the client has no public cancellation API at all, so no pathway "
281-
"exists that could cancel initialize; there is no distinct behaviour to pin beyond that absence."
282-
),
283280
),
284281
"protocol:cancel:late-response-ignored": Requirement(
285282
source=f"{SPEC_BASE_URL}/basic/utilities/cancellation#behavior-requirements",
@@ -342,6 +339,26 @@ def __post_init__(self) -> None:
342339
source=f"{SPEC_BASE_URL}/basic#responses",
343340
behavior="A request whose method has no registered handler is answered with a METHOD_NOT_FOUND error.",
344341
),
342+
"protocol:error:null-id": Requirement(
343+
source="sdk",
344+
behavior=(
345+
"An error response carrying a null id — the JSON-RPC shape for a peer reporting a failure it "
346+
"could not attribute to a request, such as a parse error — is surfaced to the application "
347+
"rather than silently discarded."
348+
),
349+
divergence=Divergence(
350+
note=(
351+
"The dispatcher drops null-id error responses with a debug log; v1 surfaced them to "
352+
"message_handler as an MCPError. A typed fault channel restoring visibility is planned "
353+
"before v2 stable."
354+
),
355+
),
356+
deferred=(
357+
"Not yet covered here: the current drop is pinned at the dispatcher level by "
358+
"tests/shared/test_jsonrpc_dispatcher.py; an interaction-level test waits on the planned "
359+
"fault channel."
360+
),
361+
),
345362
"protocol:meta:related-task": Requirement(
346363
source=f"{SPEC_BASE_URL}/basic/utilities/tasks#related-task-metadata",
347364
behavior="Messages may carry related-task _meta associating them with a task.",

tests/interaction/lowlevel/test_cancellation.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from mcp.shared.memory import MessageStream, create_client_server_memory_streams
1717
from mcp.shared.message import SessionMessage
1818
from mcp.types import (
19+
REQUEST_TIMEOUT,
1920
CallToolResult,
2021
EmptyResult,
2122
ErrorData,
@@ -196,7 +197,8 @@ async def sample() -> None:
196197
raise NotImplementedError # unreachable: the scope is cancelled
197198

198199
abandon_scope.start_soon(sample)
199-
await callback_started.wait()
200+
with anyio.fail_after(5):
201+
await callback_started.wait()
200202
abandon_scope.cancel_scope.cancel()
201203
with anyio.fail_after(5):
202204
await callback_cancelled.wait()
@@ -284,3 +286,60 @@ async def message_handler(message: IncomingMessage) -> None:
284286
assert pong == snapshot(EmptyResult())
285287
# The fabricated response was dropped silently: the ping after it still
286288
# round-tripped, and the message handler (a tripwire) was never invoked.
289+
290+
291+
@requirement("protocol:cancel:initialize-not-cancellable")
292+
async def test_timed_out_initialize_sends_no_cancellation() -> None:
293+
"""An abandoned initialize is not followed by notifications/cancelled on the wire.
294+
295+
Spec-mandated: the initialize request MUST NOT be cancelled. Abandoning any other request
296+
sends a courtesy notifications/cancelled (see protocol:timeout:sends-cancellation); this
297+
test pins that initialize opts out. A real Server always answers initialize, so the test
298+
plays a stalling server by hand: it never answers initialize, the client's read timeout
299+
fires, and the ping the test sends next is the marker — the in-memory stream is ordered and
300+
a courtesy cancel goes out before the timeout error reaches the caller, so a regression
301+
would put notifications/cancelled ahead of the ping in the recorded sequence.
302+
"""
303+
received_methods: list[str] = []
304+
305+
async def scripted_server(streams: MessageStream) -> None:
306+
server_read, server_write = streams
307+
308+
# Hold the initialize request unanswered until the client's read timeout fires.
309+
init = await server_read.receive()
310+
assert isinstance(init, SessionMessage)
311+
assert isinstance(init.message, JSONRPCRequest)
312+
received_methods.append(init.message.method)
313+
314+
follow_up = await server_read.receive()
315+
assert isinstance(follow_up, SessionMessage)
316+
assert isinstance(follow_up.message, JSONRPCRequest)
317+
received_methods.append(follow_up.message.method)
318+
await server_write.send(
319+
SessionMessage(
320+
JSONRPCResponse(
321+
jsonrpc="2.0",
322+
id=follow_up.message.id,
323+
# Serialized exactly as a real server serializes results onto the wire.
324+
result=EmptyResult().model_dump(by_alias=True, mode="json", exclude_none=True),
325+
)
326+
)
327+
)
328+
329+
async with (
330+
create_client_server_memory_streams() as ((client_read, client_write), server_streams),
331+
anyio.create_task_group() as task_group,
332+
# The session-level read timeout is the only public pathway that abandons initialize;
333+
# the response never arrives, so any positive value fires on the next event-loop pass.
334+
ClientSession(client_read, client_write, read_timeout_seconds=0.000001) as session,
335+
):
336+
task_group.start_soon(scripted_server, server_streams)
337+
with anyio.fail_after(5):
338+
with pytest.raises(MCPError) as exc_info:
339+
await session.initialize()
340+
assert exc_info.value.error.code == REQUEST_TIMEOUT
341+
# Override the session-level timeout: this ping must round-trip normally.
342+
pong = await session.send_request(PingRequest(), EmptyResult, request_read_timeout_seconds=5)
343+
344+
assert pong == snapshot(EmptyResult())
345+
assert received_methods == snapshot(["initialize", "ping"])

tests/interaction/lowlevel/test_timeouts.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ async def sampling_callback(
107107
context: ClientRequestContext, params: types.CreateMessageRequestParams
108108
) -> types.CreateMessageResult:
109109
callback_started.set()
110-
await release.wait()
110+
with anyio.fail_after(5):
111+
await release.wait()
111112
return types.CreateMessageResult(role="assistant", content=TextContent(text="too late"), model="test-model")
112113

113114
async with Client(recording, sampling_callback=sampling_callback) as client:
@@ -146,7 +147,7 @@ async def list_tools(
146147
async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult:
147148
if params.name == "echo":
148149
return CallToolResult(content=[TextContent(text="still alive")])
149-
await anyio.Event().wait() # blocks until the session is torn down
150+
await anyio.Event().wait() # blocks until the courtesy cancellation interrupts it
150151
raise NotImplementedError # unreachable
151152

152153
server = Server("blocker", on_list_tools=list_tools, on_call_tool=call_tool)
@@ -178,7 +179,7 @@ async def test_session_level_timeout_applies_to_every_request() -> None:
178179

179180
async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult:
180181
assert params.name == "block"
181-
await anyio.Event().wait() # blocks until the session is torn down
182+
await anyio.Event().wait() # blocks until the courtesy cancellation interrupts it
182183
raise NotImplementedError # unreachable
183184

184185
server = Server("blocker", on_call_tool=call_tool)

tests/issues/test_88_random_error.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,14 @@
1212
from mcp.server import Server, ServerRequestContext
1313
from mcp.shared.exceptions import MCPError
1414
from mcp.shared.message import SessionMessage
15-
from mcp.types import CallToolRequestParams, CallToolResult, ListToolsResult, PaginatedRequestParams, TextContent
15+
from mcp.types import (
16+
REQUEST_TIMEOUT,
17+
CallToolRequestParams,
18+
CallToolResult,
19+
ListToolsResult,
20+
PaginatedRequestParams,
21+
TextContent,
22+
)
1623

1724

1825
@pytest.mark.anyio
@@ -97,7 +104,7 @@ async def client(
97104
# Use very small timeout to trigger quickly without waiting
98105
with pytest.raises(MCPError) as exc_info:
99106
await session.call_tool("slow", read_timeout_seconds=0.000001) # artificial timeout that always fails
100-
assert "timed out" in str(exc_info.value)
107+
assert exc_info.value.error.code == REQUEST_TIMEOUT
101108

102109
# No-op if the courtesy cancellation already interrupted the handler.
103110
slow_request_lock.set()

0 commit comments

Comments
 (0)