Keep the STDIO bridge alive when standard input is a socket - #309
Keep the STDIO bridge alive when standard input is a socket#309Alexrydder wants to merge 1 commit into
Conversation
When STDIN is a socket rather than a pipe, PHP applies default_socket_timeout (60 seconds by default) to each blocking read in the serve loop. Once that passes with no input, fgets() returns false, the loop takes it as end of input, and the bridge exits. Claude Code connects STDIO servers over a socket pair, so the server dropped a minute into every idle period. Set the read timeout on STDIN to -1 before entering the loop, which means wait indefinitely, the same value the ini setting accepts. Pipes and files have no read timeout and stream_set_timeout() simply returns false for them. The new test puts a 200 ms timeout on a socket pair, shows an idle read gives up, applies the fix, and reads a line written by a child process 500 ms later. A second test covers a non-socket stream. Changelog entry added.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @Alexrydder. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the WP-CLI STDIO server bridge could exit after ~60 seconds of idle time when STDIN is a socket (due to PHP applying default_socket_timeout to blocking socket reads), by disabling the read timeout on the input stream and adding unit coverage.
Changes:
- Disable socket read timeouts for the bridge input stream via
stream_set_timeout( STDIN, -1 )before entering the serve loop. - Add unit tests covering both socket-pair behavior (timeout would have triggered) and non-socket streams (no behavior change).
- Document the fix in the Unreleased changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| includes/Cli/StdioServerBridge.php | Sets the STDIN stream to wait indefinitely before the blocking fgets() serve loop. |
| tests/phpunit/Unit/Cli/StdioServerBridgeTest.php | Adds unit tests validating the timeout behavior on sockets and no-op behavior on non-socket streams. |
| CHANGELOG.md | Adds an Unreleased “Fixed” entry describing the STDIO socket timeout issue and resolution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #309 +/- ##
============================================
- Coverage 88.17% 88.16% -0.02%
- Complexity 1259 1260 +1
============================================
Files 54 54
Lines 4120 4123 +3
============================================
+ Hits 3633 3635 +2
- Misses 487 488 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What?
Closes #308
Stops the WP-CLI STDIO bridge from exiting after 60 seconds without input when its standard input is a socket.
Why?
When stdin is a socket rather than a pipe (Claude Code connects stdio MCP servers over a socket pair), PHP applies
default_socket_timeoutto the blockingfgets( STDIN )in the serve loop. After 60 idle seconds the read returnsfalsewith the client still connected, the loop treats that as end of input, and the server exits. In practice the server dropped a minute into every idle period and the client reported it as failed. Run from a shell with a pipe on stdin the same command lives forever, which is why it was easy to miss. Details and a standalone repro are in the linked issue.How?
stream_set_timeout( STDIN, -1 )is called once before entering the loop. -1 means wait indefinitely, the same valuedefault_socket_timeoutaccepts. It goes through a small private method so it can be unit tested. On a pipe or a filestream_set_timeout()returns false without a warning and nothing changes, so the plain pipe case is untouched. I did not use PHP_INT_MAX because PHP converts the timeout to milliseconds in an int before polling.The test puts a 200 ms timeout on a socket pair, first shows an idle read gives up with
timed_outset, then applies the fix and reads a line that a child PHP process writes 500 ms later. The parent closes its copy of the writing end right after starting the child, so a child that exits without writing produces EOF and the read fails instead of hanging. A second test runs the call againstphp://memoryfor the non-socket path. Both skip ifproc_open()or Unix socket pairs are unavailable. With thestream_set_timeout()call stubbed out the first test fails in about two seconds.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude
Used for: tracing the cause (strace on the running bridge, checking PHP socket timeout behavior), and drafting the patch, tests, and this write-up. I reviewed and ran everything myself and take responsibility for it.
Testing Instructions
wp mcp-adapter serve --server=mcp-adapter-default --user=1.pingrequest after two or more idle minutes still gets{"jsonrpc":"2.0","id":...,"result":{}}.composer lint,composer phpstan, andcomposer testpass (994 unit and 42 integration tests locally on PHP 8.4 with WordPress 7.0.4).Changelog Entry
Fixed: The STDIO bridge no longer exits after 60 seconds without input when its standard input is a socket.