-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Acknowledge notification POSTs with 202 on the 2026-07-28 HTTP entry #3326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,13 +20,15 @@ | |
| HEADER_MISMATCH, | ||
| INTERNAL_ERROR, | ||
| INVALID_PARAMS, | ||
| INVALID_REQUEST, | ||
| METHOD_NOT_FOUND, | ||
| MISSING_REQUIRED_CLIENT_CAPABILITY, | ||
| SERVER_INFO_META_KEY, | ||
| CallToolRequestParams, | ||
| CallToolResult, | ||
| DiscoverResult, | ||
| EmptyResult, | ||
| ErrorData, | ||
| Implementation, | ||
| JSONRPCError, | ||
| JSONRPCResponse, | ||
|
|
@@ -153,6 +155,39 @@ | |
| assert "mcp-session-id" not in response.headers | ||
|
|
||
|
|
||
| @requirement("hosting:http:modern:notification-post-202") | ||
| @pytest.mark.parametrize("json_response", [True, False], ids=["json", "sse"]) | ||
| @pytest.mark.parametrize("stateless_http", [True, False], ids=["stateless-flag", "default"]) | ||
| async def test_modern_notification_post_is_acknowledged_202_and_a_posted_response_is_rejected( | ||
| json_response: bool, stateless_http: bool | ||
| ) -> None: | ||
| """A 2026-07-28 notification POST is answered 202 with no body; a posted response is 400 INVALID_REQUEST. | ||
|
|
||
| Spec-permitted (streamable-http §Sending Messages item 5): the server may accept (202) or refuse | ||
| (4xx) a notification POST, and the SDK accepts -- the same answer the legacy leg gives, so a | ||
| client's courtesy `notifications/cancelled` is not met with an error on one era only. | ||
| Spec-mandated (item 4): clients MUST NOT post responses, so one is refused. Driven through the | ||
| mounted app so the manager's header routing is in the path, under both response modes and both | ||
| values of the legacy-only `stateless_http` flag (neither is read before the modern entry answers). | ||
|
Check warning on line 171 in tests/interaction/transports/test_hosting_http_modern.py
|
||
|
Comment on lines
+169
to
+171
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [quality] nit: the new test's docstring claims "(neither is read before the modern entry answers)" about the json_response/stateless_http parametrization, but json_response IS read before the notification 202 — handle_modern_request's Accept gate ( Extended reasoning...Concrete cost: a documented-but-false invariant in the test that .claude/skills/test-quality/SKILL.md-style provenance docstrings are supposed to state accurately. A maintainer extending the notification arm (e.g. deciding whether a bare Verification: nit — the test docstring at tests/interaction/transports/test_hosting_http_modern.py:170-171 says "under both response modes and both values of the legacy-only |
||
| """ | ||
| notification = {"jsonrpc": "2.0", "method": "notifications/cancelled", "params": {"requestId": 1}} | ||
| posted_response: dict[str, Any] = {"jsonrpc": "2.0", "id": 1, "result": {}} | ||
| async with mounted_app(_server(), json_response=json_response, stateless_http=stateless_http) as (http, _): | ||
| acknowledged = await http.post( | ||
| "/mcp", json=notification, headers=_modern_headers(method="notifications/cancelled") | ||
| ) | ||
| refused = await http.post("/mcp", json=posted_response, headers=_modern_headers(method="tools/list")) | ||
|
|
||
| assert (acknowledged.status_code, acknowledged.content) == (202, b"") | ||
| assert "mcp-session-id" not in acknowledged.headers | ||
| assert refused.status_code == 400 | ||
| assert JSONRPCError.model_validate(refused.json()) == JSONRPCError( | ||
| jsonrpc="2.0", | ||
| id=None, | ||
| error=ErrorData(code=INVALID_REQUEST, message="Body must be a single JSON-RPC request or notification object"), | ||
| ) | ||
|
|
||
|
|
||
| @requirement("hosting:http:modern:initialize-removed") | ||
| async def test_modern_initialize_is_method_not_found() -> None: | ||
| """A 2026-07-28 initialize request that carries a valid envelope is answered METHOD_NOT_FOUND at HTTP 404. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When a notification POST includes duplicate routing headers, this path skips duplicate-header rejection and reads one folded
mcp-protocol-versionvalue. Checkfind_duplicated_routing_headerin_acknowledge_notificationand returnHEADER_MISMATCHbefore reading the version header.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deliberate for now. The duplicate check guards the header↔body cross-check, which only requests get; the notification arm reads the routing header once (first value, same as the manager) and acts on nothing, so rejecting here would only turn a 202-and-drop into a 400 for a message we drop either way, on a POST whose header requirements the revision leaves undefined. If the notification arm ever grows real rungs it will get its own classifier and the check belongs there.
AI Disclaimer