Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
-----

* [BC Break] `Mcp\Schema\JsonRpc\Error` accepts `null` as its `$id`, and `getId()` may return it. An error response whose id could not be read now omits the member instead of sending `"id": ""` — which claimed the peer had issued a request with an empty-string id. All the `for*()` factories default to `null`, `fromArray()` accepts a missing or explicitly-null id, and `MessageFactory` decodes both as an id-less error rather than rejecting them.
* Preserve the original request `id` on an invalid-but-parseable message (`-32600`) instead of answering it id-less: `InvalidInputMessageException` now carries the recoverable id via `getRequestId()`/`setRequestId()`, threaded from `MessageFactory` through to the error response.
* [BC Break] Drop the SDK-only name pattern on `ResourceDefinition`/`ResourceTemplate` `$name` — the spec allows any string (its own examples use `main.rs` and `Project Files`). URI/URI-template validation is unchanged.
* Add `ClientGateway::supportsExtension()`, `Client\Builder::enableExtension()`, and `ClientCapabilities::withExtensions()` so clients can negotiate and check protocol extensions (e.g. MCP Apps) the same way servers already do. [BC Break] `ServerExtensionInterface` is replaced by the side-agnostic `Mcp\Schema\Extension\ExtensionInterface`.
* Deprecate Roots, Sampling and Logging per SEP-2577 (protocol revision `2026-07-28`, earliest removal `2027-07-28`). They keep working but using them now triggers a deprecation notice — migrate to tool arguments/resource URIs, a direct LLM provider API, and stderr/OpenTelemetry respectively.
Expand Down
13 changes: 13 additions & 0 deletions src/Exception/InvalidInputMessageException.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,17 @@
*/
class InvalidInputMessageException extends \InvalidArgumentException implements ExceptionInterface
{
private string|int|null $requestId = null;

public function getRequestId(): string|int|null
{
return $this->requestId;
}

public function setRequestId(string|int|null $requestId): self
{
$this->requestId = $requestId;

return $this;
}
}
5 changes: 5 additions & 0 deletions src/JsonRpc/MessageFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ public function create(string $input): array

$messages[] = $this->createMessage($message);
} catch (InvalidInputMessageException $e) {
// Recover the id only when it's a valid JSON-RPC scalar;
// a null or malformed id is left at the exception's null default.
if (\is_array($message) && isset($message['id']) && (\is_string($message['id']) || \is_int($message['id']))) {
Comment thread
valeriudev marked this conversation as resolved.
$e->setRequestId($message['id']);
}
$messages[] = $e;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Schema/JsonRpc/Error.php
Comment thread
valeriudev marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*
* @phpstan-type ErrorData array{
* jsonrpc: string,
* id: string|int,
* id: string|int|null,
* code: int,
* message: string,
* data?: mixed,
Expand Down
2 changes: 1 addition & 1 deletion src/Server/Protocol.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ private function handleInvalidMessage(TransportInterface $transport, InvalidInpu
{
$this->logger->warning('Failed to create message.', ['exception' => $exception]);

$error = Error::forInvalidRequest($exception->getMessage());
$error = Error::forInvalidRequest($exception->getMessage(), $exception->getRequestId());
$this->sendResponse($transport, $error, $session);
}

Expand Down
14 changes: 14 additions & 0 deletions tests/Unit/Client/ProtocolTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,20 @@ public function testIgnoresIdLessErrorResponse(): void
$this->assertCount(1, $logger->warnings);
}

#[TestDox('stores an error response under its id so the pending request can be correlated')]
public function testErrorResponseWithIdIsStoredForItsPendingRequest(): void
{
$protocol = new Protocol();

$protocol->processMessage('{"jsonrpc": "2.0", "id": 7, "error": {"code": -32601, "message": "Method not found"}}');

$response = $protocol->getState()->consumeResponse(7);

$this->assertInstanceOf(Error::class, $response);
$this->assertSame(7, $response->getId());
$this->assertSame(Error::METHOD_NOT_FOUND, $response->code);
}

private function createConfiguration(ProtocolVersion $protocolVersion): Configuration
{
return new Configuration(
Expand Down
95 changes: 95 additions & 0 deletions tests/Unit/Server/ProtocolTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Mcp\Server\Session\SessionManagerInterface;
use Mcp\Server\Transport\TransportInterface;
use Mcp\Tests\Unit\Fixtures\ThrowingRequest;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\TestDox;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -323,6 +324,40 @@ public function testInvalidJsonReturnsParseError(): void
);
}

#[TestDox('Unrecoverable parse error does not fabricate an empty-string id')]
public function testParseErrorDoesNotFabricateEmptyStringId(): void
{
$sentPayload = null;
$this->transport->expects($this->once())
->method('send')
->willReturnCallback(static function ($data) use (&$sentPayload) {
$sentPayload = $data;
});

$protocol = new Protocol(
requestHandlers: [],
notificationHandlers: [],
messageFactory: MessageFactory::make(),
sessionManager: $this->sessionManager,
);

// Well-formed JSON nested past PHP's json_decode() depth limit (512), mirroring
// issue #333: json_decode() throws "Maximum stack depth exceeded" so the request
// carries a real numeric id (900512) that cannot be recovered once decoding fails.
$deeplyNested = str_repeat('[', 600).str_repeat(']', 600);
$input = '{"jsonrpc":"2.0","id":900512,"method":"initialize","params":'.$deeplyNested.'}';

$protocol->processInput($this->transport, $input, null);

$this->assertNotNull($sentPayload);
$decoded = json_decode($sentPayload, true);
$this->assertSame(Error::PARSE_ERROR, $decoded['error']['code']);
// The original id is genuinely unrecoverable after a parse failure: it must never be
// fabricated as an empty string, and — per the MCP `RequestId` schema, which never
// allows `null` — the key must be omitted rather than sent as `id: null`.
$this->assertArrayNotHasKey('id', $decoded, 'Unrecoverable parse error must omit id, not fabricate one');
}

#[TestDox('Invalid message structure returns error')]
public function testInvalidMessageStructureReturnsError(): void
{
Expand Down Expand Up @@ -530,6 +565,66 @@ public function testFailingNotificationListenerDoesNotProduceResponse(): void
$this->assertSame([], $protocol->consumeOutgoingMessages($sessionId));
}

/**
* @return iterable<string, array{string, string|int}>
*/
public static function recoverableIdProvider(): iterable
{
yield 'positive int' => ['{"jsonrpc": "2.0", "id": 42, "params": {}}', 42];
yield 'zero int (truthiness trap)' => ['{"jsonrpc": "2.0", "id": 0, "params": {}}', 0];
yield 'string id' => ['{"jsonrpc": "2.0", "id": "req-1", "params": {}}', 'req-1'];
}

#[DataProvider('recoverableIdProvider')]
#[TestDox('Invalid but parseable message preserves its recoverable id')]
public function testInvalidMessagePreservesRecoverableId(string $input, string|int $expectedId): void
{
$session = $this->createMock(SessionInterface::class);

$this->sessionManager->method('createWithId')->willReturn($session);
$this->sessionManager->method('exists')->willReturn(true);

// Configure session mock for queue operations (mirrors testInvalidMessageStructureReturnsError).
$queue = [];
$session->method('get')->willReturnCallback(static function ($key, $default = null) use (&$queue) {
if ('_mcp.outgoing_queue' === $key) {
return $queue;
}

return $default;
});

$session->method('set')->willReturnCallback(static function ($key, $value) use (&$queue) {
if ('_mcp.outgoing_queue' === $key) {
$queue = $value;
}
});

$protocol = new Protocol(
requestHandlers: [],
notificationHandlers: [],
messageFactory: MessageFactory::make(),
sessionManager: $this->sessionManager,
);

$sessionId = Uuid::v4();
// Valid JSON carrying a real id but missing method/result/error: the message is
// structurally invalid, yet its id IS recoverable from the decoded payload.
$protocol->processInput(
$this->transport,
$input,
$sessionId
);

$outgoing = $protocol->consumeOutgoingMessages($sessionId);
$this->assertCount(1, $outgoing);

$message = json_decode($outgoing[0]['message'], true);
$this->assertArrayHasKey('error', $message);
$this->assertEquals(Error::INVALID_REQUEST, $message['error']['code']);
$this->assertSame($expectedId, $message['id'], 'Invalid-but-parseable message must preserve its recoverable id, not return ""');
}

#[TestDox('Request without handler returns method not found error')]
public function testRequestWithoutHandlerReturnsMethodNotFoundError(): void
{
Expand Down