feat(coding-agent-tools): implemented browser execution safety controls
- Implemented cell budget clamping for timeouts to prevent stalled browser operations from exceeding execution limits. - Added `recover` capabilities for tab workers to clear blocking dialogs and safely terminate hung navigations. - Introduced `//!world=main` support for `tab.evaluate` via Puppeteer patch to allow execution in main execution contexts. - Improved failure attribution for tab terminations by tracking dialogs, stalled operations, and specific termination reasons.
This commit is contained in:
@@ -3,12 +3,23 @@
|
||||
## [Unreleased]
|
||||
|
||||
## [16.4.8] - 2026-07-12
|
||||
### Added
|
||||
|
||||
- Added a predicate form to the browser run's `wait()` helper: `wait(fn, { timeout?, interval? })` polls the function (sync or async) until truthy and resolves with that value, failing with a named timeout error (deadline clamped under the cell budget so it always beats the opaque whole-cell timeout) instead of Bun's `sleep expects a number` or a whole-cell stall from in-page polling Promises; both `wait` forms now register in the stall diagnosis of cell timeouts
|
||||
|
||||
### Changed
|
||||
|
||||
- Improved tab recovery after timeouts by automatically clearing pending navigation and JS dialogs
|
||||
- Made `tab.goto` navigation failures catchable with a named error instead of triggering a whole-cell timeout
|
||||
- Added support for `//!world=main` in `tab.evaluate` to access page-level globals
|
||||
- Enhanced cell timeout messages to include identification of stalled operations and blocking JS dialogs
|
||||
- Browser `run` on a tab the supervisor force-killed now reports the kill reason instead of a bare "not alive"
|
||||
- Refined agent workflow to prioritize smoke testing and reduce mandatory upfront test generation
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the `//!world=main` evaluate directive being silently ignored for string expressions passed through `page.evaluate`/`tab.evaluate`: puppeteer's source-URL tagging boxed the string into a `String` object before the world check
|
||||
- Fixed tab reuse issues where hung navigation or unhandled modals would cause initialization to stall and trigger a force-kill
|
||||
- Improved search reliability for Perplexity provider by forcing retrieval for all queries
|
||||
- Fixed JS eval cells losing top-level `function` and `var` declarations across cells when the defining cell contained top-level `await` — the async wrapper scoped them to the cell's IIFE instead of publishing them to the worker global
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ Drives real Chromium tab; full puppeteer access via JS.
|
||||
- Three actions:
|
||||
- `open` — acquire/reuse named tab (`name` defaults `"main"`). Optional `url` (navigate once ready), `viewport`, `dialogs: "accept" | "dismiss"` (auto-handle `alert`/`confirm`/`beforeunload`; else page hangs till you wire `page.on('dialog', …)`).
|
||||
- `close` — release tab by `name`, or all with `all: true`. `kill: true` also kills spawned-app process trees.
|
||||
- `run` — execute JS in existing tab. `code` = async function body; `page`, `browser`, `tab`, `display`, `assert`, `wait` in scope. Return value JSON-stringified into result; `display(value)` accumulates text/images. `wait(ms)` sleeps; `wait(fn, { timeout?, interval? })` polls `fn` (sync or async) until truthy and resolves with that value (default 30s timeout, 100ms interval; named error on timeout) — use it instead of in-page polling Promises inside `tab.evaluate`.
|
||||
- `run` — execute JS in existing tab. `code` = async function body; `page`, `browser`, `tab`, `display`, `assert`, `wait` in scope. Return value JSON-stringified into result; `display(value)` accumulates text/images. `wait(ms)` sleeps; `wait(fn, { timeout?, interval? })` polls `fn` (sync or async) until truthy and resolves with that value (default 100ms interval; deadline min(30s, cell budget − 1s), named error on timeout) — use it instead of in-page polling Promises inside `tab.evaluate`.
|
||||
- Tabs survive `run` calls and in-process subagents — open once, reuse.
|
||||
- Browser kinds (`app` on `open`):
|
||||
- default (no `app`) → headless Chromium with stealth patches.
|
||||
@@ -13,7 +13,7 @@ Drives real Chromium tab; full puppeteer access via JS.
|
||||
- `app.cdp_url` → connect to existing CDP endpoint (e.g. `http://127.0.0.1:9222`).
|
||||
- `app.target` (with `path`/`cdp_url`) — substring on url+title picks BrowserWindow.
|
||||
- `tab` helpers; drop to raw puppeteer `page` for anything uncovered:
|
||||
- `tab.goto(url, { waitUntil? })` — navigate.
|
||||
- `tab.goto(url, { waitUntil? })` — navigate. A hung load fails ~1s before the cell budget with a named, catchable error and the pending navigation is stopped; for slow pages raise `timeout` or use `waitUntil: "domcontentloaded"`.
|
||||
- `tab.observe({ includeAll?, viewportOnly? })` — accessibility snapshot: `{ url, title, viewport, scroll, elements: [{ id, role, name, value, states, … }] }`. Ids stable until next observe/goto.
|
||||
- `tab.ariaSnapshot(selector?, { depth?, boxes? })` — Playwright-format ARIA-tree YAML (nested roles + accessible names + `/url`/`/placeholder`), scoped to `selector` or the whole document. Every node carries a `[ref=eN]` id; `[cursor=pointer]` flags clickables. Captures dense, hierarchical structure/text that `observe()`'s flat list flattens away. Refs renumber from e1 each call and stay valid until the next `ariaSnapshot()`.
|
||||
- `tab.ref("e5")` — `[ref=eN]` from the last ariaSnapshot → element handle with the common action methods (`.click()`, `.type()`, `.fill()`, `.hover()`, `.evaluate()`, …); the primary way to act on a ref. For convenience `aria-ref=e5` also works inline in `tab.click`/`type`/`fill`/`waitFor`/`scrollIntoView` (e.g. `tab.click("aria-ref=e5")`).
|
||||
@@ -27,10 +27,10 @@ Drives real Chromium tab; full puppeteer access via JS.
|
||||
- `tab.waitForUrl(pattern, { timeout? })` — substring or `RegExp` (matches SPA pushState nav); returns matched URL.
|
||||
- `tab.waitForResponse(pattern, { timeout? })` — substring, `RegExp`, or `(response) => boolean`; returns puppeteer `HTTPResponse` (`.text()`/`.json()`/`.status()`/`.headers()`).
|
||||
- `tab.waitForNavigation({ waitUntil?, timeout? })` — resolves on the next navigation. Start it BEFORE the click/submit that triggers it; after `tab.goto` (which already waits) use `tab.waitForUrl`/`tab.waitForSelector` instead.
|
||||
- `tab.evaluate(fn, …args)` — `page.evaluate` for ad-hoc DOM reads.
|
||||
- `tab.evaluate(fn, …args)` — `page.evaluate` for ad-hoc DOM reads. Runs in an ISOLATED world: DOM, `document.title`, `document.fonts` are shared, but the page's OWN JS globals (`window.myFlag`, app state, framework internals) are NOT visible and reads return `undefined`. To touch page globals, start the function body with the main-world directive: `tab.evaluate(() => { //!world=main` `return window.__done; })` (or `/*!world=main*/`; also honored by string expressions and `page.waitForFunction`).
|
||||
- `tab.screenshot({ selector?, fullPage?, save?, silent? })` — capture + attach for viewing (`silent: true` skips). Pass `save` only when a later step needs the file.
|
||||
- `tab.extract(format = "markdown")` — readable page content (`"markdown"` | `"text"`); throws when nothing readable.
|
||||
- Selectors: CSS + puppeteer handlers `aria/Sign in`, `text/Continue`, `xpath/…`, `pierce/…`; also Playwright-style `p-aria/…`, `p-text/…`. Playwright-only engines/pseudos (`:has-text()`, `:visible`, …) are rejected — use `text/…` or `aria/…`. A stalled action/wait fails fast with a named `tab.<op>` error carrying a match-count diagnosis, never the whole-cell timeout; a selector matching nothing fails in ~2s (pass an explicit `{ timeout }` to `waitFor`/`waitForSelector` to wait out slow-appearing elements).
|
||||
- Selectors: CSS + puppeteer handlers `aria/Sign in`, `text/Continue`, `xpath/…`, `pierce/…`; also Playwright-style `p-aria/…`, `p-text/…`. Playwright-only engines/pseudos (`:has-text()`, `:visible`, …) are rejected — use `text/…` or `aria/…`. A stalled action/wait fails fast with a named `tab.<op>` error carrying a match-count diagnosis, never the whole-cell timeout; a selector matching nothing fails in ~2s (pass an explicit `{ timeout }` to `waitFor`/`waitForSelector` to wait out slow-appearing elements). A whole-cell timeout names the stalled op (including `wait(...)`) and any unhandled dialog blocking the page.
|
||||
</instruction>
|
||||
|
||||
<critical>
|
||||
|
||||
@@ -12,7 +12,12 @@ import { ToolAbortError, ToolError, throwIfAborted } from "../../tool-errors";
|
||||
import { type AriaSnapshotOptions, buildAriaSnapshotScript } from "../aria/aria-snapshot";
|
||||
import { DEFAULT_VIEWPORT } from "../launch";
|
||||
import { extractReadableFromHtml, type ReadableFormat } from "../readable";
|
||||
import { bindBrowserRunFacade, type WaitPredicateOptions, waitForBrowserRun } from "../run-cancellation";
|
||||
import {
|
||||
bindBrowserRunFacade,
|
||||
resolvePredicateTimeout,
|
||||
type WaitPredicateOptions,
|
||||
waitForBrowserRun,
|
||||
} from "../run-cancellation";
|
||||
import { cloneSafe, RunOutput } from "../run-output";
|
||||
import type { Observation, ReadyInfo, RunResultOk, ScreenshotResult, SessionSnapshot } from "../tab-protocol";
|
||||
import {
|
||||
@@ -1352,8 +1357,17 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi
|
||||
assert: (cond: unknown, text?: string): void => {
|
||||
if (!cond) throw new ToolError(text ?? "Assertion failed");
|
||||
},
|
||||
wait: (msOrPredicate: number | (() => unknown), opts?: WaitPredicateOptions): Promise<unknown> =>
|
||||
waitForBrowserRun(msOrPredicate, signal, opts),
|
||||
wait: (msOrPredicate: number | (() => unknown), waitOpts?: WaitPredicateOptions): Promise<unknown> =>
|
||||
waitForBrowserRun(
|
||||
msOrPredicate,
|
||||
signal,
|
||||
typeof msOrPredicate === "number"
|
||||
? waitOpts
|
||||
: {
|
||||
timeout: resolvePredicateTimeout(opts.timeoutMs, waitOpts?.timeout),
|
||||
interval: waitOpts?.interval,
|
||||
},
|
||||
),
|
||||
});
|
||||
|
||||
const hooks: RuntimeHooks = {
|
||||
|
||||
@@ -16,14 +16,33 @@ export function markHandled<T>(promise: Promise<T>): Promise<T> {
|
||||
return promise;
|
||||
}
|
||||
|
||||
/** Headroom subtracted from the cell budget so an in-run deadline fires before the opaque whole-cell timeout. */
|
||||
export const CELL_BUDGET_SLACK_MS = 1_000;
|
||||
|
||||
/** Default poll deadline for `wait(predicate)` before clamping to the cell budget. */
|
||||
export const DEFAULT_PREDICATE_TIMEOUT_MS = 30_000;
|
||||
|
||||
/** Options for the predicate form of the run-scoped `wait()` helper. */
|
||||
export interface WaitPredicateOptions {
|
||||
/** Max time to poll before failing, in ms (default 30_000). */
|
||||
/** Max time to poll before failing, in ms (default 30s, clamped to the cell budget). */
|
||||
timeout?: number;
|
||||
/** Poll interval in ms (default 100, floor 10). */
|
||||
interval?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Effective `wait(predicate)` deadline for a given cell budget. Always strictly below
|
||||
* the cell budget so the named `wait(predicate) timed out` error wins the race against
|
||||
* the opaque whole-cell "Browser code execution timed out". `0`/`Infinity` ("disable")
|
||||
* map to the largest bounded deadline; negative/NaN garbage falls back to the default.
|
||||
*/
|
||||
export function resolvePredicateTimeout(cellTimeoutMs: number, explicit?: number): number {
|
||||
const budgetBound = Math.max(1, cellTimeoutMs - CELL_BUDGET_SLACK_MS);
|
||||
if (explicit === 0 || explicit === Number.POSITIVE_INFINITY) return budgetBound;
|
||||
if (explicit !== undefined && Number.isFinite(explicit) && explicit > 0) return Math.min(explicit, budgetBound);
|
||||
return Math.min(DEFAULT_PREDICATE_TIMEOUT_MS, budgetBound);
|
||||
}
|
||||
|
||||
/**
|
||||
* Run-scoped `wait()` helper for evaluated browser code, honoring the owning run's
|
||||
* cancellation signal.
|
||||
@@ -49,7 +68,9 @@ export function waitForBrowserRun(
|
||||
throw new ToolError("wait(...) expects milliseconds (number) or a predicate function to poll");
|
||||
}
|
||||
const timeout =
|
||||
opts?.timeout !== undefined && Number.isFinite(opts.timeout) && opts.timeout > 0 ? opts.timeout : 30_000;
|
||||
opts?.timeout !== undefined && Number.isFinite(opts.timeout) && opts.timeout > 0
|
||||
? opts.timeout
|
||||
: DEFAULT_PREDICATE_TIMEOUT_MS;
|
||||
const interval = Math.max(opts?.interval ?? 100, 10);
|
||||
const deadline = Date.now() + timeout;
|
||||
for (;;) {
|
||||
|
||||
@@ -59,6 +59,12 @@ export type WorkerInitPayload =
|
||||
safeDir: string;
|
||||
targetId: string;
|
||||
dialogs?: "accept" | "dismiss";
|
||||
/**
|
||||
* Post-timeout recycle: before adopting the page, dismiss any open JS dialog and
|
||||
* stop a pending navigation so a blocked target cannot stall worker init (which
|
||||
* previously force-killed the tab). Never set for first-time Electron attach.
|
||||
*/
|
||||
recover?: boolean;
|
||||
};
|
||||
|
||||
export type ToolReply = { ok: true; value: unknown } | { ok: false; error: RunErrorPayload };
|
||||
|
||||
@@ -131,6 +131,10 @@ const tabs = new Map<string, TabSession>();
|
||||
// awaits) cannot interleave and leak a worker + browser refCount.
|
||||
const acquireChains = new Map<string, Promise<void>>();
|
||||
const GRACE_MS = 750;
|
||||
// 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.
|
||||
const killedTabs = new Map<string, string>();
|
||||
|
||||
export function getTab(name: string): TabSession | undefined {
|
||||
return tabs.get(name);
|
||||
@@ -161,6 +165,7 @@ async function acquireTabImpl(
|
||||
if (opts.signal?.aborted) {
|
||||
throw new ToolAbortError("Browser tab open aborted");
|
||||
}
|
||||
killedTabs.delete(name);
|
||||
// Temporary refCount hold so releasing an existing tab on the SAME browser
|
||||
// below cannot drop it to refCount 0 and dispose the instance we are about
|
||||
// to reuse (e.g. reopening the sole tab with a different dialogs policy).
|
||||
@@ -386,7 +391,14 @@ async function runInTabWithSnapshot(
|
||||
snapshot: SessionSnapshot,
|
||||
): Promise<RunResultOk> {
|
||||
const tab = tabs.get(name);
|
||||
if (!tab || tab.state === "dead") throw new ToolError(`Tab ${JSON.stringify(name)} is not alive. Reopen it.`);
|
||||
if (!tab || tab.state === "dead") {
|
||||
const killed = killedTabs.get(name);
|
||||
throw new ToolError(
|
||||
killed
|
||||
? `Tab ${JSON.stringify(name)} was killed: ${killed}. Reopen it.`
|
||||
: `Tab ${JSON.stringify(name)} is not alive. Open it first with action:"open".`,
|
||||
);
|
||||
}
|
||||
if (tab.pending.size > 0) throw new ToolError(`Tab ${JSON.stringify(name)} is busy`);
|
||||
const id = Snowflake.next();
|
||||
const { promise, resolve, reject } = Promise.withResolvers<RunResultOk>();
|
||||
@@ -712,6 +724,9 @@ async function recycleTimedOutWorkerTab(tab: WorkerTabSession, timeoutMs: number
|
||||
safeDir: getPuppeteerDir(),
|
||||
targetId: tab.targetId,
|
||||
dialogs: tab.dialogPolicy,
|
||||
// Unblock a wedged page (open JS dialog, hung navigation) before adopting it —
|
||||
// otherwise init stalls, times out, and the tab gets force-killed.
|
||||
recover: true,
|
||||
};
|
||||
let worker = await spawnTabWorker();
|
||||
try {
|
||||
@@ -743,6 +758,7 @@ async function recycleTimedOutWorkerTab(tab: WorkerTabSession, timeoutMs: number
|
||||
async function forceKillTab(name: string, reason: string): Promise<void> {
|
||||
const tab = tabs.get(name);
|
||||
if (!tab) return;
|
||||
killedTabs.set(name, reason);
|
||||
tab.state = "dead";
|
||||
const error = postmortem.markExpectedCleanupError(new ToolError(reason));
|
||||
for (const pending of tab.pending.values()) pending.reject(error);
|
||||
|
||||
@@ -6,6 +6,7 @@ import { postmortem, Snowflake, untilAborted } from "@oh-my-pi/pi-utils";
|
||||
import type { HTMLElement } from "linkedom";
|
||||
import type {
|
||||
Browser,
|
||||
CDPSession,
|
||||
Dialog,
|
||||
ElementHandle,
|
||||
ElementScreenshotOptions,
|
||||
@@ -35,7 +36,13 @@ import {
|
||||
loadPuppeteerInWorker,
|
||||
} from "./launch";
|
||||
import { extractReadableFromHtml, type ReadableFormat } from "./readable";
|
||||
import { markHandled, type WaitPredicateOptions, waitForBrowserRun } from "./run-cancellation";
|
||||
import {
|
||||
CELL_BUDGET_SLACK_MS,
|
||||
markHandled,
|
||||
resolvePredicateTimeout,
|
||||
type WaitPredicateOptions,
|
||||
waitForBrowserRun,
|
||||
} from "./run-cancellation";
|
||||
import { cloneSafe, RunOutput } from "./run-output";
|
||||
import type {
|
||||
Observation,
|
||||
@@ -105,6 +112,11 @@ const PLAYWRIGHT_ONLY_SELECTOR_RE =
|
||||
type DialogPolicy = "accept" | "dismiss";
|
||||
type DragTarget = string | { readonly x: number; readonly y: number };
|
||||
type ActionabilityResult = { ok: true; x: number; y: number } | { ok: false; reason: string };
|
||||
/** Last JS dialog seen on the page; kept for timeout attribution until handled or navigation. */
|
||||
interface OpenDialogInfo {
|
||||
type: string;
|
||||
message: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-op fail-fast ceilings for `tab.*` helpers. All are kept strictly under the cell
|
||||
@@ -126,7 +138,7 @@ type ActionabilityResult = { ok: true; x: number; y: number } | { ok: false; rea
|
||||
const QUICK_OP_TIMEOUT_MS = 20_000;
|
||||
const ACTION_OP_TIMEOUT_MS = 8_000;
|
||||
/** Headroom subtracted from the cell budget so a per-op deadline fires before it. */
|
||||
const OP_DEADLINE_SLACK_MS = 1_000;
|
||||
const OP_DEADLINE_SLACK_MS = CELL_BUDGET_SLACK_MS;
|
||||
/**
|
||||
* A selector op whose selector has matched nothing for this long fails fast with the
|
||||
* zero-match hint instead of burning the rest of its deadline: a wrong selector or a
|
||||
@@ -595,6 +607,7 @@ export class WorkerCore {
|
||||
#mode?: WorkerInitPayload["mode"];
|
||||
#dialogPolicy?: DialogPolicy;
|
||||
#dialogHandler?: (dialog: Dialog) => void;
|
||||
#openDialog?: OpenDialogInfo;
|
||||
|
||||
constructor(transport: Transport) {
|
||||
this.#transport = transport;
|
||||
@@ -648,6 +661,7 @@ export class WorkerCore {
|
||||
});
|
||||
if (payload.mode === "headless") {
|
||||
this.#page = await this.#browser.newPage();
|
||||
this.#observeDialogs();
|
||||
await applyStealthPatches(this.#browser, this.#page, { browserSession: null, override: null });
|
||||
await applyViewport(this.#page, payload.viewport);
|
||||
if (payload.dialogs) this.#applyDialogPolicy(payload.dialogs);
|
||||
@@ -659,7 +673,15 @@ export class WorkerCore {
|
||||
});
|
||||
}
|
||||
} else {
|
||||
this.#page = await this.#findAttachedPage(payload.targetId);
|
||||
const target = await this.#findAttachedTarget(payload.targetId);
|
||||
// Post-timeout recycle: unblock the target BEFORE adopting the page — an open
|
||||
// modal dialog or hung navigation can stall `target.page()` / ready info, and a
|
||||
// stalled init used to time out and force-kill the tab.
|
||||
if (payload.recover) await this.#recoverAttachedTarget(target);
|
||||
const page = await target.page();
|
||||
if (!page) throw new ToolError(`Target ${payload.targetId} is no longer available on the attached browser`);
|
||||
this.#page = page;
|
||||
this.#observeDialogs();
|
||||
if (payload.dialogs) this.#applyDialogPolicy(payload.dialogs);
|
||||
}
|
||||
this.#targetId = await targetIdForPage(this.#page);
|
||||
@@ -669,17 +691,53 @@ export class WorkerCore {
|
||||
}
|
||||
}
|
||||
|
||||
async #findAttachedPage(targetId: string): Promise<Page> {
|
||||
async #findAttachedTarget(targetId: string): Promise<Target> {
|
||||
if (!this.#browser) throw new ToolError("Browser is not connected");
|
||||
for (const target of this.#browser.targets()) {
|
||||
if ((await targetIdForTarget(target).catch(() => "")) !== targetId) continue;
|
||||
const page = await target.page();
|
||||
if (!page) break;
|
||||
return page;
|
||||
return target;
|
||||
}
|
||||
throw new ToolError(`Target ${targetId} is no longer available on the attached browser`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort unblocking of a wedged target during post-timeout recovery: dismiss any
|
||||
* open JS dialog and stop a pending navigation over a raw CDP session (created on the
|
||||
* target, not the page, so it works while the page itself is unresponsive). Every step
|
||||
* tolerates "nothing to do".
|
||||
*/
|
||||
async #recoverAttachedTarget(target: Target): Promise<void> {
|
||||
let session: CDPSession | undefined;
|
||||
try {
|
||||
session = await target.createCDPSession();
|
||||
await session.send("Page.enable").catch(() => undefined);
|
||||
await session.send("Page.handleJavaScriptDialog", { accept: false }).catch(() => undefined);
|
||||
await session.send("Page.stopLoading").catch(() => undefined);
|
||||
} catch (error) {
|
||||
this.#log("debug", "Recovery CDP session failed; proceeding with attach", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
} finally {
|
||||
await session?.detach().catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Record JS dialogs for timeout attribution without handling them (semantics of an
|
||||
* unset `dialogs` policy are unchanged — the page stays blocked until user code or
|
||||
* the policy handler acts). Cleared when the policy handler settles the dialog or a
|
||||
* main-frame navigation proves the modal is gone.
|
||||
*/
|
||||
#observeDialogs(): void {
|
||||
const page = this.#requirePage();
|
||||
page.on("dialog", dialog => {
|
||||
this.#openDialog = { type: dialog.type(), message: dialog.message() };
|
||||
});
|
||||
page.on("framenavigated", frame => {
|
||||
if (frame === page.mainFrame()) this.#openDialog = undefined;
|
||||
});
|
||||
}
|
||||
|
||||
async #currentReadyInfo(): Promise<ReadyInfo> {
|
||||
const page = this.#requirePage();
|
||||
const targetId = this.#targetId ?? (await targetIdForPage(page));
|
||||
@@ -698,7 +756,11 @@ export class WorkerCore {
|
||||
if (this.#dialogHandler) page.off("dialog", this.#dialogHandler);
|
||||
const handler = (dialog: Dialog): void => {
|
||||
const action = policy === "accept" ? dialog.accept() : dialog.dismiss();
|
||||
void action.catch(err =>
|
||||
void action.then(
|
||||
() => {
|
||||
this.#openDialog = undefined;
|
||||
},
|
||||
err =>
|
||||
this.#log("debug", "Dialog auto-handler failed", {
|
||||
policy,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
@@ -761,8 +823,20 @@ export class WorkerCore {
|
||||
assert: (cond: unknown, text?: string): void => {
|
||||
if (!cond) throw new ToolError(text ?? "Assertion failed");
|
||||
},
|
||||
wait: (msOrPredicate: number | (() => unknown), opts?: WaitPredicateOptions): Promise<unknown> =>
|
||||
waitForBrowserRun(msOrPredicate, signal, opts),
|
||||
// Both wait forms register in the in-flight map so a cell that dies while
|
||||
// sleeping/polling names the culprit instead of a bare whole-cell timeout.
|
||||
wait: (msOrPredicate: number | (() => unknown), opts?: WaitPredicateOptions): Promise<unknown> => {
|
||||
const label = typeof msOrPredicate === "number" ? `wait(${msOrPredicate}ms)` : "wait(predicate)";
|
||||
const resolved =
|
||||
typeof msOrPredicate === "number"
|
||||
? undefined
|
||||
: { timeout: resolvePredicateTimeout(msg.timeoutMs, opts?.timeout), interval: opts?.interval };
|
||||
return markHandled(
|
||||
this.#runOp(active, label, signal, Number.POSITIVE_INFINITY, sig =>
|
||||
waitForBrowserRun(msOrPredicate, sig, resolved),
|
||||
),
|
||||
);
|
||||
},
|
||||
});
|
||||
const { promise: cancelRejection, reject: rejectCancel } = Promise.withResolvers<never>();
|
||||
const onCancel = (): void => {
|
||||
@@ -772,9 +846,13 @@ export class WorkerCore {
|
||||
: new ToolAbortError(undefined, { cause: signal.reason });
|
||||
if (timeoutSignal.aborted) {
|
||||
const stalled = describeInflight(active.inflight);
|
||||
const dialog = this.#openDialog;
|
||||
const dialogNote = dialog
|
||||
? `; a ${dialog.type}(${JSON.stringify(dialog.message.slice(0, 80))}) dialog opened during this run and may still block the page — reopen the tab with dialogs:"accept"|"dismiss" or handle page.on('dialog')`
|
||||
: "";
|
||||
rejectCancel(
|
||||
new ToolError(
|
||||
`Browser code execution timed out after ${msg.timeoutMs}ms${stalled ? ` (stalled on ${stalled})` : ""}`,
|
||||
`Browser code execution timed out after ${msg.timeoutMs}ms${stalled ? ` (stalled on ${stalled})` : ""}${dialogNote}`,
|
||||
),
|
||||
);
|
||||
} else {
|
||||
@@ -986,7 +1064,7 @@ export class WorkerCore {
|
||||
active: ActiveRun,
|
||||
): TabApi {
|
||||
const page = this.#requirePage();
|
||||
const { quickOpMs, actionOpMs } = resolveOpTimeouts(timeoutMs);
|
||||
const { budgetBound, quickOpMs, actionOpMs } = resolveOpTimeouts(timeoutMs);
|
||||
const waitMs = (explicit?: number): number => resolveWaitTimeout(timeoutMs, explicit);
|
||||
const INF = Number.POSITIVE_INFINITY;
|
||||
const op = <T>(
|
||||
@@ -1004,10 +1082,24 @@ export class WorkerCore {
|
||||
goto: (url, opts) =>
|
||||
op(`tab.goto(${JSON.stringify(url)})`, INF, async sig => {
|
||||
this.#clearElementCache();
|
||||
try {
|
||||
// Default to "load" because dev servers with HMR/WS never reach networkidle.
|
||||
// budgetBound (not the full cell) so a hung navigation fails named and
|
||||
// catchable inside the run instead of dying with the whole cell.
|
||||
await untilAborted(sig, () =>
|
||||
page.goto(url, { waitUntil: opts?.waitUntil ?? "load", timeout: timeoutMs }),
|
||||
page.goto(url, { waitUntil: opts?.waitUntil ?? "load", timeout: budgetBound }),
|
||||
);
|
||||
} catch (err) {
|
||||
if (err instanceof Error && err.name === "TimeoutError") {
|
||||
// Abandon the hung navigation NOW — a still-pending load stalls every
|
||||
// later op on this page and cascades into more opaque timeouts.
|
||||
await this.#stopLoading();
|
||||
throw new ToolError(
|
||||
`tab.goto(${JSON.stringify(url)}) timed out after ${budgetBound}ms; pending navigation stopped — retry with a longer tool timeout or waitUntil:"domcontentloaded"`,
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}),
|
||||
observe: opts => op("tab.observe()", quickOpMs, sig => this.#collectObservation({ ...opts, signal: sig })),
|
||||
ariaSnapshot: (selector, opts) =>
|
||||
@@ -1554,6 +1646,22 @@ export class WorkerCore {
|
||||
for (const handle of handles) void handle.dispose().catch(() => undefined);
|
||||
}
|
||||
|
||||
/** Best-effort `Page.stopLoading` so an abandoned navigation cannot stall later ops. */
|
||||
async #stopLoading(): Promise<void> {
|
||||
try {
|
||||
const session = await this.#requirePage().createCDPSession();
|
||||
try {
|
||||
await session.send("Page.stopLoading");
|
||||
} finally {
|
||||
await session.detach().catch(() => undefined);
|
||||
}
|
||||
} catch (error) {
|
||||
this.#log("debug", "Page.stopLoading failed", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async #close(): Promise<void> {
|
||||
this.#unsub();
|
||||
this.#clearElementCache();
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { resolvePredicateTimeout } from "@oh-my-pi/pi-coding-agent/tools/browser/run-cancellation";
|
||||
import {
|
||||
normalizeSelector,
|
||||
resolveOpTimeouts,
|
||||
@@ -77,6 +78,31 @@ describe("browser wait-helper timeout resolution", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("browser wait(predicate) deadline resolution", () => {
|
||||
it("keeps the default deadline strictly under the cell budget so the named error wins", () => {
|
||||
// Default cell (30s): the old 30s predicate default tied the cell timer and lost the
|
||||
// race, surfacing the opaque whole-cell timeout instead of the named wait error.
|
||||
for (const cell of [5_000, 30_000, 120_000]) {
|
||||
expect(resolvePredicateTimeout(cell)).toBeLessThan(cell);
|
||||
}
|
||||
expect(resolvePredicateTimeout(120_000)).toBe(30_000);
|
||||
expect(resolvePredicateTimeout(5_000)).toBe(4_000);
|
||||
});
|
||||
|
||||
it("honors an explicit deadline but clamps it under the cell budget", () => {
|
||||
expect(resolvePredicateTimeout(30_000, 5_000)).toBe(5_000);
|
||||
expect(resolvePredicateTimeout(30_000, 90_000)).toBe(29_000);
|
||||
expect(resolvePredicateTimeout(120_000, 90_000)).toBe(90_000);
|
||||
});
|
||||
|
||||
it("maps disable sentinels to the largest bounded deadline and garbage to the default", () => {
|
||||
expect(resolvePredicateTimeout(30_000, 0)).toBe(29_000);
|
||||
expect(resolvePredicateTimeout(30_000, Number.POSITIVE_INFINITY)).toBe(29_000);
|
||||
expect(resolvePredicateTimeout(30_000, -5)).toBe(29_000);
|
||||
expect(resolvePredicateTimeout(30_000, Number.NaN)).toBe(29_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("browser selector guard", () => {
|
||||
it("rejects Playwright-only selector engines with an actionable message", () => {
|
||||
expect(() => normalizeSelector('button:has-text("Allow all")')).toThrow(/Playwright-only/);
|
||||
|
||||
@@ -1,16 +1,24 @@
|
||||
diff --git a/node_modules/puppeteer-core/.bun-tag-2e714b457f0bd8e8 b/.bun-tag-2e714b457f0bd8e8
|
||||
new file mode 100644
|
||||
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
|
||||
diff --git a/node_modules/puppeteer-core/.bun-tag-a797aeb3ca2bd69f b/.bun-tag-a797aeb3ca2bd69f
|
||||
new file mode 100644
|
||||
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
|
||||
diff --git a/lib/puppeteer/api/ElementHandle.js b/lib/puppeteer/api/ElementHandle.js
|
||||
index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415e6271e28 100644
|
||||
index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..ee6b042c87861f00d7fcbb6e3eb6e87af232ebbd 100644
|
||||
--- a/lib/puppeteer/api/ElementHandle.js
|
||||
+++ b/lib/puppeteer/api/ElementHandle.js
|
||||
@@ -102,6 +102,31 @@ import { throwIfDisposed } from '../util/decorators.js';
|
||||
@@ -102,6 +102,36 @@ import { throwIfDisposed } from '../util/decorators.js';
|
||||
import { _isElementHandle } from './ElementHandleSymbol.js';
|
||||
import { JSHandle } from './JSHandle.js';
|
||||
import { NodeLocator } from './locators/locators.js';
|
||||
+const MAIN_WORLD_DIRECTIVE = /^\s*(?:(?:\/\/!world=main(?=$|\s))|(?:\/\*!world=main\s*\*\/))/;
|
||||
+const shouldEvaluateInMainWorld = (pageFunction) => {
|
||||
+ if (pageFunction instanceof String) {
|
||||
+ // withSourcePuppeteerURLIfNone tags string page functions via Object.assign,
|
||||
+ // which boxes the primitive; unbox so the directive check still sees a string.
|
||||
+ pageFunction = pageFunction.toString();
|
||||
+ }
|
||||
+ if (typeof pageFunction !== 'function' && typeof pageFunction !== 'string') {
|
||||
+ return false;
|
||||
+ }
|
||||
@@ -33,11 +41,11 @@ index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415
|
||||
+ const arrowIndex = source.indexOf('=>');
|
||||
+ const bodyStart = source.indexOf('{', arrowIndex >= 0 ? arrowIndex : 0);
|
||||
+ return bodyStart >= 0 && MAIN_WORLD_DIRECTIVE.test(source.slice(bodyStart + 1));
|
||||
+};
|
||||
+}
|
||||
/**
|
||||
* A given method will have it's `this` replaced with an isolated version of
|
||||
* `this` when decorated with this decorator.
|
||||
@@ -299,6 +324,7 @@ let ElementHandle = (() => {
|
||||
@@ -299,6 +329,7 @@ let ElementHandle = (() => {
|
||||
* trying to adopt it multiple times
|
||||
*/
|
||||
isolatedHandle = __runInitializers(this, _instanceExtraInitializers);
|
||||
@@ -45,7 +53,7 @@ index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415
|
||||
/**
|
||||
* @internal
|
||||
*/
|
||||
@@ -335,19 +361,37 @@ let ElementHandle = (() => {
|
||||
@@ -335,19 +366,37 @@ let ElementHandle = (() => {
|
||||
async getProperties() {
|
||||
return await this.handle.getProperties();
|
||||
}
|
||||
@@ -85,7 +93,7 @@ index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415
|
||||
}
|
||||
/**
|
||||
* @internal
|
||||
@@ -371,7 +415,7 @@ let ElementHandle = (() => {
|
||||
@@ -371,7 +420,7 @@ let ElementHandle = (() => {
|
||||
* @internal
|
||||
*/
|
||||
async dispose() {
|
||||
@@ -94,7 +102,7 @@ index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415
|
||||
}
|
||||
/**
|
||||
* @internal
|
||||
@@ -555,15 +599,27 @@ let ElementHandle = (() => {
|
||||
@@ -555,15 +604,27 @@ let ElementHandle = (() => {
|
||||
async $$eval(selector, pageFunction, ...args) {
|
||||
const env_2 = { stack: [], error: void 0, hasError: false };
|
||||
try {
|
||||
@@ -127,15 +135,20 @@ index 81454fe2af73518f7e76f04cbe3a7f170d735fa5..6f9eedb9fc1344901705bc83cb3cf415
|
||||
]);
|
||||
return result;
|
||||
diff --git a/lib/puppeteer/api/Frame.js b/lib/puppeteer/api/Frame.js
|
||||
index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4265ca15f 100644
|
||||
index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..62c3b7ca8ec987cc62ceacec69acf6545bb59b86 100644
|
||||
--- a/lib/puppeteer/api/Frame.js
|
||||
+++ b/lib/puppeteer/api/Frame.js
|
||||
@@ -119,6 +119,31 @@ export var FrameEvent;
|
||||
@@ -119,6 +119,36 @@ export var FrameEvent;
|
||||
export const throwIfDetached = throwIfDisposed(frame => {
|
||||
return `Attempted to use detached Frame '${frame._id}'.`;
|
||||
});
|
||||
+const MAIN_WORLD_DIRECTIVE = /^\s*(?:(?:\/\/!world=main(?=$|\s))|(?:\/\*!world=main\s*\*\/))/;
|
||||
+const shouldEvaluateInMainWorld = (pageFunction) => {
|
||||
+ if (pageFunction instanceof String) {
|
||||
+ // withSourcePuppeteerURLIfNone tags string page functions via Object.assign,
|
||||
+ // which boxes the primitive; unbox so the directive check still sees a string.
|
||||
+ pageFunction = pageFunction.toString();
|
||||
+ }
|
||||
+ if (typeof pageFunction !== 'function' && typeof pageFunction !== 'string') {
|
||||
+ return false;
|
||||
+ }
|
||||
@@ -158,11 +171,11 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
+ const arrowIndex = source.indexOf('=>');
|
||||
+ const bodyStart = source.indexOf('{', arrowIndex >= 0 ? arrowIndex : 0);
|
||||
+ return bodyStart >= 0 && MAIN_WORLD_DIRECTIVE.test(source.slice(bodyStart + 1));
|
||||
+};
|
||||
+}
|
||||
/**
|
||||
* Represents a DOM frame.
|
||||
*
|
||||
@@ -277,12 +302,21 @@ let Frame = (() => {
|
||||
@@ -277,12 +307,21 @@ let Frame = (() => {
|
||||
super();
|
||||
}
|
||||
#_document;
|
||||
@@ -186,7 +199,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
return document;
|
||||
});
|
||||
}
|
||||
@@ -295,6 +329,7 @@ let Frame = (() => {
|
||||
@@ -295,6 +334,7 @@ let Frame = (() => {
|
||||
*/
|
||||
clearDocumentHandle() {
|
||||
this.#_document = undefined;
|
||||
@@ -194,7 +207,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
}
|
||||
/**
|
||||
* @returns The frame element associated with this frame (if any).
|
||||
@@ -345,8 +380,9 @@ let Frame = (() => {
|
||||
@@ -345,8 +385,9 @@ let Frame = (() => {
|
||||
* See {@link Page.evaluateHandle} for details.
|
||||
*/
|
||||
async evaluateHandle(pageFunction, ...args) {
|
||||
@@ -205,7 +218,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
}
|
||||
/**
|
||||
* Behaves identically to {@link Page.evaluate} except it's run within
|
||||
@@ -355,8 +391,9 @@ let Frame = (() => {
|
||||
@@ -355,8 +396,9 @@ let Frame = (() => {
|
||||
* See {@link Page.evaluate} for details.
|
||||
*/
|
||||
async evaluate(pageFunction, ...args) {
|
||||
@@ -216,7 +229,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
}
|
||||
/**
|
||||
* @internal
|
||||
@@ -458,9 +495,10 @@ let Frame = (() => {
|
||||
@@ -458,9 +500,10 @@ let Frame = (() => {
|
||||
* @returns A promise to the result of the function.
|
||||
*/
|
||||
async $eval(selector, pageFunction, ...args) {
|
||||
@@ -228,7 +241,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
return await document.$eval(selector, pageFunction, ...args);
|
||||
}
|
||||
/**
|
||||
@@ -498,9 +536,10 @@ let Frame = (() => {
|
||||
@@ -498,9 +541,10 @@ let Frame = (() => {
|
||||
* @returns A promise to the result of the function.
|
||||
*/
|
||||
async $$eval(selector, pageFunction, ...args) {
|
||||
@@ -240,7 +253,7 @@ index 8698d2a5a976b277344fe42e90ffe985bdf0fbd7..01614143cba96055d0f0c22a1eb903d4
|
||||
return await document.$$eval(selector, pageFunction, ...args);
|
||||
}
|
||||
/**
|
||||
@@ -577,7 +616,8 @@ let Frame = (() => {
|
||||
@@ -577,7 +621,8 @@ let Frame = (() => {
|
||||
* @returns the promise which resolve when the `pageFunction` returns a truthy value.
|
||||
*/
|
||||
async waitForFunction(pageFunction, options = {}, ...args) {
|
||||
|
||||
Reference in New Issue
Block a user