diff --git a/packages/coding-agent/src/cli.ts b/packages/coding-agent/src/cli.ts index d115544c9..0450aa5fb 100755 --- a/packages/coding-agent/src/cli.ts +++ b/packages/coding-agent/src/cli.ts @@ -159,7 +159,10 @@ async function runWorkerEntrypoint(arg: string | undefined): Promise { return true; } if (arg === JS_EVAL_PROCESS_ARG) { - await runIpcSubprocessWorker(startJsEvalProcess); + // The JS evaluator forwards user-controlled payloads (tool-call args, + // display outputs); a non-serializable one must fail that cell, not + // SIGKILL the kernel and erase the eval session's state. + await runIpcSubprocessWorker(startJsEvalProcess, { rethrowConnectedSendErrors: true }); return true; } if (arg === STT_WORKER_ARG) { @@ -202,6 +205,18 @@ async function runIpcSubprocessWorker( sendAndFlush(message: Out): Promise; onMessage(handler: (message: In) => void): () => void; }) => void, + options?: { + /** + * Rethrow send failures while the IPC channel is still connected instead + * of shutting down. A connected-channel failure means this particular + * message could not be serialized (e.g. a JS eval cell passed a function + * into tool args, a DataCloneError under advanced serialization) — the + * caller must see that error, exactly as Worker `postMessage` would + * deliver it, rather than losing the whole worker and its state. + * Channel-gone failures still shut down. + */ + rethrowConnectedSendErrors?: boolean; + }, ): Promise { const { promise: shuttingDown, resolve: shutdown } = Promise.withResolvers(); type IpcSend = (this: NodeJS.Process, message: unknown, callback?: (error: Error | null) => void) => boolean; @@ -217,7 +232,8 @@ async function runIpcSubprocessWorker( } try { sender.call(process, message); - } catch { + } catch (error) { + if (options?.rethrowConnectedSendErrors && process.connected) throw error; shutdown(); } }; diff --git a/packages/coding-agent/src/eval/js/worker-core.ts b/packages/coding-agent/src/eval/js/worker-core.ts index 354f0c981..69222f885 100644 --- a/packages/coding-agent/src/eval/js/worker-core.ts +++ b/packages/coding-agent/src/eval/js/worker-core.ts @@ -297,7 +297,15 @@ export class WorkerCore { const id = `tc-${active.runId}-${crypto.randomUUID()}`; const { promise, resolve, reject } = Promise.withResolvers(); active.pendingTools.set(id, { runId: active.runId, resolve, reject }); - this.#transport.send({ type: "tool-call", id, runId: active.runId, name, args }); + try { + this.#transport.send({ type: "tool-call", id, runId: active.runId, name, args }); + } catch (error) { + // Non-serializable args (DataCloneError from postMessage / IPC send). + // No reply will ever arrive; fail this call instead of stranding a + // pending entry until close. + active.pendingTools.delete(id); + reject(error); + } return await promise; } diff --git a/packages/coding-agent/test/tools/eval-clone-error.test.ts b/packages/coding-agent/test/tools/eval-clone-error.test.ts new file mode 100644 index 000000000..607e02d0f --- /dev/null +++ b/packages/coding-agent/test/tools/eval-clone-error.test.ts @@ -0,0 +1,67 @@ +import { afterAll, describe, expect, it } from "bun:test"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { disposeAllVmContexts } from "@oh-my-pi/pi-coding-agent/eval/js/context-manager"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { EvalTool } from "@oh-my-pi/pi-coding-agent/tools/eval"; + +function makeSession(): ToolSession { + return { + cwd: process.cwd(), + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => null, + settings: Settings.isolated(), + } as unknown as ToolSession; +} + +function textOf(result: { content: Array<{ type: string; text?: string }> }): string { + return result.content + .filter( + (block): block is { type: "text"; text: string } => block.type === "text" && typeof block.text === "string", + ) + .map(block => block.text) + .join("\n"); +} + +/** + * Tool-call args cross the worker boundary by structured clone, and functions + * are not cloneable. The clone failure must reject the awaiting cell — the + * Worker `postMessage` contract — not shut down the subprocess kernel and + * erase the eval session's accumulated state. + */ +describe("EvalTool non-serializable tool args", () => { + afterAll(async () => { + await disposeAllVmContexts(); + }); + + it("fails the tool call inside the cell and keeps kernel state alive", async () => { + const tool = new EvalTool(makeSession()); + + const seed = await tool.execute("call-clone-seed", { + language: "js", + code: "globalThis.__cloneProbe = 41; console.log('seeded');", + }); + expect(textOf(seed)).toContain("seeded"); + + const bad = await tool.execute("call-clone-bad", { + language: "js", + code: [ + "try {", + '\tawait __omp_call_tool__("read", { path: () => "x" });', + '\tconsole.log("no-throw");', + "} catch (error) {", + '\tconsole.log("caught " + error.name);', + "}", + ].join("\n"), + }); + const badText = textOf(bad); + expect(badText).toContain("caught DataCloneError"); + expect(badText).not.toContain("JS eval worker exited"); + + const after = await tool.execute("call-clone-after", { + language: "js", + code: "console.log('probe=' + (globalThis.__cloneProbe + 1));", + }); + expect(textOf(after)).toContain("probe=42"); + }); +});