Return a current revision from browser_open and report batch progress on act failures

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.

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. |

packages/extension/src/background.ts

@@ -33,6 +33,10 @@ let automationCleanupPending = false;
 let automationCleanupDelayMs = AUTOMATION_CLEANUP_RETRY_BASE_MS;
 let automationCleanupTimer: ReturnType<typeof setTimeout> | undefined;
 let automationCleanupQueue = Promise.resolve();
+const OPEN_SETTLE_TIMEOUT_MS = 10_000;
+const tabLoadWaiters = new Map<number, Set<() => void>>();
+/** Tabs whose `complete` event the tab-event queue has processed; removal prunes it to open tabs. */
+const processedTabLoads = new Set<number>();
 
 const PRE_DISPATCH_ERRORS: Record<string, true> = {
   invalid_request: true,
@@ -388,7 +392,11 @@ async function dispatch(command: NativeDispatchCommand): Promise<NativeResponse>
       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<NativeResponse>
       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<void> = () => undefined):
   startupOperations.enqueue(operation);
 }
 
+function errorDetails(error: unknown): Record<string, unknown> | 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<string, unknown>,
+  created: boolean,
+): Promise<Record<string, unknown>> {
+  const tabId = opened.tab_id;
+  if (typeof tabId !== "number") return opened;
+  let release!: () => void;
+  const loaded = new Promise<void>((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<void>((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<void>((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<void> {
   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();
     }
   });
 });