fix(mcp-servers,tests): defer stdio rebind to main(); fix test isolation so full suite runs under pytest fd-capture

Adding a full-suite CI job (prior commit) exposed two latent bugs that only
surface when the whole suite runs under pytest's DEFAULT fd-level capture —
which had never happened, because collection aborted early and most runs used
--capture=no.

1. Import-time stdio seizure (4 MCP servers). codex-image2, llm-chat,
   minimax-chat, and gemini-review each ran, at MODULE TOP LEVEL,
   `sys.stdout = os.fdopen(sys.stdout.fileno(), "wb")`. os.fdopen defaults to
   closefd=True, so importing the module SEIZES ownership of the current stdout
   fd — under pytest that is the capture tmpfile fd, and when the wrapper is GC'd
   it closes it, corrupting capture for every subsequent test ("OSError: Bad
   file descriptor" cascade; 395 spurious errors). Fixed by deferring the rebind
   into an idempotent `_init_stdio()` called at the top of each `main()` — the
   exact pattern manual-review already used. Real server launch
   (`python server.py`) is unchanged (main() runs it first); only import is now
   side-effect-free. test_codex_image2_server.py reverts to a plain import.

2. Module-name collision in tests (test_manual_review.py). It loaded the server
   via bare `import server as srv` (x9). Every mcp-server is named server.py, and
   test_minimax_chat_server does `sys.path.insert(0, <minimax dir>)` at
   collection time, so by run time a bare `import server` resolved to minimax's
   server → `AttributeError: module 'server' has no attribute 'create_thread'`
   (passed in isolation, failed in the full suite). Fixed to load by explicit
   path under a unique module name via spec_from_file_location, matching every
   other server test in the suite.

Full suite now: 406 passed, 16 skipped under default fd-capture (was: aborted /
395 errors / 9 failures).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mcp-servers/codex-image2/server.py

@@ -23,8 +23,26 @@ from pathlib import Path
 from typing import Any
 
 
-sys.stdout = os.fdopen(sys.stdout.fileno(), "wb", buffering=0)
-sys.stdin = os.fdopen(sys.stdin.fileno(), "rb", buffering=0)
+_stdio_initialized = False
+
+
+def _init_stdio() -> None:
+    """Rebind stdio to raw unbuffered binary streams for MCP framing.
+
+    Deferred into a function (called at the top of main()) so that merely
+    IMPORTING this module has no stdio side effects. os.fdopen(fileno) defaults
+    to closefd=True and thus seizes ownership of the fd; doing that at import
+    time under a test harness that captures stdio (pytest fd-capture) closes the
+    harness's capture fd and corrupts capture for every subsequent test. Real
+    server launch (python server.py) still calls this first via main(), so
+    runtime behavior is unchanged. Idempotent."""
+    global _stdio_initialized
+    if _stdio_initialized:
+        return
+    sys.stdout = os.fdopen(sys.stdout.fileno(), "wb", buffering=0)
+    sys.stdin = os.fdopen(sys.stdin.fileno(), "rb", buffering=0)
+    _stdio_initialized = True
+
 
 SERVER_NAME = os.environ.get("CODEX_IMAGE2_SERVER_NAME", "codex-image2")
 CODEX_BIN = os.environ.get("CODEX_IMAGE2_CODEX_BIN", "codex")
@@ -858,6 +876,7 @@ def handle_request(request: dict[str, Any]) -> dict[str, Any] | None:
 
 
 def main() -> int:
+    _init_stdio()
     if len(sys.argv) == 3 and sys.argv[1] == "--run-job":
         return run_async_job(sys.argv[2])
 

mcp-servers/gemini-review/server.py

@@ -33,8 +33,26 @@ from pathlib import Path
 from typing import Any
 
 
-sys.stdout = os.fdopen(sys.stdout.fileno(), "wb", buffering=0)
-sys.stdin = os.fdopen(sys.stdin.fileno(), "rb", buffering=0)
+_stdio_initialized = False
+
+
+def _init_stdio() -> None:
+    """Rebind stdio to raw unbuffered binary streams for MCP framing.
+
+    Deferred into a function (called at the top of main()) so that merely
+    IMPORTING this module has no stdio side effects. os.fdopen(fileno) defaults
+    to closefd=True and thus seizes ownership of the fd; doing that at import
+    time under a test harness that captures stdio (pytest fd-capture) closes the
+    harness's capture fd and corrupts capture for every subsequent test. Real
+    server launch (python server.py) still calls this first via main(), so
+    runtime behavior is unchanged. Idempotent."""
+    global _stdio_initialized
+    if _stdio_initialized:
+        return
+    sys.stdout = os.fdopen(sys.stdout.fileno(), "wb", buffering=0)
+    sys.stdin = os.fdopen(sys.stdin.fileno(), "rb", buffering=0)
+    _stdio_initialized = True
+
 
 SERVER_NAME = os.environ.get("GEMINI_REVIEW_SERVER_NAME", "gemini-review")
 GEMINI_BIN = os.environ.get("GEMINI_BIN", "gemini")
@@ -1815,6 +1833,7 @@ def handle_request(request: dict[str, Any]) -> dict[str, Any] | None:
 
 
 def main() -> None:
+    _init_stdio()
     if len(sys.argv) == 3 and sys.argv[1] == "--run-job":
         raise SystemExit(run_async_job(sys.argv[2]))
 

mcp-servers/llm-chat/server.py

@@ -22,9 +22,26 @@ import sys
 import tempfile
 import httpx
 
-# Force unbuffered stdout/stdin
-sys.stdout = os.fdopen(sys.stdout.fileno(), 'wb', buffering=0)
-sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', buffering=0)
+_stdio_initialized = False
+
+
+def _init_stdio():
+    """Rebind stdio to raw unbuffered binary streams for MCP framing.
+
+    Deferred into a function (called at the top of main()) so that merely
+    IMPORTING this module has no stdio side effects. os.fdopen(fileno) defaults
+    to closefd=True and thus seizes ownership of the fd; doing that at import
+    time under a test harness that captures stdio (pytest fd-capture) closes the
+    harness's capture fd and corrupts capture for every subsequent test. Real
+    server launch (python server.py) still calls this first via main(), so
+    runtime behavior is unchanged. Idempotent."""
+    global _stdio_initialized
+    if _stdio_initialized:
+        return
+    # Force unbuffered stdout/stdin
+    sys.stdout = os.fdopen(sys.stdout.fileno(), 'wb', buffering=0)
+    sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', buffering=0)
+    _stdio_initialized = True
 
 # Configuration from environment
 API_KEY = os.environ.get("LLM_API_KEY", "")
@@ -283,6 +300,7 @@ def read_message():
 
 def main():
     """Main loop - read JSON-RPC messages from stdin"""
+    _init_stdio()
     debug_log("Entering main loop")
 
     while True:

mcp-servers/minimax-chat/server.py

@@ -8,9 +8,26 @@ import sys
 import tempfile
 import httpx
 
-# Force unbuffered stdout/stdin
-sys.stdout = os.fdopen(sys.stdout.fileno(), 'wb', buffering=0)
-sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', buffering=0)
+_stdio_initialized = False
+
+
+def _init_stdio():
+    """Rebind stdio to raw unbuffered binary streams for MCP framing.
+
+    Deferred into a function (called at the top of main()) so that merely
+    IMPORTING this module has no stdio side effects. os.fdopen(fileno) defaults
+    to closefd=True and thus seizes ownership of the fd; doing that at import
+    time under a test harness that captures stdio (pytest fd-capture) closes the
+    harness's capture fd and corrupts capture for every subsequent test. Real
+    server launch (python server.py) still calls this first via main(), so
+    runtime behavior is unchanged. Idempotent."""
+    global _stdio_initialized
+    if _stdio_initialized:
+        return
+    # Force unbuffered stdout/stdin
+    sys.stdout = os.fdopen(sys.stdout.fileno(), 'wb', buffering=0)
+    sys.stdin = os.fdopen(sys.stdin.fileno(), 'rb', buffering=0)
+    _stdio_initialized = True
 
 # Debug logging
 DEBUG_LOG = os.path.join(tempfile.gettempdir(), "minimax-mcp-debug.log")
@@ -329,6 +346,7 @@ def read_message():
 
 def main():
     """Main loop - read JSON-RPC messages from stdin"""
+    _init_stdio()
     debug_log("Entering main loop")
 
     while True:

tests/test_codex_image2_server.py

@@ -13,6 +13,8 @@ from unittest import mock
 
 ROOT = Path(__file__).resolve().parent.parent
 SERVER_PATH = ROOT / "mcp-servers" / "codex-image2" / "server.py"
+# server.py defers its stdio rebind into _init_stdio() (called from main()), so
+# a plain import has no stdio side effects and is safe under pytest fd-capture.
 SPEC = importlib.util.spec_from_file_location("codex_image2_server", SERVER_PATH)
 assert SPEC and SPEC.loader
 MODULE = importlib.util.module_from_spec(SPEC)

tests/test_manual_review.py

@@ -10,6 +10,7 @@ Covers:
 7. Concurrency (fail-fast on second review)
 """
 
+import importlib.util
 import json
 import os
 import subprocess
@@ -23,9 +24,18 @@ from pathlib import Path
 
 import pytest
 
-# Add the server directory to path for import
+# Load the server by explicit path under a UNIQUE module name. A bare
+# `import server` is unsafe here: every mcp-server is named server.py, and
+# sibling test modules (e.g. test_minimax_chat_server) do
+# `sys.path.insert(0, <their-server-dir>)` at collection time — so by the time
+# these tests run, a bare `import server` can resolve to the wrong server and
+# fail with AttributeError. spec_from_file_location sidesteps sys.path entirely.
 SERVER_DIR = Path(__file__).parent.parent / "mcp-servers" / "manual-review"
-sys.path.insert(0, str(SERVER_DIR))
+_SRV_SPEC = importlib.util.spec_from_file_location(
+    "manual_review_server", SERVER_DIR / "server.py")
+assert _SRV_SPEC and _SRV_SPEC.loader
+srv = importlib.util.module_from_spec(_SRV_SPEC)
+_SRV_SPEC.loader.exec_module(srv)
 
 # Prevent auto-open browser during tests
 os.environ["MANUAL_REVIEW_AUTO_OPEN"] = "false"
@@ -104,7 +114,6 @@ def _start_server(**extra_env):
 # Test 1: Module import
 # ============================================================
 def test_import():
-    import server as srv
     assert hasattr(srv, "handle_request")
     assert hasattr(srv, "create_thread")
     assert hasattr(srv, "do_review")
@@ -156,7 +165,6 @@ def test_tools_list():
 # Test 4: Thread management
 # ============================================================
 def test_thread_management():
-    import server as srv
     tid = srv.create_thread()
     assert tid and len(tid) == 12, f"bad thread id: {tid}"
     srv.append_exchange(tid, "user", "hello")
@@ -174,7 +182,6 @@ import socketserver
 # Test 5: Browser mode — HTTP server + submit flow
 # ============================================================
 def test_browser_mode_http():
-    import server as srv
 
     prompt = "Test review prompt for unit testing"
     config = {"model_reasoning_effort": "xhigh"}
@@ -267,7 +274,6 @@ def test_browser_mode_http():
 # Test 6: File mode — prompt + response + cross-model warning
 # ============================================================
 def test_file_mode():
-    import server as srv
 
     with tempfile.TemporaryDirectory() as tmpdir:
         original_dir = srv.PENDING_DIR
@@ -339,7 +345,6 @@ def test_file_mode():
 # Test 7: File mode — empty file rejected
 # ============================================================
 def test_file_mode_empty_rejected():
-    import server as srv
 
     with tempfile.TemporaryDirectory() as tmpdir:
         original_dir = srv.PENDING_DIR
@@ -405,7 +410,6 @@ def test_file_mode_empty_rejected():
 # Test 8: review rejects empty prompt
 # ============================================================
 def test_review_missing_prompt():
-    import server as srv
     resp = srv.handle_review({"prompt": ""}, 99, threading.Event(), "")
     assert resp["result"].get("isError") is True, f"unexpected: {resp}"
 
@@ -414,7 +418,6 @@ def test_review_missing_prompt():
 # Test 9: review_reply rejects unknown threadId
 # ============================================================
 def test_review_reply_unknown_thread():
-    import server as srv
     resp = srv.handle_review_reply(
         {"threadId": "nonexistent", "prompt": "hi"}, 100, threading.Event(), "",
     )
@@ -425,7 +428,6 @@ def test_review_reply_unknown_thread():
 # Test 10: Pending state file
 # ============================================================
 def test_pending_state():
-    import server as srv
 
     with tempfile.TemporaryDirectory() as tmpdir:
         original_dir = srv.PENDING_DIR
@@ -448,7 +450,6 @@ def test_pending_state():
 # Test 11: File mode cancellation via _PendingCall
 # ============================================================
 def test_file_mode_cancelled():
-    import server as srv
 
     with tempfile.TemporaryDirectory() as tmpdir:
         original_dir = srv.PENDING_DIR