[Schema][Server] Preserve request id in JSON-RPC error responses instead of fabricating id:""

[valeriudev]

The bug

When the server receives input it cannot turn into a valid message, the JSON-RPC error response carries a fabricated empty-string id that was never in the request:

{"jsonrpc":"2.0","id":"","error":{"code":-32700,...}}

The reporter sent an initialize request with a real numeric id (900512) nested past PHP's json_decode() depth limit; the server replied with id:"". An empty string is not a valid JSON-RPC / MCP RequestId, so this breaks client correlation.

Root cause

Two paths both collapsed to "":

• Error::forParseError() and Error::forInvalidRequest() defaulted $id to '', and Error::$id was typed string|int (no null). On an unrecoverable parse failure (\JsonException in Protocol::processInput()) the factory was called with no id, so "" reached the wire.
• On an invalid-but-parseable message, MessageFactory threw InvalidInputMessageException without carrying the decoded id, so Protocol::handleInvalidMessage() also fell back to "" — even though the real id was recoverable from the decoded payload.

The fix

• Unrecoverable parse errors (-32700) now return id: null, per JSON-RPC 2.0 (id is null when it cannot be determined).
• Invalid-but-parseable requests (-32600) now preserve the original request id.
• Error::$id / Error::getId() and the $id parameter of Error::forParseError() / Error::forInvalidRequest() are widened to string|int|null. jsonSerialize() is unchanged — it now emits "id":null instead of "id":"".
• The recoverable id is threaded from MessageFactory through InvalidInputMessageException via new getRequestId()/setRequestId(). The id is only attached when the decoded value is a valid string|int (mirroring Request/Error::fromArray validation); id:0 is preserved (type-checked, not truthiness), while null/true/array/missing stay null.

No [BC Break]: the type is widened, not narrowed, and the only behavioral change is replacing an invalid fabricated id with a spec-compliant one.

Tests

Two regression tests in tests/Unit/Server/ProtocolTest.php:

• testParseErrorDoesNotFabricateEmptyStringId — feeds JSON nested 600 deep so json_decode() throws, asserts error.code === -32700, id !== '', and that id is null or omitted.
• testInvalidMessagePreservesRecoverableId — feeds valid JSON with id 42 but no method/result/error, asserts error.code === -32600 and id === 42.

Both were red for the right reason (got "") before the fix and are green after. Full unit suite (766 tests), phpstan (level 6), and php-cs-fixer all pass.

Fixes #333

[soyuka]

Automatic review worth a look.

[chr-hertel]

Correct me if I'm wrong, but I think you missed Mcp\Client\State\ClientState::storeResponse here.

Sorry about PHPStan not catching that - somehow expected with this kind of refactoring tbh.

[valeriudev]

Correct me if I'm wrong, but I think you missed Mcp\Client\State\ClientState::storeResponse here.

Sorry about PHPStan not catching that - somehow expected with this kind of refactoring tbh.

You're right, fixed - thanks. A null-id error can't be matched to any pending request, so handleResponse() now logs and drops it before it reaches the state. Added a test in tests/Unit/Client/ProtocolTest.php

[chr-hertel]

Hey @valeriudev — thanks for tracking this down, the empty-string id bug was a real problem.

There's some overlap with #442, which also fixes #333 and just merged. The two took different stances on what an unrecoverable id should serialize as: #442 omits the id key, this PR emitted id: null. I checked the actual MCP JSON schema (RequestId in modelcontextprotocol/modelcontextprotocol) across every revision including 2026-07-28, and RequestId never permits null — it's always type: ["string", "integer"]. As of 2025-11-25, id was made optional on JSONRPCErrorResponse rather than nullable, which is exactly the omit approach.

Since #442 already landed with that semantics, I rebased this branch onto current main:

• Dropped the Error.php/fromArray()/jsonSerialize() changes — already done by [Schema] Omit the id of an error response that never had one #442, so they were pure conflicts.
• Kept the part that's genuinely additive and not covered by [Schema] Omit the id of an error response that never had one #442: recovering the real request id for an invalid-but-parseable message (valid JSON, bad/missing method) instead of answering it id-less. That's InvalidInputMessageException::getRequestId()/setRequestId() threaded from MessageFactory through Protocol::handleInvalidMessage().
• Updated the parse-error test assertion to require the id be omitted, not null.
• Kept your authorship on every commit.

Full unit suite, phpstan, and cs-fixer all pass on the rebased branch, force-pushed to this PR. Let me know if you want any of that reworked — happy to iterate.