From 0209c6ebdaf95d8475d9763d7c724b3991a6df8e Mon Sep 17 00:00:00 2001 From: jeomon Date: Sat, 1 Aug 2026 12:43:27 +0530 Subject: [PATCH] fix: stop writing to stdout, which carries the stdio JSON-RPC protocol The default transport is stdio, where stdout is the JSON-RPC channel -- as __main__.py already notes: "Written to stderr because stdout carries the stdio protocol". Two live code paths wrote to it anyway, interleaving plain text into the framing the client is parsing. tree/service.py printed every word of every EditControl, DocumentControl and ImageControl encountered during traversal: word_elements.append((word,box)) print(word,box) # <- removed This runs on any snapshot containing an editable control, once per word, so a single text field can emit dozens of lines into the protocol stream. It is debug leftover from the word-bounding-box work: the value is already accumulated on the line above and the print does nothing else. infrastructure/analytics.py printed a per-tool-call summary that the very next line already sends to the logger, so removing the print loses nothing -- the logger writes to stderr, where this belongs. Adds tests/test_no_stdout_writes.py, a static AST scan of the source tree. It counts only ast.Call nodes, so prints inside docstring examples are excluded for free, and it ignores print(..., file=...) since that is not a stdout write. Two narrow exemptions are encoded: prints lexically inside an `if ...debug...:` block, and threadFunc inside RunByHotKey, the upstream uiautomation hotkey utility the server never invokes. Those cover the three remaining prints in uia/controls.py; verified that an ungated print added anywhere in that same file is still caught. Not run locally: the test suite needs pywin32, which has no macOS wheels, so this was verified by static analysis and by exercising the scanner directly. CI runs on windows-latest. --- src/windows_mcp/infrastructure/analytics.py | 2 - src/windows_mcp/tree/service.py | 1 - tests/test_no_stdout_writes.py | 105 ++++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/test_no_stdout_writes.py diff --git a/src/windows_mcp/infrastructure/analytics.py b/src/windows_mcp/infrastructure/analytics.py index fe78ce5e..b25f859e 100644 --- a/src/windows_mcp/infrastructure/analytics.py +++ b/src/windows_mcp/infrastructure/analytics.py @@ -115,8 +115,6 @@ async def track_tool(self, tool_name: str, result: Dict[str, Any]) -> None: duration = result.get("duration_ms", 0) success_mark = "SUCCESS" if result.get("success") else "FAILED" - # Using print for immediate visibility in console during debugging - print(f"[Analytics] {tool_name}: {success_mark} ({duration}ms)") logger.info(f"{tool_name}: {success_mark} ({duration}ms)") async def track_error(self, error: Exception, context: Dict[str, Any]) -> None: diff --git a/src/windows_mcp/tree/service.py b/src/windows_mcp/tree/service.py index 57cd0c9e..fefffa73 100755 --- a/src/windows_mcp/tree/service.py +++ b/src/windows_mcp/tree/service.py @@ -568,7 +568,6 @@ def tree_traversal(self, node: Control, window_bounding_box:Rect, window_name:st for word,boxes in words: for box in boxes: word_elements.append((word,box)) - print(word,box) except Exception: pass diff --git a/tests/test_no_stdout_writes.py b/tests/test_no_stdout_writes.py new file mode 100644 index 00000000..b2f849bb --- /dev/null +++ b/tests/test_no_stdout_writes.py @@ -0,0 +1,105 @@ +"""Guard against bare `print()` in the server's source. + +The default transport is stdio (`__main__.py`), where stdout *is* the +JSON-RPC channel -- as the codebase already notes elsewhere: "Written to +stderr because stdout carries the stdio protocol". Anything printed to +stdout is interleaved into that stream and corrupts the framing the client +is parsing. + +This is a static scan rather than a runtime check, so it needs no Windows +imports and runs anywhere. + +Two narrow exemptions, both deliberate: + +* prints lexically inside an `if ...debug...:` block -- opt-in, off by + default, and never reached in normal operation; +* `RunByHotKey`, a standalone hotkey-runner utility carried over from the + upstream `uiautomation` library that the MCP server never invokes. + +Anything else should use `logger`, which is already configured to write to +stderr. +""" + +import ast +from pathlib import Path + +import pytest + +SRC_ROOT = Path(__file__).resolve().parent.parent / "src" / "windows_mcp" + +# Enclosing functions exempted wholesale. Keyed by name rather than line +# number so the allowlist survives edits elsewhere in the file. +EXEMPT_FUNCTIONS = { + "threadFunc", # nested inside RunByHotKey; upstream uiautomation utility +} + + +def _is_debug_gated(ancestors: list[ast.AST]) -> bool: + """True if any enclosing `if` tests something named like a debug flag.""" + for node in ancestors: + if isinstance(node, ast.If): + test_source = ast.dump(node.test).lower() + if "debug" in test_source: + return True + return False + + +def _enclosing_function(ancestors: list[ast.AST]) -> str | None: + for node in reversed(ancestors): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + return node.name + return None + + +def _find_stdout_prints(path: Path) -> list[tuple[int, str | None]]: + """Return (lineno, enclosing function) for each non-exempt print call. + + Only `ast.Call` nodes count, so prints appearing inside docstring + examples are excluded for free -- those are strings, not calls. + """ + tree = ast.parse(path.read_text(encoding="utf-8")) + offenders: list[tuple[int, str | None]] = [] + + def walk(node: ast.AST, ancestors: list[ast.AST]) -> None: + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "print" + ): + # `print(..., file=sys.stderr)` is not a stdout write. + writes_elsewhere = any(kw.arg == "file" for kw in node.keywords) + function = _enclosing_function(ancestors) + if ( + not writes_elsewhere + and not _is_debug_gated(ancestors) + and function not in EXEMPT_FUNCTIONS + ): + offenders.append((node.lineno, function)) + + for child in ast.iter_child_nodes(node): + walk(child, ancestors + [node]) + + walk(tree, []) + return offenders + + +def _source_files() -> list[Path]: + return sorted(SRC_ROOT.rglob("*.py")) + + +class TestNoStdoutWrites: + def test_source_tree_was_found(self): + """Fail loudly rather than passing vacuously on a bad path.""" + assert _source_files(), f"no Python sources under {SRC_ROOT}" + + @pytest.mark.parametrize( + "path", _source_files(), ids=lambda p: str(p.relative_to(SRC_ROOT)) + ) + def test_no_bare_print(self, path: Path): + offenders = _find_stdout_prints(path) + assert offenders == [], "\n".join( + f"{path.relative_to(SRC_ROOT)}:{line} in {func or ''} " + "writes to stdout, which carries the stdio JSON-RPC protocol; " + "use logger instead" + for line, func in offenders + )