[Client] Send HTTP requests from outside the request fiber

[aton-of-data]

Fixes #504

Reproduction and mechanism: #504 (comment). Lines are on main at d1e8091.

Why it fails

connect() runs the handshake in a fiber (HttpTransport.php:141-148), and send() calls sendRequest() inside it (line 211). An amphp-based client does a bare Fiber::suspend() there and waits for the Revolt loop, which only runs from {main}. So the request never leaves the process. processFiber() then resumes it with the timeout Error (line 1027), and Amp throws the FutureIterator::consume() TypeError. Body reads (line 261) suspend the same way.

Change

• Inside the request fiber, send() suspends with the message.
• tick() first calls processSends(), which sends it from {main} and resumes the fiber. A send failure is thrown back into the fiber.
• processFiber() skips a fiber that waits for a send.
• connect() keeps what start() returns, as runRequest() does.
• checkInterruption() keeps what throw() returns: the notifications/cancelled sent since [Client] Throw TimeoutException on request timeout and cancel the request #567 now steps out too.

The errors from #577 are stored during the send, so the fiber still finds them as an immediate response. Every client takes this path, so the existing transport tests cover it too.

Verification

PHP 8.3.6, composer update --prefer-stable.

• New test: its fake client suspends like Revolt in sendRequest() and body reads. On main it fails with HTTP request failed: Resumed by something other than the event loop, with Mcp\Schema\JsonRpc\Error. With the change it passes.
• Unit: 2558 tests (4 skipped) on main, 2559 (4 skipped) with the change, all green.
• phpstan and php-cs-fixer fix --dry-run: clean.
• Integration: 108 tests, the same 8 DualEraElicitationTest errors on main and with the change. That example prints its shebang under php -S here.
• Interop: 175 tests, the same 44 Interop\Server\Http errors on main and with the change. All 72 Interop\Client tests pass (1 skipped).
• End to end with amphp's PSR-18 adapter (http-client 5.3.6) against the discovery-calculator example: main fails after 3.00s with the TypeError, the change connects and lists tools. Curl, native and discovery still connect.

Not verified

• Symfony's Psr18Client(new AmpHttpClient()) on this base (needs PHP 8.4).
• ReactPHP clients, listen: true with Amp.
• Conformance suites (Docker), ci-lowest, PHP other than 8.3.

[Copilot]

🟡 Changes recommended

Interruption handling can enter a blocking SSE read before sending the yielded cancellation notification.

1 open finding

• Drain cancellation send before processing the SSE stream · New

What changed in this PR

Moves HTTP I/O outside SDK-managed request fibers to support Amp-based PSR-18 clients.

Changes:

• Defers fiber-originated sends to the transport loop.
• Preserves suspension state during initialization and cancellation.
• Adds Amp-like fiber suspension coverage and changelog documentation.

File	Description
src/​Client/​Transport/​HttpTransport.php	Processes HTTP sends outside request fibers.
tests/​Unit/​Client/​Transport/​HttpTransportTest.php	Tests fiber-suspending HTTP clients and response bodies.
CHANGELOG.md	Documents the compatibility fix.

🧠 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]

haven't setup the AMP for reproduction, but @ineersa can you confirm that this fixes the issue for you?

[ineersa]

@chr-hertel Apart from review comment should be good to go

Tested PR #599 with PHP 8.5.10, Symfony HttpClient 8.1.9, and amphp/http-client 5.3.6 against a local MCP server.

SDK revision	Symfony Amp client	cURL client	Default discovery
Hatfield’s locked SDK bb74bbe	FAIL	PASS	PASS, selected cURL
PR base d1e8091	FAIL	PASS	PASS, selected cURL
PR head 9cb9a0c	PASS	PASS	PASS, selected cURL

Successful cases verified:

• MCP initialization.
• Tool listing.
• Calling calculate with 2 + 3 and asserting the result is 5.

Both baseline Amp failures reproduced the original error:

Amp\Internal\FutureIterator::consume(): Return value must be of type ?array,
Mcp\Schema\JsonRpc\Error returned

Hatfield regressions with the PR installed: 28 tests, 197 assertions, zero failures, errors, or skips.

Limitations: HTTP cancellation, listen:true, concurrency, and ReactPHP were not tested. Amp took approximately one second per request phase in this setup; that delay remains unexplained. The full QA gate was not run, and Hatfield’s production cURL default remains unchanged.

[chr-hertel]

Thanks for checking @ineersa!

[Copilot]

🟡 Changes recommended

Stream processing can perform another blocking read after initialization completes, hanging connect().

1 open issue

• Avoid blocking SSE read after initialization completes · New

1 resolved since last review

• Drain cancellation send before processing the SSE stream

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No unresolved findings remain, and regression coverage addresses fiber ownership and the prior cancellation-ordering concern.

No open issues 🎉

1 resolved since last review

• Avoid blocking SSE read after initialization completes

_{Review effort: Balanced}

[chr-hertel]

Thanks for jumping on this @aton-of-data!