diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5bcee1054..ba25b3dd4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -142,6 +142,9 @@ - Fixed heavily branched conversation trees shifting linear continuations into disconnected columns. - Fixed plugin installation validation failures for legacy compatibility shims. - Removed hard-coded references to disabled or absent agents in system and tool prompts. +### Fixed + +- Fixed unobserved promise continuations from browser helpers such as `tab.waitForResponse()` wedging or killing the tab worker when they reject; browser facade promises now retain native promise behavior while observing every `then`, `catch`, and `finally` continuation. ## [17.2.4] - 2026-08-01 diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index 8f257feac..7eed8a823 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -24,6 +24,7 @@ import { bindRunFacade, CELL_BUDGET_SLACK_MS, markHandled, + markBrowserRunRejection, resolvePredicateTimeout, type WaitPredicateOptions, waitForRun, @@ -44,6 +45,7 @@ import { loadPuppeteerInWorker, } from "./launch"; import { extractReadableFromHtml, type ReadableFormat } from "./readable"; + import { cloneSafe, RunOutput } from "./run-output"; import type { Observation, @@ -1182,9 +1184,9 @@ export class WorkerCore { (opTimeout?.aborted || (err instanceof Error && err.name === "TimeoutError")) ) { const hint = selector ? await this.#selectorTimeoutHint(selector) : ""; - throw new ToolError(`${label} timed out after ${perOpTimeoutMs}ms${hint}`); + throw markBrowserRunRejection(new ToolError(`${label} timed out after ${perOpTimeoutMs}ms${hint}`)); } - throw err; + throw markBrowserRunRejection(err); } finally { earlyAc.abort(); active.inflight.delete(opId); diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index 742c8daf8..5d44bfc06 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -1,6 +1,79 @@ +import { logger, postmortem } from "@oh-my-pi/pi-utils"; import { untilAborted } from "@oh-my-pi/pi-utils/abortable"; import { ToolError, throwIfAborted } from "./tool-errors"; +const BROWSER_RUN_REJECTION = Symbol.for("omp.browserRunRejection"); + +export function markBrowserRunRejection(reason: T): T { + if (reason !== null && (typeof reason === "object" || typeof reason === "function")) { + Reflect.set(reason, BROWSER_RUN_REJECTION, true); + } + return reason; +} + +function isBrowserRunRejection(reason: unknown): boolean { + let current = reason; + for (let depth = 0; depth < 8 && current !== null && typeof current === "object"; depth++) { + if (Reflect.get(current, BROWSER_RUN_REJECTION) === true) return true; + current = "cause" in current ? current.cause : undefined; + } + return false; +} + +function consumeBrowserRunRejection(reason: unknown): boolean { + if (!isBrowserRunRejection(reason)) return false; + logger.warn("Contained unhandled browser-run rejection (missing await?)", { + error: reason instanceof Error ? reason.message : String(reason), + }); + return true; +} + +postmortem.interceptUnhandledRejections(consumeBrowserRunRejection); + +/** + * Observe every continuation created from a browser facade promise. A catch on + * only the original promise does not cover `tab.waitForResponse(...).then(...)`: + * `then` creates a fresh rejection that can otherwise kill or wedge the worker. + */ +const observedPromiseTrees = new WeakSet>(); + +function observePromiseTree(promise: Promise): Promise { + if (observedPromiseTrees.has(promise)) return promise; + observedPromiseTrees.add(promise); + markHandled(promise); + const originalThen = promise.then.bind(promise); + const originalCatch = promise.catch.bind(promise); + const originalFinally = promise.finally.bind(promise); + Object.defineProperties(promise, { + // biome-ignore lint/suspicious/noThenProperty: native Promise continuations must remain thenable. + then: { + configurable: true, + value: ( + onFulfilled?: ((value: T) => TResult1 | PromiseLike) | null, + onRejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, + ): Promise => observePromiseTree(originalThen(onFulfilled, onRejected)), + }, + catch: { + configurable: true, + value: ( + onRejected?: ((reason: unknown) => TResult | PromiseLike) | null, + ): Promise => observePromiseTree(originalCatch(onRejected)), + }, + finally: { + configurable: true, + value: (onFinally?: (() => void) | null): Promise => observePromiseTree(originalFinally(onFinally)), + }, + }); + return promise; +} + +function trackBrowserRunPromise(promise: Promise): Promise { + const tracked = promise.catch(error => { + throw markBrowserRunRejection(error); + }); + return observePromiseTree(tracked); +} + /** * Marks a run-scoped promise as observed without changing its behavior for awaited callers. * @@ -83,7 +156,7 @@ export function waitForRun( await untilAborted(signal, async () => await Bun.sleep(interval)); } })(); - return markHandled(promise); + return trackBrowserRunPromise(promise); } /** Binds a long-lived scope facade (page/tab/desktop objects) to one evaluated run's abort signal. */ @@ -102,11 +175,16 @@ export function bindRunFacade(target: T, signal: AbortSignal): if (result && typeof result === "object") { const then = Reflect.get(result, "then"); if (typeof then === "function") { - return markHandled( - Promise.resolve(result).then(resolved => { - throwIfAborted(signal); - return resolved; - }), + return trackBrowserRunPromise( + Promise.resolve(result).then( + resolved => { + throwIfAborted(signal); + return resolved; + }, + error => { + throw markBrowserRunRejection(error); + }, + ), ); } } diff --git a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts index 52f2c24ca..0fa306232 100644 --- a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts +++ b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts @@ -4,6 +4,9 @@ import { JsRuntime, type RuntimeHooks } from "../../src/eval/js/shared/runtime"; import { bindRunFacade, markHandled, waitForRun } from "../../src/tools/run-scope"; import { ToolAbortError } from "../../src/tools/tool-errors"; +const runCancellationModuleUrl = new URL("../../src/tools/browser/run-cancellation.ts", import.meta.url).href; +const toolErrorsModuleUrl = new URL("../../src/tools/tool-errors.ts", import.meta.url).href; + async function collectUnhandledRejections(action: () => void | Promise): Promise { const reasons: unknown[] = []; const onUnhandled = (reason: unknown) => reasons.push(reason); @@ -127,6 +130,40 @@ describe("browser run cancellation", () => { expect(reasons).toEqual([]); }); + it("contains an unhandled descendant of a timed-out browser helper promise", async () => { + const script = ` + import { bindBrowserRunFacade } from ${JSON.stringify(runCancellationModuleUrl)}; + import { ToolError } from ${JSON.stringify(toolErrorsModuleUrl)}; + + const facade = bindBrowserRunFacade( + { + waitForResponse() { + return Promise.reject(new ToolError("tab.waitForResponse() timed out after 15ms")); + }, + }, + new AbortController().signal, + ); + void facade.waitForResponse().then(() => undefined); + await Promise.resolve(); + await Promise.resolve(); + console.log("browser probe survived"); + `; + const proc = Bun.spawn([process.execPath, "-e", script], { + cwd: process.cwd(), + stdout: "pipe", + stderr: "pipe", + }); + const [exitCode, stdout, stderr] = await Promise.all([ + proc.exited, + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + + expect(exitCode, stderr).toBe(0); + expect(stdout).toContain("browser probe survived"); + expect(stderr).not.toContain("[Unhandled Rejection]"); + }); + it("rejects awaited facade method calls that settle after abort", async () => { const controller = new AbortController(); const deferred = Promise.withResolvers(); diff --git a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts index 5d6234d30..fea1670fc 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -208,6 +208,44 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("keeps the tab worker alive after an unhandled waitForResponse timeout descendant", async () => { + const tool = new BrowserTool(makeSession()); + const name = `response-timeout-descendant-${process.pid}`; + + try { + 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" }]); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 30_000); + it("observes floating raw page promises when the target closes", async () => { const tool = new BrowserTool(makeSession()); const name = `target-close-${process.pid}`;