From f03581dc22ec8c6bcfced78005e36c1bcd4ec544 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:04:21 +0000 Subject: [PATCH 1/2] fix(coding-agent): stop eval shell children inheriting the runner control channel; resolve Git Bash on Windows --- packages/coding-agent/CHANGELOG.md | 4 +++ packages/coding-agent/src/eval/py/runner.py | 33 ++++++++++++++++++- .../coding-agent/test/eval/py/prelude.test.ts | 6 ++-- .../test/eval/py/runner-shell-output.test.ts | 13 ++++++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7ba445a2b..dadcf5908 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### 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 ### Breaking Changes diff --git a/packages/coding-agent/src/eval/py/runner.py b/packages/coding-agent/src/eval/py/runner.py index 7190594c3..c006dea1e 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,31 @@ 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 + return "bash" + + @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 +805,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 +850,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..114725be2 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) { From 85177a29ecb665524d64a914e2d8d38a34236d57 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:42:36 +0000 Subject: [PATCH 2/2] fix(coding-agent): fail loudly when no POSIX bash exists; add stdin-isolation regression test --- packages/coding-agent/src/eval/py/runner.py | 8 +++++++- .../test/eval/py/runner-shell-output.test.ts | 20 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/eval/py/runner.py b/packages/coding-agent/src/eval/py/runner.py index c006dea1e..a87f6ab71 100644 --- a/packages/coding-agent/src/eval/py/runner.py +++ b/packages/coding-agent/src/eval/py/runner.py @@ -759,7 +759,13 @@ def _resolve_bash() -> str: found = shutil.which("bash") if found and "system32" not in found.lower(): return found - return "bash" + # 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") 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 114725be2..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 @@ -148,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",