From 9ebc2392801ed129fd7f6038e1ca0e75aecae12d Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 11 Jul 2026 06:21:07 +0200 Subject: [PATCH] feat(coding-agent/tools): added fail-fast watchdog for browser selector ops - Implement a zero-match watchdog for browser selector operations that aborts after ~2s if no elements are found, preventing actions from unnecessarily consuming the full deadline. - Lower the default interactive action operation ceiling from 15s to 8s. - Update `waitFor` and `waitForSelector` to conditionally opt out of the fast-fail mechanism when explicit timeouts or hidden-state expectations are provided. --- packages/coding-agent/CHANGELOG.md | 2 + .../coding-agent/src/prompts/tools/browser.md | 2 +- .../src/tools/browser/tab-worker.ts | 96 ++++++++++++++++--- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 39fae7d7c..6abda2377 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,7 @@ ### Changed +- Reduced browser action timeout from 15s to 8s to improve agent iteration speed - Refined agent delegation logic to prioritize top-level planning and scoping by the primary agent - Optimized subagent usage to discourage single-agent delegation and improve parallel execution flows - Clarified that prerequisite work for subagent tasks should be handled inline by the main agent @@ -27,6 +28,7 @@ - Fixed browser runs silently dropping `display("string")`, `console.log`, and `print` output: the runtime emits those as stream text, which the browser embedders (worker and cmux) routed to the debug log only, so the tool result showed a bare "Ran code on tab". Stream text is now buffered and surfaced as ordered display entries alongside `display()` payloads and screenshots. - Fixed `input.fill is not a function` on element handles from `tab.id()`/`tab.ref()`/`tab.waitFor()`: raw puppeteer ElementHandles expose `type()` but not the `fill()` the tool docs promise. Handles handed to user code now carry a `fill()` matching `tab.fill()` semantics (focus, clear, retype). - Browser selector ops (`click`/`type`/`fill`/`waitFor`/…) that hit their fail-fast timeout now append a match-count diagnosis — "matches no elements" (wrong page, consent wall) vs "matches N element(s) but the action never became possible" (hidden/covered) — instead of a bare `timed out after 15000ms`. +- Browser selector ops no longer burn their whole deadline on a selector that matches nothing: a watchdog polls while the action runs and aborts after ~2s of confirmed zero matches with the inspection hint. `waitFor`/`waitForSelector` opt out via an explicit `{ timeout }` (and `hidden: true` waits are exempt — zero matches is their success condition). The per-action ceiling for present-but-unactionable elements also dropped from 15s to 8s. - Fixed silent failures in ACP mode when provider errors occurred before streaming assistant text - Prevented duplicate error messages in ACP when a provider error was both streamed and final - Fixed `glob` reporting the contradictory "No files found matching pattern" next to a "timed out; returning 0 partial matches" notice. A timed-out empty scan now states explicitly that the result is incomplete (not proof of absence) and suggests scoping to a deeper directory, and the TUI renders it as "No matches before timeout (scan incomplete)" instead of a definitive no-files claim. diff --git a/packages/coding-agent/src/prompts/tools/browser.md b/packages/coding-agent/src/prompts/tools/browser.md index 51780cbc0..e0f6d22ff 100644 --- a/packages/coding-agent/src/prompts/tools/browser.md +++ b/packages/coding-agent/src/prompts/tools/browser.md @@ -30,7 +30,7 @@ Drives real Chromium tab; full puppeteer access via JS. - `tab.evaluate(fn, …args)` — `page.evaluate` for ad-hoc DOM reads. - `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. timed out` error, never the whole-cell timeout. +- 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.` 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). diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index c10c4c4f2..546cc8e35 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -115,15 +115,27 @@ type ActionabilityResult = { ok: true; x: number; y: number } | { ok: false; rea * - `QUICK_OP_TIMEOUT_MS`: page-coupled reads that should resolve fast (`observe`, * `screenshot`, `extract`, `ariaSnapshot`). * - `ACTION_OP_TIMEOUT_MS`: interactive point actions (`click`, `fill`, `type`, …) and - * the default for wait helpers when no explicit `{ timeout }` is given. + * the default for wait helpers when no explicit `{ timeout }` is given. Selector ops + * additionally fail fast after `ZERO_MATCH_FAIL_FAST_MS` of confirmed zero matches + * (see `#zeroMatchWatchdog`), so the full ceiling is only spent on elements that + * exist but are not yet actionable. * * `goto` and `evaluate` stay uncapped (`Number.POSITIVE_INFINITY`): navigation and user * code legitimately use the full cell budget. */ const QUICK_OP_TIMEOUT_MS = 20_000; -const ACTION_OP_TIMEOUT_MS = 15_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; +/** + * 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 + * wrong page (consent wall, pre-navigation document) is the common agent failure and + * should cost ~2s, not the full action ceiling. Explicit `{ timeout }` waits opt out. + */ +const ZERO_MATCH_FAIL_FAST_MS = 2_000; +/** Poll cadence for the zero-match watchdog. */ +const ZERO_MATCH_POLL_MS = 250; export interface OpTimeouts { /** Largest per-op deadline allowed — strictly below the cell budget. */ @@ -857,7 +869,9 @@ export class WorkerCore { * `Number.POSITIVE_INFINITY` for `perOpTimeoutMs` to bound the op only by the cell * budget (used for `evaluate` running user code and for locator helpers that already * carry puppeteer's own `.setTimeout(timeoutMs)`). When the op targets a `selector`, - * the fail-fast timeout carries a best-effort match-count hint. + * the fail-fast timeout carries a best-effort match-count hint, and — when + * `zeroMatchAfterMs` is set — a watchdog aborts the op early once the selector has + * matched nothing for that long. */ async #runOp( active: ActiveRun, @@ -865,15 +879,28 @@ export class WorkerCore { cellSignal: AbortSignal, perOpTimeoutMs: number, fn: (signal: AbortSignal) => Promise, - selector?: string, + opts?: { selector?: string; zeroMatchAfterMs?: number }, ): Promise { const opId = active.opCounter++; active.inflight.set(opId, { label, startedAt: Date.now() }); const capped = Number.isFinite(perOpTimeoutMs) && perOpTimeoutMs > 0; const opTimeout = capped ? AbortSignal.timeout(perOpTimeoutMs) : undefined; const opSignal = opTimeout ? AbortSignal.any([cellSignal, opTimeout]) : cellSignal; + const selector = opts?.selector; + const watchdog = + selector !== undefined && opts?.zeroMatchAfterMs !== undefined && parseAriaRefSelector(selector) === null + ? { selector, afterMs: opts.zeroMatchAfterMs } + : undefined; + // Fired when the watchdog wins the race (tears down the in-flight action) and in + // the finally (stops the watchdog's polling once the op settles either way). + const earlyAc = new AbortController(); try { - return await fn(opSignal); + if (!watchdog) return await fn(opSignal); + const racedSignal = AbortSignal.any([opSignal, earlyAc.signal]); + return await Promise.race([ + fn(racedSignal), + this.#zeroMatchWatchdog(watchdog.selector, label, watchdog.afterMs, racedSignal), + ]); } catch (err) { // Fail fast with a named, attributable error instead of the opaque whole-cell timeout: // our per-op deadline fired, or puppeteer's own (equal) timeout fired first — having @@ -889,10 +916,45 @@ export class WorkerCore { } throw err; } finally { + earlyAc.abort(); active.inflight.delete(opId); } } + /** + * Fail-fast arm raced against a selector op: rejects once the selector has matched + * nothing for the whole `afterMs` window, so a wrong selector or wrong page (consent + * wall, pre-navigation document) costs ~2s instead of the full action deadline. + * Disarms — hangs until the settled race drops it — the moment at least one element + * matches; an inconclusive probe (mid-navigation, detached frame) never counts + * toward the zero-match window. + */ + async #zeroMatchWatchdog(selector: string, label: string, afterMs: number, signal: AbortSignal): Promise { + const page = this.#requirePage(); + const resolved = normalizeSelector(selector); + const deadline = Date.now() + afterMs; + while (!signal.aborted) { + let count: number | null = null; + try { + const handles = await page.$$(resolved); + count = handles.length; + for (const handle of handles) void handle.dispose().catch(() => undefined); + } catch { + // Inconclusive probe — keep polling without advancing toward failure. + } + if (count !== null && count > 0) break; + if (count === 0 && Date.now() >= deadline) { + throw new ToolError(`${label} failed fast after ${afterMs}ms${formatSelectorMatchHint(0)}`); + } + try { + await untilAborted(signal, () => Bun.sleep(ZERO_MATCH_POLL_MS)); + } catch { + break; + } + } + return await new Promise(() => {}); + } + /** * Best-effort match-count probe for a timed-out selector op. Never throws; * empty string when the probe fails, stalls, or the selector is an aria-ref. @@ -930,8 +992,8 @@ export class WorkerCore { label: string, perOpMs: number, fn: (sig: AbortSignal) => Promise, - selector?: string, - ): Promise => markHandled(this.#runOp(active, label, signal, perOpMs, fn, selector)); + selectorOpts?: { selector?: string; zeroMatchAfterMs?: number }, + ): Promise => markHandled(this.#runOp(active, label, signal, perOpMs, fn, selectorOpts)); return { name, page, @@ -1011,7 +1073,7 @@ export class WorkerCore { page.locator(resolved).setTimeout(actionOpMs).click({ signal: sig }), ); }, - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), type: (selector, text) => op( @@ -1025,7 +1087,7 @@ export class WorkerCore { await handle.dispose().catch(() => undefined); } }, - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), fill: (selector, value) => op( @@ -1045,7 +1107,7 @@ export class WorkerCore { page.locator(normalizeSelector(selector)).setTimeout(actionOpMs).fill(value, { signal: sig }), ); }, - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), press: (key, opts) => op(`tab.press(${JSON.stringify(key)})`, actionOpMs, async sig => { @@ -1062,7 +1124,7 @@ export class WorkerCore { `tab.waitFor(${JSON.stringify(selector)})`, w, async sig => toActionableHandle(await this.#resolveActionHandle(selector, w, sig)), - selector, + { selector, zeroMatchAfterMs: opts?.timeout === undefined ? ZERO_MATCH_FAIL_FAST_MS : undefined }, ); }, waitForSelector: (selector, opts) => { @@ -1083,7 +1145,11 @@ export class WorkerCore { )) as ElementHandle | null; return handle ? toActionableHandle(handle) : null; }, - selector, + { + selector, + // `hidden: true` waits for zero matches — that is success, never a fast-fail. + zeroMatchAfterMs: opts?.timeout === undefined && !opts?.hidden ? ZERO_MATCH_FAIL_FAST_MS : undefined, + }, ); }, waitForNavigation: opts => { @@ -1121,21 +1187,21 @@ export class WorkerCore { await handle.dispose().catch(() => undefined); } }, - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), select: (selector, ...values) => op( `tab.select(${JSON.stringify(selector)})`, actionOpMs, sig => this.#select(selector, values, actionOpMs, sig), - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), uploadFile: (selector, ...filePaths) => op( `tab.uploadFile(${JSON.stringify(selector)})`, actionOpMs, sig => this.#uploadFile(selector, filePaths, actionOpMs, sig, session), - selector, + { selector, zeroMatchAfterMs: ZERO_MATCH_FAIL_FAST_MS }, ), waitForUrl: (pattern, opts) => { const w = waitMs(opts?.timeout);