From eeb1f94752483b4db2730ffc2e04abf9517e99cb Mon Sep 17 00:00:00 2001
From: Som Tripathi <somtri@iastate.edu>
Date: Tue, 29 Sep 2026 05:32:02 -0500
Subject: [PATCH] fix(tools/mcp): stop launcher subprocesses inheriting server
 stdin

modelopt-mcp runs as a stdio MCP server, so its stdin is the MCP
transport. The three modelopt-launcher calls in submit_job (Docker
Popen, Slurm run, dry-run run) left stdin unset, and the child
inherited it. Pass stdin=subprocess.DEVNULL at those calls.

Add a test that runs the real submit_job_impl and subprocess calls
with data waiting on stdin and checks that a stand-in launcher reads
0 bytes.

Fixes #2551

Signed-off-by: Som Tripathi <somtri@iastate.edu>
---
 tools/mcp/modelopt_mcp/bridge.py |  3 ++
 tools/mcp/tests/test_bridge.py   | 59 ++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/tools/mcp/modelopt_mcp/bridge.py b/tools/mcp/modelopt_mcp/bridge.py
index 5ac251e83b1..b6789962bdb 100644
--- a/tools/mcp/modelopt_mcp/bridge.py
+++ b/tools/mcp/modelopt_mcp/bridge.py
@@ -1257,6 +1257,7 @@ def submit_job_impl(
             proc = subprocess.Popen(  # nosec B603 - fixed launcher argv list; no shell.
                 argv,
                 env=child_env,
+                stdin=subprocess.DEVNULL,  # the server's stdin is the MCP transport
                 stdout=log_file,
                 stderr=subprocess.STDOUT,
                 start_new_session=True,
@@ -1298,6 +1299,7 @@ def submit_job_impl(
         proc = subprocess.run(  # nosec B603 - fixed launcher argv list; no shell.
             argv,
             env=child_env,
+            stdin=subprocess.DEVNULL,  # the server's stdin is the MCP transport
             capture_output=True,
             text=True,
             timeout=300,
@@ -1507,6 +1509,7 @@ def _submit_job_dry_run(
         proc = subprocess.run(  # nosec B603 - fixed dry-run launcher argv list; no shell.
             argv,
             env=child_env,
+            stdin=subprocess.DEVNULL,  # the server's stdin is the MCP transport
             capture_output=True,
             text=True,
             timeout=60,
diff --git a/tools/mcp/tests/test_bridge.py b/tools/mcp/tests/test_bridge.py
index 69d57822119..9fcd1bc32d7 100644
--- a/tools/mcp/tests/test_bridge.py
+++ b/tools/mcp/tests/test_bridge.py
@@ -18,7 +18,10 @@
 from __future__ import annotations
 
 import json
+import os
 import subprocess
+import sys
+import time
 
 import pytest
 
@@ -1072,6 +1075,62 @@ def fake_verify(**kwargs):
     assert verify_called["n"] == 0  # verify_setup never invoked in dry-run
 
 
+# Fake launcher: reports how many bytes it could read from its own stdin.
+_STDIN_PROBE = (
+    "import os, sys\n"
+    "data = sys.stdin.buffer.read()\n"
+    "with open(os.environ['STDIN_PROBE_OUT'], 'w') as f:\n"
+    "    f.write(str(len(data)))\n"
+)
+
+
+@pytest.mark.parametrize(
+    "mode_kwargs",
+    [{"dry_run": True}, {"cluster_host": "cluster.example.com"}, {"hf_local": "/tmp/hf"}],
+    ids=["dry_run", "slurm", "docker"],
+)
+def test_launcher_children_do_not_inherit_server_stdin(monkeypatch, tmp_path, mode_kwargs):
+    """The launcher child must not read the MCP server's stdin (issue #2551)."""
+    yaml_dir = tmp_path / "examples"
+    yaml_dir.mkdir()
+    (yaml_dir / "config.yaml").write_text("job_name: t\npipeline: []\n")
+    probe = tmp_path / "probe.py"
+    probe.write_text(_STDIN_PROBE)
+    out = tmp_path / "stdin_bytes"
+    monkeypatch.setenv("MODELOPT_LAUNCHER_EXAMPLES_DIR", str(yaml_dir))
+    monkeypatch.setenv("NEMORUN_HOME", str(tmp_path / "nemo"))
+    monkeypatch.setenv("MODELOPT_MCP_DOCKER_ID_TIMEOUT_SEC", "0")
+    monkeypatch.setenv("STDIN_PROBE_OUT", str(out))
+    monkeypatch.setattr(bridge, "_launcher_argv", lambda *a, **k: [sys.executable, str(probe)])
+
+    # Point this process's stdin at a pipe that already holds data, as a live MCP transport would.
+    read_fd, write_fd = os.pipe()
+    os.write(write_fd, b"leak\n")
+    os.close(write_fd)
+    saved_fd = os.dup(0)
+    os.dup2(read_fd, 0)
+    try:
+        bridge.submit_job_impl(
+            yaml_path="config.yaml",
+            **{"hf_local": None, "cluster_host": None, **mode_kwargs},
+            cluster_user=None,
+            identity=None,
+            job_dir=None,
+            job_name=None,
+            extra_overrides=None,
+            skip_verify=True,
+        )
+    finally:
+        os.dup2(saved_fd, 0)
+        os.close(saved_fd)
+        os.close(read_fd)
+
+    deadline = time.monotonic() + 5  # the Docker launcher is detached, so wait for its report
+    while time.monotonic() < deadline and not (out.exists() and out.read_text()):
+        time.sleep(0.05)
+    assert out.exists() and out.read_text() == "0"
+
+
 # ---------------------------------------------------------------------------
 # job_status / job_logs — filesystem-based
 # ---------------------------------------------------------------------------