Skip to content

fix(mcp): isolate notifier subprocess stdin and sanitize desktop notification strings - #58

Open
rksharma-owg wants to merge 1 commit into
google:mainfrom
rksharma-owg:fix/mcp-subprocess-stdio-isolation
Open

rksharma-owg wants to merge 1 commit into
google:mainfrom
rksharma-owg:fix/mcp-subprocess-stdio-isolation

Conversation

@rksharma-owg

@rksharma-owg rksharma-owg commented Sep 12, 2026 •

Copy link
Copy Markdown

Summary

When Artemis runs as an MCP stdio server (e.g. within Antigravity, Cursor, or Claude Desktop), child subprocesses spawned without stdin=subprocess.DEVNULL can inherit the parent process's MCP stdio stream, creating a risk of protocol interference or channel stalls if a subprocess interacts with standard input.

Additionally, desktop notifications on macOS via osascript constructed inline AppleScript strings using raw f-strings. If a message or title contained double quotes ("), backslashes (\\), or newlines, osascript failed with AppleScript syntax errors (such as error -2740), causing notifications to be dropped.

This PR addresses both areas:

  1. Notifier Subprocess Stdio Isolation: Sets stdin=subprocess.DEVNULL across all subprocess.run invocations in AgentApiNotifier, DesktopNotifier (Linux, macOS, and Windows notification commands), and ScriptNotifier, ensuring compatibility with existing stdout/stderr behavior while isolating standard input.
  2. AppleScript & PowerShell String Sanitization: Introduces helper functions escape_applescript_string and escape_powershell_string in mcp_server/notifiers/desktop.py to safely escape quotes and backslashes and normalize newlines into spaces while preserving apostrophes and Unicode.
  3. Unit & Regression Testing: Expands tests/unit/mcp/test_mcp_stdio_lifecycle.py with tests verifying stdin isolation across AgentApiNotifier, DesktopNotifier (on Linux, Darwin, and Windows), and ScriptNotifier, alongside string sanitization tests.

Changes

  • mcp_server/notifiers/agentapi.py: Added stdin=subprocess.DEVNULL to both subprocess.run invocations.
  • mcp_server/notifiers/desktop.py: Added escape_applescript_string and escape_powershell_string; added stdin=subprocess.DEVNULL to notify-send, osascript, and powershell subprocess calls.
  • mcp_server/notifiers/script.py: Added stdin=subprocess.DEVNULL to custom script runner subprocess.run.
  • tests/unit/mcp/test_mcp_stdio_lifecycle.py: Added regression test coverage for stdin isolation and string escaping, and hermetic adb mocks.
  • tests/unit/mcp/test_notifiers.py: Updated mock_run signature in test_agentapi_notifier_candidate_recovery_and_retry to accept **kwargs / stdin.

Validation Performed

  • uv run pytest -v tests/unit/mcp/test_mcp_stdio_lifecycle.py (14/14 passed)
  • uv run pytest -v tests/unit/mcp (228 passed, 1 skipped)
  • uv run ruff check mcp_server/notifiers tests/unit/mcp (All checks passed)
  • uv run ruff format --check mcp_server/notifiers tests/unit/mcp/test_mcp_stdio_lifecycle.py tests/unit/mcp/test_notifiers.py (Clean)
  • uv run pyright --project pyright-core.json (0 errors, 0 warnings)

… strings

When Artemis runs as an MCP stdio server, child processes spawned without stdin=subprocess.DEVNULL inherit sys.stdin and can consume incoming JSON-RPC bytes, causing protocol corruption and communication hangs.

This change:
- Sets stdin=subprocess.DEVNULL on all subprocess.run calls in AgentApiNotifier, DesktopNotifier, and ScriptNotifier.
- Adds string escaping and newline sanitization for macOS AppleScript and Windows PowerShell desktop notifications to prevent syntax breakage and execution errors when notifications contain quotes, backslashes, or multiline strings.
- Adds comprehensive unit regression tests in test_mcp_stdio_lifecycle.py.
giveUphope added a commit to giveUphope/autoGamer that referenced this pull request Oct 12, 2026
用户点名的失效正是本会话我自己撞的三种,google#58 逐条带命令:
① 未核实就叙述"已完成"(我写过"方案甲已落地 / google#50 / G63 / 188 项测试",git 核对全不存在,
   当时 HEAD 8eecb39、条目 49 条)——只留在对话里,但照它继续做就会去验收不存在的东西;
② 新事实推翻旧行而旧行还在(§零"CI 用 spike-mcp 做比对"grep 0 命中、"CI 侧全绿"那次 run 整体
   failure,google#53 已更正);
③ 派生数不回抄与替换不校验命中(google#45/google#56)。

四条判据:当轮必落一次文档提交并改到受影响那一行(验收带 git grep 反判据,本身要能红);
否定旧表述必须改掉或标作废加出处;说"已完成 X"必须指到 commit 或命令输出,否则写"待做";
派生计数只留复测命令。与 R24(从哪取证)、R27(锁会不会红)同族 —— R28 管"取到的证有没有
落到别人会读的地方"。
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.

1 participant