[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.

[soyuka]

🟡 for*() factories left behind (L89–L111). forMethodNotFound, forInvalidParams, forInternalError, forServerError, and forResourceNotFound still default $id = ''. Any caller that forgets to pass the id reintroduces the exact bug this PR fixes. Suggest widening them to string|int|null with null default for consistency, even if no current call site hits that path.

🟡 fromArray() no longer round-trips its own output (L60–L65). The ctor and jsonSerialize() now accept/emit id: null, but fromArray() still throws on null (isset($data['id']) is false for null, then the type guard rejects it). Encoding a parse-error response and decoding it back via fromArray() blows up. Either allow null here or document the asymmetry.

🔵 @phpstan-type ErrorData is stale (L19–L25). Still declares id: string|int; widen to string|int|null to match the ctor / getId() / jsonSerialize() shape.

[soyuka]

❓ JSON-RPC 2.0 allows id: null in requests. Here isset() drops it, so a null-id invalid request falls back to the null default in forInvalidRequest() — same outcome, fine. Intentional? If so, worth a one-line comment so the next reader doesn't "fix" it by switching to array_key_exists.

[soyuka]

🔵 nit: assertion intent and actual behavior don't quite line up. jsonSerialize() always emits the id key, so the response is always id: null, never omitted — the "or omitted" branch can never trigger. Tighter:

$this->assertArrayHasKey('id', $decoded);
$this->assertNull($decoded['id'], 'Unrecoverable parse error must emit id: null');

That also catches a future regression where someone drops the key entirely (which would break clients that strictly validate the JSON-RPC shape).

[soyuka]

🔵 nit: PR description specifically calls out that id: 0 and string ids are preserved (type-checked, not truthiness), but only int 42 is exercised here. Worth adding a data provider or two extra cases for id: 0 (the truthiness trap) and a string id like "req-1" — both are cheap to assert and lock down the two explicit promises.