[huong8373tt-beep]

Summary

• avoid replacing sys.stdout when pytest imports the MCP bridge scripts on Windows
• retain UTF-8 output setup only for direct __main__ script execution
• add regression coverage that guards the three MCP test modules against import-time stdout replacement

Validation

• uv run --with pytest python -m pytest --confcutdir=tests/unit/mcp -q tests/unit/mcp/test_mcp_stdout_configuration.py — 2 passed
• python -m pytest --collect-only -q tests/mcp/test_mcp_simple.py tests/mcp/test_mcp_integration.py tests/mcp/test_mcp_http_validation.py — 13 tests collected after the fix
• uvx --from black black --check ... — passed
• uvx --from isort isort --check-only ... — passed
• git diff --check — passed

Full tests/mcp collection is not claimed as passing locally because this checkout's ad-hoc environment lacks the optional mcp dependency for test_engineering_mcp.py.

Disclosure

Slai Agent Runtime assisted with the analysis, implementation, testing, and preparation of this contribution. The account owner authorized this work and assumes full responsibility for the submission.

Closes #4443

[github-actions]

Verdict: Approve with suggestions

This stops three MCP bridge test scripts from swapping out the console stream when pytest imports them on Windows. That swap broke pytest's output capture. The UTF-8 setup now runs only when someone executes the script directly. The fix is correct and tightly scoped. The one suggestion: the new regression test looks for the three scripts relative to the folder pytest is run from, so it fails if you run pytest from anywhere except the repo root.

Real-world evidence

N/A: test-only change with no user-facing surface. No evidence bundle was produced, so this verdict rests on static review. The author's PR body reports the new test passing and the three MCP modules collecting cleanly (13 tests).

🔍 Technical details

Issues Found

🟢 Regression test depends on the working directory (tests/unit/mcp/test_mcp_stdout_configuration.py:8-17): Path("tests/mcp/...") resolves against the current directory. Run from tests/ or an IDE runner rooted elsewhere, both tests raise FileNotFoundError. Other unit tests anchor on __file__ (e.g. REPO_ROOT = Path(__file__).resolve().parents[2] in tests/unit/test_agent_hub_worker_workflow.py:18).

REPO_ROOT = Path(__file__).resolve().parents[3]

MCP_TEST_MODULES = (
    "tests/mcp/test_mcp_simple.py",
    "tests/mcp/test_mcp_integration.py",
    "tests/mcp/test_mcp_http_validation.py",
)


def _module_tree(relative_path: str) -> ast.Module:
    path = REPO_ROOT / relative_path
    return ast.parse(path.read_text(encoding="utf-8"), filename=str(path))

🟢 First test is broader than its name (test_mcp_stdout_configuration.py:20-41): ast.walk over every top-level node also covers function bodies and the __main__ guard. It therefore rejects any sys.stdout = anywhere in the file, not just at import time. That strictness is harmless, but the test name and the assertion message ("during import") overstate what the test pins down. Consider renaming it to ..._never_replace_stdout.

Strengths

• sys.stdout.reconfigure(encoding="utf-8") behind a hasattr guard is the right fix. It changes the encoding in place instead of replacing the stream that pytest's capture holds a reference to.
• The patch touches only the three affected modules and drops the now-unused io imports.
• The AST-based guard is fast and needs no Windows runner to catch the regression.

[kovtcharov-amd]

Thanks for this. It fixes the Windows "I/O operation on closed file" crash that stopped pytest --collect-only partway through the suite. I reproduced the crash on main, and with your change all 15 tests in those modules collect.

I pushed one small commit that covers the review bot's suggestion. The guard test now finds the scripts from its own location, so it passes no matter which folder pytest runs from. It also adds a behavioural check that imports each script as if on Windows and confirms sys.stdout is untouched. That catches a replacement the AST scan would miss. I also linked #4443 in the description. Nothing else is needed from you.

[kovtcharov-amd]

Merged — thanks @huong8373tt-beep. The MCP test scripts no longer swap out stdout when pytest imports them, so collection on Windows stops crashing with 'I/O operation on closed file', and they still print UTF-8 correctly when run directly.