diff --git a/packages/coding-agent/src/utils/external-editor.ts b/packages/coding-agent/src/utils/external-editor.ts index 8185854f0..35cea1c87 100644 --- a/packages/coding-agent/src/utils/external-editor.ts +++ b/packages/coding-agent/src/utils/external-editor.ts @@ -1,7 +1,6 @@ /** * Utilities for launching an external text editor ($VISUAL / $EDITOR). */ -import { spawn } from "node:child_process"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -51,17 +50,17 @@ export async function openInEditor( try { await Bun.write(tmpFile, content); - const [editor, ...editorArgs] = editorCmd.split(" "); - const stdio = options?.stdio ?? ["inherit", "inherit", "inherit"]; - const child = + const [stdin, stdout, stderr] = options?.stdio ?? ["inherit", "inherit", "inherit"]; + const cmd = process.platform === "win32" - ? spawn(editor, [...editorArgs, tmpFile], { stdio, shell: true }) - : spawn($which("sh") ?? "sh", ["-c", `${editorCmd} "$1"`, "sh", tmpFile], { stdio }); - const { promise, reject, resolve } = Promise.withResolvers(); - child.once("exit", (code, signal) => resolve(code ?? (signal ? -1 : 0))); - child.once("error", error => reject(error)); - const exitCode = await promise; - + ? ["cmd", "/c", `${editorCmd} "${tmpFile}"`] + : [$which("sh") ?? "sh", "-c", `${editorCmd} "$1"`, "sh", tmpFile]; + const child = Bun.spawn(cmd, { + stdin, + stdout, + stderr, + }); + const exitCode = await child.exited; if (exitCode === 0) { const text = await Bun.file(tmpFile).text(); if (options?.trimTrailingNewline === false) { diff --git a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts deleted file mode 100644 index a1adb63c3..000000000 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ /dev/null @@ -1,514 +0,0 @@ -import { afterAll, beforeAll, describe, expect, it, vi } from "bun:test"; -import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; -import type { ToolSession } from "@oh-my-pi/pi-coding-agent/sdk"; -import { BrowserTool } from "@oh-my-pi/pi-coding-agent/tools/browser"; -import { getTabsMapForTest } from "@oh-my-pi/pi-coding-agent/tools/browser/tab-supervisor"; -import * as logger from "@oh-my-pi/pi-utils/logger"; -import { chromiumAvailable } from "./chromium-probe"; - -const CHROMIUM_AVAILABLE = await chromiumAvailable(); - -function makeSession(): ToolSession { - return { - cwd: "/tmp/test", - hasUI: false, - getSessionFile: () => null, - getSessionSpawns: () => "*", - settings: Settings.isolated({ "browser.headless": true }), - }; -} - -describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { - const suiteTool = new BrowserTool(makeSession()); - const suiteTabName = `evaluation-suite-${process.pid}`; - - // Hold one browser and one tab worker across the suite. Each case navigates - // the shared page before exercising its run-level isolation contract. - beforeAll(async () => { - await suiteTool.execute("open", { - action: "open", - name: suiteTabName, - url: "data:text/html,Browser evaluation suite", - }); - }, 30_000); - - afterAll(async () => { - await suiteTool.execute("close", { action: "close", name: suiteTabName, kill: true }); - }, 30_000); - - // Launches real headless Chromium; CI cold start easily exceeds bun's 5s default. - it("runs tab.evaluate in the page's main JavaScript world", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,", - }); - const result = await tool.execute("run", { - action: "run", - name, - code: "return await tab.evaluate(() => globalThis.__ompMainWorld);", - }); - - expect(result.content).toEqual([{ type: "text", text: "42" }]); - }, 30_000); - - it("clears request interception and held requests between runs, including thrown runs", async () => { - const server = Bun.serve({ - port: 0, - fetch(request) { - const { pathname } = new URL(request.url); - if (pathname === "/held") return new Response("normal-held"); - if (pathname === "/mock") return new Response("normal-mock"); - return new Response("Interception lifecycle", { - headers: { "content-type": "text/html" }, - }); - }, - }); - const tool = suiteTool; - const name = suiteTabName; - - try { - await tool.execute("open", { - action: "open", - name, - url: `http://127.0.0.1:${server.port}/`, - }); - let setupError: unknown; - try { - await tool.execute("run", { - action: "run", - name, - code: ` - globalThis.__requestListenerBaseline = page.listenerCount("request"); - await page.setRequestInterception(true); - page.on("request", request => void request.abort()); - throw new Error("setup failed"); - `, - }); - } catch (error) { - setupError = error; - } - expect(setupError).toBeInstanceOf(Error); - expect(setupError).toHaveProperty("message", "setup failed"); - - const afterThrow = await tool.execute("run", { - action: "run", - name, - code: ` - return { - clean: page.listenerCount("request") === globalThis.__requestListenerBaseline, - body: await tab.evaluate(async () => await (await fetch("/mock")).text()), - }; - `, - }); - expect(afterThrow.content).toEqual([ - { type: "text", text: '{\n "clean": true,\n "body": "normal-mock"\n}' }, - ]); - - const intercepted = await tool.execute("run", { - action: "run", - name, - code: ` - let heldSeen = false; - await page.setRequestInterception(true); - page.on("request", request => { - const pathname = new URL(request.url()).pathname; - if (pathname === "/held") { - heldSeen = true; - return; - } - if (pathname === "/mock") { - void request.respond({ status: 200, body: "mocked" }); - return; - } - void request.continue(); - }); - await tab.evaluate(() => { - globalThis.__heldFetch = fetch("/held").then(async response => await response.text()); - }); - await wait(() => heldSeen); - return await tab.evaluate(async () => await (await fetch("/mock")).text()); - `, - }); - expect(intercepted.content).toEqual([{ type: "text", text: "mocked" }]); - - const resumed = await tool.execute("run", { - action: "run", - name, - code: ` - const listeners = page.listenerCount("request"); - if (listeners !== globalThis.__requestListenerBaseline) { - return { clean: false, listeners, baseline: globalThis.__requestListenerBaseline }; - } - return { - clean: true, - values: await tab.evaluate(async () => [ - await globalThis.__heldFetch, - await (await fetch("/mock")).text(), - ]), - }; - `, - }); - expect(resumed.content).toEqual([ - { - type: "text", - text: '{\n "clean": true,\n "values": [\n "normal-held",\n "normal-mock"\n ]\n}', - }, - ]); - } finally { - server.stop(true); - } - }, 30_000); - - it("fires a once request handler exactly once and clears it between runs", async () => { - const server = Bun.serve({ - port: 0, - fetch(request) { - const { pathname } = new URL(request.url); - if (pathname === "/mock") return new Response("normal-mock"); - return new Response("Once interception", { - headers: { "content-type": "text/html" }, - }); - }, - }); - const tool = suiteTool; - const name = suiteTabName; - - try { - await tool.execute("open", { - action: "open", - name, - url: `http://127.0.0.1:${server.port}/`, - }); - const fired = await tool.execute("run", { - action: "run", - name, - code: ` - const baseline = page.listenerCount("request"); - globalThis.__requestListenerBaseline = baseline; - await page.setRequestInterception(true); - page.once("request", request => { - if (new URL(request.url()).pathname === "/mock") { - void request.respond({ status: 200, body: "mocked-once" }); - return; - } - void request.continue(); - }); - const body = await tab.evaluate(async () => await (await fetch("/mock")).text()); - // A once handler that fired must unregister from the emitter, not just the tracker. - return { body, leaked: page.listenerCount("request") - baseline }; - `, - }); - expect(fired.content).toEqual([{ type: "text", text: '{\n "body": "mocked-once",\n "leaked": 0\n}' }]); - - const resumed = await tool.execute("run", { - action: "run", - name, - code: ` - return { - clean: page.listenerCount("request") === globalThis.__requestListenerBaseline, - body: await tab.evaluate(async () => await (await fetch("/mock")).text()), - }; - `, - }); - expect(resumed.content).toEqual([{ type: "text", text: '{\n "clean": true,\n "body": "normal-mock"\n}' }]); - } finally { - server.stop(true); - } - }, 30_000); - - it("keeps the tab worker alive after an unhandled waitForResponse timeout descendant", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - const tabSession = getTabsMapForTest().get(name); - if (tabSession?.backend !== "worker") throw new Error("Worker tab was not created"); - expect(tabSession.worker.mode).toBe("worker"); - const result = await tool.execute("run", { - action: "run", - name, - timeout: 2, - // Real worker timers are intentional: the rejection must cross an - // unhandledRejection turn while the browser run remains active. - code: ` - void tab.waitForResponse("/never", { timeout: 10 }).then(() => undefined); - await Bun.sleep(50); - return "survived timeout"; - `, - }); - expect(result.content).toEqual([{ type: "text", text: "survived timeout" }]); - - const followup = await tool.execute("run", { - action: "run", - name, - code: "return 42;", - }); - expect(followup.content).toEqual([{ type: "text", text: "42" }]); - }, 30_000); - - it("fails floated user continuations without killing the tab worker", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - let failure = ""; - try { - await tool.execute("run", { - action: "run", - name, - timeout: 2, - code: ` - void tab.title().then(() => { - throw new Error("continuation failed"); - }); - await Bun.sleep(500); - return "incorrect success"; - `, - }); - } catch (error) { - failure = error instanceof Error ? error.message : String(error); - } - expect(failure).toContain("Unhandled rejection (missing await?): continuation failed"); - - let rethrowFailure = ""; - try { - await tool.execute("run", { - action: "run", - name, - timeout: 2, - code: ` - void tab.waitForResponse("/never", { timeout: 10 }).catch(reason => { - throw reason; - }); - await Bun.sleep(500); - return "incorrect success"; - `, - }); - } catch (error) { - rethrowFailure = error instanceof Error ? error.message : String(error); - } - expect(rethrowFailure).toContain( - "Unhandled rejection (missing await?): tab.waitForResponse() timed out after 10ms", - ); - - const followup = await tool.execute("run", { - action: "run", - name, - code: "return 42;", - }); - expect(followup.content).toEqual([{ type: "text", text: "42" }]); - }, 30_000); - - it("fails a browser error rethrown through a native promise combinator", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - let failure = ""; - try { - await tool.execute("run", { - action: "run", - name, - timeout: 2, - code: ` - void Promise.all([ - tab.waitForResponse("/never", { timeout: 10 }), - ]).catch(reason => { - throw reason; - }); - await Bun.sleep(500); - return "incorrect success"; - `, - }); - } catch (error) { - failure = error instanceof Error ? error.message : String(error); - } - expect(failure).toContain("Unhandled rejection (missing await?): tab.waitForResponse() timed out after 10ms"); - - const followup = await tool.execute("run", { - action: "run", - name, - code: "return 42;", - }); - expect(followup.content).toEqual([{ type: "text", text: "42" }]); - }, 30_000); - - it("restores promise tracking after evaluated code freezes Promise", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - const frozen = await tool.execute("run", { - action: "run", - name, - code: ` - Object.freeze(Promise); - return Object.isFrozen(Promise); - `, - }); - expect(frozen.content).toEqual([{ type: "text", text: "true" }]); - - const followup = await tool.execute("run", { - action: "run", - name, - code: "return (await Promise.all([42]))[0];", - }); - expect(followup.content).toEqual([{ type: "text", text: "42" }]); - }, 30_000); - - it("aborts the run facade before draining floated continuations", async () => { - const tool = suiteTool; - const name = suiteTabName; - const url = "data:text/html,original

ready

"; - - await tool.execute("open", { - action: "open", - name, - url, - }); - const result = await tool.execute("run", { - action: "run", - name, - code: ` - page.title = async () => { - await Bun.sleep(0); - return "ready"; - }; - void tab.title().then(() => tab.goto("data:text/html,late")); - return "completed"; - `, - }); - expect(result.content).toEqual([{ type: "text", text: "completed" }]); - - await Bun.sleep(100); - const followup = await tool.execute("run", { - action: "run", - name, - code: "return tab.url();", - }); - expect(followup.content).toEqual([{ type: "text", text: url }]); - }, 30_000); - - it("folds a user continuation rejection that settles during cleanup", async () => { - const tool = suiteTool; - const name = suiteTabName; - - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - let failure = ""; - try { - await tool.execute("run", { - action: "run", - name, - code: ` - await page.setRequestInterception(true); - page.setRequestInterception = async () => { - await Bun.sleep(50); - }; - const continuationStarted = Promise.withResolvers(); - void tab.title().then(async () => { - continuationStarted.resolve(); - await Bun.sleep(10); - throw new Error("cleanup continuation failed"); - }); - await continuationStarted.promise; - return "incorrect success"; - `, - }); - } catch (error) { - failure = error instanceof Error ? error.message : String(error); - } - expect(failure).toContain("Unhandled rejection (missing await?): cleanup continuation failed"); - }, 30_000); - - it("logs a user continuation rejection after its browser run ends", async () => { - const warningLogged = Promise.withResolvers(); - const warn = vi.spyOn(logger, "warn").mockImplementation(message => { - if (message === "Unhandled rejection after browser run ended") warningLogged.resolve(); - }); - const tool = suiteTool; - const name = suiteTabName; - - try { - await tool.execute("open", { - action: "open", - name, - url: "data:text/html,

ready

", - }); - const result = await tool.execute("run", { - action: "run", - name, - code: ` - const continuationStarted = Promise.withResolvers(); - void tab.title().then(async () => { - continuationStarted.resolve(); - await Bun.sleep(50); - throw new Error("late continuation failed"); - }); - await continuationStarted.promise; - return "completed"; - `, - }); - expect(result.content).toEqual([{ type: "text", text: "completed" }]); - - await warningLogged.promise; - expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { - runId: expect.any(String), - error: "late continuation failed", - }); - } finally { - warn.mockRestore(); - } - }, 30_000); - - it("observes floating raw page promises when the target closes", async () => { - const tool = suiteTool; - const name = suiteTabName; - const url = `data:text/html,

ready

#${name}`; - - await tool.execute("open", { action: "open", name, url }); - const tabSession = getTabsMapForTest().get(name); - if (tabSession?.backend !== "worker") throw new Error("Worker tab was not created"); - const pages = await tabSession.browser.browser.pages(); - const targetPage = pages.find(page => page.url() === url); - if (!targetPage) throw new Error(`Target page was not found for ${url}`); - - const started = targetPage.waitForFunction("document.documentElement.dataset.floating === 'true'", { - polling: "mutation", - }); - const run = tool.execute("run", { - action: "run", - name, - code: "page.evaluate(() => { document.documentElement.dataset.floating = 'true'; return Promise.withResolvers().promise; }); try { await tab.waitForSelector('#never'); } catch {} return 'survived';", - }); - const startedHandle = await started; - await startedHandle.dispose(); - await targetPage.close(); - - const result = await run; - expect(result.content).toEqual([{ type: "text", text: "survived" }]); - }, 30_000); -});