Repository navigation
fix: preserve stdio streams in server transport - #2734
pranjalbhatia710 wants to merge 3 commits into
Conversation
|
Thanks for checking this on Windows. I kept the real-stdio path on duplicated file descriptors, but added a fallback for fileno-less in-memory stdin/stdout replacements so the old test/embedding shape continues to work. Added a regression test for that path too.\n\nValidated locally:\n\n |
|
One more CI follow-up: the Python 3.14 lowest-direct jobs exposed an AnyIO subprocess startup SyntaxWarning being forwarded through stderr before the expected clean-exit marker. I adjusted the stdio interaction test to keep proving the child exited cleanly while tolerating dependency startup warnings before that marker.\n\nAdditional local validation:\n\n |
|
Thanks for rechecking this on Windows. That remaining edge is real: the current fallback fixes the fileno-less crash, but it does not fully preserve replacement in-memory wrappers after the context exits. I think the stronger guarantee is worth keeping, especially for embedders/tests that temporarily replace stdio. I’ll update the fallback so it avoids closing the caller-provided fileno-less wrappers, then rerun the focused stdio tests plus ruff/pyright. Also noted on the existing |
|
Updated this edge case in I also extended Validated locally: uv run --frozen pytest tests/server/test_stdio.py -q
# 3 passed
uv run --frozen pytest tests/server/test_stdio.py tests/interaction/transports/test_stdio.py -q
# 5 passed
uv run --frozen ruff check src/mcp/server/stdio.py tests/server/test_stdio.py tests/interaction/transports/test_stdio.py
# All checks passed
uv run --frozen pyright src/mcp/server/stdio.py tests/server/test_stdio.py
# 0 errors
git diff --check origin/main...HEAD |
|
Cross-reference / consolidation note (verified independently from CI, not a fix request) The
This is currently being patched, in parallel, inside 6 open PRs that each bundle it as collateral with unrelated work:
All six are functionally equivalent for the test's stated intent (the docstring/comment says the assertion's purpose is to prove the child wrote Recommendation: land one focused fix for this (the |
e626a5e to
cd2acc8
Compare
|
Rebased this on latest |
|
Thanks for the PR. Tracking this in #2040 instead. I'm closing this out as part of a backlog cleanup now that v2 is out. Feel free to reopen if this is still relevant. |
Summary
sys.stdin/sys.stdoutwrappers remain usable after the server exitsFixes #1933
Test plan
uv run --frozen pytest tests/server/test_stdio.py -quv run --frozen ruff check src/mcp/server/stdio.py tests/server/test_stdio.pyuv run --frozen pyright src/mcp/server/stdio.py tests/server/test_stdio.py