Skip to content

fix(stdio): drain responses after stdin EOF#2680

Open
Epochex wants to merge 5 commits into
modelcontextprotocol:mainfrom
Epochex:fix/2678-stdio-drain-writes
Open

fix(stdio): drain responses after stdin EOF#2680
Epochex wants to merge 5 commits into
modelcontextprotocol:mainfrom
Epochex:fix/2678-stdio-drain-writes

Conversation

@Epochex
Copy link
Copy Markdown

@Epochex Epochex commented May 25, 2026

Fixes #2678.

When stdin hits EOF (e.g. python server.py < payload.jsonl > response.jsonl), the client is done sending requests but is still reading stdout for results. On main, BaseSession._receive_loop() closes the write stream as soon as the read stream ends, and Server.run() cancels the handler task group on transport close. That combination can cancel in-flight tool handlers before they respond, dropping the last ools/call responses.

This change lets ServerSession keep the write stream open when the read side closes, so Server.run() can drain already-started handlers and flush their responses before shutdown.

Tests:

  • uv run --frozen pytest tests/server/test_cancel_handling.py tests/server/test_stdio.py

Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

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

I ran this locally against the branch. The focused stdio/cancel tests pass:

TMPDIR=.codex-tmp/tmp UV_CACHE_DIR=.codex-tmp/uv-cache uv run --frozen pytest tests/server/test_cancel_handling.py tests/server/test_stdio.py -q

One shutdown edge looks worth tightening before merge. With drain_on_read_close=True, stdin EOF now waits for in-flight handlers to finish, but there is no bounded drain window. In a local memory-stream probe, I started a tools/call handler that does await anyio.sleep_forever(), sent initialize / notifications/initialized / tools/call, closed the input stream, and then waited for server.run(..., drain_on_read_close=True) to return. It did not return before anyio.fail_after(0.75).

That leaves redirected-stdin callers with the opposite failure mode: the previous branch can drop a response, while this one can keep the process alive forever if an accepted tool hangs after EOF. I think the drain path needs a small timeout or cancellation window so already-started handlers get a chance to flush, but EOF still remains a bounded shutdown signal.

@Epochex Epochex force-pushed the fix/2678-stdio-drain-writes branch from 16d44a2 to a8efe77 Compare June 2, 2026 17:24
@Epochex Epochex force-pushed the fix/2678-stdio-drain-writes branch from a8efe77 to 721dd43 Compare June 2, 2026 17:30
@StantonMatt
Copy link
Copy Markdown

Rechecked the updated head 721dd43. The new read_eof_drain_timeout_seconds path covers the hanging-handler case I was worried about: in-flight handlers get a drain window, and the server cancels the task group if they never finish after read EOF.

Local validation:

  • git diff --check origin/main...HEAD
  • TMPDIR=/Users/mjstanton/Programming/open-source-maintainer4/.codex-tmp/python-sdk-tmp UV_CACHE_DIR=/Users/mjstanton/Programming/open-source-maintainer4/.codex-tmp/python-sdk-uv-cache uv run --frozen pytest tests/server/test_cancel_handling.py tests/server/test_stdio.py -q -> 9 passed

The visible CI matrix is also green on this head.

Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

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

The new read_eof_drain_timeout_seconds path addresses the bounded-shutdown concern I raised.

I re-ran the focused stdio/cancel suite on head 721dd43:

TMPDIR=.codex-tmp/tmp UV_CACHE_DIR=.codex-tmp/uv-cache uv run --frozen pytest tests/server/test_cancel_handling.py tests/server/test_stdio.py -q

That passed with 9 passed, including test_server_bounds_drain_on_read_eof_when_handler_never_finishes. git diff --check origin/main...HEAD is also clean, and the current GitHub Actions/Conformance/Zizmor checks are green.

Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

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

I retested the latest head (721dd43) against the shutdown case I called out earlier.

The new bounded drain path covers both sides of that tradeoff for me: test_server_drains_in_flight_handlers_on_transport_read_eof still lets already-started handlers flush responses after stdin EOF, and test_server_bounds_drain_on_read_eof_when_handler_never_finishes covers the hung-handler case so EOF stays bounded.

Local focused check passed:

TMPDIR=.codex-tmp/tmp UV_CACHE_DIR=.codex-tmp/uv-cache uv run --frozen pytest tests/server/test_cancel_handling.py tests/server/test_stdio.py -q

GitHub checks are green on the current head as well. No further concern from my earlier note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FastMCP/stdio: in-flight tool responses dropped on stdin EOF when input is bash-redirected from a file

2 participants