Skip to content

[Server] Serve both protocol eras over stdio - #546

Open
chr-hertel wants to merge 11 commits into
mainfrom
server-stdio-dual-era
Open

chr-hertel wants to merge 11 commits into
mainfrom
server-stdio-dual-era

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

Split out of #537.

  • StdioTransport settles the era on the client's first request: a 2026-07-28 _meta envelope gets the modern dispatcher, anything else the handshake. A request from the other era is refused afterwards (-32022 / -32600)
  • Modern requests share the one channel: progress & logs before the result, subscriptions/listen next to other requests, notifications/cancelled to stop one
  • A bare initialize on a modern-only endpoint gets -32022 naming the served revisions, and the handshake leg echoes the id when refusing a request without a session - so a probing client fails fast instead of timing out

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Stream frames are advanced before being written, which can delay progress notifications until subsequent work completes.

1 open finding
What changed in this PR

Adds dual-era stdio support, selecting the handshake or modern dispatcher from the first request.

Changes:

  • Routes stdio traffic by protocol era and supports modern streams and cancellation.
  • Adds headerless stateless dispatch and correlated protocol errors.
  • Expands tests and documentation for dual-era behavior.
File Description
src/​Server/​Transport/​StdioTransport.php Implements dual-era routing and stream handling.
src/​Server/​Stateless/​StatelessProtocol.php Adds headerless inline dispatch.
src/​Server/​Protocol.php Correlates missing-session errors by request ID.
tests/​Unit/​Server/​Transport/​StdioDualEraTest.php Tests stdio era selection and streams.
tests/​Unit/​Server/​Stateless/​StatelessProtocolTest.php Tests inline modern dispatch.
tests/​Unit/​Server/​ProtocolTest.php Verifies correlated handshake errors.
examples/​server/​bootstrap.php Documents dual-era example behavior.
docs/​run/​protocol-eras.md Documents stdio era selection.
docs/​protocol-versions.md Updates protocol compatibility guidance.
CHANGELOG.md Records the new behavior.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Server/Transport/StdioTransport.php Outdated
@chr-hertel
chr-hertel added this pull request to stack #549 October 7, 2026 21:49
@chr-hertel chr-hertel added Server Issues & PRs related to the Server component improves spec compliance Improves consistency with other SDKs such as TyepScript 2026-07-28 All issues and PRs related to the spec release 2026-07-28 labels Oct 7, 2026
@chr-hertel
chr-hertel force-pushed the server-stdio-dual-era branch from 62c01f5 to 6a22720 Compare October 7, 2026 22:35
@chr-hertel
chr-hertel force-pushed the server-stdio-dual-era branch from 6a22720 to 87e15e5 Compare October 7, 2026 22:52
@chr-hertel
chr-hertel requested a balanced review from Copilot October 7, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Revision advertising and cancellation handling can produce incorrect protocol behavior.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

if ($classification->modern && null === $this->stateless) {
// Served nothing but the handshake: say which revisions that
// is, the way the HTTP entry does, and leave the era open.
$this->writeError(Error::forUnsupportedProtocolVersion((string) $classification->claimedVersion, ProtocolVersion::handshakeVersions(), $request['id']));
Comment on lines +241 to +247
if (\is_array($decoded) && self::CANCELLED_NOTIFICATION === ($decoded['method'] ?? null) && !isset($decoded['id'])) {
$requestId = $decoded['params']['requestId'] ?? null;

if ((\is_string($requestId) || \is_int($requestId)) && isset($this->streams[$key = self::streamKey($requestId)])) {
unset($this->streams[$key]);
$this->logger->debug('StdioTransport dropped a cancelled request.', ['request_id' => $requestId]);
}
@chr-hertel
chr-hertel force-pushed the server-stdio-dual-era branch from 87e15e5 to 087aaac Compare October 8, 2026 00:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A leading JSON-RPC response incorrectly produces an id-less error before era selection.

3 open findings

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

return;
}

$this->handleMessage($message, $this->sessionId);

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2026-07-28 All issues and PRs related to the spec release 2026-07-28 improves spec compliance Improves consistency with other SDKs such as TyepScript Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants