Skip to content

Conversation

@CodeWithKyrian
Copy link
Contributor

This PR introduces a new Protocol layer, reshapes message parsing, and redefines how custom handlers plug into the server. The Protocol exposes handleRequest, handleNotification, and a placeholder handleResponse, along with sendRequest, sendResponse, and sendNotification. Only response sending is wired for now, but the other direction-facing methods lay the foundation for server-initiated messaging.

Motivation and Context

  • The old JsonRpcHandler/MethodHandlerInterface blended request and notification semantics, making it impossible to enforce the JSON‑RPC contract (requests must yield Response|Error, notifications return nothing).
  • Preparing for server‑initiated messaging (sampling, logging, custom notifications) required a bidirectional protocol layer that can send as well as receive.
  • The existing message factory only understood requests/notifications; it could not parse responses or errors, blocking any future server‑initiated flows.
  • To keep responsibilities clear, the server should manage the transport lifecycle (initializeconnectlistenclose) while the protocol owns all transport callbacks (onMessage, onSessionEnd, and any future hooks) and output routing.

What’s Changed

  • Renamed JsonRpcHandler to Protocol and made it the transport-facing façade that registers callbacks, parses all JSON‑RPC message types, dispatches request/notification handlers, and emits responses.
  • Shifted Server::run() into a small lifecycle orchestrator: it initializes the transport, connects the protocol, runs listen(), and guarantees close(). No more callback wiring in Server.
  • Replaced the coarse MethodHandlerInterface with RequestHandlerInterface and NotificationHandlerInterface, enforcing that requests always return Response|Error and notifications remain fire-and-forget.
  • Refactored MessageFactory to emit typed Request, Notification, Response, and Error objects (or InvalidInputMessageException), laying the groundwork for bidirectional messaging.
  • Added new builder hooks, addRequestHandler(s) and addNotificationHandler(s), and updated docs, examples, and tests to use the new abstractions.

Breaking Changes

  • Builder::addMethodHandler(s) and MethodHandlerInterface have been removed.
  • Use addRequestHandler(s)/RequestHandlerInterface for JSON‑RPC requests and
    addNotificationHandler(s)/NotificationHandlerInterface for notifications.

@chr-hertel
Copy link
Member

a bit funny to see that because having notification and request handlers separate was exactly how i started:
https://github.com/php-llm/mcp-sdk/tree/5bfd6bf757da756501b73242e11cf386de0c6ddc/src/Server

i treated both similar, that why i brought it up one level and unified. buuut, the context here is different - so i'm in for doing it 👍

Co-authored-by: Christopher Hertel <mail@christopher-hertel.de>
Comment on lines 263 to 274
public function testNotificationMethodUsedAsRequest(): void
{
$json = '{"jsonrpc": "2.0", "result": {"status": "ok"}}';
// When a notification method is used with an id, it should still create the notification
// The fromArray validation will handle any issues
$json = '{"jsonrpc": "2.0", "method": "notifications/initialized", "id": 1}';

$results = $this->factory->create($json);

$this->assertCount(1, $results);
// The notification class will reject the id in fromArray validation
$this->assertInstanceOf(InvalidInputMessageException::class, $results[0]);
$this->assertStringContainsString('id', $results[0]->getMessage());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra mile 😎 👍

@CodeWithKyrian CodeWithKyrian merged commit 54a5d14 into modelcontextprotocol:main Oct 12, 2025
10 checks passed
@CodeWithKyrian CodeWithKyrian deleted the feat/jsonrpc-handler-to-protocol branch October 12, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants