Skip to content

[Client] Probe for 2026-07-28 and fall back to the handshake - #547

Merged
chr-hertel merged 23 commits into
mainfrom
client-version-negotiation
Oct 10, 2026
Merged

chr-hertel merged 23 commits into
mainfrom
client-version-negotiation

Conversation

@chr-hertel

@chr-hertel chr-hertel commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Follows #546, split out of #537.

  • A client configured with 2026-07-28 probes with server/discover and falls back to initialize on 2025-11-25 unless the server proves to be modern, following the backward compatibility section of the spec - setFallbackProtocolVersion() picks the revision, null makes it modern-only
  • HTTP refusals and a dead stdio server fail the request at once instead of waiting out the timeout - a lost stdio connection fails the probe instead of falling back, flagged via the new TransportInterface::CONNECTION_LOST key in the error data
  • On a modern connection setLoggingLevel() rides on every request, ping() uses server/discover and the roots notification is skipped

The default stays 2025-11-25, the bump itself is #537.

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

Malformed HTTP refusals can still time out, and modern-only negotiation can accept responses without evidence of protocol support.

2 open findings
What changed in this PR

Adds modern-protocol probing and handshake fallback to the PHP client while retaining the 2025-11-25 default.

Changes:

  • Adds configurable fallback from modern discovery to legacy initialization.
  • Adapts logging, ping, and roots behavior to the negotiated protocol.
  • Handles transport refusals promptly and expands cross-era tests and documentation.
File Description
tests/​Unit/​ClientTest.php Tests disconnected logging calls.
tests/​Unit/​Client/​Transport/​StdioTransportTest.php Tests closed output and process exits.
tests/​Unit/​Client/​Transport/​HttpTransportTest.php Tests HTTP probe refusals.
tests/​Unit/​Client/​ProtocolTest.php Covers probing and fallback negotiation.
tests/​Unit/​Client/​ConfigurationTest.php Validates fallback configuration.
tests/​Integration/​SamplingTest.php Checks modern sampling rejection.
tests/​Integration/​NotificationTest.php Tests notifications across eras.
tests/​Integration/​IntegrationTestCase.php Adds shared protocol-era cases.
tests/​Integration/​HttpNegotiationTest.php Adds HTTP negotiation coverage.
tests/​Integration/​HandshakeTest.php Expands stdio negotiation coverage.
tests/​Integration/​Fixture/​http.php Adds configurable HTTP fixture.
tests/​Integration/​Fixture/​handshake.php Supports handshake-only fixtures.
tests/​Integration/​ElicitationTest.php Tests unsupported modern input handling.
src/​Client/​Transport/​StdioTransport.php Fails requests on closed output.
src/​Client/​Transport/​HttpTransport.php Converts HTTP refusals into errors.
src/​Client/​Stateless/​RequestEnvelope.php Carries per-request logging levels.
src/​Client/​Protocol.php Implements probing and fallback.
src/​Client/​Configuration.php Defines and validates fallback revision.
src/​Client/​Builder.php Exposes fallback configuration.
src/​Client.php Adapts calls to protocol era.
docs/​protocol-versions.md Explains negotiation and modern behavior.
docs/​client/​connecting.md Documents fallback configuration.
CHANGELOG.md Records client compatibility changes.

🧠 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/Client/Protocol.php
Comment thread src/Client/Transport/HttpTransport.php Outdated
@chr-hertel
chr-hertel added this pull request to stack #549 October 7, 2026 21:49
@chr-hertel chr-hertel added Client Issues & PRs related to the Client component 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 client-version-negotiation branch from 1577a81 to f04e149 Compare October 7, 2026 22:35
@chr-hertel
chr-hertel force-pushed the client-version-negotiation branch from f04e149 to cebcefb Compare October 7, 2026 22:58
@chr-hertel
chr-hertel requested a balanced review from Copilot October 7, 2026 23:15

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

Stdio EOF handling can overwrite completed responses, and malformed HTTP refusals can still leave requests waiting for timeout.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/Client/Transport/StdioTransport.php Outdated
Comment thread src/Client/Transport/HttpTransport.php Outdated

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

Stdio end-of-file handling can overwrite valid buffered replies with errors.

3 open findings

🧠 Review effort: Balanced


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

Comment thread src/Client/Transport/StdioTransport.php Outdated
@chr-hertel
chr-hertel force-pushed the client-version-negotiation branch from 17befc5 to f776209 Compare October 9, 2026 22:15
Base automatically changed from server-stdio-dual-era to main October 9, 2026 23:25
@chr-hertel
chr-hertel force-pushed the client-version-negotiation branch from f776209 to 75fd305 Compare October 9, 2026 23:34

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

Batched progress and logging notifications can still be delivered out of order, and the modern roots no-op lacks coverage.

1 open finding
3 resolved since last review

🧠 Review effort: Balanced

Comment thread src/Client/Transport/StdioTransport.php Outdated

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.

🔵 Needs a closer look

Modern roots handling and uncorrelated HTTP errors can produce behavior contrary to the intended negotiation semantics.

0 open findings

1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Allow modern clients to skip removed notifications without capability checks

src/​Client.php:380

The modern no-op is reached only after the roots.listChanged capability guard, so a modern client built with the default capabilities still throws instead of skipping this removed notification. Check the connection/era first, then enforce the legacy capability only for handshake-era connections.

Medium severity Prevent uncorrelated error bodies from overriding probe results

src/​Client/​Transport/​HttpTransport.php:260

Once the same-ID/well-formed check above fails, this body may be malformed or belong to another request, but its error code and data are still reassigned to the current request. For example, an id-less or mismatched -32022 body can make a probe fail as an authoritative modern-version refusal instead of falling back. Treat every uncorrelated body as the generic HTTP refusal; only the validated branch above should preserve JSON-RPC error details.

🧠 Review effort: Balanced

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 Client Issues & PRs related to the Client component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants