FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[Client] Support cooperative tool-call cancellation and per-call deadlines by ineersa · Pull Request #518 · modelcontextprotocol/php-sdk · GitHub

Repository navigation

[Client] Support cooperative tool-call cancellation and per-call deadlines - #518

Open
ineersa wants to merge 1 commit into
modelcontextprotocol:mainfrom
ineersa:task/add-proper-mcp-tool-call-cancellation
Open

ineersa wants to merge 1 commit into
modelcontextprotocol:mainfrom
ineersa:task/add-proper-mcp-tool-call-cancellation

Conversation

ineersa commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Closes #517.

Changes

Adds optional cancellation and timeoutSeconds arguments to Client::callTool(). Existing calls remain valid. An explicit per-call timeout replaces the configured request timeout and uses one deadline across protocol exchanges.

  • Check interruption before sending, after synchronous I/O returns, and after a suspended request resumes. An interrupted call cannot return a buffered success.
  • Clear pending state, discard late responses, and leave the connection available for subsequent calls.
  • Send notifications/cancelled over STDIO and handshake-era HTTP. HTTP protocol 2026-07-28 uses response-stream closure instead.
  • Log notification-delivery failures without replacing the original interruption. Discard notification response bodies so they cannot replace an active SSE stream.
  • Document the API and transport tradeoffs in docs/client/transports.md.

Async behavior and limitations

The API remains synchronous and framework-agnostic. No async HTTP client or event loop is added.

HTTP cancellation is cooperative. Blocking PSR-18 requests and PSR-7 body reads must return before the SDK can observe interruption. A legacy HTTP cancellation POST can itself block. Per-call deadlines are therefore not hard HTTP wall-clock limits; callers still need HTTP-client network timeouts. Cancellation does not guarantee that server-side work stops or rolls back.

Validation

  • make ci: passed, 1,775 tests and 4,646 assertions, with 7 existing Inspector skips. Those skips are explicitly declared in HttpClientCommunicationTest for logging/sampling limitations.
  • make docs-guides: strict build passed.
  • make conformance-tests: client matched its expected-failure baseline, 12 checks passed and 42 expected failures. The initial server run was blocked by fixture-directory permissions; after applying the setup used by CI, make conformance-server passed all 80 checks.
  • Hatfield integration against this SDK: 189 MCP tests and 727 assertions passed.

New regression coverage uses protocol/HTTP unit tests and a real STDIO integration fixture. It covers synchronous JSON interruption, cancellation during body reads, interrupted suspended requests with buffered replies, version-dependent signaling, notification failures, late-response cleanup, and connection reuse. No new Inspector scenarios were added.

chr-hertel added Client Issues & PRs related to the Client component enhancement Request for a new feature that's not currently supported labels Oct 6, 2026
Comment thread src/Client/Protocol.php
Comment on lines +566 to +568
if ($this->transport instanceof HttpTransport && true === $this->state->getProtocolVersion()?->isModern()) {
return;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

those are the ugly parts of this SDK - global layer, but concrete transport and version specific ... don't have an idea that's worth the effort, somehow honest: that's MCP :D
let's keep it unless you have a cheap alternative

chr-hertel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor comment in case you have an idea - thanks already @ineersa!

This branch has not been deployed

No deployments
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

Labels

Client Issues & PRs related to the Client component enhancement Request for a new feature that's not currently supported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Client] Support cancellation and per-call deadlines in Client::callTool()

2 participants


Back | FazBrowse Home | New Git URL