Skip to content

Commit 1fa5992

Browse files
authored
Merge branch 'main' into mime-type-validation-1756
2 parents 7672848 + c7cbfbb commit 1fa5992

File tree

3 files changed

+123
-7
lines changed

3 files changed

+123
-7
lines changed

CONTRIBUTING.md

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,43 @@
22

33
Thank you for your interest in contributing to the MCP Python SDK! This document provides guidelines and instructions for contributing.
44

5+
## Before You Start
6+
7+
We welcome contributions! These guidelines exist to save everyone time, yours included. Following them means your work is more likely to be accepted.
8+
9+
**All pull requests require a corresponding issue.** Unless your change is trivial (typo, docs tweak, broken link), create an issue first. Every merged feature becomes ongoing maintenance, so we need to agree something is worth doing before reviewing code. PRs without a linked issue will be closed.
10+
11+
Having an issue doesn't guarantee acceptance. Wait for maintainer feedback or a `ready for work` label before starting. PRs for issues without buy-in may also be closed.
12+
13+
Use issues to validate your idea before investing time in code. PRs are for execution, not exploration.
14+
15+
### The SDK is Opinionated
16+
17+
Not every contribution will be accepted, even with a working implementation. We prioritize maintainability and consistency over adding capabilities. This is at maintainers' discretion.
18+
19+
### What Needs Discussion
20+
21+
These always require an issue first:
22+
23+
- New public APIs or decorators
24+
- Architectural changes or refactoring
25+
- Changes that touch multiple modules
26+
- Features that might require spec changes (these need a [SEP](https://github.com/modelcontextprotocol/modelcontextprotocol) first)
27+
28+
Bug fixes for clear, reproducible issues are welcome—but still create an issue to track the fix.
29+
30+
### Finding Issues to Work On
31+
32+
| Label | For | Description |
33+
|-------|-----|-------------|
34+
| [`good first issue`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | Newcomers | Can tackle without deep codebase knowledge |
35+
| [`help wanted`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) | Experienced contributors | Maintainers probably won't get to this |
36+
| [`ready for work`](https://github.com/modelcontextprotocol/python-sdk/issues?q=is%3Aopen+is%3Aissue+label%3A%22ready+for+work%22) | Maintainers | Triaged and ready for a maintainer to pick up |
37+
38+
Issues labeled `needs confirmation` or `needs maintainer action` are **not** ready for work—wait for maintainer input first.
39+
40+
Before starting, comment on the issue so we can assign it to you. This prevents duplicate effort.
41+
542
## Development Setup
643

744
1. Make sure you have Python 3.10+ installed
@@ -76,13 +113,29 @@ pre-commit run --all-files
76113
- Add type hints to all functions
77114
- Include docstrings for public APIs
78115

79-
## Pull Request Process
116+
## Pull Requests
117+
118+
By the time you open a PR, the "what" and "why" should already be settled in an issue. This keeps reviews focused on implementation.
119+
120+
### Scope
121+
122+
Small PRs get reviewed fast. Large PRs sit in the queue.
123+
124+
A few dozen lines can be reviewed in minutes. Hundreds of lines across many files takes real effort and things slip through. If your change is big, break it into smaller PRs or get alignment from a maintainer first.
125+
126+
### What Gets Rejected
127+
128+
- **No prior discussion**: Features or significant changes without an approved issue
129+
- **Scope creep**: Changes that go beyond what was discussed
130+
- **Misalignment**: Even well-implemented features may be rejected if they don't fit the SDK's direction
131+
- **Overengineering**: Unnecessary complexity for simple problems
132+
133+
### Checklist
80134

81135
1. Update documentation as needed
82136
2. Add tests for new functionality
83137
3. Ensure CI passes
84-
4. Maintainers will review your code
85-
5. Address review feedback
138+
4. Address review feedback
86139

87140
## Code of Conduct
88141

src/mcp/server/streamable_http_manager.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
StreamableHTTPServerTransport,
2323
)
2424
from mcp.server.transport_security import TransportSecuritySettings
25+
from mcp.types import INVALID_REQUEST, ErrorData, JSONRPCError
2526

2627
logger = logging.getLogger(__name__)
2728

@@ -276,10 +277,21 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
276277

277278
# Handle the HTTP request and return the response
278279
await http_transport.handle_request(scope, receive, send)
279-
else: # pragma: no cover
280-
# Invalid session ID
280+
else:
281+
# Unknown or expired session ID - return 404 per MCP spec
282+
# TODO: Align error code once spec clarifies
283+
# See: https://github.com/modelcontextprotocol/python-sdk/issues/1821
284+
error_response = JSONRPCError(
285+
jsonrpc="2.0",
286+
id="server-error",
287+
error=ErrorData(
288+
code=INVALID_REQUEST,
289+
message="Session not found",
290+
),
291+
)
281292
response = Response(
282-
"Bad Request: No valid session ID provided",
283-
status_code=HTTPStatus.BAD_REQUEST,
293+
content=error_response.model_dump_json(by_alias=True, exclude_none=True),
294+
status_code=HTTPStatus.NOT_FOUND,
295+
media_type="application/json",
284296
)
285297
await response(scope, receive, send)

tests/server/test_streamable_http_manager.py

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

3+
import json
34
from typing import Any
45
from unittest.mock import AsyncMock, patch
56

@@ -11,6 +12,7 @@
1112
from mcp.server.lowlevel import Server
1213
from mcp.server.streamable_http import MCP_SESSION_ID_HEADER, StreamableHTTPServerTransport
1314
from mcp.server.streamable_http_manager import StreamableHTTPSessionManager
15+
from mcp.types import INVALID_REQUEST
1416

1517

1618
@pytest.mark.anyio
@@ -262,3 +264,52 @@ async def mock_receive():
262264

263265
# Verify internal state is cleaned up
264266
assert len(transport._request_streams) == 0, "Transport should have no active request streams"
267+
268+
269+
@pytest.mark.anyio
270+
async def test_unknown_session_id_returns_404():
271+
"""Test that requests with unknown session IDs return HTTP 404 per MCP spec."""
272+
app = Server("test-unknown-session")
273+
manager = StreamableHTTPSessionManager(app=app)
274+
275+
async with manager.run():
276+
sent_messages: list[Message] = []
277+
response_body = b""
278+
279+
async def mock_send(message: Message):
280+
nonlocal response_body
281+
sent_messages.append(message)
282+
if message["type"] == "http.response.body":
283+
response_body += message.get("body", b"")
284+
285+
# Request with a non-existent session ID
286+
scope = {
287+
"type": "http",
288+
"method": "POST",
289+
"path": "/mcp",
290+
"headers": [
291+
(b"content-type", b"application/json"),
292+
(b"accept", b"application/json, text/event-stream"),
293+
(b"mcp-session-id", b"non-existent-session-id"),
294+
],
295+
}
296+
297+
async def mock_receive():
298+
return {"type": "http.request", "body": b"{}", "more_body": False} # pragma: no cover
299+
300+
await manager.handle_request(scope, mock_receive, mock_send)
301+
302+
# Find the response start message
303+
response_start = next(
304+
(msg for msg in sent_messages if msg["type"] == "http.response.start"),
305+
None,
306+
)
307+
assert response_start is not None, "Should have sent a response"
308+
assert response_start["status"] == 404, "Should return HTTP 404 for unknown session ID"
309+
310+
# Verify JSON-RPC error format
311+
error_data = json.loads(response_body)
312+
assert error_data["jsonrpc"] == "2.0"
313+
assert error_data["id"] == "server-error"
314+
assert error_data["error"]["code"] == INVALID_REQUEST
315+
assert error_data["error"]["message"] == "Session not found"

0 commit comments

Comments
 (0)