Skip to content

Commit 393ee87

Browse files
authored
Merge branch 'main' into feat/store-server-info-on-client-session
2 parents 1657034 + 7ba41dc commit 393ee87

File tree

8 files changed

+42
-25
lines changed

8 files changed

+42
-25
lines changed

.github/workflows/shared.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ jobs:
7070
- name: Run pytest with coverage
7171
shell: bash
7272
run: |
73+
uv run --frozen --no-sync coverage erase
7374
uv run --frozen --no-sync coverage run -m pytest -n auto
7475
uv run --frozen --no-sync coverage combine
7576
uv run --frozen --no-sync coverage report

CLAUDE.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,19 @@ This document contains critical information about working with this codebase. Fo
2828
- Bug fixes require regression tests
2929
- IMPORTANT: The `tests/client/test_client.py` is the most well designed test file. Follow its patterns.
3030
- IMPORTANT: Be minimal, and focus on E2E tests: Use the `mcp.client.Client` whenever possible.
31-
- IMPORTANT: Before pushing, verify 100% branch coverage on changed files by running
32-
`uv run --frozen pytest -x` (coverage is configured in `pyproject.toml` with `fail_under = 100`
33-
and `branch = true`). If any branch is uncovered, add a test for it before pushing.
31+
- Coverage: CI requires 100% (`fail_under = 100`, `branch = true`).
32+
- Full check: `./scripts/test` (~20s, matches CI exactly)
33+
- Targeted check while iterating:
34+
35+
```bash
36+
uv run --frozen coverage erase
37+
uv run --frozen coverage run -m pytest tests/path/test_foo.py
38+
uv run --frozen coverage combine
39+
uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0
40+
```
41+
42+
Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0`
43+
and `--include` scope the report to what you actually changed.
3444
- Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead:
3545
- Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test
3646
- For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()`

scripts/test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
set -ex
44

5+
uv run --frozen coverage erase
56
uv run --frozen coverage run -m pytest -n auto $@
67
uv run --frozen coverage combine
78
uv run --frozen coverage report

src/mcp/server/mcpserver/resources/base.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,7 @@ class Resource(BaseModel, abc.ABC):
2323
name: str | None = Field(description="Name of the resource", default=None)
2424
title: str | None = Field(description="Human-readable title of the resource", default=None)
2525
description: str | None = Field(description="Description of the resource", default=None)
26-
mime_type: str = Field(
27-
default="text/plain",
28-
description="MIME type of the resource content",
29-
pattern=r"^[a-zA-Z0-9]+/[a-zA-Z0-9\-+.]+(;\s*[a-zA-Z0-9\-_.]+=[a-zA-Z0-9\-_.]+)*$",
30-
)
26+
mime_type: str = Field(default="text/plain", description="MIME type of the resource content")
3127
icons: list[Icon] | None = Field(default=None, description="Optional list of icons for this resource")
3228
annotations: Annotations | None = Field(default=None, description="Optional annotations for the resource")
3329
meta: dict[str, Any] | None = Field(default=None, description="Optional metadata for this resource")

src/mcp/server/streamable_http_manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
272272
# Unknown or expired session ID - return 404 per MCP spec
273273
# TODO: Align error code once spec clarifies
274274
# See: https://github.com/modelcontextprotocol/python-sdk/issues/1821
275+
logger.info(f"Rejected request with unknown or expired session ID: {request_mcp_session_id[:64]}")
275276
error_response = JSONRPCError(
276277
jsonrpc="2.0",
277278
id=None,

tests/server/mcpserver/resources/test_resources.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ def dummy_func() -> str: # pragma: no cover
9191
)
9292
assert resource.mime_type == "application/json"
9393

94+
# RFC 2045 quoted parameter value (gh-1756)
95+
resource = FunctionResource(
96+
uri="resource://test",
97+
fn=dummy_func,
98+
mime_type='text/plain; charset="utf-8"',
99+
)
100+
assert resource.mime_type == 'text/plain; charset="utf-8"'
101+
94102
@pytest.mark.anyio
95103
async def test_resource_read_abstract(self):
96104
"""Test that Resource.read() is abstract."""

tests/server/test_streamable_http_manager.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for StreamableHTTPSessionManager."""
22

33
import json
4+
import logging
45
from typing import Any
56
from unittest.mock import AsyncMock, patch
67

@@ -269,7 +270,7 @@ async def mock_receive():
269270

270271

271272
@pytest.mark.anyio
272-
async def test_unknown_session_id_returns_404():
273+
async def test_unknown_session_id_returns_404(caplog: pytest.LogCaptureFixture):
273274
"""Test that requests with unknown session IDs return HTTP 404 per MCP spec."""
274275
app = Server("test-unknown-session")
275276
manager = StreamableHTTPSessionManager(app=app)
@@ -299,7 +300,8 @@ async def mock_send(message: Message):
299300
async def mock_receive():
300301
return {"type": "http.request", "body": b"{}", "more_body": False} # pragma: no cover
301302

302-
await manager.handle_request(scope, mock_receive, mock_send)
303+
with caplog.at_level(logging.INFO):
304+
await manager.handle_request(scope, mock_receive, mock_send)
303305

304306
# Find the response start message
305307
response_start = next(
@@ -315,6 +317,7 @@ async def mock_receive():
315317
assert error_data["id"] is None
316318
assert error_data["error"]["code"] == INVALID_REQUEST
317319
assert error_data["error"]["message"] == "Session not found"
320+
assert "Rejected request with unknown or expired session ID: non-existent-session-id" in caplog.text
318321

319322

320323
@pytest.mark.anyio

tests/test_examples.py

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
# pyright: reportUnknownArgumentType=false
66
# pyright: reportUnknownMemberType=false
77

8-
import sys
98
from pathlib import Path
109

1110
import pytest
@@ -65,12 +64,17 @@ async def test_direct_call_tool_result_return():
6564

6665

6766
@pytest.mark.anyio
68-
async def test_desktop(monkeypatch: pytest.MonkeyPatch):
67+
async def test_desktop(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
6968
"""Test the desktop server"""
70-
# Mock desktop directory listing
71-
mock_files = [Path("/fake/path/file1.txt"), Path("/fake/path/file2.txt")]
72-
monkeypatch.setattr(Path, "iterdir", lambda self: mock_files) # type: ignore[reportUnknownArgumentType]
73-
monkeypatch.setattr(Path, "home", lambda: Path("/fake/home"))
69+
# Build a real Desktop directory under tmp_path rather than patching
70+
# Path.iterdir — a class-level patch breaks jsonschema_specifications'
71+
# import-time schema discovery when this test happens to be the first
72+
# tool call in an xdist worker.
73+
desktop = tmp_path / "Desktop"
74+
desktop.mkdir()
75+
(desktop / "file1.txt").touch()
76+
(desktop / "file2.txt").touch()
77+
monkeypatch.setattr(Path, "home", lambda: tmp_path)
7478

7579
from examples.mcpserver.desktop import mcp
7680

@@ -85,15 +89,8 @@ async def test_desktop(monkeypatch: pytest.MonkeyPatch):
8589
content = result.contents[0]
8690
assert isinstance(content, TextResourceContents)
8791
assert isinstance(content.text, str)
88-
if sys.platform == "win32": # pragma: no cover
89-
file_1 = "/fake/path/file1.txt".replace("/", "\\\\") # might be a bug
90-
file_2 = "/fake/path/file2.txt".replace("/", "\\\\") # might be a bug
91-
assert file_1 in content.text
92-
assert file_2 in content.text
93-
# might be a bug, but the test is passing
94-
else: # pragma: lax no cover
95-
assert "/fake/path/file1.txt" in content.text
96-
assert "/fake/path/file2.txt" in content.text
92+
assert "file1.txt" in content.text
93+
assert "file2.txt" in content.text
9794

9895

9996
# TODO(v2): Change back to README.md when v2 is released

0 commit comments

Comments
 (0)