From 364e13c49e669257d760335cabb8fe80a080a4d2 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:16:38 +0800 Subject: [PATCH 01/14] 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. --- packages/coding-agent/CHANGELOG.md | 3 + .../src/tools/browser/tab-worker.ts | 6 +- packages/coding-agent/src/tools/run-scope.ts | 90 +++++++++++++++++-- .../tools/browser-run-cancellation.test.ts | 37 ++++++++ .../test/tools/browser-tab-evaluate.test.ts | 38 ++++++++ 5 files changed, 166 insertions(+), 8 deletions(-) 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}`; From 56931915f11fbeb13ffc05907cb54f2b7df1c22c Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:16:55 +0800 Subject: [PATCH 02/14] feat(coding-agent): added fatal session recovery hints Registered live session resume commands with postmortem handling so a fatal rejection or exception identifies every recoverable agent before cleanup. Escaped terminal control characters in recovery output. --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/session/agent-session.ts | 12 ++++++ packages/utils/CHANGELOG.md | 3 ++ packages/utils/src/postmortem.ts | 42 ++++++++++++++++++- .../test/postmortem-cleanup-error.test.ts | 17 ++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ba25b3dd4..bc59f7951 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -142,6 +142,10 @@ - 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. +### Added + +- Added resumable session details to fatal crash output, including an `omp --resume ` command for every persisted live agent session. + ### 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. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 7573608a9..9a34d5da4 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -82,6 +82,7 @@ import { modelsAreEqual } from "@oh-my-pi/pi-catalog/models"; import { MacOSPowerAssertion } from "@oh-my-pi/pi-natives"; import { $env, + APP_NAME, escapeXmlText, formatDuration, getAgentDbPath, @@ -445,6 +446,7 @@ export class AgentSession { // Event subscription state #unsubscribeAgent?: () => void; #cancelExitRecorder?: () => void; + #cancelFatalRecoveryHint?: () => void; #exitRecorded = false; #unsubscribeAppendOnly?: () => void; #unsubscribeModelRoles?: () => void; @@ -1376,6 +1378,14 @@ export class AgentSession { this.#cancelExitRecorder = postmortem.register(`agent-session:${this.sessionManager.getSessionId()}`, reason => { this.#recordSessionExit(reason); }); + this.#cancelFatalRecoveryHint = postmortem.registerFatalRecoveryHint(() => { + const sessionId = this.sessionManager.getSessionId(); + if (!sessionId || !this.sessionManager.getSessionFile()) return undefined; + return { + label: this.#agentId ?? (this.#agentKind === "main" ? "Main" : "Agent"), + command: `${APP_NAME} --resume ${sessionId}`, + }; + }); const advisorsHost: SessionAdvisorsHost = { agent: this.agent, @@ -3758,6 +3768,8 @@ export class AgentSession { this.#recordSessionExit(options.reason ?? "dispose"); this.#cancelExitRecorder?.(); this.#cancelExitRecorder = undefined; + this.#cancelFatalRecoveryHint?.(); + this.#cancelFatalRecoveryHint = undefined; try { await emitSessionShutdownEvent(this.#extensionRunner); } catch (error) { diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index a05d90562..6e093b703 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -28,6 +28,9 @@ ### Changed - Updated the lightweight CLI runner to support static command metadata, allowing root help to render without importing full command implementations. +### Added + +- Added postmortem fatal recovery hint providers so applications can print actionable recovery commands before cleanup starts. ## [17.2.4] - 2026-08-01 diff --git a/packages/utils/src/postmortem.ts b/packages/utils/src/postmortem.ts index 62d3c76b4..2df9a7e8f 100644 --- a/packages/utils/src/postmortem.ts +++ b/packages/utils/src/postmortem.ts @@ -67,6 +67,14 @@ function exitProcess(code: number): never { let cleanupPromise: Promise | undefined; let stdioDisconnectRegistrations = 0; +export interface FatalRecoveryHint { + label: string; + command: string; +} + +type FatalRecoveryHintProvider = () => FatalRecoveryHint | undefined; +const fatalRecoveryHintProviders = new Set(); + /** * Internal: runs all registered cleanup callbacks for the given reason. * Ensures each callback is invoked at most once. Handles errors and prevents reentrancy. @@ -196,6 +204,38 @@ export function interceptUnhandledRejections(interceptor: (reason: unknown) => b return () => rejectionInterceptors.delete(interceptor); } +/** + * Register a synchronous recovery command to print when the process exits + * through an uncaught exception or unhandled rejection. + */ +export function registerFatalRecoveryHint(provider: FatalRecoveryHintProvider): () => void { + fatalRecoveryHintProviders.add(provider); + return () => fatalRecoveryHintProviders.delete(provider); +} + +function escapeFatalHintText(value: string): string { + return value.replace(/[\u0000-\u001f\u007f-\u009f]/gu, char => { + const code = char.codePointAt(0) ?? 0; + return `\\u${code.toString(16).padStart(4, "0")}`; + }); +} + +function formatFatalRecoveryHints(): string { + const lines: string[] = []; + const seenCommands = new Set(); + for (const provider of fatalRecoveryHintProviders) { + try { + const hint = provider(); + if (!hint?.command || seenCommands.has(hint.command)) continue; + seenCommands.add(hint.command); + lines.push(` ${escapeFatalHintText(hint.label)}: ${escapeFatalHintText(hint.command)}`); + } catch (err) { + logger.warn("Fatal recovery hint provider failed", { err }); + } + } + return lines.length > 0 ? `\n[Recovery]\n${lines.join("\n")}\n` : ""; +} + function formatFatalError(label: string, err: Error): string { const name = err.name || "Error"; const message = err.message || "(no message)"; @@ -212,7 +252,7 @@ async function exitAfterFatal(label: string, logMessage: string, err: Error, rea // A revoked terminal can make stream writes raise another fatal error. Use // the descriptor directly so failure stays synchronous and contained. try { - fs.writeSync(2, formatFatalError(label, err)); + fs.writeSync(2, `${formatFatalError(label, err)}${formatFatalRecoveryHints()}`); } catch {} logger.error(logMessage, { err }); await runCleanup(reason); diff --git a/packages/utils/test/postmortem-cleanup-error.test.ts b/packages/utils/test/postmortem-cleanup-error.test.ts index 840b0e760..fc8c93dcc 100644 --- a/packages/utils/test/postmortem-cleanup-error.test.ts +++ b/packages/utils/test/postmortem-cleanup-error.test.ts @@ -123,6 +123,23 @@ describe("postmortem expected cleanup errors", () => { expect(result.stderr).toContain("[Unhandled Rejection] Error: unexpected cleanup rejection"); }); + it("prints registered recovery commands before fatal cleanup", async () => { + const result = await runPostmortemProbe(` + import { postmortem } from "${postmortemModuleUrl}"; + + postmortem.registerFatalRecoveryHint(() => ({ + label: "Main", + command: "omp --resume 019cafe0-dead-beef", + })); + Promise.reject(new Error("session crashed")); + await Promise.resolve(); + `); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("[Unhandled Rejection] Error: session crashed"); + expect(result.stderr).toContain("[Recovery]\n Main: omp --resume 019cafe0-dead-beef"); + }); + it("exits after an uncaught exception when terminal stderr is revoked", async () => { const result = await runPostmortemProbe(` import { spyOn } from "bun:test"; From 01ed58d684b86400d66ce1080ae0a66b7e35e307 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:43:17 +0800 Subject: [PATCH 03/14] docs(utils): documented fatal recovery hint contract --- packages/utils/src/postmortem.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/utils/src/postmortem.ts b/packages/utils/src/postmortem.ts index 2df9a7e8f..63fa30d02 100644 --- a/packages/utils/src/postmortem.ts +++ b/packages/utils/src/postmortem.ts @@ -67,8 +67,11 @@ function exitProcess(code: number): never { let cleanupPromise: Promise | undefined; let stdioDisconnectRegistrations = 0; +/** User-facing command printed before fatal cleanup so interrupted work can be resumed. */ export interface FatalRecoveryHint { + /** Stable label identifying the recoverable session or process. */ label: string; + /** Complete shell command the user can execute to resume the interrupted work. */ command: string; } From 2fd469de75256c48597aa8c7ee3cff3fa50dbef9 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:43:32 +0800 Subject: [PATCH 04/14] fix(coding-agent): corrected browser rejection containment Scoped browser-error markers to each run and contained only propagated browser failures. Routed floated user continuations into failed runs and added worker coverage for native await plus every continuation method. --- .../src/tools/browser/cmux/cmux-tab.ts | 65 ++++-- .../src/tools/browser/tab-supervisor.ts | 2 +- .../src/tools/browser/tab-worker-entry.ts | 2 +- .../src/tools/browser/tab-worker.ts | 90 +++++++- packages/coding-agent/src/tools/run-scope.ts | 129 +++++++---- .../tools/browser-run-cancellation.test.ts | 214 +++++++++++++++--- .../test/tools/browser-tab-evaluate.test.ts | 40 ++++ 7 files changed, 448 insertions(+), 94 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index 49e783d0a..f13ecc13a 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -8,11 +8,20 @@ import { resizeImage } from "../../../utils/image-resize"; import type { ToolSession } from "../../index"; import { resolveToCwd } from "../../path-utils"; import { formatScreenshot } from "../../render-utils"; -import { bindRunFacade, resolvePredicateTimeout, type WaitPredicateOptions, waitForRun } from "../../run-scope"; +import { + bindRunFacade, + isBrowserRunRejection, + markBrowserRunRejection, + observeBrowserRunPromise, + resolvePredicateTimeout, + type WaitPredicateOptions, + waitForRun, +} from "../../run-scope"; import { ToolAbortError, ToolError, throwIfAborted } from "../../tool-errors"; import { type AriaSnapshotOptions, assertSelectorString, buildAriaSnapshotScript } from "../aria/aria-snapshot"; import { DEFAULT_VIEWPORT } from "../launch"; import { extractReadableFromHtml, type ReadableFormat } from "../readable"; + import { cloneSafe, RunOutput } from "../run-output"; import type { Observation, ReadyInfo, RunResultOk, ScreenshotResult, SessionSnapshot } from "../tab-protocol"; import { @@ -1382,6 +1391,26 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi // handler to this promise; keep its armed rejection from surfacing as an // unhandled rejection — the postmortem-fatal path this run guards against. cancelRejection.catch(() => {}); + const rejectionOwner = {}; + const { promise: floatingFailure, reject: rejectFloatingFailure } = Promise.withResolvers(); + floatingFailure.catch(() => {}); + let runActive = true; + let hasFloatingFailure = false; + const recordFloatingFailure = (reason: unknown): void => { + if (!runActive || hasFloatingFailure || postmortem.isExpectedCleanupError(reason)) return; + hasFloatingFailure = true; + const message = reason instanceof Error ? reason.message : String(reason); + const error = new Error(`Unhandled rejection (missing await?): ${message}`, { cause: reason }); + if (reason instanceof Error) error.name = reason.name; + rejectFloatingFailure(error); + }; + const uninstallRejectionInterceptor = postmortem.interceptUnhandledRejections(reason => { + if (isBrowserRunRejection(reason, rejectionOwner)) return true; + const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; + if (stack?.includes(`cmux-run-${runId}.js`) !== true) return false; + recordFloatingFailure(reason); + return true; + }); const onAbort = (): void => { if (timeoutSignal.aborted) { reject(new ToolError(`Browser code execution timed out after ${opts.timeoutMs}ms`)); @@ -1402,24 +1431,30 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi // Keep both inside try so a concurrent in-process eval/browser run surfaces as // a rejected promise the supervisor can report, never an unhandled rejection. runtime.setCwd(opts.snapshot.cwd); - const runTab = bindRunFacade(tab, signal); + const runTab = bindRunFacade(tab, signal, rejectionOwner, recordFloatingFailure); runtime.setRunScope({ - page: bindRunFacade(tab.page, signal), - browser: bindRunFacade(tab.browser, signal), + page: bindRunFacade(tab.page, signal, rejectionOwner, recordFloatingFailure), + browser: bindRunFacade(tab.browser, signal, rejectionOwner, recordFloatingFailure), tab: runTab, assert: (cond: unknown, text?: string): void => { if (!cond) throw new ToolError(text ?? "Assertion failed"); }, wait: (msOrPredicate: number | (() => unknown), waitOpts?: WaitPredicateOptions): Promise => - waitForRun( - msOrPredicate, - signal, - typeof msOrPredicate === "number" - ? waitOpts - : { - timeout: resolvePredicateTimeout(opts.timeoutMs, waitOpts?.timeout), - interval: waitOpts?.interval, - }, + observeBrowserRunPromise( + waitForRun( + msOrPredicate, + signal, + typeof msOrPredicate === "number" + ? waitOpts + : { + timeout: resolvePredicateTimeout(opts.timeoutMs, waitOpts?.timeout), + interval: waitOpts?.interval, + }, + ).catch(error => { + throw markBrowserRunRejection(error, rejectionOwner); + }), + rejectionOwner, + recordFloatingFailure, ), }); @@ -1447,6 +1482,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi returnValue = await Promise.race([ runtime.run(opts.code, filename, hooks, { runId, cwd: opts.snapshot.cwd }), cancelRejection, + floatingFailure, ]); } catch (error) { runFailed = true; @@ -1454,6 +1490,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi } // Let rejection callbacks run while this run can still own guest-created promises. await Bun.sleep(0); + if (hasFloatingFailure && !runFailed) await floatingFailure; if (runFailed) { for (const reason of activeRun.floatingRejections) { logger.warn("Unhandled rejection accompanied a failed cmux browser run", { filename, error: reason }); @@ -1470,6 +1507,8 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi } return { displays: output.finish(), returnValue: cloneSafe(returnValue), screenshots }; } finally { + runActive = false; + uninstallRejectionInterceptor(); signal.removeEventListener("abort", onAbort); runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); activeCmuxRuns.delete(filename); diff --git a/packages/coding-agent/src/tools/browser/tab-supervisor.ts b/packages/coding-agent/src/tools/browser/tab-supervisor.ts index 6deea3931..6da6b7be0 100644 --- a/packages/coding-agent/src/tools/browser/tab-supervisor.ts +++ b/packages/coding-agent/src/tools/browser/tab-supervisor.ts @@ -1014,7 +1014,7 @@ async function spawnInlineWorker(): Promise { close: () => {}, }; const { WorkerCore } = await import("./tab-worker"); - new WorkerCore(workerTransport); + new WorkerCore(workerTransport, false); return { mode: "inline", send: msg => diff --git a/packages/coding-agent/src/tools/browser/tab-worker-entry.ts b/packages/coding-agent/src/tools/browser/tab-worker-entry.ts index 38c0d1c7d..9c572c874 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker-entry.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker-entry.ts @@ -26,4 +26,4 @@ const transport: Transport = { }, }; -new WorkerCore(transport); +new WorkerCore(transport, true); diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index 7eed8a823..db02740dd 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -23,8 +23,11 @@ import { formatScreenshot } from "../render-utils"; import { bindRunFacade, CELL_BUDGET_SLACK_MS, - markHandled, + installBrowserWorkerRejectionGuard, + isBrowserRunRejection, markBrowserRunRejection, + markHandled, + observeBrowserRunPromise, resolvePredicateTimeout, type WaitPredicateOptions, waitForRun, @@ -703,6 +706,9 @@ interface ActiveRun { output: RunOutput; screenshots: ScreenshotResult[]; pendingTools: Map; + rejectionOwner: object; + floatingRejections: unknown[]; + floatingFailure: { promise: Promise; reject(reason?: unknown): void }; /** Helper invocations currently awaiting the page/network, keyed by op id. */ inflight: Map; opCounter: number; @@ -750,17 +756,71 @@ export class WorkerCore { #active: ActiveRun | null = null; #runtime: JsRuntime | null = null; #unsub: () => void; + #isolated: boolean; + #uninstallRejectionGuard: () => void; #mode?: WorkerInitPayload["mode"]; #activateForScreenshot = true; #dialogPolicy?: DialogPolicy; #dialogHandler?: (dialog: Dialog) => void; #openDialog?: OpenDialogInfo; - constructor(transport: Transport) { + constructor(transport: Transport, isolated: boolean) { this.#transport = transport; + this.#isolated = isolated; this.#unsub = this.#transport.onMessage(msg => { void this.#handleMessage(msg as WorkerInbound); }); + this.#uninstallRejectionGuard = this.#installRejectionGuard(); + } + + #installRejectionGuard(): () => void { + if (!this.#isolated) { + return postmortem.interceptUnhandledRejections(reason => this.#consumeUnhandledRejection(reason)); + } + return installBrowserWorkerRejectionGuard(reason => this.#consumeUnhandledRejection(reason)); + } + + #consumeUnhandledRejection(reason: unknown): boolean { + const active = this.#active; + if (!active) return false; + if (isBrowserRunRejection(reason, active.rejectionOwner)) return true; + const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; + const fromRun = stack?.includes(`browser-run-${active.id}.js`) === true; + if (!this.#isolated && !fromRun) return false; + this.#recordFloatingRejection(active, reason); + return true; + } + + #recordFloatingRejection(active: ActiveRun, reason: unknown): void { + if (this.#active !== active || postmortem.isExpectedCleanupError(reason)) return; + const isFirst = active.floatingRejections.length === 0; + active.floatingRejections.push(reason); + if (isFirst) active.floatingFailure.reject(this.#floatingRejectionError(reason)); + } + + #floatingRejectionError(reason: unknown): Error { + const message = reason instanceof Error ? reason.message : String(reason); + const error = new Error(`Unhandled rejection (missing await?): ${message}`, { cause: reason }); + if (reason instanceof Error) error.name = reason.name; + return error; + } + + #foldFloatingRejections(active: ActiveRun, failure: { error: unknown } | undefined): { error: unknown } | undefined { + const rejections = active.floatingRejections; + if (rejections.length === 0) return failure; + let reported = rejections; + if (!failure) { + failure = { error: this.#floatingRejectionError(rejections[0]) }; + reported = rejections.slice(1); + } else if (failure.error instanceof Error && failure.error.cause === rejections[0]) { + reported = rejections.slice(1); + } + for (const reason of reported) { + this.#log("warn", "Additional unhandled browser-run rejection", { + error: reason instanceof Error ? reason.message : String(reason), + }); + } + return failure; } nextElementId(): number { @@ -975,6 +1035,7 @@ export class WorkerCore { const signal = AbortSignal.any([timeoutSignal, ac.signal, runAc.signal]); const output = new RunOutput(); const screenshots: ScreenshotResult[] = []; + const floatingFailure = Promise.withResolvers(); const active: ActiveRun = { id: msg.id, ac, @@ -982,6 +1043,9 @@ export class WorkerCore { output, screenshots, pendingTools: new Map(), + rejectionOwner: {}, + floatingRejections: [], + floatingFailure, inflight: new Map(), opCounter: 0, }; @@ -997,10 +1061,11 @@ export class WorkerCore { const tabApi = this.#createTabApi(msg.name, msg.timeoutMs, signal, msg.session, output, screenshots, active); const runtime = this.#ensureRuntime(msg.session); runtime.setCwd(msg.session.cwd); + const onFloatingRejection = (reason: unknown): void => this.#recordFloatingRejection(active, reason); runtime.setRunScope({ - page: bindRunFacade(runPage.page, signal), - browser: bindRunFacade(browser, signal), - tab: bindRunFacade(tabApi, signal), + page: bindRunFacade(runPage.page, signal, active.rejectionOwner, onFloatingRejection), + browser: bindRunFacade(browser, signal, active.rejectionOwner, onFloatingRejection), + tab: bindRunFacade(tabApi, signal, active.rejectionOwner, onFloatingRejection), assert: (cond: unknown, text?: string): void => { if (!cond) throw new ToolError(text ?? "Assertion failed"); }, @@ -1012,10 +1077,12 @@ export class WorkerCore { typeof msOrPredicate === "number" ? undefined : { timeout: resolvePredicateTimeout(msg.timeoutMs, opts?.timeout), interval: opts?.interval }; - return markHandled( + return observeBrowserRunPromise( this.#runOp(active, label, signal, Number.POSITIVE_INFINITY, sig => waitForRun(msOrPredicate, sig, resolved), ), + active.rejectionOwner, + onFloatingRejection, ); }, }); @@ -1056,6 +1123,7 @@ export class WorkerCore { returnValue = await Promise.race([ runtime.run(msg.code, `browser-run-${msg.id}.js`, hooks, { runId: msg.id, cwd: msg.session.cwd }), cancelRejection, + floatingFailure.promise, ]); completed = true; } finally { @@ -1064,6 +1132,8 @@ export class WorkerCore { } catch (error) { failure = { error }; } finally { + await Bun.sleep(0); + failure = this.#foldFloatingRejections(active, failure); runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); try { await runPage?.cleanup(); @@ -1184,9 +1254,12 @@ export class WorkerCore { (opTimeout?.aborted || (err instanceof Error && err.name === "TimeoutError")) ) { const hint = selector ? await this.#selectorTimeoutHint(selector) : ""; - throw markBrowserRunRejection(new ToolError(`${label} timed out after ${perOpTimeoutMs}ms${hint}`)); + throw markBrowserRunRejection( + new ToolError(`${label} timed out after ${perOpTimeoutMs}ms${hint}`), + active.rejectionOwner, + ); } - throw markBrowserRunRejection(err); + throw markBrowserRunRejection(err, active.rejectionOwner); } finally { earlyAc.abort(); active.inflight.delete(opId); @@ -1870,6 +1943,7 @@ export class WorkerCore { async #close(): Promise { this.#unsub(); + this.#uninstallRejectionGuard(); this.#clearElementCache(); const page = this.#page; if (this.#dialogHandler && page && !page.isClosed()) page.off("dialog", this.#dialogHandler); diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index 5d44bfc06..2568c7826 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -1,77 +1,114 @@ -import { logger, postmortem } from "@oh-my-pi/pi-utils"; +import { 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"); +const browserRunRejections = new WeakMap(); -export function markBrowserRunRejection(reason: T): T { +/** Associates a browser operation failure with its owning evaluated run. */ +export function markBrowserRunRejection(reason: T, owner: object): T { if (reason !== null && (typeof reason === "object" || typeof reason === "function")) { - Reflect.set(reason, BROWSER_RUN_REJECTION, true); + browserRunRejections.set(reason, owner); } 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; +/** Returns whether a rejection was marked for the specified evaluated run. */ +export function isBrowserRunRejection(reason: unknown, owner: object): boolean { + return ( + reason !== null && + (typeof reason === "object" || typeof reason === "function") && + browserRunRejections.get(reason) === owner + ); } -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; +type FloatingRejectionHandler = (reason: unknown) => void; + +interface ObservedPromiseState { + handled: boolean; } -postmortem.interceptUnhandledRejections(consumeBrowserRunRejection); +const observedBrowserPromises = new WeakMap, ObservedPromiseState>(); +const observedPromiseConstructor = { [Symbol.species]: Promise }; /** - * 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. + * Observes every explicit continuation of a browser promise without replacing + * the native promise. Browser failures remain contained; an unhandled error + * created by user continuation code is reported to the owning run. */ -const observedPromiseTrees = new WeakSet>(); - -function observePromiseTree(promise: Promise): Promise { - if (observedPromiseTrees.has(promise)) return promise; - observedPromiseTrees.add(promise); - markHandled(promise); +export function observeBrowserRunPromise( + promise: Promise, + owner: object, + onFloatingRejection: FloatingRejectionHandler, +): Promise { + if (observedBrowserPromises.has(promise)) return promise; + const state: ObservedPromiseState = { handled: false }; + observedBrowserPromises.set(promise, state); const originalThen = promise.then.bind(promise); - const originalCatch = promise.catch.bind(promise); const originalFinally = promise.finally.bind(promise); + void originalThen(undefined, reason => { + if (isBrowserRunRejection(reason, owner)) return; + setTimeout(() => { + if (!state.handled) onFloatingRejection(reason); + }, 0); + }); Object.defineProperties(promise, { + constructor: { configurable: true, value: observedPromiseConstructor }, // 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)), + ): Promise => { + state.handled = true; + return observeBrowserRunPromise(originalThen(onFulfilled, onRejected), owner, onFloatingRejection); + }, }, catch: { configurable: true, value: ( onRejected?: ((reason: unknown) => TResult | PromiseLike) | null, - ): Promise => observePromiseTree(originalCatch(onRejected)), + ): Promise => { + state.handled = true; + return observeBrowserRunPromise(originalThen(undefined, onRejected), owner, onFloatingRejection); + }, }, finally: { configurable: true, - value: (onFinally?: (() => void) | null): Promise => observePromiseTree(originalFinally(onFinally)), + value: (onFinally?: (() => void) | null): Promise => { + state.handled = true; + return observeBrowserRunPromise(originalFinally(onFinally), owner, onFloatingRejection); + }, }, }); return promise; } -function trackBrowserRunPromise(promise: Promise): Promise { +function trackBrowserRunPromise( + promise: Promise, + owner?: object, + onFloatingRejection?: FloatingRejectionHandler, +): Promise { + if (!owner) return markHandled(promise); const tracked = promise.catch(error => { - throw markBrowserRunRejection(error); + throw markBrowserRunRejection(error, owner); }); - return observePromiseTree(tracked); + return onFloatingRejection ? observeBrowserRunPromise(tracked, owner, onFloatingRejection) : tracked; +} + +/** + * Installs worker-realm rejection routing. Consumed browser-run failures stay in + * the worker; unrelated failures retain the default fatal worker behavior. + */ +export function installBrowserWorkerRejectionGuard(consume: (reason: unknown) => boolean): () => void { + const onRejection = (reason: unknown): void => { + if (postmortem.isExpectedCleanupError(reason) || consume(reason)) return; + setTimeout(() => { + throw reason; + }, 0); + }; + process.on("unhandledRejection", onRejection); + return () => process.off("unhandledRejection", onRejection); } /** @@ -160,7 +197,12 @@ export function waitForRun( } /** Binds a long-lived scope facade (page/tab/desktop objects) to one evaluated run's abort signal. */ -export function bindRunFacade(target: T, signal: AbortSignal): T { +export function bindRunFacade( + target: T, + signal: AbortSignal, + rejectionOwner?: object, + onFloatingRejection?: FloatingRejectionHandler, +): T { const cache = new Map(); return new Proxy(target, { get(current, prop) { @@ -176,15 +218,12 @@ export function bindRunFacade(target: T, signal: AbortSignal): const then = Reflect.get(result, "then"); if (typeof then === "function") { return trackBrowserRunPromise( - Promise.resolve(result).then( - resolved => { - throwIfAborted(signal); - return resolved; - }, - error => { - throw markBrowserRunRejection(error); - }, - ), + Promise.resolve(result).then(resolved => { + throwIfAborted(signal); + return resolved; + }), + rejectionOwner, + onFloatingRejection, ); } } @@ -199,7 +238,7 @@ export function bindRunFacade(target: T, signal: AbortSignal): // brand-check internal slots that a Proxy cannot forward, and reading a // signal needs no abort gating anyway. if (value instanceof AbortSignal) return value; - const wrapped = bindRunFacade(value, signal); + const wrapped = bindRunFacade(value, signal, rejectionOwner, onFloatingRejection); cache.set(prop, wrapped); return wrapped; } 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 0fa306232..a3cec3b1e 100644 --- a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts +++ b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts @@ -1,11 +1,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; import { postmortem } from "@oh-my-pi/pi-utils"; import { JsRuntime, type RuntimeHooks } from "../../src/eval/js/shared/runtime"; -import { bindRunFacade, markHandled, waitForRun } from "../../src/tools/run-scope"; +import { + bindRunFacade, + isBrowserRunRejection, + markBrowserRunRejection, + 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; +const runScopeModuleUrl = new URL("../../src/tools/run-scope.ts", import.meta.url).href; async function collectUnhandledRejections(action: () => void | Promise): Promise { const reasons: unknown[] = []; @@ -130,38 +136,194 @@ 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)}; + it("scopes browser rejection markers to the owning run and direct reason", () => { + const owner = {}; + const browserFailure = new Error("browser failed"); + markBrowserRunRejection(browserFailure, owner); - const facade = bindBrowserRunFacade( - { - waitForResponse() { - return Promise.reject(new ToolError("tab.waitForResponse() timed out after 15ms")); - }, - }, - new AbortController().signal, - ); - void facade.waitForResponse().then(() => undefined); + expect(isBrowserRunRejection(browserFailure, owner)).toBe(true); + expect(isBrowserRunRejection(browserFailure, {})).toBe(false); + expect(isBrowserRunRejection(new Error("unrelated", { cause: browserFailure }), owner)).toBe(false); + }); + + it("keeps a later cause-wrapped rejection on the fatal path", async () => { + vi.useRealTimers(); + const script = ` + import { markBrowserRunRejection } from ${JSON.stringify(runScopeModuleUrl)}; + + const browserFailure = new Error("browser failure"); + markBrowserRunRejection(browserFailure, {}); + Promise.reject(new Error("unrelated fatal", { cause: browserFailure })); 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(), - ]); + const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]); - expect(exitCode, stderr).toBe(0); - expect(stdout).toContain("browser probe survived"); - expect(stderr).not.toContain("[Unhandled Rejection]"); + expect(exitCode).toBe(1); + expect(stderr).toContain("[Unhandled Rejection] Error: unrelated fatal"); + }); + + it("preserves a browser rejection marker through native await", async () => { + const owner = {}; + const browserFailure = new Error("browser failed"); + const facade = bindRunFacade( + { + fail(): Promise { + return Promise.reject(browserFailure); + }, + }, + new AbortController().signal, + owner, + ); + + let caught: unknown; + try { + await (async () => await facade.fail())(); + } catch (error) { + caught = error; + } + + expect(caught).toBe(browserFailure); + expect(isBrowserRunRejection(caught, owner)).toBe(true); + }); + + it("keeps a real worker alive after floating browser and continuation rejections", async () => { + vi.useRealTimers(); + const workerPath = `/tmp/omp-browser-rejections-${process.pid}.ts`; + await Bun.write( + workerPath, + ` + import { + bindRunFacade, + installBrowserWorkerRejectionGuard, + } from ${JSON.stringify(runScopeModuleUrl)}; + + const failures = []; + const uninstall = installBrowserWorkerRejectionGuard(reason => { + failures.push(reason instanceof Error ? reason.message : String(reason)); + return true; + }); + const facade = bindRunFacade( + { + waitForResponse() { + return Promise.reject(new Error("browser timeout")); + }, + title() { + return Promise.resolve("ready"); + }, + }, + new AbortController().signal, + {}, + ); + void (async () => { + await facade.waitForResponse(); + })(); + void facade.title().then(() => { + throw new Error("continuation failed"); + }); + setTimeout(() => { + uninstall(); + postMessage({ alive: true, failures }); + }, 50); + `, + ); + try { + const script = ` + const worker = new Worker(${JSON.stringify(workerPath)}, { type: "module" }); + const done = Promise.withResolvers(); + worker.onmessage = event => done.resolve(event.data); + worker.onerror = event => done.reject(new Error(event.message)); + try { + const result = await Promise.race([ + done.promise, + Bun.sleep(1000).then(() => { + throw new Error("worker timed out"); + }), + ]); + console.log(JSON.stringify(result)); + } finally { + await worker.terminate(); + } + `; + 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 timeout"); + expect(stdout).toContain("continuation failed"); + } finally { + await fs.rm(workerPath, { force: true }); + } + }); + + it("does not mark errors thrown by user continuations", async () => { + const owner = {}; + const continuationFailure = new Error("continuation failed"); + const floatingRejections: unknown[] = []; + const facade = bindRunFacade( + { + ok: async (): Promise => "ok", + }, + new AbortController().signal, + owner, + reason => floatingRejections.push(reason), + ); + + const root = facade.ok(); + expect(root).toBeInstanceOf(Promise); + expect(Object.getPrototypeOf(root)).toBe(Promise.prototype); + const continuation = root.then(() => { + throw continuationFailure; + }); + + await expect(continuation).rejects.toBe(continuationFailure); + expect(isBrowserRunRejection(continuationFailure, owner)).toBe(false); + expect(floatingRejections).toEqual([]); + }); + + it("reports unhandled errors from then, catch, and finally continuations", async () => { + vi.useRealTimers(); + const owner = {}; + const floatingRejections: unknown[] = []; + const facade = bindRunFacade( + { + fail: async (): Promise => { + throw new Error("browser failure"); + }, + ok: async (): Promise => "ok", + }, + new AbortController().signal, + owner, + reason => floatingRejections.push(reason), + ); + + void facade.ok().then(() => { + throw new Error("then failed"); + }); + void facade.fail().catch(() => { + throw new Error("catch failed"); + }); + void facade.ok().finally(() => { + throw new Error("finally failed"); + }); + await Bun.sleep(20); + + const messages = floatingRejections + .map(reason => (reason instanceof Error ? reason.message : String(reason))) + .sort(); + expect(messages).toEqual(["catch failed", "finally failed", "then failed"]); }); it("rejects awaited facade method calls that settle after abort", async () => { 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 fea1670fc..c16118a1c 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -246,6 +246,46 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("fails a floated user continuation without killing the tab worker", async () => { + const tool = new BrowserTool(makeSession()); + const name = `continuation-rejection-${process.pid}`; + + try { + 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(50); + return "incorrect success"; + `, + }); + } catch (error) { + failure = error instanceof Error ? error.message : String(error); + } + expect(failure).toContain("Unhandled rejection (missing await?): continuation failed"); + + 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}`; From 71c26e5958452b54a1f0540608703848dd51e9c6 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:07:37 +0800 Subject: [PATCH 05/14] fix(coding-agent): reported rethrown browser continuations Tracked whether user continuation callbacks create each descendant rejection. Browser errors that user code rethrows now fail the owning run instead of being contained as propagated helper failures. --- packages/coding-agent/src/tools/run-scope.ts | 75 +++++++++++++++++-- .../tools/browser-run-cancellation.test.ts | 23 ++++++ .../test/tools/browser-tab-evaluate.test.ts | 23 +++++- 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index 2568c7826..19b1123aa 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -25,6 +25,7 @@ type FloatingRejectionHandler = (reason: unknown) => void; interface ObservedPromiseState { handled: boolean; + userContinuationFailed: boolean; } const observedBrowserPromises = new WeakMap, ObservedPromiseState>(); @@ -39,16 +40,28 @@ export function observeBrowserRunPromise( promise: Promise, owner: object, onFloatingRejection: FloatingRejectionHandler, +): Promise { + return observeBrowserRunPromiseWithState(promise, owner, onFloatingRejection, { + handled: false, + userContinuationFailed: false, + }); +} + +function observeBrowserRunPromiseWithState( + promise: Promise, + owner: object, + onFloatingRejection: FloatingRejectionHandler, + state: ObservedPromiseState, ): Promise { if (observedBrowserPromises.has(promise)) return promise; - const state: ObservedPromiseState = { handled: false }; observedBrowserPromises.set(promise, state); const originalThen = promise.then.bind(promise); const originalFinally = promise.finally.bind(promise); void originalThen(undefined, reason => { - if (isBrowserRunRejection(reason, owner)) return; setTimeout(() => { - if (!state.handled) onFloatingRejection(reason); + if (!state.handled && (state.userContinuationFailed || !isBrowserRunRejection(reason, owner))) { + onFloatingRejection(reason); + } }, 0); }); Object.defineProperties(promise, { @@ -61,7 +74,16 @@ export function observeBrowserRunPromise( onRejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, ): Promise => { state.handled = true; - return observeBrowserRunPromise(originalThen(onFulfilled, onRejected), owner, onFloatingRejection); + const childState = createContinuationState(); + return observeBrowserRunPromiseWithState( + originalThen( + recordContinuationFailure(onFulfilled, childState), + recordContinuationFailure(onRejected, childState), + ), + owner, + onFloatingRejection, + childState, + ); }, }, catch: { @@ -70,20 +92,61 @@ export function observeBrowserRunPromise( onRejected?: ((reason: unknown) => TResult | PromiseLike) | null, ): Promise => { state.handled = true; - return observeBrowserRunPromise(originalThen(undefined, onRejected), owner, onFloatingRejection); + const childState = createContinuationState(); + return observeBrowserRunPromiseWithState( + originalThen(undefined, recordContinuationFailure(onRejected, childState)), + owner, + onFloatingRejection, + childState, + ); }, }, finally: { configurable: true, value: (onFinally?: (() => void) | null): Promise => { state.handled = true; - return observeBrowserRunPromise(originalFinally(onFinally), owner, onFloatingRejection); + const childState = createContinuationState(); + return observeBrowserRunPromiseWithState( + originalFinally(recordContinuationFailure(onFinally, childState)), + owner, + onFloatingRejection, + childState, + ); }, }, }); return promise; } +function createContinuationState(): ObservedPromiseState { + return { handled: false, userContinuationFailed: false }; +} + +function recordContinuationFailure( + continuation: ((...args: TArgs) => TResult | PromiseLike) | null | undefined, + state: ObservedPromiseState, +): ((...args: TArgs) => TResult | PromiseLike) | null | undefined { + if (!continuation) return continuation; + return (...args) => { + try { + const result = continuation(...args); + if (!isThenable(result)) return result; + return Promise.resolve(result).catch(reason => { + state.userContinuationFailed = true; + throw reason; + }) as PromiseLike; + } catch (reason) { + state.userContinuationFailed = true; + throw reason; + } + }; +} + +function isThenable(value: unknown): value is PromiseLike { + if (value === null || (typeof value !== "object" && typeof value !== "function")) return false; + return typeof Reflect.get(value, "then") === "function"; +} + function trackBrowserRunPromise( promise: Promise, owner?: object, 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 a3cec3b1e..98d67847b 100644 --- a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts +++ b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts @@ -326,6 +326,29 @@ describe("browser run cancellation", () => { expect(messages).toEqual(["catch failed", "finally failed", "then failed"]); }); + it("reports a browser error rethrown by a user rejection continuation", async () => { + vi.useRealTimers(); + const owner = {}; + const browserFailure = new Error("browser failure"); + const floatingRejections: unknown[] = []; + const facade = bindBrowserRunFacade( + { + fail: (): Promise => Promise.reject(browserFailure), + }, + new AbortController().signal, + owner, + reason => floatingRejections.push(reason), + ); + + void facade.fail().catch(reason => { + throw reason; + }); + await Bun.sleep(20); + + expect(isBrowserRunRejection(browserFailure, owner)).toBe(true); + expect(floatingRejections).toEqual([browserFailure]); + }); + 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 c16118a1c..13eb30cad 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -246,7 +246,7 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); - it("fails a floated user continuation without killing the tab worker", async () => { + it("fails floated user continuations without killing the tab worker", async () => { const tool = new BrowserTool(makeSession()); const name = `continuation-rejection-${process.pid}`; @@ -275,6 +275,27 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } 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(50); + 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, From dd0b7b3176a792c1d38fe4ad382164269d2d51d5 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:24:46 +0800 Subject: [PATCH 06/14] fix(coding-agent): reported late browser continuations Logged user continuation rejections that settle after their browser run has ended. This preserves the completed result while making missing awaits visible instead of silently dropping them. --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/tools/browser/tab-worker.ts | 9 +++- .../test/tools/browser-tab-evaluate.test.ts | 41 ++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bc59f7951..d043a1845 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -148,7 +148,7 @@ ### 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. +- 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, and late user continuation errors are logged instead of dropped after the run ends. ## [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 db02740dd..fc81deacc 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -792,7 +792,14 @@ export class WorkerCore { } #recordFloatingRejection(active: ActiveRun, reason: unknown): void { - if (this.#active !== active || postmortem.isExpectedCleanupError(reason)) return; + if (postmortem.isExpectedCleanupError(reason)) return; + if (this.#active !== active) { + this.#log("warn", "Unhandled rejection after browser run ended", { + runId: active.id, + error: reason instanceof Error ? reason.message : String(reason), + }); + return; + } const isFirst = active.floatingRejections.length === 0; active.floatingRejections.push(reason); if (isFirst) active.floatingFailure.reject(this.#floatingRejectionError(reason)); 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 13eb30cad..8b8a5e10d 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, it } from "bun:test"; +import { 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(); @@ -307,6 +308,44 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("logs a user continuation rejection after its browser run ends", async () => { + const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const tool = new BrowserTool(makeSession()); + const name = `late-continuation-rejection-${process.pid}`; + + 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 Bun.sleep(150); + expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { + runId: expect.any(String), + error: "late continuation failed", + }); + } finally { + warn.mockRestore(); + 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}`; From bb5d64557db3474ff4bb217cc47d17501edf54f7 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:54:32 +0800 Subject: [PATCH 07/14] fix(coding-agent): preserved late browser rejections Logged late user continuation failures in cmux runs and delayed worker rejection folding until request-interception cleanup completed. This closes both windows where missing awaits could be silently dropped. --- .../src/tools/browser/cmux/cmux-tab.ts | 8 ++- .../src/tools/browser/tab-worker.ts | 2 +- .../browser-cmux-release-mid-run.test.ts | 50 +++++++++++++++++++ .../test/tools/browser-tab-evaluate.test.ts | 39 +++++++++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index f13ecc13a..7e48bd140 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -1397,9 +1397,13 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi let runActive = true; let hasFloatingFailure = false; const recordFloatingFailure = (reason: unknown): void => { - if (!runActive || hasFloatingFailure || postmortem.isExpectedCleanupError(reason)) return; - hasFloatingFailure = true; + if (hasFloatingFailure || postmortem.isExpectedCleanupError(reason)) return; const message = reason instanceof Error ? reason.message : String(reason); + if (!runActive) { + logger.warn("Unhandled rejection after browser run ended", { runId, error: message }); + return; + } + hasFloatingFailure = true; const error = new Error(`Unhandled rejection (missing await?): ${message}`, { cause: reason }); if (reason instanceof Error) error.name = reason.name; rejectFloatingFailure(error); diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index fc81deacc..ebb1747fe 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -1140,13 +1140,13 @@ export class WorkerCore { failure = { error }; } finally { await Bun.sleep(0); - failure = this.#foldFloatingRejections(active, failure); runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); try { await runPage?.cleanup(); } catch (error) { failure = { error }; } + failure = this.#foldFloatingRejections(active, failure); if (this.#active?.id === msg.id) this.#active = null; } if (failure) { diff --git a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts index 51df6907e..94ec1e6af 100644 --- a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts +++ b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts @@ -42,6 +42,7 @@ import { runInTab, } from "@oh-my-pi/pi-coding-agent/tools/browser/tab-supervisor"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools/index"; +import * as logger from "@oh-my-pi/pi-utils/logger"; function makeKind(socketSuffix: string): CmuxKind { return { @@ -286,6 +287,55 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => { } }); + it("logs a user continuation rejection after its cmux run ends", async () => { + spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); + spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); + spyOn(CmuxSocketClient.prototype, "request").mockImplementation( + async (method: string): Promise> => { + switch (method) { + case "browser.open_split": + return { surface_id: "surface-late-rejection", url: "about:blank" }; + case "browser.url.get": + return { url: "about:blank" }; + case "browser.snapshot": + return { page: { html: "" } }; + case "browser.eval": + return { value: "" }; + default: + return {}; + } + }, + ); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + const browser = await acquireBrowser(makeKind("late-rejection"), { cwd: "/tmp" }); + await acquireTab("late-rejection", browser, { + timeoutMs: 5_000, + ownerSessionId: "session-late-rejection", + }); + + const result = await runInTab("late-rejection", { + code: ` + const continuationStarted = Promise.withResolvers(); + void tab.title().then(async () => { + continuationStarted.resolve(); + await Bun.sleep(50); + throw new Error("late cmux continuation failed"); + }); + await continuationStarted.promise; + return "completed"; + `, + timeoutMs: 5_000, + session: makeSession("/tmp"), + }); + expect(result.returnValue).toBe("completed"); + + await Bun.sleep(150); + expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { + runId: expect.any(String), + error: "late cmux continuation failed", + }); + }); + it("ignores the daemon screenshot path when no screenshot directory is configured", async () => { spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); 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 8b8a5e10d..d042c8f97 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -308,6 +308,45 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("folds a user continuation rejection that settles during cleanup", async () => { + const tool = new BrowserTool(makeSession()); + const name = `cleanup-continuation-rejection-${process.pid}`; + + try { + 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"); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 30_000); + it("logs a user continuation rejection after its browser run ends", async () => { const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); const tool = new BrowserTool(makeSession()); From af35be11050c5508d233893a35a3cff21269d681 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:27:04 +0800 Subject: [PATCH 08/14] fix(coding-agent): tracked browser promise combinators Observed Promise.all and Promise.race results derived from browser calls during each evaluated run. User catch continuations that rethrow browser failures now fail the owning run without changing native await behavior. --- .../src/tools/browser/cmux/cmux-tab.ts | 21 +++-- .../src/tools/browser/tab-worker.ts | 23 ++++-- packages/coding-agent/src/tools/run-scope.ts | 77 +++++++++++++++++++ .../browser-cmux-release-mid-run.test.ts | 44 +++++++++++ .../tools/browser-run-cancellation.test.ts | 72 ++++++++++++++++- .../test/tools/browser-tab-evaluate.test.ts | 42 ++++++++++ 6 files changed, 263 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index 7e48bd140..85e5e3619 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -16,12 +16,12 @@ import { resolvePredicateTimeout, type WaitPredicateOptions, waitForRun, + withBrowserPromiseCombinatorTracking, } from "../../run-scope"; import { ToolAbortError, ToolError, throwIfAborted } from "../../tool-errors"; import { type AriaSnapshotOptions, assertSelectorString, buildAriaSnapshotScript } from "../aria/aria-snapshot"; import { DEFAULT_VIEWPORT } from "../launch"; import { extractReadableFromHtml, type ReadableFormat } from "../readable"; - import { cloneSafe, RunOutput } from "../run-output"; import type { Observation, ReadyInfo, RunResultOk, ScreenshotResult, SessionSnapshot } from "../tab-protocol"; import { @@ -1409,9 +1409,9 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi rejectFloatingFailure(error); }; const uninstallRejectionInterceptor = postmortem.interceptUnhandledRejections(reason => { - if (isBrowserRunRejection(reason, rejectionOwner)) return true; + const browserRunRejection = isBrowserRunRejection(reason, rejectionOwner); const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; - if (stack?.includes(`cmux-run-${runId}.js`) !== true) return false; + if (!browserRunRejection && stack?.includes(`cmux-run-${runId}.js`) !== true) return false; recordFloatingFailure(reason); return true; }); @@ -1483,11 +1483,16 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi let runError: unknown; let runFailed = false; try { - returnValue = await Promise.race([ - runtime.run(opts.code, filename, hooks, { runId, cwd: opts.snapshot.cwd }), - cancelRejection, - floatingFailure, - ]); + returnValue = await withBrowserPromiseCombinatorTracking( + rejectionOwner, + recordFloatingFailure, + async () => + await Promise.race([ + runtime.run(opts.code, filename, hooks, { runId, cwd: opts.snapshot.cwd }), + cancelRejection, + floatingFailure, + ]), + ); } catch (error) { runFailed = true; runError = error; diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index ebb1747fe..cd50e8795 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -31,6 +31,7 @@ import { resolvePredicateTimeout, type WaitPredicateOptions, waitForRun, + withBrowserPromiseCombinatorTracking, } from "../run-scope"; import { ToolAbortError, ToolError, throwIfAborted } from "../tool-errors"; import { @@ -783,10 +784,10 @@ export class WorkerCore { #consumeUnhandledRejection(reason: unknown): boolean { const active = this.#active; if (!active) return false; - if (isBrowserRunRejection(reason, active.rejectionOwner)) return true; + const browserRunRejection = isBrowserRunRejection(reason, active.rejectionOwner); const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; const fromRun = stack?.includes(`browser-run-${active.id}.js`) === true; - if (!this.#isolated && !fromRun) return false; + if (!browserRunRejection && !this.#isolated && !fromRun) return false; this.#recordFloatingRejection(active, reason); return true; } @@ -1127,11 +1128,19 @@ export class WorkerCore { try { const hooks = this.#hooksForActiveRun(); if (!hooks) throw new ToolError("Browser runtime started without an active run"); - returnValue = await Promise.race([ - runtime.run(msg.code, `browser-run-${msg.id}.js`, hooks, { runId: msg.id, cwd: msg.session.cwd }), - cancelRejection, - floatingFailure.promise, - ]); + returnValue = await withBrowserPromiseCombinatorTracking( + active.rejectionOwner, + onFloatingRejection, + async () => + await Promise.race([ + runtime.run(msg.code, `browser-run-${msg.id}.js`, hooks, { + runId: msg.id, + cwd: msg.session.cwd, + }), + cancelRejection, + floatingFailure.promise, + ]), + ); completed = true; } finally { signal.removeEventListener("abort", onCancel); diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index 19b1123aa..07ac2a697 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -1,3 +1,4 @@ +import { AsyncLocalStorage } from "node:async_hooks"; import { postmortem } from "@oh-my-pi/pi-utils"; import { untilAborted } from "@oh-my-pi/pi-utils/abortable"; import { ToolError, throwIfAborted } from "./tool-errors"; @@ -31,6 +32,82 @@ interface ObservedPromiseState { const observedBrowserPromises = new WeakMap, ObservedPromiseState>(); const observedPromiseConstructor = { [Symbol.species]: Promise }; +type PromiseCombinatorName = "all" | "race"; + +interface PromiseCombinatorTrackingContext { + owner: object; + onFloatingRejection: FloatingRejectionHandler; +} + +const PROMISE_COMBINATORS: readonly PromiseCombinatorName[] = ["all", "race"]; +const promiseCombinatorTracking = new AsyncLocalStorage(); +const originalPromiseCombinatorDescriptors = new Map(); +let promiseCombinatorTrackingScopes = 0; + +/** + * Observes native promise-combinator results derived from browser promises for + * the duration of one evaluated run. Native `await` remains unchanged; dropped + * user continuations from `Promise.all` and `Promise.race` are routed to the + * owning run. + */ +export async function withBrowserPromiseCombinatorTracking( + owner: object, + onFloatingRejection: FloatingRejectionHandler, + run: () => Promise, +): Promise { + installPromiseCombinatorTracking(); + try { + return await promiseCombinatorTracking.run({ owner, onFloatingRejection }, run); + } finally { + restorePromiseCombinatorTracking(); + } +} + +function installPromiseCombinatorTracking(): void { + promiseCombinatorTrackingScopes++; + if (promiseCombinatorTrackingScopes !== 1) return; + for (const name of PROMISE_COMBINATORS) { + const descriptor = Object.getOwnPropertyDescriptor(Promise, name); + if (!descriptor || typeof descriptor.value !== "function") continue; + originalPromiseCombinatorDescriptors.set(name, descriptor); + const original = descriptor.value as (this: PromiseConstructor, values: Iterable) => Promise; + Object.defineProperty(Promise, name, { + ...descriptor, + value(this: PromiseConstructor, values: Iterable): Promise { + let hasObservedInput = false; + const result = Reflect.apply(original, this, [ + tapObservedBrowserPromises(values, () => { + hasObservedInput = true; + }), + ]) as Promise; + const context = promiseCombinatorTracking.getStore(); + return hasObservedInput && context + ? observeBrowserRunPromise(result, context.owner, context.onFloatingRejection) + : result; + }, + }); + } +} + +function restorePromiseCombinatorTracking(): void { + promiseCombinatorTrackingScopes--; + if (promiseCombinatorTrackingScopes !== 0) return; + for (const [name, descriptor] of originalPromiseCombinatorDescriptors) { + Object.defineProperty(Promise, name, descriptor); + } + originalPromiseCombinatorDescriptors.clear(); +} + +function* tapObservedBrowserPromises( + values: Iterable, + onObserved: () => void, +): Generator { + for (const value of values) { + if (observedBrowserPromises.has(value as Promise)) onObserved(); + yield value; + } +} + /** * Observes every explicit continuation of a browser promise without replacing * the native promise. Browser failures remain contained; an unhandled error diff --git a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts index 94ec1e6af..e41cc9103 100644 --- a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts +++ b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts @@ -336,6 +336,50 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => { }); }); + it("fails a browser error rethrown through a native promise combinator", async () => { + spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); + spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); + spyOn(CmuxSocketClient.prototype, "request").mockImplementation( + async (method: string): Promise> => { + switch (method) { + case "browser.open_split": + return { surface_id: "surface-combinator-rejection", url: "about:blank" }; + case "browser.url.get": + return { url: "about:blank" }; + case "browser.snapshot": + return { page: { html: "" } }; + case "browser.eval": + return { value: "" }; + case "browser.navigate": + throw new Error("navigation failed"); + default: + return {}; + } + }, + ); + const browser = await acquireBrowser(makeKind("combinator-rejection"), { cwd: "/tmp" }); + await acquireTab("combinator-rejection", browser, { + timeoutMs: 5_000, + ownerSessionId: "session-combinator-rejection", + }); + + const run = runInTab("combinator-rejection", { + code: ` + void Promise.all([ + tab.goto("https://example.test"), + ]).catch(reason => { + throw reason; + }); + await wait(50); + return "incorrect success"; + `, + timeoutMs: 5_000, + session: makeSession("/tmp"), + }); + + await expect(run).rejects.toThrow("Unhandled rejection (missing await?): navigation failed"); + }); + it("ignores the daemon screenshot path when no screenshot directory is configured", async () => { spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); 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 98d67847b..5c85bf566 100644 --- a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts +++ b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts @@ -8,6 +8,7 @@ import { markBrowserRunRejection, markHandled, waitForRun, + withBrowserPromiseCombinatorTracking, } from "../../src/tools/run-scope"; import { ToolAbortError } from "../../src/tools/tool-errors"; @@ -191,6 +192,75 @@ describe("browser run cancellation", () => { expect(isBrowserRunRejection(caught, owner)).toBe(true); }); + it("reports user rethrows from native browser-promise combinators", async () => { + vi.useRealTimers(); + for (const name of ["all", "race"] as const) { + const owner = {}; + const browserFailure = new Error(`${name} browser failure`); + const floatingRejections: unknown[] = []; + const facade = bindRunFacade( + { + fail(): Promise { + return Promise.reject(browserFailure); + }, + }, + new AbortController().signal, + owner, + reason => floatingRejections.push(reason), + ); + const originalCombinator = Promise[name]; + + await withBrowserPromiseCombinatorTracking( + owner, + reason => floatingRejections.push(reason), + async () => { + const combined = name === "all" ? Promise.all([facade.fail()]) : Promise.race([facade.fail()]); + void combined.catch(reason => { + throw reason; + }); + await Bun.sleep(20); + }, + ); + + expect(floatingRejections).toEqual([browserFailure]); + expect(Promise[name]).toBe(originalCombinator); + } + }); + + it("preserves native await through a tracked browser-promise combinator", async () => { + vi.useRealTimers(); + const owner = {}; + const browserFailure = new Error("browser failed"); + const floatingRejections: unknown[] = []; + const facade = bindRunFacade( + { + fail(): Promise { + return Promise.reject(browserFailure); + }, + }, + new AbortController().signal, + owner, + reason => floatingRejections.push(reason), + ); + + let caught: unknown; + await withBrowserPromiseCombinatorTracking( + owner, + reason => floatingRejections.push(reason), + async () => { + try { + await Promise.all([facade.fail()]); + } catch (error) { + caught = error; + } + await Bun.sleep(10); + }, + ); + + expect(caught).toBe(browserFailure); + expect(floatingRejections).toEqual([]); + }); + it("keeps a real worker alive after floating browser and continuation rejections", async () => { vi.useRealTimers(); const workerPath = `/tmp/omp-browser-rejections-${process.pid}.ts`; @@ -331,7 +401,7 @@ describe("browser run cancellation", () => { const owner = {}; const browserFailure = new Error("browser failure"); const floatingRejections: unknown[] = []; - const facade = bindBrowserRunFacade( + const facade = bindRunFacade( { fail: (): Promise => Promise.reject(browserFailure), }, 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 d042c8f97..53a34f693 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -308,6 +308,48 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("fails a browser error rethrown through a native promise combinator", async () => { + const tool = new BrowserTool(makeSession()); + const name = `combinator-rejection-${process.pid}`; + + try { + 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(50); + 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" }]); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 30_000); + it("folds a user continuation rejection that settles during cleanup", async () => { const tool = new BrowserTool(makeSession()); const name = `cleanup-continuation-rejection-${process.pid}`; From 5c0187dec5e629dc862972b30cef1189ab4772de Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:39:58 +0800 Subject: [PATCH 09/14] fix(coding-agent): isolated promise combinator tracking Used a run-scoped Promise subclass instead of mutating native combinator methods. Evaluated code can now freeze its Promise constructor without breaking cleanup or later browser runs. --- packages/coding-agent/src/tools/run-scope.ts | 59 +++++++++++++------ .../test/tools/browser-tab-evaluate.test.ts | 31 ++++++++++ 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index 07ac2a697..f519565c8 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -33,6 +33,7 @@ const observedBrowserPromises = new WeakMap, ObservedPromiseSta const observedPromiseConstructor = { [Symbol.species]: Promise }; type PromiseCombinatorName = "all" | "race"; +type PromiseCombinator = (this: PromiseConstructor, values: Iterable) => Promise; interface PromiseCombinatorTrackingContext { owner: object; @@ -40,8 +41,13 @@ interface PromiseCombinatorTrackingContext { } const PROMISE_COMBINATORS: readonly PromiseCombinatorName[] = ["all", "race"]; +const NativePromise = Promise; +const nativePromiseCombinators: Record = { + all: Promise.all, + race: Promise.race, +}; const promiseCombinatorTracking = new AsyncLocalStorage(); -const originalPromiseCombinatorDescriptors = new Map(); +let previousPromiseDescriptor: PropertyDescriptor | undefined; let promiseCombinatorTrackingScopes = 0; /** @@ -64,15 +70,40 @@ export async function withBrowserPromiseCombinatorTracking( } function installPromiseCombinatorTracking(): void { - promiseCombinatorTrackingScopes++; - if (promiseCombinatorTrackingScopes !== 1) return; + if (promiseCombinatorTrackingScopes > 0) { + promiseCombinatorTrackingScopes++; + return; + } + const descriptor = Object.getOwnPropertyDescriptor(globalThis, "Promise"); + if (!descriptor) throw new Error("Global Promise descriptor is unavailable"); + const trackedPromise = createTrackedPromiseConstructor(); + Object.defineProperty(globalThis, "Promise", { ...descriptor, value: trackedPromise }); + previousPromiseDescriptor = descriptor; + promiseCombinatorTrackingScopes = 1; +} + +function restorePromiseCombinatorTracking(): void { + if (promiseCombinatorTrackingScopes > 1) { + promiseCombinatorTrackingScopes--; + return; + } + const descriptor = previousPromiseDescriptor; + try { + if (!descriptor) throw new Error("Global Promise tracking scope is not installed"); + Object.defineProperty(globalThis, "Promise", descriptor); + } finally { + previousPromiseDescriptor = undefined; + promiseCombinatorTrackingScopes = 0; + } +} + +function createTrackedPromiseConstructor(): PromiseConstructor { + class TrackedPromise extends NativePromise {} for (const name of PROMISE_COMBINATORS) { - const descriptor = Object.getOwnPropertyDescriptor(Promise, name); - if (!descriptor || typeof descriptor.value !== "function") continue; - originalPromiseCombinatorDescriptors.set(name, descriptor); - const original = descriptor.value as (this: PromiseConstructor, values: Iterable) => Promise; - Object.defineProperty(Promise, name, { - ...descriptor, + const original = nativePromiseCombinators[name]; + Object.defineProperty(TrackedPromise, name, { + configurable: true, + writable: true, value(this: PromiseConstructor, values: Iterable): Promise { let hasObservedInput = false; const result = Reflect.apply(original, this, [ @@ -87,15 +118,7 @@ function installPromiseCombinatorTracking(): void { }, }); } -} - -function restorePromiseCombinatorTracking(): void { - promiseCombinatorTrackingScopes--; - if (promiseCombinatorTrackingScopes !== 0) return; - for (const [name, descriptor] of originalPromiseCombinatorDescriptors) { - Object.defineProperty(Promise, name, descriptor); - } - originalPromiseCombinatorDescriptors.clear(); + return TrackedPromise; } function* tapObservedBrowserPromises( 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 53a34f693..08d0ed3b4 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -350,6 +350,37 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("restores promise tracking after evaluated code freezes Promise", async () => { + const tool = new BrowserTool(makeSession()); + const name = `frozen-promise-${process.pid}`; + + try { + 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" }]); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 30_000); + it("folds a user continuation rejection that settles during cleanup", async () => { const tool = new BrowserTool(makeSession()); const name = `cleanup-continuation-rejection-${process.pid}`; From a51b69ee85b7dd98fd7f3228e0daabea57d7e619 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:49:52 +0800 Subject: [PATCH 10/14] fix(coding-agent): kept unrelated worker failures fatal Classified only marked browser failures and evaluated-run stack frames as run-owned rejections. Unrelated tab-worker failures now remain on the worker guard's fatal path. --- .../src/tools/browser/cmux/cmux-tab.ts | 6 ++---- .../coding-agent/src/tools/browser/tab-worker.ts | 7 ++----- packages/coding-agent/src/tools/run-scope.ts | 6 ++++++ .../test/tools/browser-run-cancellation.test.ts | 15 +++++++++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index 85e5e3619..f307cae69 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -10,7 +10,7 @@ import { resolveToCwd } from "../../path-utils"; import { formatScreenshot } from "../../render-utils"; import { bindRunFacade, - isBrowserRunRejection, + isBrowserRunOwnedRejection, markBrowserRunRejection, observeBrowserRunPromise, resolvePredicateTimeout, @@ -1409,9 +1409,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi rejectFloatingFailure(error); }; const uninstallRejectionInterceptor = postmortem.interceptUnhandledRejections(reason => { - const browserRunRejection = isBrowserRunRejection(reason, rejectionOwner); - const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; - if (!browserRunRejection && stack?.includes(`cmux-run-${runId}.js`) !== true) return false; + if (!isBrowserRunOwnedRejection(reason, rejectionOwner, `cmux-run-${runId}.js`)) return false; recordFloatingFailure(reason); return true; }); diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index cd50e8795..f8d442afe 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -24,7 +24,7 @@ import { bindRunFacade, CELL_BUDGET_SLACK_MS, installBrowserWorkerRejectionGuard, - isBrowserRunRejection, + isBrowserRunOwnedRejection, markBrowserRunRejection, markHandled, observeBrowserRunPromise, @@ -784,10 +784,7 @@ export class WorkerCore { #consumeUnhandledRejection(reason: unknown): boolean { const active = this.#active; if (!active) return false; - const browserRunRejection = isBrowserRunRejection(reason, active.rejectionOwner); - const stack = reason instanceof Error && typeof reason.stack === "string" ? reason.stack : undefined; - const fromRun = stack?.includes(`browser-run-${active.id}.js`) === true; - if (!browserRunRejection && !this.#isolated && !fromRun) return false; + if (!isBrowserRunOwnedRejection(reason, active.rejectionOwner, `browser-run-${active.id}.js`)) return false; this.#recordFloatingRejection(active, reason); return true; } diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index f519565c8..db83e1bc7 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -22,6 +22,12 @@ export function isBrowserRunRejection(reason: unknown, owner: object): boolean { ); } +/** Returns whether a rejection belongs to the marked browser run or its evaluated source file. */ +export function isBrowserRunOwnedRejection(reason: unknown, owner: object, filename: string): boolean { + if (isBrowserRunRejection(reason, owner)) return true; + return reason instanceof Error && typeof reason.stack === "string" && reason.stack.includes(filename); +} + type FloatingRejectionHandler = (reason: unknown) => void; interface ObservedPromiseState { 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 5c85bf566..133a7bdd9 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,7 @@ import { postmortem } from "@oh-my-pi/pi-utils"; import { JsRuntime, type RuntimeHooks } from "../../src/eval/js/shared/runtime"; import { bindRunFacade, + isBrowserRunOwnedRejection, isBrowserRunRejection, markBrowserRunRejection, markHandled, @@ -147,6 +148,20 @@ describe("browser run cancellation", () => { expect(isBrowserRunRejection(new Error("unrelated", { cause: browserFailure }), owner)).toBe(false); }); + it("keeps unrelated worker rejections outside the active browser run", () => { + const owner = {}; + const workerFailure = new Error("transport failed"); + workerFailure.stack = "Error: transport failed\n at tab-worker.ts:1:1"; + const evaluatedFailure = new Error("evaluated failure"); + evaluatedFailure.stack = "Error: evaluated failure\n at browser-run-run-1.js:1:1"; + + expect(isBrowserRunOwnedRejection(workerFailure, owner, "browser-run-run-1.js")).toBe(false); + expect(isBrowserRunOwnedRejection(evaluatedFailure, owner, "browser-run-run-1.js")).toBe(true); + expect( + isBrowserRunOwnedRejection(markBrowserRunRejection(workerFailure, owner), owner, "browser-run-run-1.js"), + ).toBe(true); + }); + it("keeps a later cause-wrapped rejection on the fatal path", async () => { vi.useRealTimers(); const script = ` From fe086df7bfda2f3bed7c55ba70015aea28063ade Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:01:17 +0800 Subject: [PATCH 11/14] fix(coding-agent): aborted browser runs before draining Closed worker and cmux run signals before yielding for floating-rejection drainage. Stale promise continuations can no longer begin page navigation after evaluated code returns. --- .../src/tools/browser/cmux/cmux-tab.ts | 4 +- .../src/tools/browser/tab-worker.ts | 2 +- .../browser-cmux-release-mid-run.test.ts | 44 +++++++++++++++++++ .../test/tools/browser-tab-evaluate.test.ts | 37 ++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index f307cae69..e57bf97f1 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -1377,6 +1377,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi const signal = AbortSignal.any( opts.signal ? [timeoutSignal, opts.signal, runAc.signal] : [timeoutSignal, runAc.signal], ); + const runEndedError = postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended")); const output = new RunOutput(); const screenshots: ScreenshotResult[] = []; const runId = crypto.randomUUID(); @@ -1495,6 +1496,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi runFailed = true; runError = error; } + runAc.abort(runEndedError); // Let rejection callbacks run while this run can still own guest-created promises. await Bun.sleep(0); if (hasFloatingFailure && !runFailed) await floatingFailure; @@ -1517,7 +1519,7 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi runActive = false; uninstallRejectionInterceptor(); signal.removeEventListener("abort", onAbort); - runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); + runAc.abort(runEndedError); activeCmuxRuns.delete(filename); rememberCmuxRunFile(filename); tab.clearRunContext(); diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index f8d442afe..97eb8e497 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -1145,8 +1145,8 @@ export class WorkerCore { } catch (error) { failure = { error }; } finally { - await Bun.sleep(0); runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); + await Bun.sleep(0); try { await runPage?.cleanup(); } catch (error) { diff --git a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts index e41cc9103..8e130c14b 100644 --- a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts +++ b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts @@ -380,6 +380,50 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => { await expect(run).rejects.toThrow("Unhandled rejection (missing await?): navigation failed"); }); + it("aborts the cmux run facade before draining floated continuations", async () => { + spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); + spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); + const navigatedUrls: string[] = []; + spyOn(CmuxSocketClient.prototype, "request").mockImplementation( + async (method: string, params: Record): Promise> => { + switch (method) { + case "browser.open_split": + return { surface_id: "surface-drain-abort", url: "about:blank" }; + case "browser.url.get": + return { url: "about:blank" }; + case "browser.snapshot": + return { page: { html: "" } }; + case "browser.eval": + await Bun.sleep(0); + return { value: "ready" }; + case "browser.navigate": + navigatedUrls.push(String(params.url)); + return { url: params.url }; + default: + return {}; + } + }, + ); + const browser = await acquireBrowser(makeKind("drain-abort"), { cwd: "/tmp" }); + await acquireTab("drain-abort", browser, { + timeoutMs: 5_000, + ownerSessionId: "session-drain-abort", + }); + + const result = await runInTab("drain-abort", { + code: ` + void tab.title().then(() => tab.goto("https://late.example")); + return "completed"; + `, + timeoutMs: 5_000, + session: makeSession("/tmp"), + }); + expect(result.returnValue).toBe("completed"); + + await Bun.sleep(20); + expect(navigatedUrls).toEqual([]); + }); + it("ignores the daemon screenshot path when no screenshot directory is configured", async () => { spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined); spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined); 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 08d0ed3b4..57ad531e2 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -381,6 +381,43 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { } }, 30_000); + it("aborts the run facade before draining floated continuations", async () => { + const tool = new BrowserTool(makeSession()); + const name = `drain-abort-${process.pid}`; + const url = "data:text/html,original

ready

"; + + try { + 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 }]); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 30_000); + it("folds a user continuation rejection that settles during cleanup", async () => { const tool = new BrowserTool(makeSession()); const name = `cleanup-continuation-rejection-${process.pid}`; From adee5b9991339970d8f661abb2a4d6bff99a6292 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:18:41 +0800 Subject: [PATCH 12/14] fix(coding-agent): restored lazy CLI imports Imported postmortem through its direct subpath so browser run-scope loading no longer initializes the pi-utils barrel during CLI startup. --- packages/coding-agent/src/tools/run-scope.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coding-agent/src/tools/run-scope.ts b/packages/coding-agent/src/tools/run-scope.ts index db83e1bc7..184cd367a 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -1,6 +1,6 @@ import { AsyncLocalStorage } from "node:async_hooks"; -import { postmortem } from "@oh-my-pi/pi-utils"; import { untilAborted } from "@oh-my-pi/pi-utils/abortable"; +import * as postmortem from "@oh-my-pi/pi-utils/postmortem"; import { ToolError, throwIfAborted } from "./tool-errors"; const browserRunRejections = new WeakMap(); From 5e28f856271e3e2b5672d41a9a9657beff06118c Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:34:09 +0800 Subject: [PATCH 13/14] fix(coding-agent): stabilized browser worker startup Gave browser worker initialization an infrastructure timeout floor so short navigation budgets do not expire before a loaded runner starts the worker. --- .../src/tools/browser/tab-supervisor.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/tools/browser/tab-supervisor.ts b/packages/coding-agent/src/tools/browser/tab-supervisor.ts index 6da6b7be0..767b48c64 100644 --- a/packages/coding-agent/src/tools/browser/tab-supervisor.ts +++ b/packages/coding-agent/src/tools/browser/tab-supervisor.ts @@ -134,6 +134,7 @@ const tabs = new Map(); // awaits) cannot interleave and leak a worker + browser refCount. const acquireChains = new Map>(); const GRACE_MS = 750; +const WORKER_INIT_TIMEOUT_MS = 15_000; // Names of tabs the supervisor force-killed (timeout past grace, failed recycle), // mapped to the kill reason. Lets the next `run` on that name explain WHY the tab // vanished instead of a bare "not alive". Cleared when the name is opened again. @@ -272,7 +273,11 @@ async function acquireTabImpl( } let info: ReadyInfo; try { - info = await initializeTabWorker(worker, initPayload, opts.timeoutMs + GRACE_MS); + info = await initializeTabWorker( + worker, + initPayload, + Math.max(WORKER_INIT_TIMEOUT_MS, opts.timeoutMs + GRACE_MS), + ); } catch (error) { // `BuildMessage`-class failures arrive asynchronously via the worker's `error` event, // after `spawnTabWorker`'s synchronous try/catch has already returned. Fall back to @@ -287,7 +292,11 @@ async function acquireTabImpl( }); worker = await spawnInlineWorker(); try { - info = await initializeTabWorker(worker, initPayload, opts.timeoutMs + GRACE_MS); + info = await initializeTabWorker( + worker, + initPayload, + Math.max(WORKER_INIT_TIMEOUT_MS, opts.timeoutMs + GRACE_MS), + ); } catch (inlineError) { await worker.terminate().catch(() => undefined); if (tempHold || browser.refCount === 0) await releaseBrowser(browser, { kill: false }); From 271e621a02e3e8300a802127f839b0cb121a2770 Mon Sep 17 00:00:00 2001 From: brymko <47984664+brymko@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:41:40 +0800 Subject: [PATCH 14/14] test(coding-agent): synchronized browser rejection checks Waited for the observable warning event instead of assuming late continuations settle within a fixed wall-clock delay under CI load. --- .../test/tools/browser-cmux-release-mid-run.test.ts | 7 +++++-- .../coding-agent/test/tools/browser-tab-evaluate.test.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts index 8e130c14b..768021be3 100644 --- a/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts +++ b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts @@ -306,7 +306,10 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => { } }, ); - const warn = spyOn(logger, "warn").mockImplementation(() => {}); + const warningLogged = Promise.withResolvers(); + const warn = spyOn(logger, "warn").mockImplementation(message => { + if (message === "Unhandled rejection after browser run ended") warningLogged.resolve(); + }); const browser = await acquireBrowser(makeKind("late-rejection"), { cwd: "/tmp" }); await acquireTab("late-rejection", browser, { timeoutMs: 5_000, @@ -329,7 +332,7 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => { }); expect(result.returnValue).toBe("completed"); - await Bun.sleep(150); + await warningLogged.promise; expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { runId: expect.any(String), error: "late cmux continuation failed", 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 57ad531e2..30c299a20 100644 --- a/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-evaluate.test.ts @@ -458,7 +458,10 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { }, 30_000); it("logs a user continuation rejection after its browser run ends", async () => { - const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const warningLogged = Promise.withResolvers(); + const warn = vi.spyOn(logger, "warn").mockImplementation(message => { + if (message === "Unhandled rejection after browser run ended") warningLogged.resolve(); + }); const tool = new BrowserTool(makeSession()); const name = `late-continuation-rejection-${process.pid}`; @@ -484,7 +487,7 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => { }); expect(result.content).toEqual([{ type: "text", text: "completed" }]); - await Bun.sleep(150); + await warningLogged.promise; expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { runId: expect.any(String), error: "late continuation failed",