diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2b81b48c1..487aee24f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -25,6 +25,9 @@ ### Fixed - WSL host-home resolution now builds `/mnt//...` 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 diff --git a/packages/coding-agent/src/eval/py/runner.py b/packages/coding-agent/src/eval/py/runner.py index 7190594c3..a87f6ab71 100644 --- a/packages/coding-agent/src/eval/py/runner.py +++ b/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, ) diff --git a/packages/coding-agent/test/eval/py/prelude.test.ts b/packages/coding-agent/test/eval/py/prelude.test.ts index c22e5a820..6554265e0 100644 --- a/packages/coding-agent/test/eval/py/prelude.test.ts +++ b/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", () => { diff --git a/packages/coding-agent/test/eval/py/runner-shell-output.test.ts b/packages/coding-agent/test/eval/py/runner-shell-output.test.ts index 8fc0d2978..44cfc7f9c 100644 --- a/packages/coding-agent/test/eval/py/runner-shell-output.test.ts +++ b/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 { 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",