From 7d233725b65cf810ddfe96e2b97d22c0e90faf70 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 12 May 2026 05:43:37 +0200 Subject: [PATCH] refactor(coding-agent/eval): removed eval shell run helpers from JS and Python preludes - Removed the JS and Python eval prelude `run` helpers, including their shell execution and timeout/cwd option handling. - Updated the JS VM helper set to expose `Bun` and removed the deleted `run` entry from the prelude. - Revised eval docs to drop `run` from the helper surface and note the new JS `Bun` global. --- docs/tools/eval.md | 4 +- .../src/eval/js/context-manager.ts | 39 +--------- packages/coding-agent/src/eval/js/prelude.txt | 2 - packages/coding-agent/src/eval/py/prelude.py | 75 +------------------ .../coding-agent/src/prompts/tools/eval.md | 4 +- 5 files changed, 4 insertions(+), 120 deletions(-) diff --git a/docs/tools/eval.md b/docs/tools/eval.md index 137982bdb..f10227954 100644 --- a/docs/tools/eval.md +++ b/docs/tools/eval.md @@ -129,7 +129,7 @@ Implemented in `packages/coding-agent/src/eval/js/context-manager.ts` and `packa - Top-level static `import ... from ...` is rewritten to `await import(...)` by `rewriteStaticImports()` - The prelude installs globals: - `display`, `print` - - `read`, `write`, `append`, `sort`, `uniq`, `counter`, `diff`, `tree`, `run`, `env`, `output` + - `read`, `write`, `append`, `sort`, `uniq`, `counter`, `diff`, `tree`, `env`, `output` - `tool.(args)` proxy for arbitrary session tool calls - JS helpers are async because they cross the VM/tool boundary - `display(value)` behavior: @@ -183,8 +183,6 @@ A single tool call can mix Python and JS cells. Persistence is per language runt - Subprocesses / native bindings - Python availability check runs ` -c ...`. - Python backend may start or connect to a kernel gateway; details are in `docs/python-repl.md`. - - JS `run()` helper spawns `bash -lc ` via `Bun.spawn`. - - Python `run()` helper spawns `bash` or `sh` via `subprocess.Popen`. - Session state - `session.assertEvalExecutionAllowed?.()` can block execution. - `session.trackEvalExecution?.(...)` can register cancellable eval work. diff --git a/packages/coding-agent/src/eval/js/context-manager.ts b/packages/coding-agent/src/eval/js/context-manager.ts index d865924f0..4987d1be8 100644 --- a/packages/coding-agent/src/eval/js/context-manager.ts +++ b/packages/coding-agent/src/eval/js/context-manager.ts @@ -32,9 +32,6 @@ interface VmHelperOptions { reverse?: boolean; unique?: boolean; count?: boolean; - cwd?: string; - timeoutMs?: number; - timeout?: number; } interface VmContextState { @@ -303,41 +300,6 @@ async function createHelpers(state: VmContextState) { emitStatus(state, { op: "tree", path: root, entries: entryCount, preview: result.slice(0, 1000) }); return result; }, - run: async ( - command: string, - options: VmHelperOptions = {}, - ): Promise<{ stdout: string; stderr: string; exit_code: number }> => { - const cwd = options.cwd ? resolvePath(state, options.cwd) : state.cwd; - const timeoutMs = - typeof options.timeoutMs === "number" - ? options.timeoutMs - : typeof options.timeout === "number" - ? options.timeout * 1000 - : undefined; - const timeoutSignal = - typeof timeoutMs === "number" && Number.isFinite(timeoutMs) && timeoutMs > 0 - ? AbortSignal.timeout(timeoutMs) - : undefined; - const signal = - state.currentRun?.signal && timeoutSignal - ? AbortSignal.any([state.currentRun.signal, timeoutSignal]) - : (state.currentRun?.signal ?? timeoutSignal); - const child = Bun.spawn(["bash", "-lc", command], { - cwd, - env: getMergedEnv(state), - stdout: "pipe", - stderr: "pipe", - signal, - }); - const [stdout, stderr, exit_code] = await Promise.all([ - new Response(child.stdout as ReadableStream).text(), - new Response(child.stderr as ReadableStream).text(), - child.exited, - ]); - const output = `${stdout}${stderr}`.slice(0, 500); - emitStatus(state, { op: "run", cmd: command.slice(0, 120), code: exit_code, output }); - return { stdout, stderr, exit_code }; - }, env: (key?: string, value?: string): string | Record | undefined => { if (!key) { const env = Object.fromEntries(Object.entries(getMergedEnv(state)).sort(([a], [b]) => a.localeCompare(b))); @@ -419,6 +381,7 @@ async function createVmState( atob, btoa, Buffer, + Bun, process: createProcessSubset(cwd), require: buildRequire(cwd), createRequire, diff --git a/packages/coding-agent/src/eval/js/prelude.txt b/packages/coding-agent/src/eval/js/prelude.txt index b4f6ae701..0fd503eab 100644 --- a/packages/coding-agent/src/eval/js/prelude.txt +++ b/packages/coding-agent/src/eval/js/prelude.txt @@ -12,7 +12,6 @@ if (!globalThis.__omp_js_prelude_loaded__) { const counter = (items, opts = {}) => callHelper("counter", items, toOptions(opts)); const diff = (a, b) => callHelper("diff", a, b); const tree = (path = ".", opts = {}) => callHelper("tree", path, toOptions(opts)); - const run = (cmd, opts = {}) => callHelper("run", cmd, toOptions(opts)); const env = (key, value) => callHelper("env", key, value); const tool = new Proxy( @@ -67,6 +66,5 @@ if (!globalThis.__omp_js_prelude_loaded__) { globalThis.counter = counter; globalThis.diff = diff; globalThis.tree = tree; - globalThis.run = run; globalThis.env = env; } diff --git a/packages/coding-agent/src/eval/py/prelude.py b/packages/coding-agent/src/eval/py/prelude.py index de30356a1..0c13619fe 100644 --- a/packages/coding-agent/src/eval/py/prelude.py +++ b/packages/coding-agent/src/eval/py/prelude.py @@ -3,7 +3,7 @@ from __future__ import annotations if "__omp_prelude_loaded__" not in globals(): __omp_prelude_loaded__ = True from pathlib import Path - import os, json, shutil, subprocess + import os, json from IPython.display import display as _ipy_display, JSON _PRESENTABLE_REPRS = ( @@ -79,79 +79,6 @@ if "__omp_prelude_loaded__" not in globals(): f.write(content) _emit_status("append", path=str(p), chars=len(content)) return p - class ShellResult: - """Result from shell command execution.""" - __slots__ = ("args", "stdout", "stderr", "returncode") - def __init__(self, args: str, stdout: str, stderr: str, returncode: int): - self.args = args - self.stdout = stdout - self.stderr = stderr - self.returncode = returncode - - @property - def code(self) -> int: - return self.returncode - - @property - def exit_code(self) -> int: - return self.returncode - - def check_returncode(self) -> None: - if self.returncode != 0: - raise subprocess.CalledProcessError( - self.returncode, self.args, output=self.stdout, stderr=self.stderr - ) - - def __repr__(self): - if self.returncode == 0: - return "" - return f"exit code {self.returncode}" - - def __bool__(self): - return self.returncode == 0 - - def _make_shell_result(proc: subprocess.CompletedProcess[str], cmd: str) -> ShellResult: - """Create ShellResult and emit status.""" - output = proc.stdout + proc.stderr if proc.stderr else proc.stdout - _emit_status("sh", cmd=cmd[:80], code=proc.returncode, output=output[:500]) - return ShellResult(cmd, proc.stdout, proc.stderr, proc.returncode) - - import signal as _signal - - def _run_with_interrupt(args: list[str], cwd: str | None, timeout: int | None, cmd: str) -> ShellResult: - """Run subprocess with proper interrupt handling.""" - proc = subprocess.Popen( - args, - cwd=cwd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - start_new_session=True, - ) - try: - stdout, stderr = proc.communicate(timeout=timeout) - except KeyboardInterrupt: - os.killpg(proc.pid, _signal.SIGINT) - try: - stdout, stderr = proc.communicate(timeout=2) - except subprocess.TimeoutExpired: - os.killpg(proc.pid, _signal.SIGKILL) - stdout, stderr = proc.communicate() - result = subprocess.CompletedProcess(args, -_signal.SIGINT, stdout, stderr) - return _make_shell_result(result, cmd) - except subprocess.TimeoutExpired: - os.killpg(proc.pid, _signal.SIGKILL) - stdout, stderr = proc.communicate() - result = subprocess.CompletedProcess(args, -_signal.SIGKILL, stdout, stderr) - return _make_shell_result(result, cmd) - result = subprocess.CompletedProcess(args, proc.returncode, stdout, stderr) - return _make_shell_result(result, cmd) - - def run(cmd: str, *, cwd: str | Path | None = None, timeout: int | None = None) -> ShellResult: - """Run a shell command. Returns ShellResult with stdout/stderr and returncode/exit_code fields.""" - shell_path = shutil.which("bash") or shutil.which("sh") or "/bin/sh" - args = [shell_path, "-c", cmd] - return _run_with_interrupt(args, str(cwd) if cwd else None, timeout, cmd) def sort(text: str, *, reverse: bool = False, unique: bool = False) -> str: """Sort lines of text.""" diff --git a/packages/coding-agent/src/prompts/tools/eval.md b/packages/coding-agent/src/prompts/tools/eval.md index 973a1dc97..b9c291124 100644 --- a/packages/coding-agent/src/prompts/tools/eval.md +++ b/packages/coding-agent/src/prompts/tools/eval.md @@ -46,8 +46,6 @@ tree(path?=".", max_depth?=3, show_hidden?=False) → str Render a directory tree. diff(a, b) → str Unified diff between two files. -run(cmd, cwd?=None, timeout?=None) → {stdout, stderr, exit_code} - Run a shell command. env(key?=None, value?=None) → str | None | dict No args → full environment as dict. One arg → value of `key`. Two args → set `key=value` and return value. output(*ids, format?="raw", query?=None, offset?=None, limit?=None) → str | dict | list[dict] @@ -63,7 +61,7 @@ Cells render like a Jupyter notebook. `display(value)` renders non-presentable d - In session mode, use `*** Reset` on a cell to wipe its language's kernel before running.{{#ifAll py js}} Reset is per-language: a python cell's `*** Reset` does not touch the JavaScript kernel and vice versa.{{/ifAll}} -{{#if js}}- **js**: the VM exposes a selective `process` subset, Web APIs, `Buffer`, `fs/promises`. +{{#if js}}- **js**: the VM exposes a selective `process` subset, Web APIs, `Buffer`, `fs/promises`, and the `Bun` global. {{/if}}