Repository navigation
[Client] Probe for 2026-07-28 and fall back to the handshake - #547
Open
chr-hertel wants to merge 12 commits into
Open
chr-hertel wants to merge 12 commits into
chr-hertel wants to merge 12 commits into
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
October 7, 2026 21:29
Contributor
There was a problem hiding this comment.
🟡 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.
chr-hertel
added this pull request to stack #549
October 7, 2026 21:49
chr-hertel
added
Client
chr-hertel
force-pushed
the
client-version-negotiation
branch
from
October 7, 2026 22:35
1577a81 to
f04e149
Compare
chr-hertel
force-pushed
the
client-version-negotiation
branch
from
October 7, 2026 22:58
f04e149 to
cebcefb
Compare
| // whatever is still pending can only time out, so fail it now. Failed | ||
| // as answers rather than thrown, so each waiting fiber unwinds and | ||
| // clears its request instead of leaving it to time out a later one. | ||
| if (feof($this->stdout)) { |
Comment on lines
+282
to
+288
| if (\array_key_exists('result', $answer)) { | ||
| return true; | ||
| } | ||
|
|
||
| $error = $answer['error'] ?? null; | ||
|
|
||
| return \is_array($error) && \is_int($error['code'] ?? null) && \is_string($error['message'] ?? null); |
… on the modern era
chr-hertel
force-pushed
the
client-version-negotiation
branch
from
October 8, 2026 00:55
cebcefb to
1de6989
Compare
Contributor
There was a problem hiding this comment.
🟡 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 on lines
+286
to
+287
| if (\is_resource($this->stdout) && feof($this->stdout)) { | ||
| $this->failPending('The server process closed its output; it is no longer running.'); |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #546, split out of #537.
2026-07-28probes withserver/discoverand falls back toinitializeon2025-11-25unless the server proves to be modern, following the backward compatibility section of the spec -setFallbackProtocolVersion()picks the revision,nullmakes it modern-onlysetLoggingLevel()rides on every request,ping()usesserver/discoverand the roots notification is skippedThe default stays
2025-11-25, the bump itself is #537.