Merge PR #7696: fix(coding-agent): stop Python eval shell children inheriting the runner control channel; resolve Git Bash on Windows (@metaphorics)

packages/coding-agent/CHANGELOG.md

@@ -25,6 +25,9 @@
 ### Fixed
 
 - WSL host-home resolution now builds `/mnt/<drive>/...` fallback paths with POSIX semantics regardless of the host platform, instead of the platform-dependent `path` module ([#3779](https://github.com/can1357/oh-my-pi/issues/3779)).
+### Fixed
+
+- Fixed Python eval shell helpers (`!cmd`, `%%bash`, `%pip`) letting child processes inherit the runner's stdin — the host's NDJSON control channel — which could steal protocol frames and deadlocked nested interpreters on Windows; children now get `stdin=DEVNULL`. `%%bash` also resolves Git Bash on Windows instead of hardcoding `/bin/bash`.
 
 ## [17.2.9] - 2026-08-05
 

packages/coding-agent/src/eval/py/runner.py

@@ -40,6 +40,7 @@ import os
 import re
 import runpy
 import shlex
+import shutil
 import signal
 import subprocess
 import sys
@@ -576,8 +577,10 @@ class _BoundedLineScanner:
 def _magic_pip(args: str) -> None:
     argv = shlex.split(args) if args else ["--help"]
     cmd = [sys.executable, "-m", "pip", *argv]
+    # stdin=DEVNULL: see _run_shell_body.
     proc = subprocess.Popen(
         cmd,
+        stdin=subprocess.DEVNULL,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
     )
@@ -737,9 +740,37 @@ def _magic_run(args: str) -> None:
         _STATE.user_ns[name] = value
 
 
+def _resolve_bash() -> str:
+    if os.name != "nt":
+        return "/bin/bash"
+    # Prefer Git Bash over WSL's System32 bash.exe, which runs inside a
+    # separate Linux environment and does not share the Windows filesystem
+    # layout or PATH.
+    for env_var, suffix in (
+        ("ProgramFiles", r"Git\bin\bash.exe"),
+        ("ProgramFiles(x86)", r"Git\bin\bash.exe"),
+        ("LOCALAPPDATA", r"Programs\Git\bin\bash.exe"),
+    ):
+        root = os.environ.get(env_var)
+        if root:
+            candidate = os.path.join(root, suffix)
+            if os.path.isfile(candidate):
+                return candidate
+    found = shutil.which("bash")
+    if found and "system32" not in found.lower():
+        return found
+    # WSL's System32 bash.exe runs in a separate Linux environment, so
+    # silently falling back to it would execute the cell somewhere the user
+    # did not intend; fail loudly instead.
+    raise RuntimeError(
+        "%%bash requires a POSIX bash, but none was found. "
+        "Install Git for Windows or add a non-WSL bash to PATH."
+    )
+
+
 @cell_magic("bash")
 def _magic_cell_bash(args: str, body: str) -> int:
-    return _run_shell_body(body, shell_arg="/bin/bash")
+    return _run_shell_body(body, shell_arg=_resolve_bash())
 
 
 @cell_magic("capture")
@@ -780,8 +811,12 @@ def _magic_cell_writefile(args: str, body: str) -> str:
 
 
 def _run_shell_body(body: str, *, shell_arg: str) -> int:
+    # stdin=DEVNULL: children must not inherit the runner's stdin, which is
+    # the host's NDJSON control channel (a reading child would steal frames,
+    # and inheriting the pipe deadlocks nested interpreters on Windows).
     proc = subprocess.Popen(
         [shell_arg, "-c", body],
+        stdin=subprocess.DEVNULL,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
     )
@@ -821,9 +856,11 @@ class _ShellResult(list):
 
 
 def __omp_shell(cmd: str) -> _ShellResult:
+    # stdin=DEVNULL: see _run_shell_body.
     proc = subprocess.Popen(
         cmd,
         shell=True,
+        stdin=subprocess.DEVNULL,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
     )

packages/coding-agent/test/eval/py/prelude.test.ts

@@ -1,7 +1,8 @@
 import { describe, expect, it } from "bun:test";
+import { $which } from "@oh-my-pi/pi-utils";
 import { PYTHON_PRELUDE } from "../../../src/eval/py/prelude";
 
-const pythonPath = Bun.env.PYTHON ?? "python3";
+const pythonPath = Bun.env.PYTHON ?? ($which("python3") ? "python3" : "python");
 
 async function runPrelude(
 	code: string,
@@ -22,7 +23,8 @@ async function runPrelude(
 		new Response(proc.stderr).text(),
 		proc.exited,
 	]);
-	return { stdout, stderr, exitCode };
+	// Python's text-mode stdout emits \r\n on Windows.
+	return { stdout: stdout.replaceAll("\r\n", "\n"), stderr: stderr.replaceAll("\r\n", "\n"), exitCode };
 }
 
 describe("python prelude", () => {

packages/coding-agent/test/eval/py/runner-shell-output.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, it } from "bun:test";
 import * as path from "node:path";
+import { $which } from "@oh-my-pi/pi-utils";
 
 interface RunnerFrame {
 	type?: string;
@@ -8,12 +9,17 @@ interface RunnerFrame {
 	status?: string;
 }
 
-const pythonPath = Bun.env.PYTHON ?? "python3";
+const pythonPath = Bun.env.PYTHON ?? ($which("python3") ? "python3" : "python");
 const runnerPath = path.resolve(import.meta.dir, "../../../src/eval/py/runner.py");
 const repoRoot = path.resolve(import.meta.dir, "../../../../..");
 const encoder = new TextEncoder();
 
 function shellQuote(value: string): string {
+	// `!cmd` runs through cmd.exe on Windows (shell=True) and a POSIX shell
+	// elsewhere; quote for the shell that will actually parse the command.
+	if (process.platform === "win32") {
+		return `"${value.replaceAll('"', '""')}"`;
+	}
 	return `'${value.replaceAll("'", `'"'"'`)}'`;
 }
 
@@ -41,7 +47,10 @@ async function runCell(code: string): Promise<RunnerFrame[]> {
 			if (newline >= 0) {
 				const line = pending.slice(0, newline);
 				pending = pending.slice(newline + 1);
-				return JSON.parse(line) as RunnerFrame;
+				const frame = JSON.parse(line) as RunnerFrame;
+				// Child processes write \r\n on Windows.
+				if (typeof frame.data === "string") frame.data = frame.data.replaceAll("\r\n", "\n");
+				return frame;
 			}
 			const { value, done } = await reader.read();
 			if (done) {
@@ -139,6 +148,26 @@ describe("Python runner shell output streaming", () => {
 		expect(stdout).not.toContain("capturedChars=1048593");
 	});
 
+	it("isolates !cmd children from the runner's stdin control channel", async () => {
+		// The runner's stdin carries the host's NDJSON frames. A child that
+		// inherits it can steal frames or block forever waiting for input;
+		// with stdin=DEVNULL a stdin-reading child sees immediate EOF instead.
+		const child = ["import sys", "data = sys.stdin.read()", "print('read=' + repr(data))"].join(";");
+		const frames = await runCell(
+			[
+				`result = !${pythonPath} -c ${shellQuote(child)}`,
+				"print('return=' + str(result.returncode) + ' lines=' + repr(list(result)))",
+			].join("\n"),
+		);
+		const stdout = frames
+			.filter(frame => frame.type === "stdout")
+			.map(frame => frame.data)
+			.join("");
+
+		expect(stdout).toContain("read=''");
+		expect(stdout).toContain("return=0");
+	});
+
 	it("streams newline-free %%bash output without waiting for EOF", async () => {
 		const child = [
 			"import sys,time",