Testing-standards fixes: hollow-proof, provenance docstrings, constants, assertion patterns

Fixes from the testing-standards review of the new/modified test files.

Hollow-proof (the two SERIOUS findings):
- test_standalone_outbound_defaults_to_request_outbound: drop outbound= from from_envelope so
  conn.outbound is the no-channel sentinel; traffic on the request outbound now proves the default
- test_server_info_version_falls_back_to_package: assert == importlib.metadata.version('mcp')

Assertion patterns:
- test_stateless_mode.py: 8x match= -> exc.value.method (structured attribute, not message text)
- test_streamable_http_modern.py: literal 404/400 status asserts (impl reads same table, so
  table[X]==table[X] proved nothing); restore full-object json equality with id:None pin
- test_inbound.py: assert isinstance(route, InboundModernRoute) before frozen check

Constants: replace version/header/meta-key string literals with named imports across
test_session.py / test_streamable_http_modern.py.

Hygiene: drop from __future__ import annotations from tests/shared/test_inbound.py and
src/mcp/shared/inbound.py (no forward refs); duplicate StubOutbound locally in
test_stateless_mode.py (no cross-file test imports); _StubDispatchContext methods raise
NotImplementedError; private _request_handlers -> public get_request_handler.

Docstrings: add Spec-mandated:/SDK-defined: provenance tags to ~50 tests across 7 files.

Naming: test_cacheable_defaults -> test_discover_result_defaults_to_immediately_stale_private_cache;
test_header_rung_skipped_when_headers_none -> test_header_rung_does_not_reject_when_headers_arg_is_none;
drop 'handler' from test_serve_one_maps_error_... (it's kernel METHOD_NOT_FOUND).

Author: maxisbey

src/mcp/shared/inbound.py

@@ -8,8 +8,6 @@
 status.
 """
 
-from __future__ import annotations
-
 from collections.abc import Mapping, Sequence
 from dataclasses import dataclass
 from types import MappingProxyType

tests/server/lowlevel/test_server_discover.py

@@ -1,10 +1,11 @@
 """Direct-handler tests for the auto-derived `server/discover` handler.
 
-These call the registered handler straight from `Server._request_handlers`
-without spinning up a `ServerRunner` or any transport, so they verify the
-handler's contract in isolation from the dispatch pipeline.
+These call the registered handler via the public `Server.get_request_handler`
+accessor without spinning up a `ServerRunner` or any transport, so they verify
+the handler's contract in isolation from the dispatch pipeline.
 """
 
+import importlib.metadata
 from typing import Any, cast
 
 import pytest
@@ -13,23 +14,26 @@
 from mcp.server import Server, ServerRequestContext
 from mcp.shared.version import MODERN_PROTOCOL_VERSIONS
 
-# The default handler ignores its `ctx` argument entirely (it derives the
+# `Server._handle_discover` ignores its `ctx` argument entirely (it derives the
 # result from server state), so a sentinel keeps the call site type-correct
 # without dragging session machinery into a unit test.
 _UNUSED_CTX = cast("ServerRequestContext[Any]", None)
 
 
 async def _discover(server: Server[Any]) -> types.DiscoverResult:
-    entry = server._request_handlers["server/discover"]
+    entry = server.get_request_handler("server/discover")
+    assert entry is not None
     result = await entry.handler(_UNUSED_CTX, types.RequestParams())
     assert isinstance(result, types.DiscoverResult)
     return result
 
 
 def test_registered_by_default() -> None:
+    """SDK-defined: a bare `Server` registers a `server/discover` handler out of
+    the box, typed for the base `RequestParams`."""
     server = Server("test-server")
-    assert "server/discover" in server._request_handlers
-    entry = server._request_handlers["server/discover"]
+    entry = server.get_request_handler("server/discover")
+    assert entry is not None
     assert entry.params_type is types.RequestParams
 
 
@@ -43,6 +47,8 @@ async def test_supported_versions_is_modern_set() -> None:
 
 @pytest.mark.anyio
 async def test_server_info_reflects_constructor_fields() -> None:
+    """SDK-defined: `serverInfo` is built field-for-field from the `Server`
+    constructor arguments."""
     icons = [types.Icon(src="https://example.test/icon.png")]
     server = Server(
         "info-server",
@@ -65,15 +71,16 @@ async def test_server_info_reflects_constructor_fields() -> None:
 
 @pytest.mark.anyio
 async def test_server_info_version_falls_back_to_package() -> None:
-    """When no explicit version is supplied, `serverInfo.version` is still a
-    non-empty string (the installed `mcp` package version)."""
+    """SDK-defined: when no explicit version is supplied, `serverInfo.version`
+    falls back to the installed `mcp` package version."""
     result = await _discover(Server("unversioned"))
-    assert isinstance(result.server_info.version, str)
-    assert result.server_info.version
+    assert result.server_info.version == importlib.metadata.version("mcp")
 
 
 @pytest.mark.anyio
 async def test_instructions_threaded_through() -> None:
+    """SDK-defined: the `instructions` constructor argument is passed through
+    verbatim, defaulting to `None` when omitted."""
     server = Server("inst-server", instructions="Read the docs first.")
     result = await _discover(server)
     assert result.instructions == "Read the docs first."
@@ -84,8 +91,9 @@ async def test_instructions_threaded_through() -> None:
 
 @pytest.mark.anyio
 async def test_capabilities_derived_from_registered_handlers() -> None:
-    """Capabilities are computed at handler call time from the live registry,
-    so post-construction `add_request_handler` calls are reflected."""
+    """SDK-defined: capabilities are computed at handler call time from the
+    live registry, so post-construction `add_request_handler` calls are
+    reflected."""
 
     async def list_tools(
         ctx: ServerRequestContext[Any], params: types.PaginatedRequestParams | None
@@ -111,16 +119,18 @@ async def list_prompts(
 
 
 @pytest.mark.anyio
-async def test_cacheable_defaults() -> None:
-    """`DiscoverResult` is cacheable; the auto-derived handler relies on the
-    model defaults (immediately-stale, private)."""
+async def test_discover_result_defaults_to_immediately_stale_private_cache() -> None:
+    """SDK-defined: `DiscoverResult` is cacheable; the auto-derived handler
+    relies on the model defaults (immediately-stale, private)."""
     result = await _discover(Server("cache-server"))
     assert result.ttl_ms == 0
     assert result.cache_scope == "private"
 
 
 @pytest.mark.anyio
 async def test_overridable_via_add_request_handler() -> None:
+    """SDK-defined: a custom `server/discover` handler registered via
+    `add_request_handler` replaces the auto-derived default wholesale."""
     server = Server("custom-server", version="1.0.0")
     custom = types.DiscoverResult(
         supported_versions=list(MODERN_PROTOCOL_VERSIONS),

tests/server/test_connection.py

@@ -68,8 +68,8 @@ async def notify(self, method: str, params: Mapping[str, Any] | None) -> None:
 
 
 def test_from_envelope_is_born_ready_with_no_back_channel():
-    """`from_envelope` populates `protocol_version`, sets `initialized`, and
-    holds the no-channel sentinel so `has_standalone_channel` derives False."""
+    """SDK-defined: `from_envelope` populates `protocol_version`, sets `initialized`,
+    and holds the no-channel sentinel so `has_standalone_channel` derives False."""
     conn = Connection.from_envelope(_MODERN, None, None)
     assert conn.protocol_version == _MODERN
     assert conn.initialized.is_set()
@@ -80,6 +80,8 @@ def test_from_envelope_is_born_ready_with_no_back_channel():
 
 
 def test_from_envelope_records_client_params_when_both_info_and_caps_supplied():
+    """SDK-defined: when both client info and capabilities are supplied,
+    `from_envelope` synthesizes `client_params` so capability checks can run."""
     caps = ClientCapabilities(sampling=SamplingCapability())
     conn = Connection.from_envelope(_MODERN, _CLIENT_INFO, caps)
     assert conn.client_params is not None
@@ -95,12 +97,14 @@ def test_from_envelope_records_client_params_when_both_info_and_caps_supplied():
 def test_from_envelope_leaves_client_params_none_when_either_is_missing(
     info: Implementation | None, caps: ClientCapabilities | None
 ):
+    """SDK-defined: `client_params` is only synthesized when both info and
+    caps are present; either missing leaves it `None`."""
     conn = Connection.from_envelope(_MODERN, info, caps)
     assert conn.client_params is None
 
 
 def test_from_envelope_with_explicit_outbound_has_standalone_channel():
-    """Duplex modern transports pass an outbound; `has_standalone_channel`
+    """SDK-defined: duplex modern transports pass an outbound; `has_standalone_channel`
     derives True since the held outbound is not the no-channel sentinel."""
     out = StubOutbound()
     conn = Connection.from_envelope(_MODERN, None, None, outbound=out)
@@ -110,6 +114,8 @@ def test_from_envelope_with_explicit_outbound_has_standalone_channel():
 
 
 def test_for_loop_seeds_version_from_hint_or_latest_and_is_not_born_ready():
+    """SDK-defined: `for_loop` seeds `protocol_version` from the hint when given,
+    else `LATEST_PROTOCOL_VERSION`; the connection awaits the initialize handshake."""
     out = StubOutbound()
     conn = Connection.for_loop(out)
     assert conn.protocol_version == LATEST_PROTOCOL_VERSION
@@ -118,9 +124,14 @@ def test_for_loop_seeds_version_from_hint_or_latest_and_is_not_born_ready():
     assert conn.initialize_accepted is False
     assert conn.client_params is None
 
-    hinted = Connection.for_loop(out, session_id="sess-1", protocol_version_hint=_MODERN)
+    hinted = Connection.for_loop(out, protocol_version_hint=_MODERN)
     assert hinted.protocol_version == _MODERN
-    assert hinted.session_id == "sess-1"
+
+
+def test_for_loop_records_session_id_when_supplied():
+    """SDK-defined: `for_loop` stores the `session_id` kwarg verbatim."""
+    conn = Connection.for_loop(StubOutbound(), session_id="sess-1")
+    assert conn.session_id == "sess-1"
 
 
 # --- outbound channel ----------------------------------------------------------
@@ -145,7 +156,7 @@ async def test_connection_notify_swallows_broken_stream_and_debug_logs(caplog: p
 
 @pytest.mark.anyio
 async def test_connection_notify_drops_when_no_standalone_channel(caplog: pytest.LogCaptureFixture):
-    """The no-channel sentinel debug-logs and drops; `notify` never raises."""
+    """SDK-defined: the no-channel sentinel debug-logs and drops; `notify` never raises."""
     caplog.set_level(logging.DEBUG, logger="mcp.server.connection")
     conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
     await conn.notify("notifications/message", {"data": "x"})  # must not raise
@@ -154,7 +165,7 @@ async def test_connection_notify_drops_when_no_standalone_channel(caplog: pytest
 
 @pytest.mark.anyio
 async def test_connection_send_raw_request_raises_nobackchannel_when_no_standalone_channel():
-    """The no-channel sentinel raises structurally; `Connection` does no pre-check."""
+    """SDK-defined: the no-channel sentinel raises structurally; `Connection` does no pre-check."""
     conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
     with pytest.raises(NoBackChannelError):
         await conn.send_raw_request("ping", None)
@@ -312,6 +323,8 @@ async def test_connection_send_tool_list_changed_with_meta_includes_meta_only_pa
 
 
 def test_connection_check_capability_false_when_no_client_params_recorded():
+    """SDK-defined: `check_capability` returns False when no `client_params`
+    were recorded, regardless of which factory built the connection."""
     conn = Connection.for_loop(StubOutbound())
     assert conn.check_capability(ClientCapabilities(sampling=SamplingCapability())) is False
     # Same for a born-ready connection that supplied neither info nor caps.

tests/server/test_runner.py

@@ -1181,6 +1181,9 @@ async def _append(i: int) -> None:
 
 @pytest.mark.anyio
 async def test_to_jsonrpc_response_wraps_success_as_jsonrpc_response():
+    """SDK-defined: a handler coroutine resolving to a result dict is wrapped as a
+    `JSONRPCResponse` carrying the supplied id and the dict verbatim as `result`."""
+
     async def ok() -> dict[str, Any]:
         return {"k": "v"}
 
@@ -1192,6 +1195,9 @@ async def ok() -> dict[str, Any]:
 
 @pytest.mark.anyio
 async def test_to_jsonrpc_response_maps_mcp_error_to_jsonrpc_error():
+    """SDK-defined: an `MCPError` raised by the handler coroutine is wrapped as a
+    `JSONRPCError` whose `error` carries the same code, message, and data."""
+
     async def fail() -> dict[str, Any]:
         raise MCPError(code=METHOD_NOT_FOUND, message="nope", data="x")
 
@@ -1203,6 +1209,10 @@ async def fail() -> dict[str, Any]:
 
 @pytest.mark.anyio
 async def test_to_jsonrpc_response_maps_validation_error_to_invalid_params():
+    """SDK-defined: a pydantic `ValidationError` escaping the handler coroutine is
+    mapped to `INVALID_PARAMS` with a generic message (validator detail does not
+    reach the wire)."""
+
     async def fail() -> dict[str, Any]:
         Tool.model_validate({"name": 123})  # raises ValidationError
         raise NotImplementedError
@@ -1216,6 +1226,10 @@ async def fail() -> dict[str, Any]:
 async def test_to_jsonrpc_response_maps_unmapped_exception_to_internal_error_and_logs(
     caplog: pytest.LogCaptureFixture,
 ):
+    """SDK-defined: an unmapped exception is logged server-side and surfaced as
+    `INTERNAL_ERROR` with a generic message; the exception text never reaches the
+    wire."""
+
     async def fail() -> dict[str, Any]:
         raise RuntimeError("boom")
 
@@ -1271,13 +1285,13 @@ def can_send_request(self) -> bool:
     async def send_raw_request(
         self, method: str, params: Mapping[str, Any] | None, opts: CallOptions | None = None
     ) -> dict[str, Any]:
-        raise AssertionError("serve_one tests do not send requests on the back-channel")
+        raise NotImplementedError
 
     async def notify(self, method: str, params: Mapping[str, Any] | None) -> None:
-        pass
+        raise NotImplementedError
 
     async def progress(self, progress: float, total: float | None = None, message: str | None = None) -> None:
-        pass
+        raise NotImplementedError
 
 
 async def _append_async(dst: list[int], v: int) -> None:
@@ -1305,7 +1319,10 @@ async def test_serve_one_runs_handler_and_returns_jsonrpc_response(server: SrvT)
 
 
 @pytest.mark.anyio
-async def test_serve_one_maps_handler_error_to_jsonrpc_error_and_still_closes_exit_stack(server: SrvT):
+async def test_serve_one_maps_error_to_jsonrpc_error_and_still_closes_exit_stack(server: SrvT):
+    """SDK-defined: a kernel-produced error (here `METHOD_NOT_FOUND` for an
+    unregistered method) is wrapped as a `JSONRPCError`, and the per-request
+    exit stack is closed on the error path too."""
     conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
     cleaned: list[int] = []
     conn.exit_stack.push_async_callback(_append_async, cleaned, 1)

tests/server/test_session.py

@@ -17,6 +17,7 @@
 from mcp.server.session import ServerSession
 from mcp.shared.dispatcher import CallOptions, Outbound
 from mcp.shared.message import ServerMessageMetadata
+from mcp.shared.version import MODERN_PROTOCOL_VERSIONS
 from mcp.types import (
     LATEST_PROTOCOL_VERSION,
     ClientCapabilities,
@@ -60,6 +61,7 @@ def _make_session(
 
 
 def _two_channel_session(request_ch: Outbound, standalone_ch: Outbound) -> ServerSession:
+    """Distinct request/standalone outbounds so routing assertions can tell the channels apart."""
     conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None, outbound=standalone_ch)
     return ServerSession(request_ch, conn, standalone_outbound=standalone_ch)
 
@@ -105,7 +107,7 @@ async def test_send_request_timeout_zero_is_forwarded():
 
 @pytest.mark.anyio
 async def test_send_request_without_related_id_routes_to_standalone_channel():
-    """No `related_request_id`: the request rides the connection's standalone channel."""
+    """SDK-defined: no `related_request_id` routes the request onto the connection's standalone channel."""
     request_ch = StubOutbound()
     standalone_ch = StubOutbound(result={"roots": []})
     session = _two_channel_session(request_ch, standalone_ch)
@@ -116,7 +118,7 @@ async def test_send_request_without_related_id_routes_to_standalone_channel():
 
 @pytest.mark.anyio
 async def test_send_request_with_related_id_routes_to_request_channel():
-    """With `related_request_id`: the request rides the per-request channel
+    """SDK-defined: with `related_request_id` the request rides the per-request channel
     (the originating POST's response stream over streamable HTTP)."""
     request_ch = StubOutbound(result={"action": "cancel"})
     standalone_ch = StubOutbound()
@@ -133,7 +135,7 @@ async def test_send_request_with_related_id_routes_to_request_channel():
 
 @pytest.mark.anyio
 async def test_send_notification_routes_by_related_request_id():
-    """Notifications select channel by `related_request_id` exactly like requests."""
+    """SDK-defined: notifications select channel by `related_request_id` exactly like requests."""
     request_ch = StubOutbound()
     standalone_ch = StubOutbound()
     session = _two_channel_session(request_ch, standalone_ch)
@@ -145,10 +147,14 @@ async def test_send_notification_routes_by_related_request_id():
 
 @pytest.mark.anyio
 async def test_standalone_outbound_defaults_to_request_outbound():
-    """Omitting `standalone_outbound` collapses both channels onto the request
-    one — the duplex case (stdio) where a single stream serves both."""
+    """SDK-defined: omitting `standalone_outbound` collapses both channels onto the
+    request one — the duplex case (stdio) where a single stream serves both.
+
+    `conn.outbound` is the no-channel sentinel here, so traffic landing on `only`
+    proves the default is the request outbound (the alternate impl — defaulting to
+    `conn.outbound` — would raise `NoBackChannelError`)."""
     only = StubOutbound(result={"roots": []})
-    conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None, outbound=only)
+    conn = Connection.from_envelope(LATEST_PROTOCOL_VERSION, None, None)
     session = ServerSession(only, conn)
     await session.send_request(types.ListRootsRequest(), types.ListRootsResult)
     await session.send_notification(types.ToolListChangedNotification())
@@ -178,7 +184,7 @@ async def test_send_request_passes_a_spec_valid_client_result():
 async def test_send_request_skips_the_surface_gate_when_method_absent_at_version():
     """Surface row absent for the connection's version: gate is bypassed and only
     `result_type` validates."""
-    session = _make_session(StubOutbound(result={}), protocol_version="2026-07-28")
+    session = _make_session(StubOutbound(result={}), protocol_version=MODERN_PROTOCOL_VERSIONS[0])
     result = await session.send_request(types.PingRequest(), types.EmptyResult)
     assert isinstance(result, types.EmptyResult)
 
@@ -219,8 +225,9 @@ def test_check_client_capability_delegates_to_connection():
 
 
 def test_protocol_version_proxies_connection():
-    """`session.protocol_version` reads through to the held `Connection`."""
-    conn = Connection.from_envelope("2025-03-26", None, None)
+    """SDK-defined: `session.protocol_version` reads through to the held `Connection`."""
+    _ARBITRARY_VERSION = "sentinel-version"  # identity-only: any string the connection holds
+    conn = Connection.from_envelope(_ARBITRARY_VERSION, None, None)
     session = ServerSession(StubOutbound(), conn, standalone_outbound=conn.outbound)
-    assert session.protocol_version == "2025-03-26"
+    assert session.protocol_version == _ARBITRARY_VERSION
     assert session.client_params is None