Skip to content

Commit 7a026f2

Browse files
committed
test: correct manifest divergence notes and route in-handler notifications deterministically
Four manifest fixes from spec/SDK re-verification: - lifecycle:capability:* divergence notes used SHOULD; spec basic/lifecycle#operation has been MUST since 2025-06-18 - mcpserver:tool:naming-validation deferred reason claimed no naming check exists; Tool.from_function calls validate_and_warn_tool_name (warns, doesn't reject) - converted to a Divergence with a pinning test - client-auth:...issuer-validation divergence's second sentence is false (OAuthMetadata types the endpoints AnyHttpUrl, so scheme is validated) - resources:annotations now records that the SDK Annotations model lacks lastModified; the round-trip test sends it via model_validate so the snapshot pins the drop Twelve lowlevel tests sent notifications from inside a tool handler without related_request_id, so on the streamable-HTTP leg they routed to the standalone GET stream and the assertion relied on cross-stream ordering the suite documents as not guaranteed. Eight now pass related_request_id; four whose senders don't accept it use anyio.Event with the snapshot still proving the delivered set. The module docstrings that overstated the ordering guarantee are corrected. README §Coverage now documents the four lax-no-cover teardown markers and the sse.py aclose() fix that landed alongside this suite.
1 parent 9f2b105 commit 7a026f2

8 files changed

Lines changed: 141 additions & 47 deletions

File tree

tests/interaction/README.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,16 @@ assert after the call, with no synchronisation. The exceptions:
209209
CI requires 100% line and branch coverage, including `tests/`, and `strict-no-cover` fails the
210210
build if a line marked `# pragma: no cover` is ever executed. When a new test starts covering a
211211
pragma'd line in `src/`, delete the pragma in the same change. Do not add new `# type: ignore` or
212-
`# noqa` comments; restructure instead. The one sanctioned pragma is `# pragma: no branch` on a
213-
`with`/`async with` line whose only fault is coverage.py mis-tracing the exit arc of a nested
214-
async context — restructure first, and reserve the pragma for shapes that cannot collapse (a sync
215-
`with` adjacent to an `async with`).
212+
`# noqa` comments; restructure instead. The one sanctioned pragma in this suite's test code is
213+
`# pragma: no branch` on a `with`/`async with` line whose only fault is coverage.py mis-tracing
214+
the exit arc of a nested async context — restructure first, and reserve the pragma for shapes
215+
that cannot collapse (a sync `with` adjacent to an `async with`).
216+
217+
A handful of `# pragma: lax no cover` markers in `src/` cover teardown exception handlers whose
218+
execution is timing-dependent under the in-process HTTP bridge — the POST-stream and
219+
stateless-session `except Exception` handlers in `server/streamable_http*.py`, the `_terminated`
220+
check in `message_router`, and the response-stream double-close guard in
221+
`BaseSession._receive_loop`. `strict-no-cover` does not check `lax` lines; do not promote them to
222+
strict `no cover` without first making the teardown ordering deterministic. The suite also relies
223+
on a one-line `src/mcp/server/sse.py` fix (`sse_stream_reader.aclose()`) that closes a stream the
224+
SSE leg would otherwise leak.

tests/interaction/_requirements.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def __post_init__(self) -> None:
9090
divergence=Divergence(
9191
note=(
9292
"The client does not check its own declared capabilities before sending notifications or "
93-
"serving callbacks; nothing prevents a caller from violating the spec's SHOULD."
93+
"serving callbacks; nothing prevents a caller from violating the spec's MUST."
9494
),
9595
),
9696
deferred=(
@@ -106,7 +106,7 @@ def __post_init__(self) -> None:
106106
divergence=Divergence(
107107
note=(
108108
"The client sends any request regardless of the server's advertised capabilities and "
109-
"surfaces whatever the server answers; the spec's SHOULD is not enforced."
109+
"surfaces whatever the server answers; the spec's MUST is not enforced."
110110
),
111111
),
112112
deferred=(
@@ -693,9 +693,12 @@ def __post_init__(self) -> None:
693693
"mcpserver:tool:naming-validation": Requirement(
694694
source="sdk",
695695
behavior="Tool names that violate the spec's naming rules are rejected at registration time.",
696-
deferred=(
697-
"Not implemented in the SDK: MCPServer accepts any string as a tool name; there is no "
698-
"spec-naming-rules check at registration time."
696+
divergence=Divergence(
697+
note=(
698+
"MCPServer runs the SEP-986 naming check at registration (validate_and_warn_tool_name at "
699+
"tools/base.py) and logs a warning for non-conforming names, but does not reject them; the "
700+
"bool result is discarded and registration proceeds."
701+
),
699702
),
700703
),
701704
"mcpserver:tool:output-schema:model": Requirement(
@@ -769,9 +772,12 @@ def __post_init__(self) -> None:
769772
# ═══════════════════════════════════════════════════════════════════════════
770773
"resources:annotations": Requirement(
771774
source=f"{SPEC_BASE_URL}/server/resources#annotations",
772-
behavior=(
773-
"Resource annotations (audience, priority) supplied by the server round-trip to the client "
774-
"in the list result."
775+
behavior="Resource annotations supplied by the server round-trip to the client in the list result.",
776+
divergence=Divergence(
777+
note=(
778+
"The SDK Annotations model is missing the schema's lastModified field; MCPModel uses the "
779+
"pydantic default extra='ignore', so the value is silently dropped on parse."
780+
),
775781
),
776782
),
777783
"resources:capability:declared": Requirement(
@@ -2413,9 +2419,7 @@ def __post_init__(self) -> None:
24132419
divergence=Divergence(
24142420
note=(
24152421
"The SDK parses authorization-server metadata without comparing issuer to the discovery "
2416-
"URL; a mismatched issuer is accepted and the flow proceeds. The SDK also does not "
2417-
"validate that the document's authorization_endpoint, token_endpoint, and "
2418-
"registration_endpoint use http(s) schemes."
2422+
"URL; a mismatched issuer is accepted and the flow proceeds."
24192423
),
24202424
),
24212425
),

tests/interaction/lowlevel/test_elicitation.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,9 @@ async def test_elicitation_complete_notification_carries_the_elicited_id_back_to
304304
The lifecycle under test: the tool elicits a URL interaction with an elicitationId, the user
305305
agrees to visit the URL, the out-of-band interaction finishes, and the server emits
306306
elicitation/complete so the client can correlate the completion with the elicitation it
307-
accepted earlier. Both messages arrive before the tool call returns, so a plain collected
308-
list needs no synchronisation.
307+
accepted earlier. The completion notification carries ``related_request_id`` so over
308+
streamable HTTP it rides the tool call's own stream and reaches the client before the call
309+
returns; the same ordering already holds on in-memory and SSE transports.
309310
"""
310311
elicitation_id = "auth-001"
311312
elicited_ids: list[str] = []
@@ -327,7 +328,7 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
327328
"Authorize access to your files.", "https://example.com/oauth/authorize", elicitation_id
328329
)
329330
assert answer.action == "accept"
330-
await ctx.session.send_elicit_complete(elicitation_id)
331+
await ctx.session.send_elicit_complete(elicitation_id, related_request_id=ctx.request_id)
331332
return CallToolResult(content=[TextContent(text="linked")])
332333

333334
server = Server("authorizer", on_list_tools=list_tools, on_call_tool=call_tool)
@@ -559,7 +560,7 @@ async def list_tools(
559560

560561
async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult:
561562
assert params.name == "noop"
562-
await ctx.session.send_elicit_complete("never-elicited")
563+
await ctx.session.send_elicit_complete("never-elicited", related_request_id=ctx.request_id)
563564
return CallToolResult(content=[TextContent(text="ok")])
564565

565566
server = Server("notifier", on_list_tools=list_tools, on_call_tool=call_tool)

tests/interaction/lowlevel/test_list_changed.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
"""List-changed notifications from the low-level Server, driven through the public Client API.
22
3-
The notifications are emitted from inside a tool call, so the ordering guarantee described in
4-
test_logging.py applies: they reach the client's message handler before the tool call returns,
5-
and the tests assert on a plain collected list with no synchronisation. The collector records
6-
every message the handler receives, so the assertions also prove nothing else was delivered.
3+
``send_*_list_changed`` does not take a ``related_request_id``, so over streamable HTTP the
4+
notification routes to the standalone GET stream and is not guaranteed to arrive before the tool
5+
result on its POST stream. Tests therefore wait on an event the collector sets, the same pattern
6+
as ``transports/test_streamable_http.py::test_unrelated_server_messages_arrive_on_the_standalone_stream``.
7+
The collector still records every message it receives, so the snapshot also proves nothing else
8+
was delivered.
79
"""
810

11+
import anyio
912
import pytest
1013
from inline_snapshot import snapshot
1114

@@ -29,9 +32,11 @@
2932
async def test_tool_list_changed_notification(connect: Connect) -> None:
3033
"""A tools/list_changed notification sent during a tool call reaches the client's message handler."""
3134
received: list[IncomingMessage] = []
35+
seen = anyio.Event()
3236

3337
async def collect(message: IncomingMessage) -> None:
3438
received.append(message)
39+
seen.set()
3540

3641
async def list_tools(
3742
ctx: ServerRequestContext, params: types.PaginatedRequestParams | None
@@ -47,6 +52,8 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
4752

4853
async with connect(server, message_handler=collect) as client:
4954
await client.call_tool("install", {})
55+
with anyio.fail_after(5):
56+
await seen.wait()
5057

5158
assert received == snapshot([ToolListChangedNotification()])
5259

@@ -55,9 +62,11 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
5562
async def test_resource_list_changed_notification(connect: Connect) -> None:
5663
"""A resources/list_changed notification sent during a tool call reaches the client's message handler."""
5764
received: list[IncomingMessage] = []
65+
seen = anyio.Event()
5866

5967
async def collect(message: IncomingMessage) -> None:
6068
received.append(message)
69+
seen.set()
6170

6271
async def list_tools(
6372
ctx: ServerRequestContext, params: types.PaginatedRequestParams | None
@@ -73,6 +82,8 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
7382

7483
async with connect(server, message_handler=collect) as client:
7584
await client.call_tool("mount", {})
85+
with anyio.fail_after(5):
86+
await seen.wait()
7687

7788
assert received == snapshot([ResourceListChangedNotification()])
7889

@@ -81,9 +92,11 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
8192
async def test_prompt_list_changed_notification(connect: Connect) -> None:
8293
"""A prompts/list_changed notification sent during a tool call reaches the client's message handler."""
8394
received: list[IncomingMessage] = []
95+
seen = anyio.Event()
8496

8597
async def collect(message: IncomingMessage) -> None:
8698
received.append(message)
99+
seen.set()
87100

88101
async def list_tools(
89102
ctx: ServerRequestContext, params: types.PaginatedRequestParams | None
@@ -99,5 +112,7 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
99112

100113
async with connect(server, message_handler=collect) as client:
101114
await client.call_tool("learn", {})
115+
with anyio.fail_after(5):
116+
await seen.wait()
102117

103118
assert received == snapshot([PromptListChangedNotification()])

tests/interaction/lowlevel/test_logging.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
33
Notification ordering: the in-memory transport delivers every server-to-client message on one
44
ordered stream, and the client's receive loop dispatches each incoming message to completion
5-
before reading the next one. Together these guarantee that every notification the server sends
6-
before its response reaches the client callback before the originating request returns, so tests
7-
collect notifications into a plain list and assert after the request completes -- no events, no
8-
waiting. This does not generalise to transports that split messages across streams (the
9-
streamable HTTP standalone GET stream); tests over those transports must synchronise explicitly.
5+
before reading the next one. Over streamable HTTP that ordered single-stream guarantee holds
6+
only for messages that carry a ``related_request_id`` (they ride the originating request's POST
7+
stream); without it the message routes to the standalone GET stream and may arrive after the
8+
response. These tests pass ``related_request_id`` so they can collect into a plain list and
9+
assert after the request completes on every transport leg -- no events, no waiting.
1010
"""
1111

1212
import pytest
@@ -68,8 +68,12 @@ async def list_tools(
6868

6969
async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult:
7070
assert params.name == "chatty"
71-
await ctx.session.send_log_message(level="info", data="starting up", logger="app.lifecycle")
72-
await ctx.session.send_log_message(level="error", data={"code": 502, "retryable": True})
71+
await ctx.session.send_log_message(
72+
level="info", data="starting up", logger="app.lifecycle", related_request_id=ctx.request_id
73+
)
74+
await ctx.session.send_log_message(
75+
level="error", data={"code": 502, "retryable": True}, related_request_id=ctx.request_id
76+
)
7377
return CallToolResult(content=[TextContent(text="done")])
7478

7579
server = Server("logger", on_list_tools=list_tools, on_call_tool=call_tool)
@@ -102,7 +106,9 @@ async def list_tools(
102106
async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult:
103107
assert params.name == "siren"
104108
for level in ALL_LEVELS:
105-
await ctx.session.send_log_message(level=level, data=f"a {level} message")
109+
await ctx.session.send_log_message(
110+
level=level, data=f"a {level} message", related_request_id=ctx.request_id
111+
)
106112
return CallToolResult(content=[TextContent(text="logged")])
107113

108114
server = Server("logger", on_list_tools=list_tools, on_call_tool=call_tool)

tests/interaction/lowlevel/test_progress.py

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
"""Progress interactions against the low-level Server, driven through the public Client API.
22
33
Server-to-client progress emitted during a request follows the same ordering guarantee as
4-
logging notifications (see test_logging.py): everything the server sends before its response is
5-
dispatched to the progress callback before the request returns, so no synchronisation is needed.
6-
The client-to-server direction is a standalone notification with no response to await, so that
7-
test waits on an event set by the server's handler.
4+
logging notifications (see test_logging.py) -- on the in-memory transport unconditionally, and
5+
over streamable HTTP only when sent with ``related_request_id`` so the notification rides the
6+
originating request's POST stream rather than the standalone GET stream. These tests pass
7+
``related_request_id`` so no synchronisation is needed. The client-to-server direction is a
8+
standalone notification with no response to await, so that test waits on an event set by the
9+
server's handler.
810
"""
911

1012
import anyio
@@ -42,9 +44,15 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
4244
assert ctx.meta is not None
4345
token = ctx.meta.get("progress_token")
4446
assert token is not None
45-
await ctx.session.send_progress_notification(token, 1.0, total=3.0, message="first chunk")
46-
await ctx.session.send_progress_notification(token, 2.0, total=3.0, message="second chunk")
47-
await ctx.session.send_progress_notification(token, 3.0, total=3.0, message="done")
47+
await ctx.session.send_progress_notification(
48+
token, 1.0, total=3.0, message="first chunk", related_request_id=str(ctx.request_id)
49+
)
50+
await ctx.session.send_progress_notification(
51+
token, 2.0, total=3.0, message="second chunk", related_request_id=str(ctx.request_id)
52+
)
53+
await ctx.session.send_progress_notification(
54+
token, 3.0, total=3.0, message="done", related_request_id=str(ctx.request_id)
55+
)
4856
return CallToolResult(content=[TextContent(text="downloaded")])
4957

5058
server = Server("downloader", on_list_tools=list_tools, on_call_tool=call_tool)
@@ -166,10 +174,14 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
166174
# The two handlers interleave by waiting on alternating turns: a takes 0 and 2, b takes 1 and 3.
167175
first, second = (0, 2) if label == "a" else (1, 3)
168176
await turns[first].wait()
169-
await ctx.session.send_progress_notification(token, progress_values[label][0])
177+
await ctx.session.send_progress_notification(
178+
token, progress_values[label][0], related_request_id=str(ctx.request_id)
179+
)
170180
turns[first + 1].set()
171181
await turns[second].wait()
172-
await ctx.session.send_progress_notification(token, progress_values[label][1])
182+
await ctx.session.send_progress_notification(
183+
token, progress_values[label][1], related_request_id=str(ctx.request_id)
184+
)
173185
if second + 1 < len(turns):
174186
turns[second + 1].set()
175187
return CallToolResult(content=[TextContent(text="done")])
@@ -227,7 +239,7 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
227239
token = ctx.meta.get("progress_token")
228240
assert token is not None
229241
captured.append((ctx.session, token))
230-
await ctx.session.send_progress_notification(token, 0.5)
242+
await ctx.session.send_progress_notification(token, 0.5, related_request_id=str(ctx.request_id))
231243
return CallToolResult(content=[TextContent(text="done")])
232244

233245
server = Server("reporter", on_list_tools=list_tools, on_call_tool=call_tool)
@@ -276,9 +288,9 @@ async def call_tool(ctx: ServerRequestContext, params: types.CallToolRequestPara
276288
assert ctx.meta is not None
277289
token = ctx.meta.get("progress_token")
278290
assert token is not None
279-
await ctx.session.send_progress_notification(token, 0.5)
280-
await ctx.session.send_progress_notification(token, 0.3)
281-
await ctx.session.send_progress_notification(token, 0.9)
291+
await ctx.session.send_progress_notification(token, 0.5, related_request_id=str(ctx.request_id))
292+
await ctx.session.send_progress_notification(token, 0.3, related_request_id=str(ctx.request_id))
293+
await ctx.session.send_progress_notification(token, 0.9, related_request_id=str(ctx.request_id))
282294
return CallToolResult(content=[TextContent(text="done")])
283295

284296
server = Server("zigzagger", on_list_tools=list_tools, on_call_tool=call_tool)

0 commit comments

Comments
 (0)