Skip to content

Feature/mcp spec polish#57

Merged
RomanEmreis merged 3 commits intomainfrom
feature/mcp-spec-polish
Mar 15, 2026
Merged

Feature/mcp spec polish#57
RomanEmreis merged 3 commits intomainfrom
feature/mcp-spec-polish

Conversation

@RomanEmreis
Copy link
Copy Markdown
Owner

Summary

What does this change do and why?

Fix JSON-RPC 2.0 protocol violation: the server was sending a response to every client notification, and notifications/cancelled was silently broken on both transports.

Root causes:

  1. message_middleware unconditionally called sender.send(resp.into()) for all message types, including Message::Notification. This wrote
    {"jsonrpc":"2.0","id":"(no id)","result":{}} to the wire for every incoming notification — a direct violation of JSON-RPC 2.0 §4 ("A Notification is a Request object
    without an 'id' member ... The Server MUST NOT reply to a Notification").

  2. handle_notification never dispatched notifications/cancelled to the cancellation logic. The notifications_cancel handler was registered via map_handler (the
    request handler map), but notifications are routed to handle_notification, not through that map — so cancel_request() was never called.

  3. Streamable HTTP handle_message returned 202 Accepted for notifications without forwarding them to the app at all, so they were silently dropped before reaching
    handle_notification.

Changes:

  • handle_notification now takes runtime: ServerRuntime, dispatches notifications/cancelled to runtime.options().cancel_request() directly, and returns () instead
    of a dummy Response.
  • handle_message returns Option<Response> — notifications yield None, requests and received responses yield Some(_).
  • message_middleware only calls sender.send() when handle_message returns Some.
  • HTTP handle_message forwards notifications to the app via manager.sender before returning 202.
  • Removed dead map_handler registrations and the now-unreachable notifications_init / notifications_cancel handler functions.

Type

  • Bug fix

Checklist

  • I added/updated tests where it makes sense
  • I updated docs/examples if needed
  • This change is backwards-compatible (or clearly marked as breaking)
  • I ran formatting/lints locally (if applicable)

Notes for reviewers

The Message::Response path in message_middleware is intentionally left unchanged — when an HTTP client POSTs a response (e.g. answering a server-initiated sampling
request), handle_response returns Response::empty(resp_id) which the HTTP dispatch loop uses to complete the pending oneshot channel and unblock the HTTP handler.
Changing that would require reworking the HTTP request/response correlation mechanism and is a separate concern.

@RomanEmreis RomanEmreis self-assigned this Mar 15, 2026
@RomanEmreis RomanEmreis added bug Something isn't working chore labels Mar 15, 2026
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@RomanEmreis RomanEmreis merged commit 45482bb into main Mar 15, 2026
3 checks passed
@RomanEmreis RomanEmreis deleted the feature/mcp-spec-polish branch March 15, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant