diff --git a/CHANGELOG.md b/CHANGELOG.md index d888aba..b981e20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## Unreleased +- Fixed `browser_open` returning a page revision that was already stale. Chrome starts loading a created tab after the tab exists, and that load advanced the revision right after `browser_open` returned, so the first `browser_act` with the returned revision failed with `stale_page_revision`. `browser_open` now returns after the first load completes, up to 10 seconds, and reports the loaded page's revision and URL. +- Made failed `browser_act` batches report their progress. The error `details` now carry `failed_action_index` and the `completed_actions` results, and in multi-action batches the message names the failing step and how many steps completed before it. A failure after a completed step now reports `outcome: "unknown"` instead of `not_started`, because the earlier steps already changed the page. - Fixed the OMP adapter's tool schemas lagging behind the protocol. OMP now offers `hover`, `double_click`, `context_click`, press `modifiers`, `frame_id`, `expect_download`, dialog `prompt_text`, snapshot `frame_id` and `match`, and the download-cursor, selector-state, and value waits that the Pi adapter already accepted, and rejects duplicate `keep_tab_ids` as Pi does. A parity test now fails when the OMP schemas accept less than the Pi schemas. - Diagnosed Chrome's automatic multiple-download throttle instead of hanging on it. Chrome silently blocks every automatic download after the first one a page initiates, so a second `expect_download` action used to wait out its cursor with no lifecycle events at all; chrome.debugger sessions cannot opt into DevTools download behavior to lift that on any tested Chrome, and overriding the site's automatic-downloads permission would be a persistent browser mutation outside task ownership. A download wait that expires with an unstarted cursor on a tab that already downloaded now fails with `multiple_download_throttle` and names the recovery paths. - Stopped page popups from stealing focus. A `click` or `press` that calls `window.open` or follows a new-window link now opens each requested http or https URL as an inactive tab owned by the acting task instead of letting Chrome create and activate a window. Each action result reports those tabs in `opened_tabs` with an opened, refused, or failed outcome. The page receives `null` from `window.open`, so flows that need the returned window or `window.opener` are unsupported. diff --git a/docs/mcp.md b/docs/mcp.md index 158dcc8..0e6ffad 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -62,9 +62,9 @@ For MCP, the capability store namespace is `mcp`; OMP uses `omp`; Pi uses `pi`. | Tool | Required input and behavior | |---|---| -| `browser_open` | `mode: "create"` optionally accepts an `http`, `https`, or `about` URL, `background`, and `placement`. The default `placement: "task"` creates a tab in the task's existing window when possible. `placement: "new_window"` creates the first tab of an otherwise empty task in a separate unfocused normal window and rejects `background: false`. `mode: "adopt_active"` explicitly adopts only the currently active tab. The result includes task, tab, window, page-revision, and `automation_route` identifiers. | +| `browser_open` | `mode: "create"` optionally accepts an `http`, `https`, or `about` URL, `background`, and `placement`. The default `placement: "task"` creates a tab in the task's existing window when possible. `placement: "new_window"` creates the first tab of an otherwise empty task in a separate unfocused normal window and rejects `background: false`. `mode: "adopt_active"` explicitly adopts only the currently active tab. The result includes task, tab, window, page-revision, and `automation_route` identifiers. It returns after the tab's first load completes, up to 10 seconds, so its `page_revision` and `url` describe the loaded page and can be passed straight to `browser_act`. | | `browser_snapshot` | Requires `tab_id`. Modes are `accessibility`, `text`, `html`, and `screenshot`; each may scope to an opaque CDP `frame_id`. Only accessibility snapshots return revisioned node references, each bound to its originating frame; full-tree accessibility snapshots also append `dom_fallback` textbox nodes for editable elements that never entered Chrome's accessibility tree, so those refs still address the existing typed actions. Screenshot requests may select `png`, `jpeg`, or `webp`, set JPEG/WebP `quality`, bound dimensions with `max_width`/`max_height`, and cap compressed image size with `max_bytes`. Snapshots require the `full` automation route. | -| `browser_act` | Requires `tab_id`, `expected_page_revision`, and one to 64 typed actions. Actions are click, hover, double-click, context-click, press, type, fill, select, scroll, drag, navigate, history movement, reload, close, dialog decision, and staged file upload. Element-target actions accept exactly one `ref` or `selector` and optional `frame_id`; `press` accepts the Standard navigation/editing keys plus optional unique `Alt`, `Control`, `Meta`, and `Shift` modifiers. Lowercase `a`, `z`, and `y` require exactly one of Control or Meta, optionally Shift, never Alt. No coordinate action, generic key injection, clipboard shortcut, or cross-frame target is exposed in Standard mode. | +| `browser_act` | Requires `tab_id`, `expected_page_revision`, and one to 64 typed actions. Actions are click, hover, double-click, context-click, press, type, fill, select, scroll, drag, navigate, history movement, reload, close, dialog decision, and staged file upload. Element-target actions accept exactly one `ref` or `selector` and optional `frame_id`; `press` accepts the Standard navigation/editing keys plus optional unique `Alt`, `Control`, `Meta`, and `Shift` modifiers. Lowercase `a`, `z`, and `y` require exactly one of Control or Meta, optionally Shift, never Alt. No coordinate action, generic key injection, clipboard shortcut, or cross-frame target is exposed in Standard mode. When a step fails, the error `details` carry `failed_action_index` (zero-based) and `completed_actions`, the results of the steps that ran first; in a batch of more than one action, the message also names the failing step. A failure after at least one completed step has `outcome: "unknown"`, because those steps already changed the page. | | `browser_wait` | Requires `tab_id` and one load, URL, text, selector, value, network-idle, or download condition. Selector waits may set `attached`, `detached`, `visible`, `hidden`, or `enabled` state and scope to a `frame_id`; value waits require selector and expected value and may scope to a frame. A download wait may resume after only its opaque tab-owned `after` cursor. `timeout_ms` is at most 120 seconds. A `tab_only` route accepts load and URL conditions only; frame-scoped reads, value/selector state, network-idle, and download attribution require the tab-scoped debugger connection available on the `full` route. | | `browser_tabs` | Takes an empty object and lists only the current task's tabs, including each tab's `automation_route`. | | `browser_handoff` | Takes one of four operations. `request` requires a task tab, expected page revision, and prompt; optional `completion: {kind: "url"\|"selector", value}` and `timeout_ms` (default five minutes, reminder expiry only). It creates a `notice_id` scoped to the task and tab and returns immediately without pausing, focusing, or blocking any agent work. `status`, `resolve`, and `dismiss` take that `notice_id`. The agent tells the user in chat, verifies the page itself, then resolves or dismisses; a `resolve` against an unmet condition fails with `completion_not_met` and leaves the notice open. Works on a `tab_only` route, but selector completion requires the `full` route. | diff --git a/packages/extension/src/background.ts b/packages/extension/src/background.ts index c32d57f..7c98f2b 100644 --- a/packages/extension/src/background.ts +++ b/packages/extension/src/background.ts @@ -33,6 +33,10 @@ let automationCleanupPending = false; let automationCleanupDelayMs = AUTOMATION_CLEANUP_RETRY_BASE_MS; let automationCleanupTimer: ReturnType | undefined; let automationCleanupQueue = Promise.resolve(); +const OPEN_SETTLE_TIMEOUT_MS = 10_000; +const tabLoadWaiters = new Map void>>(); +/** Tabs whose `complete` event the tab-event queue has processed; removal prunes it to open tabs. */ +const processedTabLoads = new Set(); const PRE_DISPATCH_ERRORS: Record = { invalid_request: true, @@ -388,7 +392,11 @@ async function dispatch(command: NativeDispatchCommand): Promise throw automationRequired(); } if (command.method === "browser_open") { - return completed(command.request_id, await scheduler.enqueueGlobal(() => ownership.open(command.task_id, params))); + const opened = await scheduler.enqueueGlobal(() => ownership.open(command.task_id, params)); + return completed( + command.request_id, + await settleOpenedTab(command.task_id, opened, params.mode === "create"), + ); } if (command.method === "browser_tabs") { const result = await scheduler.enqueueGlobal(() => ownership.inventory()); @@ -515,9 +523,7 @@ async function dispatch(command: NativeDispatchCommand): Promise normalized instanceof Error ? normalized.message : String(normalized), errorOutcome(normalized, mutating, code), errorRecovery(normalized), - isRecord(normalized) && typeof normalized.currentPageRevision === "number" - ? { current_page_revision: normalized.currentPageRevision } - : undefined, + errorDetails(normalized), ); } } @@ -588,6 +594,68 @@ function runAfterStart(operation: () => void | Promise = () => undefined): startupOperations.enqueue(operation); } +function errorDetails(error: unknown): Record | undefined { + if (!isRecord(error)) return undefined; + const details = { + ...(typeof error.currentPageRevision === "number" ? { current_page_revision: error.currentPageRevision } : {}), + ...(typeof error.failedActionIndex === "number" ? { failed_action_index: error.failedActionIndex } : {}), + ...(Array.isArray(error.completedActions) ? { completed_actions: error.completedActions } : {}), + }; + return Object.keys(details).length > 0 ? details : undefined; +} + +/** + * Return an opened tab only after its first navigation is reflected in its page revision. + * Chrome starts loading a created tab after `tabs.create` returns, and the queued `loading` + * event bumps the revision; returning before that hands the caller a revision that is stale + * on first use. A created http(s) tab waits until the event queue has processed its `complete` + * event, which Chrome delivers after that `loading` event; `tabs.get` status alone can report + * `complete` before the `loading` callback is delivered. The wait runs outside the global + * scheduler because the queued tab events take ownership transitions that `browser_open` + * would otherwise hold. + */ +async function settleOpenedTab( + taskId: string, + opened: Record, + created: boolean, +): Promise> { + const tabId = opened.tab_id; + if (typeof tabId !== "number") return opened; + let release!: () => void; + const loaded = new Promise((resolve) => { + release = resolve; + }); + const waiters = tabLoadWaiters.get(tabId) ?? new Set<() => void>(); + waiters.add(release); + tabLoadWaiters.set(tabId, waiters); + let cancelDeadline: () => void = () => undefined; + const deadline = new Promise((resolve) => { + const timer = setTimeout(resolve, OPEN_SETTLE_TIMEOUT_MS); + cancelDeadline = () => clearTimeout(timer); + }); + try { + const navigating = created && /^https?:/i.test(String(opened.url)); + const tab = navigating ? undefined : await chrome.tabs.get(tabId).catch(() => null); + if (!processedTabLoads.has(tabId) && (navigating || (tab && tab.status !== "complete"))) { + await Promise.race([loaded, deadline]); + } + // Tab events drain serially, so this barrier runs after any revision bump or removal already dispatched. + await Promise.race([new Promise((resolve) => runAfterStart(resolve)), deadline]); + const settled = await ownership.describeTab(tabId).catch(() => null); + if (!settled || (await ownership.taskIdForTab(tabId)) !== taskId) { + throw Object.assign(new Error("The opened tab was closed or left its task before it finished loading"), { + code: "ownership_revoked", + recovery: "Call browser_open again.", + }); + } + return settled; + } finally { + cancelDeadline(); + waiters.delete(release); + if (waiters.size === 0) tabLoadWaiters.delete(tabId); + } +} + async function reconcileOwnership(): Promise { const revokedTabIds = await ownership.reconcile(); await Promise.all(revokedTabIds.map((tabId) => handoff.cancelForTab(tabId))); @@ -599,7 +667,10 @@ chrome.tabs.onCreated.addListener((tab: { id?: number; openerTabId?: number }) = runAfterStart(() => ownership.adoptOwnedChild(tab)); }); chrome.tabs.onRemoved.addListener((removedTabId: number) => { + for (const releaseWaiter of tabLoadWaiters.get(removedTabId) ?? []) releaseWaiter(); runAfterStart(async () => { + // Queued after any `complete` handler for this tab, so a late handler cannot re-add it. + processedTabLoads.delete(removedTabId); const owner = await ownership.taskIdForTab(removedTabId); if (!owner && !browser.tracksTab(removedTabId)) return; await handoff.cancelForTab(removedTabId); @@ -617,21 +688,28 @@ chrome.tabs.onUpdated.addListener((updatedTabId, changeInfo) => { return; } runAfterStart(async () => { - const owner = await ownership.taskIdForTab(updatedTabId); - if (!owner) return; - if (changeInfo.status === "loading") { - scheduler.invalidateTab(updatedTabId); - await revisions.markNavigation(updatedTabId); - } - if ("groupId" in changeInfo) { - const revoked = await ownership.revokeIfMoved(updatedTabId); - if (revoked) { - await handoff.cancelForTab(updatedTabId); - await browser.detach(updatedTabId); + try { + const owner = await ownership.taskIdForTab(updatedTabId); + if (!owner) return; + if (changeInfo.status === "loading") { + scheduler.invalidateTab(updatedTabId); + await revisions.markNavigation(updatedTabId); + } + if ("groupId" in changeInfo) { + const revoked = await ownership.revokeIfMoved(updatedTabId); + if (revoked) { + await handoff.cancelForTab(updatedTabId); + await browser.detach(updatedTabId); + } + } + if (typeof changeInfo.url === "string" || changeInfo.status === "complete") { + await ownership.publishInventory(); + } + } finally { + if (changeInfo.status === "complete") { + processedTabLoads.add(updatedTabId); + for (const releaseWaiter of tabLoadWaiters.get(updatedTabId) ?? []) releaseWaiter(); } - } - if (typeof changeInfo.url === "string" || changeInfo.status === "complete") { - await ownership.publishInventory(); } }); }); diff --git a/packages/extension/src/browser.ts b/packages/extension/src/browser.ts index e0dd0a7..cdeaa61 100644 --- a/packages/extension/src/browser.ts +++ b/packages/extension/src/browser.ts @@ -76,6 +76,27 @@ function validateKeyboardAction(action: Record): void { throw Object.assign(new Error(invalid.agenttab_invalid_request), { code: "invalid_request" }); } } +/** + * Name the failing step of a multi-action batch and carry the results of the steps that ran. + * Earlier steps already changed the page, so a failure after them can never be `not_started`. + */ +function batchStepFailure( + error: unknown, + index: number, + actions: ReadonlyArray>, + completedActions: Array>, +): unknown { + if (!(error instanceof Error)) return error; + if (actions.length > 1) { + const completed = completedActions.length === 1 ? "1 earlier action" : `${completedActions.length} earlier actions`; + error.message = `Action ${index + 1} of ${actions.length} (${String(actions[index]?.kind)}) failed after ${completed} completed: ${error.message}`; + } + return Object.assign(error, { + failedActionIndex: index, + completedActions, + ...(completedActions.length > 0 ? { outcome: "unknown" } : {}), + }); +} const SENSITIVE_FIELD_CHECK = "const type=String(this.getAttribute&&this.getAttribute('type')||'').toLowerCase();const autocomplete=String(this.getAttribute&&this.getAttribute('autocomplete')||'').toLowerCase().split(/\\s+/);const text=node=>String(node&&((node.innerText??node.textContent)??'')||'').trim();const ids=String(this.getAttribute&&this.getAttribute('aria-labelledby')||'')+' '+String(this.getAttribute&&this.getAttribute('aria-describedby')||'');const associated=(this.labels?Array.from(this.labels):[]).map(text).filter(Boolean);const root=this.ownerDocument||document;const accessible=[this.getAttribute&&this.getAttribute('aria-label'),...ids.trim().split(/\\s+/).filter(Boolean).map(id=>text(root.getElementById(id)))].filter(value=>typeof value==='string'&&value.trim());const role=String(this.getAttribute&&this.getAttribute('role')||'').toLowerCase();const rawDescriptor=[this.getAttribute&&this.getAttribute('name'),this.id,this.getAttribute&&this.getAttribute('aria-label'),this.getAttribute&&this.getAttribute('title'),this.getAttribute&&this.getAttribute('placeholder'),...associated,...accessible].filter(Boolean).join(' ');const descriptor=rawDescriptor.replace(/([a-z])([A-Z])/g,'$1 $2').toLowerCase().replace(/[^a-z0-9]+/g,' ').trim();const compactDescriptor=descriptor.replace(/\\s+/g,'');const tag=String(this.tagName||'').toUpperCase();const editable=tag==='INPUT'||tag==='TEXTAREA'||tag==='SELECT'||this.isContentEditable===true||role==='textbox'||role==='combobox'||role==='spinbutton';const namedSecret=/\\b(password|otp|totp|mfa(?: code|token)?|2fa(?: code)?|(?:two|multi) factor (?:authentication )?(?:code|token)|one time (?:code|password)|verification code|authentication code|auth code|(?:re ?|h ?)?captcha|(?:cvv|cvc|cvn)\\d*|cid|security code|card security code|card verification (?:value|code|number)|card number|credit card number|cc number|card (?:expiration|expiry) (?:date|month|year)|bank account number|routing number|iban)\\b/.test(descriptor)||['password','onetimecode','onetimepassword','verificationcode','authenticationcode','authcode','totp','2fa','2facode','twofactorcode','twofactortoken','twofactorauthenticationcode','multifactorcode','multifactortoken','multifactorauthenticationcode','captcha','cid','securitycode','cardverificationvalue','cardverificationcode','cardverificationnumber','cardnumber','creditcardnumber','ccnumber','cardsecuritycode','cardexpirationdate','cardexpirationmonth','cardexpirationyear','cardexpirydate','cardexpirymonth','cardexpiryyear','bankaccountnumber','routingnumber'].some(term=>compactDescriptor.includes(term));const tokenSecret=/\\b(pin|passcode)\\b/.test(descriptor);const sensitiveField=type==='password'||autocomplete.some(token=>token==='current-password'||token==='new-password'||token==='one-time-code'||token==='webauthn'||token.startsWith('cc-'))||(editable&&(namedSecret||tokenSecret));"; const DOM_SENSITIVE_ACTION_PRELUDE = `${SENSITIVE_FIELD_CHECK}if(sensitiveField){return {agenttab_sensitive_field:true}}`; @@ -888,101 +909,107 @@ export class StandardBrowserRuntime { await this.requireFullAutomationRoute(tabId, "perform page actions"); } const completedActions: Array> = []; - for (const [index, action] of validated.entries()) { - if ( - index < validated.length - 1 && - (action.kind === "navigate" || - action.kind === "go_back" || - action.kind === "go_forward" || - action.kind === "reload" || - action.kind === "close") - ) { - throw Object.assign(new Error(`${String(action.kind)} must be the final action in a batch`), { - code: "invalid_request", - }); - } - await this.revisions.assertExpected(tabId, pageRevision); - const hasRef = typeof action.ref === "string"; - const hasSelector = typeof action.selector === "string"; - if ( - action.kind === "click" || - action.kind === "hover" || - action.kind === "double_click" || - action.kind === "context_click" || - action.kind === "type" || - action.kind === "fill" || - action.kind === "select" || - action.kind === "press" || - action.kind === "upload_file" - ) { - if (hasRef === hasSelector) { - throw Object.assign(new Error(`${action.kind} requires exactly one of ref or selector`), { + let stepIndex = 0; + try { + for (const [index, action] of validated.entries()) { + stepIndex = index; + if ( + index < validated.length - 1 && + (action.kind === "navigate" || + action.kind === "go_back" || + action.kind === "go_forward" || + action.kind === "reload" || + action.kind === "close") + ) { + throw Object.assign(new Error(`${String(action.kind)} must be the final action in a batch`), { code: "invalid_request", }); } - } - validateKeyboardAction(action); - const resolvedTarget = hasSelector || hasRef - ? await this.resolveTarget(tabId, { - taskId, - pageRevision, - ...(hasRef ? { ref: String(action.ref) } : { selector: String(action.selector) }), - ...(typeof action.frame_id === "string" ? { frameId: action.frame_id } : {}), - actionKind: String(action.kind), - }) - : undefined; - const stagedConsequence = await this.consequence(tabId, action, resolvedTarget); - if (stagedConsequence && !(await readState()).skipCommitReview) { - const staged: StagedCommit = { - native_token: randomToken(), - task_id: taskId, - tab_id: tabId, - page_revision: pageRevision, - effect: stagedConsequence.effect, - fingerprint: await this.stageFingerprint( - taskId, - tabId, - pageRevision, - action, - stagedConsequence.target, - ), - expires_at_ms: Date.now() + 300_000, - action: { action }, - preview: { - effect: stagedConsequence.effect, - kind: action.kind, - target: stagedConsequence.target, - ...(typeof action.ref === "string" ? { ref: action.ref } : {}), - ...(typeof action.selector === "string" ? { selector: action.selector } : {}), - ...(typeof action.frame_id === "string" ? { frame_id: action.frame_id } : {}), - ...(typeof action.key === "string" ? { key: action.key, modifiers: action.modifiers ?? [] } : {}), - ...(typeof action.prompt_text === "string" ? { prompt_text: action.prompt_text } : {}), - ...(action.expect_download === true ? { expect_download: true } : {}), - }, - ...(stagedConsequence.dialog !== undefined ? { dialog: stagedConsequence.dialog.binding } : {}), - }; - await mutateState((state) => { - if ( - stagedConsequence.dialog !== undefined && - this.sessions.get(tabId)?.dialog !== stagedConsequence.dialog.live - ) { - throw Object.assign(new Error("JavaScript dialog changed before it could be staged"), { + await this.revisions.assertExpected(tabId, pageRevision); + const hasRef = typeof action.ref === "string"; + const hasSelector = typeof action.selector === "string"; + if ( + action.kind === "click" || + action.kind === "hover" || + action.kind === "double_click" || + action.kind === "context_click" || + action.kind === "type" || + action.kind === "fill" || + action.kind === "select" || + action.kind === "press" || + action.kind === "upload_file" + ) { + if (hasRef === hasSelector) { + throw Object.assign(new Error(`${action.kind} requires exactly one of ref or selector`), { code: "invalid_request", }); } - state.stagedCommits[staged.native_token] = staged; - }); - return { - staged, - result: { + } + validateKeyboardAction(action); + const resolvedTarget = hasSelector || hasRef + ? await this.resolveTarget(tabId, { + taskId, + pageRevision, + ...(hasRef ? { ref: String(action.ref) } : { selector: String(action.selector) }), + ...(typeof action.frame_id === "string" ? { frameId: action.frame_id } : {}), + actionKind: String(action.kind), + }) + : undefined; + const stagedConsequence = await this.consequence(tabId, action, resolvedTarget); + if (stagedConsequence && !(await readState()).skipCommitReview) { + const staged: StagedCommit = { + native_token: randomToken(), + task_id: taskId, tab_id: tabId, - page_revision: await this.revisions.current(tabId), - actions: completedActions, - staged_index: index, - }, - }; + page_revision: pageRevision, + effect: stagedConsequence.effect, + fingerprint: await this.stageFingerprint( + taskId, + tabId, + pageRevision, + action, + stagedConsequence.target, + ), + expires_at_ms: Date.now() + 300_000, + action: { action }, + preview: { + effect: stagedConsequence.effect, + kind: action.kind, + target: stagedConsequence.target, + ...(typeof action.ref === "string" ? { ref: action.ref } : {}), + ...(typeof action.selector === "string" ? { selector: action.selector } : {}), + ...(typeof action.frame_id === "string" ? { frame_id: action.frame_id } : {}), + ...(typeof action.key === "string" ? { key: action.key, modifiers: action.modifiers ?? [] } : {}), + ...(typeof action.prompt_text === "string" ? { prompt_text: action.prompt_text } : {}), + ...(action.expect_download === true ? { expect_download: true } : {}), + }, + ...(stagedConsequence.dialog !== undefined ? { dialog: stagedConsequence.dialog.binding } : {}), + }; + await mutateState((state) => { + if ( + stagedConsequence.dialog !== undefined && + this.sessions.get(tabId)?.dialog !== stagedConsequence.dialog.live + ) { + throw Object.assign(new Error("JavaScript dialog changed before it could be staged"), { + code: "invalid_request", + }); + } + state.stagedCommits[staged.native_token] = staged; + }); + return { + staged, + result: { + tab_id: tabId, + page_revision: await this.revisions.current(tabId), + actions: completedActions, + staged_index: index, + }, + }; + } + completedActions.push(await this.performAction(taskId, tabId, pageRevision, action, resolvedTarget)); } - completedActions.push(await this.performAction(taskId, tabId, pageRevision, action, resolvedTarget)); + } catch (error) { + throw batchStepFailure(error, stepIndex, validated, completedActions); } return { result: { diff --git a/packages/extension/src/ownership.ts b/packages/extension/src/ownership.ts index 8836fd2..6a6d60a 100644 --- a/packages/extension/src/ownership.ts +++ b/packages/extension/src/ownership.ts @@ -307,7 +307,7 @@ export class OwnershipLedger { await chrome.tabs.remove(createdTabId).catch(() => undefined); throw error; } - return this.tabResult(createdTabId); + return this.describeTab(createdTabId); } private async adoptActiveNow(taskId: string): Promise> { @@ -324,7 +324,7 @@ export class OwnershipLedger { await chrome.action.setBadgeText({ tabId, text: "✦" }).catch(() => undefined); setTimeout(() => void chrome.action.setBadgeText({ tabId, text: "" }).catch(() => undefined), 2000); } - return this.tabResult(tabId); + return this.describeTab(tabId); } private async adoptOwnedChildNow(tab: TabLike, sourceTabId?: number): Promise { @@ -735,7 +735,8 @@ export class OwnershipLedger { return tab; } - private async tabResult(tabId: number): Promise> { + /** The `browser_open` result for an owned tab, read from Chrome and the revision ledger now. */ + async describeTab(tabId: number): Promise> { const tab = (await chrome.tabs.get(tabId)) as TabLike; const state = await readState(); const owner = Object.values(state.tasks).find((task) => task.tabIds.includes(tabId)); diff --git a/packages/extension/test/extension.test.ts b/packages/extension/test/extension.test.ts index 48dcbe3..8df9778 100644 --- a/packages/extension/test/extension.test.ts +++ b/packages/extension/test/extension.test.ts @@ -79,6 +79,7 @@ let debuggerCalls: string[]; let tabStore: Map; let removedTabIds: number[]; let failGrouping: boolean; +let autoCompleteCreatedTabs: boolean; let nextTabId: number; let nextGroupId: number; let scriptResult: unknown; @@ -329,6 +330,7 @@ function installChromeMock(): void { tabStore = new Map(); removedTabIds = []; failGrouping = false; + autoCompleteCreatedTabs = true; nextTabId = 100; nextGroupId = 50; scriptResult = true; @@ -523,11 +525,18 @@ function installChromeMock(): void { return clone(tabs); }, async create(options: Record) { - return clone(createTab( + const tab = createTab( typeof options.url === "string" ? options.url : "about:blank", options.active === true, typeof options.windowId === "number" ? options.windowId : 1, - )); + ); + // Chrome delivers the created tab's load events after `tabs.create` resolves. + if (autoCompleteCreatedTabs) { + queueMicrotask(() => { + for (const listener of tabUpdatedListeners) listener(tab.id, { status: "complete" }); + }); + } + return clone(tab); }, async move(tabIds: number | number[], options: { windowId: number; index: number }) { const moved: MockTab[] = []; @@ -3376,6 +3385,31 @@ describe("page revision monotonicity", () => { ).rejects.toMatchObject({ code: "action_failed" }); }); + test("names the failing batch step and returns the results of the steps that ran", async () => { + const revisions = new RevisionTracker(); + const runtime = new StandardBrowserRuntime(revisions, async () => undefined, () => undefined, async () => undefined); + const pageRevision = await revisions.ensure(64); + + const failure = await runtime.act(TASK_A, 64, pageRevision, [ + { kind: "scroll", delta_x: 0, delta_y: 100 }, + { kind: "click" }, + ]).then(() => null, (error: unknown) => error); + expect(failure).toMatchObject({ + code: "invalid_request", + message: "Action 2 of 2 (click) failed after 1 earlier action completed: click requires exactly one of ref or selector", + failedActionIndex: 1, + completedActions: [{ kind: "scroll" }], + outcome: "unknown", + }); + + const firstStepFailure = await runtime.act(TASK_A, 64, pageRevision, [ + { kind: "click" }, + { kind: "scroll", delta_x: 0, delta_y: 100 }, + ]).then(() => null, (error: unknown) => error); + expect(firstStepFailure).toMatchObject({ failedActionIndex: 0, completedActions: [] }); + expect(firstStepFailure).not.toHaveProperty("outcome"); + }); + test("advances a tab revision past its generation floor after revocation and re-adoption", async () => { await seedTask(TASK_A, [62]); const tab = tabStore.get(62); @@ -5939,5 +5973,72 @@ describe("extension entrypoint admission boundaries", () => { expect(debuggerAttachedTabIds.has(101)).toBe(false); expect((await readState()).automationCleanup.tabIds).toEqual([]); expect((await readState()).tasks[TASK_B]).toMatchObject({ groupId: null, tabIds: [] }); + + // Chrome can report a created tab as complete before it delivers the initial `loading` event; + // open reports the revision only after the queue has processed that load's `complete` event. + autoCompleteCreatedTabs = false; + const loadingOpen = sendNativeCommand( + "018f47b8-2f80-7c20-9c77-f8a38c9e6311", + TASK_C, + "browser_open", + { mode: "create", url: "https://example.test/slow" }, + ); + await waitForCondition(() => JSON.stringify(port.posted).includes('"tab_id":103')); + await flushPromiseQueue(); + expect(JSON.stringify(port.posted)).not.toContain("018f47b8-2f80-7c20-9c77-f8a38c9e6311"); + const loadingTab = tabStore.get(103); + if (!loadingTab) throw new Error("loading task tab is unavailable"); + loadingTab.url = "https://example.test/slow/landed"; + for (const listener of tabUpdatedListeners) listener(103, { status: "loading" }); + for (const listener of tabUpdatedListeners) listener(103, { status: "complete" }); + expect(await loadingOpen).toMatchObject({ + outcome: "completed", + result: { tab_id: 103, page_revision: 2, url: "https://example.test/slow/landed" }, + }); + + // A tab closed while its first load is pending fails the open instead of reporting a dead tab. + const closedOpen = sendNativeCommand( + "018f47b8-2f80-7c20-9c77-f8a38c9e6312", + TASK_C, + "browser_open", + { mode: "create", url: "https://example.test/abandoned" }, + ); + await waitForCondition(() => JSON.stringify(port.posted).includes('"tab_id":104')); + tabStore.delete(104); + for (const listener of tabRemovedListeners) listener(104); + expect(await closedOpen).toMatchObject({ + outcome: "not_started", + error: { code: "ownership_revoked", recovery: "Call browser_open again." }, + }); + + // A load that never completes releases the open at the deadline with the tab's current state. + const stuckRequestId = "018f47b8-2f80-7c20-9c77-f8a38c9e6313"; + const stuckResponse = () => port.posted.find((message) => + isRecord(message) && message.kind === "response" && message.request_id === stuckRequestId); + vi.useFakeTimers(); + try { + port.receive({ + protocol: "agenttab.native", + version: 1, + kind: "command", + request_id: stuckRequestId, + task_id: TASK_C, + connection_id: NATIVE_CONNECTION_ID, + method: "browser_open", + params: { mode: "create", url: "https://example.test/stuck" }, + }); + for (let turn = 0; turn < 20 && !JSON.stringify(port.posted).includes('"tab_id":105'); turn += 1) { + await advanceTimers(0); + } + await advanceTimers(9_500); + expect(stuckResponse()).toBeUndefined(); + await advanceTimers(1_000); + expect(stuckResponse()).toMatchObject({ + outcome: "completed", + result: { tab_id: 105, page_revision: 1, url: "https://example.test/stuck" }, + }); + } finally { + vi.useRealTimers(); + } }); });