fix(coding-agent): contained browser timeout rejections

Observed every browser facade continuation so fire-and-forget helper timeouts cannot wedge or kill a tab worker. Preserved native Promise identity for callers and test matchers.
2026-08-02 13:16:38 +08:00
parent f7f8e040ee
commit 364e13c49e
5 changed files with 166 additions and 8 deletions
@@ -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

@@ -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);
@@ -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<T>(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<Promise<unknown>>();
+
+function observePromiseTree<T>(promise: Promise<T>): Promise<T> {
+	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: <TResult1 = T, TResult2 = never>(
+				onFulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | null,
+				onRejected?: ((reason: unknown) => TResult2 | PromiseLike<TResult2>) | null,
+			): Promise<TResult1 | TResult2> => observePromiseTree(originalThen(onFulfilled, onRejected)),
+		},
+		catch: {
+			configurable: true,
+			value: <TResult = never>(
+				onRejected?: ((reason: unknown) => TResult | PromiseLike<TResult>) | null,
+			): Promise<T | TResult> => observePromiseTree(originalCatch(onRejected)),
+		},
+		finally: {
+			configurable: true,
+			value: (onFinally?: (() => void) | null): Promise<T> => observePromiseTree(originalFinally(onFinally)),
+		},
+	});
+	return promise;
+}
+
+function trackBrowserRunPromise<T>(promise: Promise<T>): Promise<T> {
+	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<T extends object>(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);
+									},
+								),
 							);
 						}
 					}
@@ -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<void>): Promise<unknown[]> {
 	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<string>();
@@ -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,<h1>ready</h1>",
+			});
+			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}`;