diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7ba445a2b..456e88609 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -145,6 +145,13 @@ - 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, 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/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/coding-agent/src/tools/browser/cmux/cmux-tab.ts b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts index 49e783d0a..e57bf97f1 100644 --- a/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts +++ b/packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts @@ -8,7 +8,16 @@ 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, + isBrowserRunOwnedRejection, + markBrowserRunRejection, + observeBrowserRunPromise, + 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"; @@ -1368,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(); @@ -1382,6 +1392,28 @@ 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 (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); + }; + const uninstallRejectionInterceptor = postmortem.interceptUnhandledRejections(reason => { + if (!isBrowserRunOwnedRejection(reason, rejectionOwner, `cmux-run-${runId}.js`)) 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 +1434,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, ), }); @@ -1444,16 +1482,24 @@ 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, - ]); + 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; } + 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; if (runFailed) { for (const reason of activeRun.floatingRejections) { logger.warn("Unhandled rejection accompanied a failed cmux browser run", { filename, error: reason }); @@ -1470,8 +1516,10 @@ 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"))); + runAc.abort(runEndedError); activeCmuxRuns.delete(filename); rememberCmuxRunFile(filename); tab.clearRunContext(); diff --git a/packages/coding-agent/src/tools/browser/tab-supervisor.ts b/packages/coding-agent/src/tools/browser/tab-supervisor.ts index 6deea3931..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 }); @@ -1014,7 +1023,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 8f257feac..97eb8e497 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -23,10 +23,15 @@ import { formatScreenshot } from "../render-utils"; import { bindRunFacade, CELL_BUDGET_SLACK_MS, + installBrowserWorkerRejectionGuard, + isBrowserRunOwnedRejection, + markBrowserRunRejection, markHandled, + observeBrowserRunPromise, resolvePredicateTimeout, type WaitPredicateOptions, waitForRun, + withBrowserPromiseCombinatorTracking, } from "../run-scope"; import { ToolAbortError, ToolError, throwIfAborted } from "../tool-errors"; import { @@ -44,6 +49,7 @@ import { loadPuppeteerInWorker, } from "./launch"; import { extractReadableFromHtml, type ReadableFormat } from "./readable"; + import { cloneSafe, RunOutput } from "./run-output"; import type { Observation, @@ -701,6 +707,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; @@ -748,17 +757,75 @@ 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 (!isBrowserRunOwnedRejection(reason, active.rejectionOwner, `browser-run-${active.id}.js`)) return false; + this.#recordFloatingRejection(active, reason); + return true; + } + + #recordFloatingRejection(active: ActiveRun, reason: unknown): void { + 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)); + } + + #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 { @@ -973,6 +1040,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, @@ -980,6 +1048,9 @@ export class WorkerCore { output, screenshots, pendingTools: new Map(), + rejectionOwner: {}, + floatingRejections: [], + floatingFailure, inflight: new Map(), opCounter: 0, }; @@ -995,10 +1066,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"); }, @@ -1010,10 +1082,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, ); }, }); @@ -1051,10 +1125,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, - ]); + 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); @@ -1063,11 +1146,13 @@ export class WorkerCore { failure = { error }; } finally { runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended"))); + await Bun.sleep(0); try { await runPage?.cleanup(); } catch (error) { failure = { error }; } + failure = this.#foldFloatingRejections(active, failure); if (this.#active?.id === msg.id) this.#active = null; } if (failure) { @@ -1182,9 +1267,12 @@ 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}`), + active.rejectionOwner, + ); } - throw err; + throw markBrowserRunRejection(err, active.rejectionOwner); } finally { earlyAc.abort(); active.inflight.delete(opId); @@ -1868,6 +1956,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 742c8daf8..184cd367a 100644 --- a/packages/coding-agent/src/tools/run-scope.ts +++ b/packages/coding-agent/src/tools/run-scope.ts @@ -1,6 +1,285 @@ +import { AsyncLocalStorage } from "node:async_hooks"; 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(); + +/** 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")) { + browserRunRejections.set(reason, owner); + } + return reason; +} + +/** 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 + ); +} + +/** 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 { + handled: boolean; + userContinuationFailed: boolean; +} + +const observedBrowserPromises = new WeakMap, ObservedPromiseState>(); +const observedPromiseConstructor = { [Symbol.species]: Promise }; + +type PromiseCombinatorName = "all" | "race"; +type PromiseCombinator = (this: PromiseConstructor, values: Iterable) => Promise; + +interface PromiseCombinatorTrackingContext { + owner: object; + onFloatingRejection: FloatingRejectionHandler; +} + +const PROMISE_COMBINATORS: readonly PromiseCombinatorName[] = ["all", "race"]; +const NativePromise = Promise; +const nativePromiseCombinators: Record = { + all: Promise.all, + race: Promise.race, +}; +const promiseCombinatorTracking = new AsyncLocalStorage(); +let previousPromiseDescriptor: PropertyDescriptor | undefined; +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 { + 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 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, [ + tapObservedBrowserPromises(values, () => { + hasObservedInput = true; + }), + ]) as Promise; + const context = promiseCombinatorTracking.getStore(); + return hasObservedInput && context + ? observeBrowserRunPromise(result, context.owner, context.onFloatingRejection) + : result; + }, + }); + } + return TrackedPromise; +} + +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 + * created by user continuation code is reported to the owning run. + */ +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; + observedBrowserPromises.set(promise, state); + const originalThen = promise.then.bind(promise); + const originalFinally = promise.finally.bind(promise); + void originalThen(undefined, reason => { + setTimeout(() => { + if (!state.handled && (state.userContinuationFailed || !isBrowserRunRejection(reason, owner))) { + 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 => { + state.handled = true; + const childState = createContinuationState(); + return observeBrowserRunPromiseWithState( + originalThen( + recordContinuationFailure(onFulfilled, childState), + recordContinuationFailure(onRejected, childState), + ), + owner, + onFloatingRejection, + childState, + ); + }, + }, + catch: { + configurable: true, + value: ( + onRejected?: ((reason: unknown) => TResult | PromiseLike) | null, + ): Promise => { + state.handled = true; + const childState = createContinuationState(); + return observeBrowserRunPromiseWithState( + originalThen(undefined, recordContinuationFailure(onRejected, childState)), + owner, + onFloatingRejection, + childState, + ); + }, + }, + finally: { + configurable: true, + value: (onFinally?: (() => void) | null): Promise => { + state.handled = true; + 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, + onFloatingRejection?: FloatingRejectionHandler, +): Promise { + if (!owner) return markHandled(promise); + const tracked = promise.catch(error => { + throw markBrowserRunRejection(error, owner); + }); + 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); +} + /** * Marks a run-scoped promise as observed without changing its behavior for awaited callers. * @@ -83,11 +362,16 @@ 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. */ -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) { @@ -102,11 +386,13 @@ export function bindRunFacade(target: T, signal: AbortSignal): if (result && typeof result === "object") { const then = Reflect.get(result, "then"); if (typeof then === "function") { - return markHandled( + return trackBrowserRunPromise( Promise.resolve(result).then(resolved => { throwIfAborted(signal); return resolved; }), + rejectionOwner, + onFloatingRejection, ); } } @@ -121,7 +407,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-cmux-release-mid-run.test.ts b/packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts index 51df6907e..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 @@ -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,146 @@ 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 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, + 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 warningLogged.promise; + expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { + runId: expect.any(String), + error: "late cmux continuation failed", + }); + }); + + 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("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-run-cancellation.test.ts b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts index 52f2c24ca..133a7bdd9 100644 --- a/packages/coding-agent/test/tools/browser-run-cancellation.test.ts +++ b/packages/coding-agent/test/tools/browser-run-cancellation.test.ts @@ -1,9 +1,20 @@ 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, + isBrowserRunOwnedRejection, + isBrowserRunRejection, + markBrowserRunRejection, + markHandled, + waitForRun, + withBrowserPromiseCombinatorTracking, +} from "../../src/tools/run-scope"; import { ToolAbortError } from "../../src/tools/tool-errors"; +const runScopeModuleUrl = new URL("../../src/tools/run-scope.ts", import.meta.url).href; + async function collectUnhandledRejections(action: () => void | Promise): Promise { const reasons: unknown[] = []; const onUnhandled = (reason: unknown) => reasons.push(reason); @@ -127,6 +138,302 @@ describe("browser run cancellation", () => { expect(reasons).toEqual([]); }); + it("scopes browser rejection markers to the owning run and direct reason", () => { + const owner = {}; + const browserFailure = new Error("browser failed"); + markBrowserRunRejection(browserFailure, owner); + + expect(isBrowserRunRejection(browserFailure, owner)).toBe(true); + expect(isBrowserRunRejection(browserFailure, {})).toBe(false); + 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 = ` + 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(); + `; + const proc = Bun.spawn([process.execPath, "-e", script], { + cwd: process.cwd(), + stdout: "pipe", + stderr: "pipe", + }); + const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]); + + 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("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`; + 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("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 = bindRunFacade( + { + 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 5d6234d30..30c299a20 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(); @@ -208,6 +209,295 @@ 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("fails floated user continuations 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"); + + 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, + code: "return 42;", + }); + expect(followup.content).toEqual([{ type: "text", text: "42" }]); + } finally { + await tool.execute("close", { action: "close", name, kill: true }); + } + }, 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("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("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}`; + + 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 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}`; + + try { + await tool.execute("open", { + action: "open", + name, + url: "data:text/html,

ready

", + }); + const result = await tool.execute("run", { + action: "run", + name, + code: ` + const continuationStarted = Promise.withResolvers(); + void tab.title().then(async () => { + continuationStarted.resolve(); + await Bun.sleep(50); + throw new Error("late continuation failed"); + }); + await continuationStarted.promise; + return "completed"; + `, + }); + expect(result.content).toEqual([{ type: "text", text: "completed" }]); + + await warningLogged.promise; + expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", { + runId: expect.any(String), + error: "late continuation failed", + }); + } finally { + warn.mockRestore(); + 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}`; 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..63fa30d02 100644 --- a/packages/utils/src/postmortem.ts +++ b/packages/utils/src/postmortem.ts @@ -67,6 +67,17 @@ 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; +} + +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 +207,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 +255,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";