From c109f52048211c3cda2f28ffde01600ca7e25abe Mon Sep 17 00:00:00 2001 From: sheehanmunim Date: Thu, 8 Oct 2026 17:45:13 -0400 Subject: [PATCH 1/2] fix(browser): own tabs the agent's clicks open; make Tab move focus A target=_blank link or window.open popup the agent triggers now belongs to the task that triggered it: a tab whose opener is one of the task's tabs, created within 10 s of an agent click or key press there, joins the task's tabs and group and is closed by its cleanup. Tabs opened from an agent tab without a recent agent action stay the user's. (#14) browser_press_key Tab dispatched the keystroke, but Chrome runs focus traversal only in a focused page, and a background agent tab never is. Focus stayed put, the tool reported success, and the next browser_type landed in the original field. When the keystroke does not move focus and the page did not handle it, move focus to the next element in sequential focus order; report where focus went, and fail when it cannot move. (#20) --- chrome-extension/background.js | 167 +++++++++++++++++++++++++-- chrome-extension/background.test.mjs | 92 +++++++++++++++ macos/Sources/main.swift | 9 +- windows-linux/src/browser.rs | 25 +++- 4 files changed, 276 insertions(+), 17 deletions(-) diff --git a/chrome-extension/background.js b/chrome-extension/background.js index 65e9fa2..011f7ea 100644 --- a/chrome-extension/background.js +++ b/chrome-extension/background.js @@ -321,15 +321,65 @@ async function openTab(clientId, url) { tabOwner.set(tab.id, clientId); await ensureGroup(clientId, tab.id); await persistOwnedState(); - // Pages replace their favicon on load (Spotify, YouTube, …). Re-apply the - // pointer whenever the document finishes, and also when the tab's own icon - // changes, so the strip stays on the agent cursor rather than the site logo. + keepBadged(tab.id); + return { tabId: tab.id, url: tab.url, title: tab.title, clientId }; +} + +/// Pages replace their favicon on load (Spotify, YouTube, …). Re-apply the +/// pointer whenever the document finishes, and also when the tab's own icon +/// changes, so the strip stays on the agent cursor rather than the site logo. +function keepBadged(tabId) { chrome.tabs.onUpdated.addListener(function badge(id, info) { - if (id !== tab.id) return; - if (info.status === "complete" || info.favIconUrl) markTab(tab.id); - if (!tabOwner.has(tab.id)) chrome.tabs.onUpdated.removeListener(badge); + if (id !== tabId) return; + if (info.status === "complete" || info.favIconUrl) markTab(tabId); + if (!tabOwner.has(tabId)) chrome.tabs.onUpdated.removeListener(badge); }); - return { tabId: tab.id, url: tab.url, title: tab.title, clientId }; +} + +/** + * When the agent last clicked or pressed a key in a tab, by tab id. A tab that + * opens from one of the agent's tabs shortly after such an action was opened by + * the agent (a target=_blank link, a window.open popup), so it belongs to that + * task. Without the time bound, a link the user clicks in a tab the agent holds + * would be claimed too — and then closed by the agent's cleanup. + * + * @type {Map} + */ +const agentInputAt = new Map(); +const CHILD_TAB_WINDOW_MS = 10_000; + +function noteAgentInput(clientId, tabId) { + agentInputAt.set(tabId, { clientId, at: Date.now() }); +} + +/// Take ownership of a tab the agent's own action opened. It is agent-created, +/// not adopted, so the task's cleanup closes it like any tab it opened itself. +function claimChildTab(tab) { + const openerId = tab.openerTabId; + if (typeof tab.id !== "number" || typeof openerId !== "number") return; + if (tabOwner.has(tab.id)) return; + const input = agentInputAt.get(openerId); + if (!input || Date.now() - input.at > CHILD_TAB_WINDOW_MS) return; + const clientId = tabOwner.get(openerId); + // The opener changed hands (released, closed) since the action. + if (clientId === undefined || clientId !== input.clientId) return; + // Ownership is recorded now, synchronously, so a list_tabs or cleanup that is + // already queued sees the child; grouping waits its turn in the task's queue. + const state = clientState(clientId); + state.tabs.add(tab.id); + tabOwner.set(tab.id, clientId); + void persistOwnedState(); + keepBadged(tab.id); + void enqueue(clientId, async () => { + if (tabOwner.get(tab.id) !== clientId) return; + try { + await ensureGroup(clientId, tab.id); + } catch { + // A popup window cannot hold a tab group. The tab is still the task's, + // and cleanup closes it wherever it is. + await markTab(tab.id).catch(() => {}); + } + }).catch(() => {}); } async function listTabs(clientId, all = false) { @@ -990,18 +1040,106 @@ async function typeText(tabId, text) { return { typed: text.length }; } +/// Watch the next Tab keydown, so the follow-up can tell a page that handled +/// the key itself (an editor indenting, a widget trapping focus) from one where +/// nothing happened. +const TAB_PROBE_JS = `(() => { + const probe = { before: document.activeElement }; + probe.listener = (e) => { + if (e.key !== 'Tab') return; + probe.event = e; + }; + addEventListener('keydown', probe.listener, true); + window.__cuTabProbe = probe; + return true; +})()`; + +/// Finish a Tab press. Chrome runs its own focus traversal only in a page that +/// has focus, and an agent tab in the background never does: the keystroke +/// reaches the page's handlers and then focus stays put, so the next type lands +/// in the field the agent meant to leave. When that happens, move focus to the +/// next element in sequential focus order the way the browser would have. +const TAB_FINISH_JS = `(() => { + const probe = window.__cuTabProbe; + delete window.__cuTabProbe; + if (probe) removeEventListener('keydown', probe.listener, true); + const name = (el) => { + if (!el || el === document.body || el === document.documentElement) return null; + const label = el.getAttribute('aria-label') || el.labels?.[0]?.innerText || + el.getAttribute('placeholder') || el.getAttribute('name') || el.getAttribute('title') || + (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA' ? '' : el.innerText) || el.id || ''; + const index = el.getAttribute('data-cu-idx'); + return (index !== null ? '[' + index + '] ' : '') + el.tagName.toLowerCase() + + (label ? ' "' + String(label).trim().replace(/\\s+/g, ' ').slice(0, 60) + '"' : ''); + }; + const before = probe ? probe.before : null; + const active = document.activeElement; + if (probe && active !== before) return { moved: true, focused: name(active) }; + if (probe?.event?.defaultPrevented) return { moved: false, handled: true, focused: name(active) }; + if (active && (active.tagName === 'IFRAME' || active.tagName === 'FRAME' || active.shadowRoot)) { + return { moved: false, reason: 'focus is inside a frame or component the extension cannot step through' }; + } + const shown = (el) => { + if (el.disabled || el.closest('[inert]') || !el.getClientRects().length) return false; + const style = getComputedStyle(el); + return style.visibility !== 'hidden' && style.visibility !== 'collapse'; + }; + // A radio group is one stop: its checked button, or its first if none is. + const radioStop = (el) => { + if (el.type !== 'radio' || !el.name) return true; + const scope = el.form || document; + const group = Array.from(scope.querySelectorAll('input[type=radio]')) + .filter((r) => r.name === el.name && shown(r)); + const checked = group.find((r) => r.checked); + return checked ? checked === el : group[0] === el; + }; + const sel = 'a[href], area[href], button, input:not([type=hidden]), select, textarea, iframe, ' + + 'summary, [contenteditable]:not([contenteditable=false]), [tabindex]'; + const stops = Array.from(document.querySelectorAll(sel)) + .filter((el) => el.tabIndex >= 0 && shown(el) && radioStop(el)); + const order = stops.filter((el) => el.tabIndex > 0).sort((a, b) => a.tabIndex - b.tabIndex) + .concat(stops.filter((el) => el.tabIndex === 0)); + const at = order.indexOf(active); + let next; + if (at >= 0) next = order[at + 1]; + else if (active && active !== document.body && active !== document.documentElement) { + next = order.find((el) => active.compareDocumentPosition(el) & Node.DOCUMENT_POSITION_FOLLOWING); + } else next = order[0]; + if (!next) return { moved: false, reason: 'focus is already on the last element of the page' }; + next.focus(); + if (document.activeElement !== next) return { moved: false, reason: 'the next element refused focus' }; + // Tabbing into a text field selects its contents, as it does for a person. + if (next.tagName === 'TEXTAREA' || (next.tagName === 'INPUT' && typeof next.select === 'function' && + /^(text|search|url|tel|email|password|number)$/.test(next.type))) { + try { next.select(); } catch {} + } + return { moved: true, focused: name(next) }; +})()`; + async function pressKey(tabId, key) { const map = { - Enter: { windowsVirtualKeyCode: 13, key: "Enter", text: "\r" }, - Tab: { windowsVirtualKeyCode: 9, key: "Tab" }, - Escape: { windowsVirtualKeyCode: 27, key: "Escape" }, - Backspace: { windowsVirtualKeyCode: 8, key: "Backspace" }, + Enter: { windowsVirtualKeyCode: 13, code: "Enter", key: "Enter", text: "\r" }, + Tab: { windowsVirtualKeyCode: 9, code: "Tab", key: "Tab" }, + Escape: { windowsVirtualKeyCode: 27, code: "Escape", key: "Escape" }, + Backspace: { windowsVirtualKeyCode: 8, code: "Backspace", key: "Backspace" }, }; const spec = map[key]; if (!spec) throw new Error(`unsupported key: ${key}`); + if (key === "Tab") await send(tabId, "Runtime.evaluate", { expression: TAB_PROBE_JS, returnByValue: true }); await send(tabId, "Input.dispatchKeyEvent", { type: "keyDown", ...spec }); await send(tabId, "Input.dispatchKeyEvent", { type: "keyUp", ...spec }); - return { pressed: key }; + if (key !== "Tab") return { pressed: key }; + const res = await send(tabId, "Runtime.evaluate", { expression: TAB_FINISH_JS, returnByValue: true }); + const focus = res?.result?.value || {}; + // Reporting success when focus stayed put sent the next type into the wrong + // field, so an unmoved Tab is an error the agent can act on. + if (!focus.moved && !focus.handled) { + throw new Error( + `Tab did not move focus (${focus.reason || "the page did not respond"}) — ` + + "click the field you want with browser_click instead", + ); + } + return { pressed: key, focused: focus.focused ?? null, handledByPage: focus.handled === true }; } async function screenshot(tabId) { @@ -1690,6 +1828,7 @@ const handlers = { const clientId = requireClientId(p); assertOwned(clientId, p.tabId); await checkTab(clientId, p.tabId, p.sites, "click in"); + noteAgentInput(clientId, p.tabId); const result = p.index !== undefined ? await clickElement(p.tabId, p.index) : await clickAt(p.tabId, p.x, p.y); return withState(p, result); }, @@ -1705,6 +1844,7 @@ const handlers = { const clientId = requireClientId(p); assertOwned(clientId, p.tabId); await checkTab(clientId, p.tabId, p.sites, "press keys in"); + noteAgentInput(clientId, p.tabId); return withState(p, await pressKey(p.tabId, p.key)); }, screenshot: async (p) => { @@ -1758,8 +1898,11 @@ async function handleCommand(msg, replyPort) { } } +chrome.tabs.onCreated.addListener(claimChildTab); + chrome.tabs.onRemoved.addListener((tabId) => { siteFavicons.delete(tabId); + agentInputAt.delete(tabId); if (!tabOwner.has(tabId) && !attached.has(tabId)) return; const clientId = tabOwner.get(tabId); if (clientId) { diff --git a/chrome-extension/background.test.mjs b/chrome-extension/background.test.mjs index 8493899..f711f6a 100644 --- a/chrome-extension/background.test.mjs +++ b/chrome-extension/background.test.mjs @@ -48,6 +48,15 @@ let sent = []; /** Tabs the extension attached the debugger to, in order. */ const attaches = []; let debuggerDetachListener = null; +let tabCreatedListener = null; + +/** A tab the page opened (target=_blank, window.open), as Chrome reports it. */ +function openChild(openerTabId, url = "https://shop.example/terms") { + const child = makeTab({ url }); + child.openerTabId = openerTabId; + tabCreatedListener(child); + return child.id; +} // Most tests drive the MT Code host's port; the multi-host tests pick one. const nativeListener = (msg) => portFor(MT_HOST).onMessageListener(msg); @@ -120,6 +129,7 @@ const chrome = { ungroup: async (ids) => ids.forEach((id) => { tabs.get(id).groupId = -1; }), onUpdated: { addListener() {}, removeListener() {} }, onRemoved: { addListener() {} }, + onCreated: { addListener: (fn) => { tabCreatedListener = fn; } }, }, tabGroups: { get: async (id) => { @@ -946,6 +956,88 @@ test("credentials refuse an index that is not a text field", async () => { pageEval = () => ({ result: { value: {} } }); }); +// ── tabs the agent's own actions open ────────────────────────────────────── + +test("a tab the agent's click opens belongs to the task and is cleaned up with it", async () => { + const parent = (await call("open_tab", { url: "https://shop.example/help", sessionId: "child" })).tabId; + pageEval = () => ({ result: { value: { ok: true, tag: "a", x: 5, y: 5 } } }); + await callAndWait("click", { tabId: parent, index: 1, sessionId: "child" }); + pageEval = () => ({ result: { value: {} } }); + const child = openChild(parent); + const listed = await call("list_tabs", { sessionId: "child" }); + const row = listed.tabs.find((tab) => tab.tabId === child); + assert.ok(row, "the child is one of the task's tabs"); + assert.equal(row.adopted, false, "it is agent-created, not adopted"); + await call("ping", { sessionId: "child" }); + assert.equal(tabs.get(child).groupId, tabs.get(parent).groupId, "it joins the task's group"); + const cleaned = await call("close_all_tabs", { sessionId: "child" }); + assert.equal(cleaned.closed, 2); + assert.ok(!tabs.has(child), "cleanup closed the child"); +}); + +test("a tab opened from an agent tab with no recent agent action stays the user's", async () => { + const parent = (await call("open_tab", { url: "https://shop.example/help", sessionId: "idle" })).tabId; + const child = openChild(parent); + const listed = await call("list_tabs", { sessionId: "idle" }); + assert.ok(!listed.tabs.some((tab) => tab.tabId === child), "the user's own click is not claimed"); + await call("close_all_tabs", { sessionId: "idle" }); + assert.ok(tabs.has(child), "cleanup left the user's tab open"); + tabs.delete(child); +}); + +test("a child of another task's tab is not claimed by this one", async () => { + const parent = (await call("open_tab", { url: "https://shop.example/help", sessionId: "mine" })).tabId; + pageEval = () => ({ result: { value: { ok: true, tag: "a", x: 5, y: 5 } } }); + await callAndWait("click", { tabId: parent, index: 1, sessionId: "mine" }); + pageEval = () => ({ result: { value: {} } }); + const child = openChild(parent); + const theirs = await call("list_tabs", { sessionId: "theirs" }); + assert.ok(!theirs.tabs.some((tab) => tab.tabId === child)); + await call("close_all_tabs", { sessionId: "mine" }); +}); + +// ── Tab key ──────────────────────────────────────────────────────────────── + +let formTab; +test("a fresh agent tab for the Tab key tests", async () => { + formTab = (await call("open_tab", { url: "https://console.example/branding" })).tabId; +}); + +test("Tab in a background tab moves focus itself and says where it went", async () => { + const keys = []; + pageEval = (method, params) => { + if (method === "Input.dispatchKeyEvent") keys.push(params.type); + if ((params.expression ?? "").includes("__cuTabProbe;")) { + return { result: { value: { moved: true, focused: '[7] input "Privacy policy link"' } } }; + } + return { result: { value: {} } }; + }; + const result = await callAndWait("press", { tabId: formTab, key: "Tab" }); + assert.deepEqual([...keys], ["keyDown", "keyUp"], "the page still sees the keystroke"); + assert.equal(result.focused, '[7] input "Privacy policy link"'); + pageEval = () => ({ result: { value: {} } }); +}); + +test("a Tab that moves nothing is an error, not a success", async () => { + pageEval = (method, params) => + (params.expression ?? "").includes("__cuTabProbe;") + ? { result: { value: { moved: false, reason: "focus is already on the last element of the page" } } } + : { result: { value: {} } }; + const error = await refuses("press", { tabId: formTab, key: "Tab" }); + assert.match(error, /Tab did not move focus \(focus is already on the last element/); + pageEval = () => ({ result: { value: {} } }); +}); + +test("a Tab the page handled itself is a success", async () => { + pageEval = (method, params) => + (params.expression ?? "").includes("__cuTabProbe;") + ? { result: { value: { moved: false, handled: true, focused: "textarea" } } } + : { result: { value: {} } }; + const result = await callAndWait("press", { tabId: formTab, key: "Tab" }); + assert.equal(result.handledByPage, true); + pageEval = () => ({ result: { value: {} } }); +}); + // ── runner ────────────────────────────────────────────────────────────────── let failed = 0; diff --git a/macos/Sources/main.swift b/macos/Sources/main.swift index a14e909..63ef70d 100644 --- a/macos/Sources/main.swift +++ b/macos/Sources/main.swift @@ -2777,7 +2777,14 @@ func toolBrowserPressKey(_ args: [String: Any]) -> String { guard let tabId = args["tab_id"] as? Int else { return "error: missing required argument 'tab_id'" } guard let key = args["key"] as? String else { return "error: missing required argument 'key'" } return bridgeText(browserSessionCall("press", withReturnState(["tabId": tabId, "key": key], args), args: args)) { payload in - withPageAfter("pressed \(key) in tab \(tabId)", payload) + var line = "pressed \(key) in tab \(tabId)" + // Tab says where focus went, so the next type is not a guess. + if payload["handledByPage"] as? Bool == true { + line += " — the page handled it itself" + } else if let focused = payload["focused"] as? String { + line += " — focus is now on \(focused)" + } + return withPageAfter(line, payload) } } diff --git a/windows-linux/src/browser.rs b/windows-linux/src/browser.rs index a492b19..5305207 100644 --- a/windows-linux/src/browser.rs +++ b/windows-linux/src/browser.rs @@ -774,10 +774,19 @@ fn describe(command: &str, result: &Value, args: &Value) -> String { "typed {} characters into tab {tab}", args.get("text").and_then(Value::as_str).unwrap_or("").chars().count() ), - "press" => format!( - "pressed {} in tab {tab}", - args.get("key").and_then(Value::as_str).unwrap_or("?") - ), + "press" => { + let mut line = format!( + "pressed {} in tab {tab}", + args.get("key").and_then(Value::as_str).unwrap_or("?") + ); + // Tab says where focus went, so the next type is not a guess. + if result.get("handledByPage").and_then(Value::as_bool) == Some(true) { + line.push_str(" — the page handled it itself"); + } else if let Some(focused) = result.get("focused").and_then(Value::as_str) { + line.push_str(&format!(" — focus is now on {focused}")); + } + line + } "navigate" => format!( "navigated tab {tab} to {}", args.get("url").and_then(Value::as_str).unwrap_or("") @@ -1342,6 +1351,14 @@ mod tests { describe("press", &json!({}), &json!({ "tab_id": 9, "key": "Enter" })), "pressed Enter in tab 9" ); + assert_eq!( + describe( + "press", + &json!({ "pressed": "Tab", "focused": "[7] input \"Privacy policy\"" }), + &json!({ "tab_id": 9, "key": "Tab" }) + ), + "pressed Tab in tab 9 — focus is now on [7] input \"Privacy policy\"" + ); assert_eq!( describe("type", &json!({}), &json!({ "tab_id": 9, "text": "hello" })), "typed 5 characters into tab 9" From aa1d3cf3bbedc4b62b9893a1b3e1644051b8fcc3 Mon Sep 17 00:00:00 2001 From: sheehanmunim Date: Thu, 8 Oct 2026 17:53:49 -0400 Subject: [PATCH 2/2] fix: follow-ups to the browser state contract (#21) - Snapshot modal scoping kept only controls inside the dialog, so a select or combobox whose options render in a portal at the end of (Radix, MUI, Headless UI) lost every option. Roots the modal's controls point at via aria-controls/aria-owns, open popovers, and controls painted on top of the modal stay in scope. - wait_for_selector refuses selectors that test the value attribute. React mirrors controlled inputs' values there, password fields included, so met/timeout answered a password one guessed character at a time. Escapes and comments are undone before the check. - null for an optional argument means absent again, as it did before offset/limit/wait_for_selector existed. - A transient evaluate error (a navigation destroying the context) polls again until the deadline instead of ending the wait. - When an extension older than the server ignores the new arguments, both hosts say so and ask for a reload instead of printing nothing. - The native-host relay exits on backend EOF on Linux too (#16): it blocks on Chrome's stdin the same way the Windows relay did. - e2e: click a prompt's button after the evaluation has answered, since the click closes the window; report the live targets when no prompt answers. - browser_click / browser_press_key / browser_close_all_tabs descriptions cover child tabs and Tab's focus report. --- README.md | 15 ++++-- chrome-extension/background.js | 68 ++++++++++++++++++++++++++-- chrome-extension/background.test.mjs | 15 ++++-- macos/Sources/main.swift | 15 ++++-- scripts/e2e-browser.mjs | 16 +++++-- scripts/snapshot-dom.test.mjs | 12 +++++ windows-linux/src/browser.rs | 22 +++++++-- windows-linux/src/tools.rs | 6 +-- 8 files changed, 139 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 9db546f..2eb0209 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ Also looked at: [mediar-ai/mcp-server-macos-use](https://github.com/mediar-ai/mc - **Coordinates that land.** Every screenshot and zoom carries its screen origin and pixels-per-point. `zoom` captures any region at full physical resolution. - **Your browser, your logins.** The Chrome extension gives the agent its own labelled tab group in your signed-in Chrome, and leaves your tabs alone unless you point it at one. - **Or the tab you already have open.** `browser_list_tabs all=true` shows every tab in the browser and `browser_use_tab` takes one over in place — useful when the page is already signed in or mid-flow and re-opening the URL would throw that away. An adopted tab is not moved into the agent's group, not activated and not reloaded; cleanup releases it rather than closing it, and `browser_release_tab` hands it back early. +- **Tabs the agent's clicks open are the agent's too.** A `target=_blank` link or popup opened by an agent click or key press within 10 seconds joins that task's tabs and group, and its cleanup closes it. A tab opened from an agent tab without a recent agent action stays yours. - **Parallel tasks, one extension.** Every MCP process gets its own tab group, and one process can run several tasks by passing a stable `session_id` on its browser calls. A task cannot drive or adopt another task's tabs, and cleanup (including a process exiting) closes only its own. Any number of MCP processes share the one extension: the first owns it and the rest go through it, and if the owner exits another takes over without closing anyone's tabs. Tasks share Chrome's cookies and logins, and desktop apps and the clipboard are not isolated. - **Model-agnostic.** No vision model is required for interaction; local models work too. - **Look → act → verify.** `hover` for mouse-over menus, `wait` for loads, `query` to find a control by label without reading a whole tree. @@ -220,7 +221,9 @@ This is not a guarantee of clickability or lack of occlusion. Frames and shadow roots are not searched. Choose a selector specific to the **new expected UI**; an element already present can satisfy it immediately. No arbitrary page script -is accepted. +is accepted, and a selector may not test the `value` attribute: React mirrors a +controlled input's value there, password fields included, so a wait could +otherwise confirm a password one guessed character at a time. - `wait_timeout_ms`: defaults to `8000`, accepts integers `0`–`10000`; `0` checks once. Requires `wait_for_selector`. The budget covers polling, not browser/API @@ -248,8 +251,9 @@ Send this to `browser_snapshot`, inspect its readiness and fresh indices, then choose the next action. A timeout or tool error does not roll back a click, submission, typing, or navigation. Use current state to decide whether a retry is safe. Invalid wait types/bounds or an action wait without `return_state: true` are -rejected before acting. These options require an updated extension as well as the -native server; older extensions may omit readiness metadata. +rejected before acting. `null` for an optional argument means the same as leaving +it out. These options require an updated extension as well as the native server; +when an older extension ignores them, the result says so and asks for a reload. Regression checks: `node --test chrome-extension/background.test.mjs` covers the wait contract and site-policy guards. The real-browser CI harness @@ -267,7 +271,10 @@ node scripts/snapshot-dom.test.mjs --chrome /path/to/chrome --source original-ba ``` `browser_snapshot` automatically scopes to the topmost visible dialog when one is -open, so covered background controls cannot consume the dialog's budget. Hidden +open, so covered background controls cannot consume the dialog's budget. A +dialog's portalled popups stay in scope: elements its controls point at through +`aria-controls`/`aria-owns`, open popovers, and controls painted on top of the +dialog, such as a select's options rendered at the end of ``. Hidden and inert subtrees are excluded; reachable offscreen controls remain available. Native modeless dialogs and explicit `aria-modal="false"` do not scope the page. Native modal dialogs and explicit `aria-modal="true"` are definitive candidates. diff --git a/chrome-extension/background.js b/chrome-extension/background.js index 011f7ea..4b97c3f 100644 --- a/chrome-extension/background.js +++ b/chrome-extension/background.js @@ -677,9 +677,39 @@ const SNAPSHOT_JS = `((options = {}) => { const r = dialog.getBoundingClientRect(); return r.right > 0 && r.bottom > 0 && r.left < innerWidth && r.top < innerHeight; }) || null; - const scope = modal || document; - for (const el of scope.querySelectorAll(sel)) { - if (!visible(el)) continue; + // A select, combobox or menu inside the modal often renders its options in a + // portal at the end of , outside the dialog. Those stay in scope: roots + // the modal's controls point at (aria-controls/aria-owns), open popovers, and + // any control painted on top of the modal: inside the modal's rectangle, and + // the topmost element at its own centre. + const floating = []; + if (modal) { + for (const owner of modal.querySelectorAll('[aria-controls],[aria-owns]')) { + const ids = ((owner.getAttribute('aria-controls') || '') + ' ' + (owner.getAttribute('aria-owns') || '')).split(/\s+/); + for (const id of ids) { + const root = id && document.getElementById(id); + if (root && !modal.contains(root)) floating.push(root); + } + } + try { + for (const pop of document.querySelectorAll(':popover-open')) { + if (!modal.contains(pop) && !pop.contains(modal)) floating.push(pop); + } + } catch {} + } + const bounds = modal ? modal.getBoundingClientRect() : null; + const inScope = (el) => { + if (!modal || modal.contains(el)) return true; + if (floating.some((root) => root.contains(el))) return true; + const r = el.getBoundingClientRect(); + const x = r.left + r.width / 2, y = r.top + r.height / 2; + if (x < Math.max(0, bounds.left) || y < Math.max(0, bounds.top) || + x >= Math.min(innerWidth, bounds.right) || y >= Math.min(innerHeight, bounds.bottom)) return false; + const hit = document.elementFromPoint(x, y); + return !!hit && (hit === el || el.contains(hit)); + }; + for (const el of document.querySelectorAll(sel)) { + if (!inScope(el) || !visible(el)) continue; const index = i++; // Tag the full selected scope so indices stay global and earlier pages remain // actionable when paging an unchanged UI. Only label/serialize this page. @@ -1322,12 +1352,33 @@ function validateReadiness(p, action = false) { if (typeof p.waitForSelector !== "string" || !p.waitForSelector.trim() || p.waitForSelector.length > 2000) { throw new Error("wait_for_selector must be a nonempty CSS selector (at most 2000 characters)"); } + if (testsValueAttribute(p.waitForSelector)) { + throw new Error("wait_for_selector cannot test an element's value attribute"); + } if (action && p.returnState !== true) throw new Error("wait_for_selector requires return_state=true on actions"); if (p.waitTimeoutMs !== undefined && (!Number.isInteger(p.waitTimeoutMs) || p.waitTimeoutMs < 0 || p.waitTimeoutMs > 10000)) { throw new Error("wait_timeout_ms must be an integer from 0 to 10000"); } } +/** + * Whether a selector reads the `value` attribute. React mirrors a controlled + * input's value into it, password fields included, so a wait on + * `input[type=password][value^="a"]` would answer met/timeout one guessed + * character at a time — the password the agent is never meant to see. + * Escapes and comments are undone first, so `[\76 alue]` is caught too. + */ +function testsValueAttribute(selector) { + const plain = selector + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\\([0-9a-fA-F]{1,6})[ \t\n\r\f]?/g, (_, hex) => { + const code = parseInt(hex, 16); + return code > 0 && code <= 0x10ffff ? String.fromCodePoint(code) : "\ufffd"; + }) + .replace(/\\([\s\S])/g, "$1"); + return /\[\s*(?:[\w*-]*\|)?value\s*(?:[~|^$*]?=|\])/i.test(plain); +} + /** A visible match in the top-level document, not arbitrary application readiness. */ function visibleSelectorInPage(selector, visible) { let matches; @@ -1363,7 +1414,11 @@ async function waitForReadiness(p) { returnByValue: true, }); } catch (error) { - return outcome("error", { error: error.message || "evaluate failed" }); + // A navigation committing mid-wait destroys the execution context; the + // new document is the one to observe, so poll again until the deadline. + if (Date.now() >= deadline) return outcome("error", { error: error.message || "evaluate failed" }); + await sleep(Math.min(100, deadline - Date.now())); + continue; } if (res?.exceptionDetails) return outcome("error", { error: res.exceptionDetails.text || "evaluate failed" }); const value = res?.result?.value; @@ -1863,7 +1918,10 @@ const handlers = { async function handleCommand(msg, replyPort) { await ensureStateReady(); - const { id, command, params = {} } = msg || {}; + const { id, command, params: raw = {} } = msg || {}; + // MCP clients may send null for an optional argument they did not set, and + // the hosts pass it through; null means absent everywhere below. + const params = Object.fromEntries(Object.entries(raw ?? {}).filter(([, value]) => value !== null)); try { const processId = requireClientId(params); if (command === "close_client_tabs") { diff --git a/chrome-extension/background.test.mjs b/chrome-extension/background.test.mjs index f711f6a..2b71c0a 100644 --- a/chrome-extension/background.test.mjs +++ b/chrome-extension/background.test.mjs @@ -627,7 +627,11 @@ test("invalid wait contracts are rejected before any stateful action", async () { returnState: true, waitForSelector: 1 }, { returnState: true, waitForSelector: "x".repeat(2001) }, { returnState: true, waitTimeoutMs: 100 }, - ...[-1, 10001, 0.5, "100", null].map((waitTimeoutMs) => ({ returnState: true, waitForSelector: "#ready", waitTimeoutMs })), + { returnState: true, waitForSelector: 'input[type=password][value^="a"]' }, + { returnState: true, waitForSelector: 'form:has(input[VALUE*="x"])' }, + { returnState: true, waitForSelector: "input[\\76 alue^=a]" }, + { returnState: true, waitForSelector: "input[*|value]" }, + ...[-1, 10001, 0.5, "100"].map((waitTimeoutMs) => ({ returnState: true, waitForSelector: "#ready", waitTimeoutMs })), ]; for (const command of ["click", "type", "press", "navigate"]) { for (const params of invalid) { @@ -670,7 +674,7 @@ test("a readiness transport failure preserves the successful action and current if (expression.includes("visibleSelectorInPage")) throw new Error("readiness transport unavailable"); return { result: { value: { title: "Current", elements: [] } } }; }; - const result = await callAndWait("click", { tabId: pageTab, index: 3, returnState: true, waitForSelector: "#ready" }); + const result = await callAndWait("click", { tabId: pageTab, index: 3, returnState: true, waitForSelector: "#ready", waitTimeoutMs: 300 }); assert.equal(result.ok, true); assert.equal(result.readiness.status, "error"); assert.equal(result.readiness.error, "readiness transport unavailable"); @@ -791,9 +795,9 @@ test("snapshot forwards bounded pagination and refuses malformed options before assert.ok(expressions[0].endsWith('({"offset":250,"limit":20})')); const count = expressions.length; const malformed = [ - { offset: -1 }, { offset: "250" }, { offset: 1.5 }, { offset: null }, + { offset: -1 }, { offset: "250" }, { offset: 1.5 }, { offset: 2147483648 }, { offset: Number.MAX_SAFE_INTEGER + 1 }, - { limit: 0 }, { limit: 251 }, { limit: "10" }, { limit: null }, + { limit: 0 }, { limit: 251 }, { limit: "10" }, ]; for (const args of malformed) { assert.match(await refuses("snapshot", { tabId: pageTab, ...args }), /offset must|limit must/); @@ -801,6 +805,9 @@ test("snapshot forwards bounded pagination and refuses malformed options before assert.equal(expressions.length, count, "malformed pagination reached page script"); await call("snapshot", { tabId: pageTab }); assert.ok(expressions.at(-1).endsWith('({"offset":0,"limit":250})')); + // A client that sends null for an unset optional argument gets the default. + await call("snapshot", { tabId: pageTab, offset: null, limit: null, waitForSelector: null, waitTimeoutMs: null }); + assert.ok(expressions.at(-1).endsWith('({"offset":0,"limit":250})')); pageEval = () => ({ result: { value: {} } }); }); diff --git a/macos/Sources/main.swift b/macos/Sources/main.swift index 63ef70d..9562358 100644 --- a/macos/Sources/main.swift +++ b/macos/Sources/main.swift @@ -2473,7 +2473,14 @@ func withPageAfter(_ line: String, _ payload: [String: Any]) -> String { /// Readiness is separate from action success; never imply a retry is safe. func describeReadiness(_ payload: [String: Any]) -> String { - guard let readiness = payload["readiness"] as? [String: Any] else { return "" } + guard let readiness = payload["readiness"] as? [String: Any] else { + // A current extension always reports readiness with an action's + // snapshot, and a total with a snapshot. Without them the extension is + // older than this server and silently ignored the new arguments; say so + // rather than let the agent believe it waited or paged. + let stale = payload["snapshot"] != nil || (payload["elements"] != nil && payload["total"] == nil) + return stale ? "note: the Chrome extension is older than this server, so wait_for_selector, offset and limit were ignored — update it and reload it at chrome://extensions.\n" : "" + } let status = readiness["status"] as? String ?? "unknown" let condition = readiness["condition"] as? String ?? "unknown" let elapsed = readiness["elapsedMs"] as? Int ?? 0 @@ -3740,7 +3747,7 @@ let toolDefs: [[String: Any]] = [ ], [ "name": "browser_click", - "description": "Click in one of the agent's tabs, either an element by its index from browser_snapshot (preferred) or a point given in page coordinates. Pass index or x and y, not both. Works on a background tab. Use click for native app windows. A click can submit forms or follow links, so snapshot first.", + "description": "Click in one of the agent's tabs, either an element by its index from browser_snapshot (preferred) or a point given in page coordinates. Pass index or x and y, not both. Works on a background tab. A tab the click opens (a target=_blank link or a popup) becomes one of the agent's tabs; find it with browser_list_tabs. Use click for native app windows. A click can submit forms or follow links, so snapshot first.", "inputSchema": [ "type": "object", "properties": [ @@ -3896,7 +3903,7 @@ let toolDefs: [[String: Any]] = [ ], [ "name": "browser_press_key", - "description": "Press Enter, Tab, Escape or Backspace in one of the agent's tabs, for example Enter to submit a form after browser_type. Only these four keys are supported; use browser_type for characters. Enter can submit forms and Backspace deletes, so check the page state with browser_snapshot first.", + "description": "Press Enter, Tab, Escape or Backspace in one of the agent's tabs, for example Enter to submit a form after browser_type. Only these four keys are supported; use browser_type for characters. Tab moves focus to the next field and says which element has it; it fails rather than leave focus where it was. Enter can submit forms and Backspace deletes, so check the page state with browser_snapshot first.", "inputSchema": [ "type": "object", "properties": [ @@ -3944,7 +3951,7 @@ let toolDefs: [[String: Any]] = [ ], [ "name": "browser_close_all_tabs", - "description": "Close every tab the agent opened and remove its tab group. Tabs taken over with browser_use_tab are released back to the user, not closed. Call this when finished with the browser so no empty group is left in the user's tab strip. The MCP process also runs this automatically when the Computer Use session ends. Unsaved state in the agent's tabs is lost.", + "description": "Close every tab the agent opened, including tabs its clicks opened, and remove its tab group. Tabs taken over with browser_use_tab are released back to the user, not closed. Call this when finished with the browser so no empty group is left in the user's tab strip. The MCP process also runs this automatically when the Computer Use session ends. Unsaved state in the agent's tabs is lost.", "inputSchema": [ "type": "object", "properties": [ diff --git a/scripts/e2e-browser.mjs b/scripts/e2e-browser.mjs index 0cb00ef..58485c7 100644 --- a/scripts/e2e-browser.mjs +++ b/scripts/e2e-browser.mjs @@ -280,8 +280,9 @@ function evaluateTarget(webSocketUrl, expression) { } async function evaluateInLive(match, expression) { + let targets = []; for (let attempt = 0; attempt < 100; attempt++) { - const targets = await (await fetch(`http://127.0.0.1:${devtoolsPort}/json`)).json(); + targets = await (await fetch(`http://127.0.0.1:${devtoolsPort}/json`)).json(); for (const target of targets.filter((entry) => entry.type === "page" && match(entry.url))) { if (!target.webSocketDebuggerUrl) continue; const value = await evaluateTarget(target.webSocketDebuggerUrl, expression); @@ -289,7 +290,10 @@ async function evaluateInLive(match, expression) { } await sleep(100); } - throw new Error("no live DevTools target answered"); + // Say what was there, so a CI failure shows whether the window never opened, + // was already gone, or was open but never matched. + const seen = targets.map((target) => `${target.type} ${target.url}`).join("; "); + throw new Error(`no live DevTools target answered (last prompt ${lastPrompt}; targets: ${seen})`); } // Each prompt window is prompt.html#, ids counting up. An answered one can // linger in the target list for a moment, so always take the newest. @@ -447,7 +451,7 @@ check("browser_request_credentials fills the page without returning the values", const [email, password] = document.querySelectorAll('input'); email.value = 'person@example.com'; password.value = ${JSON.stringify(SECRET)}; - document.getElementById('ok').click(); + setTimeout(() => document.getElementById('ok').click(), 0); return true; })()`); const result = await pending; @@ -466,7 +470,9 @@ check("closing the sign-in window reports a cancel and fills nothing", async () const pending = tool("browser_request_credentials", { tab_id: tab, fields: [{ index: email }] }); await inNewPrompt(`(() => { if (!document.querySelector('input')) return null; - document.getElementById('cancel').click(); + // Cancelling closes this window. Click after the reply is on its way, or + // the window can close first and the evaluation never answers. + setTimeout(() => document.getElementById('cancel').click(), 0); return true; })()`); const result = await pending; @@ -498,7 +504,7 @@ check("an ask rule shows an approval prompt, once", async () => { `(() => { const title = document.getElementById('title')?.textContent ?? ''; if (!title.startsWith('Let')) return null; - document.getElementById('ok').click(); + setTimeout(() => document.getElementById('ok').click(), 0); return title; })()`, ); diff --git a/scripts/snapshot-dom.test.mjs b/scripts/snapshot-dom.test.mjs index 698e94e..d6822b9 100644 --- a/scripts/snapshot-dom.test.mjs +++ b/scripts/snapshot-dom.test.mjs @@ -199,6 +199,18 @@ try { assert.equal(await evaluate('document.querySelectorAll(\'button[hidden][data-cu-idx]\').length'), 0); console.log("PASS bounded pagination, unique actionable global indices, stale-index cleanup"); + // Radix, MUI and Headless UI render an open select's options in a portal + // at the end of , outside the dialog that owns the select. + const dialog = '
'; + await set(`${buttons(10)}${dialog}
Canada
France
`); + assert.deepEqual((await snapshot()).elements.map((el) => el.label), ["Country", "Save", "Canada France", "Canada", "France"]); + await set(`${buttons(10)}${dialog.replace(' aria-controls="countries"', "")}
Painted on top
`); + assert.deepEqual((await snapshot()).elements.map((el) => el.label), ["Country", "Save", "Painted on top", "Painted on top"]); + await set(`${buttons(10)}${dialog.replace(' aria-controls="countries"', "")}`); + await evaluate('document.getElementById("menu").showPopover(); true'); + assert.deepEqual((await snapshot()).elements.map((el) => el.label), ["Country", "Save", "Popover item"]); + console.log("PASS a modal keeps its portalled listbox, painted-on-top options and open popovers"); + } if (suite === "all" || suite === "audit-r1") { diff --git a/windows-linux/src/browser.rs b/windows-linux/src/browser.rs index 5305207..5a70030 100644 --- a/windows-linux/src/browser.rs +++ b/windows-linux/src/browser.rs @@ -814,7 +814,15 @@ fn describe(command: &str, result: &Value, args: &Value) -> String { /// Readiness is separate from action success; never imply a retry is safe. fn describe_readiness(result: &Value) -> String { - let Some(readiness) = result.get("readiness") else { return String::new() }; + let Some(readiness) = result.get("readiness") else { + // A current extension always reports readiness with an action's + // snapshot, and a total with a snapshot. Without them the extension is + // older than this server and silently ignored the new arguments; say so + // rather than let the agent believe it waited or paged. + let stale = result.get("snapshot").is_some() + || (result.get("elements").is_some() && result.get("total").is_none()); + return if stale { "note: the Chrome extension is older than this server, so wait_for_selector, offset and limit were ignored — update it and reload it at chrome://extensions.\n".to_string() } else { String::new() }; + }; let status = readiness.get("status").and_then(Value::as_str).unwrap_or("unknown"); let condition = readiness.get("condition").and_then(Value::as_str).unwrap_or("unknown"); let elapsed = readiness.get("elapsedMs").and_then(Value::as_u64).unwrap_or(0); @@ -1031,10 +1039,10 @@ pub fn run_native_host() -> std::io::Result<()> { break; } } - // Chrome keeps stdin open for the native port. On Windows, backend EOF - // must end this relay process too, or Chrome never observes disconnect - // and the extension retains a dead port instead of reconnecting. - #[cfg(windows)] + // Chrome keeps stdin open for the native port, so backend EOF must end + // this relay process too, or Chrome never observes disconnect and the + // extension retains a dead port instead of reconnecting. Linux relays + // block on stdin the same way Windows ones do. std::process::exit(0); }); @@ -1301,6 +1309,10 @@ mod tests { assert!(text.contains("truncated — continue with browser_snapshot offset=270, limit=20")); assert!(text.contains("[250] button \"Next\"")); let last = describe_snapshot(&json!({"total": 300, "offset": 300, "truncated": false})); + // An extension older than the server answers without readiness/total. + assert!(super::describe_readiness(&json!({ "elements": [] })).contains("older than this server")); + assert!(super::describe_readiness(&json!({ "snapshot": {} })).contains("older than this server")); + assert_eq!(super::describe_readiness(&json!({ "elements": [], "total": 0 })), ""); assert!(!last.contains("continue with")); } diff --git a/windows-linux/src/tools.rs b/windows-linux/src/tools.rs index 048931f..1a71b88 100644 --- a/windows-linux/src/tools.rs +++ b/windows-linux/src/tools.rs @@ -882,7 +882,7 @@ fn all_tool_defs() -> Value { }, { "name": "browser_click", - "description": "Click in one of the agent's tabs, either an element by its index from browser_snapshot (preferred) or a point given in page coordinates. Pass index or x and y, not both. Works on a background tab. Use click for native app windows. A click can submit forms or follow links, so snapshot first.", + "description": "Click in one of the agent's tabs, either an element by its index from browser_snapshot (preferred) or a point given in page coordinates. Pass index or x and y, not both. Works on a background tab. A tab the click opens (a target=_blank link or a popup) becomes one of the agent's tabs; find it with browser_list_tabs. Use click for native app windows. A click can submit forms or follow links, so snapshot first.", "inputSchema": { "type": "object", "properties": { @@ -1038,7 +1038,7 @@ fn all_tool_defs() -> Value { }, { "name": "browser_press_key", - "description": "Press Enter, Tab, Escape or Backspace in one of the agent's tabs, for example Enter to submit a form after browser_type. Only these four keys are supported; use browser_type for characters. Enter can submit forms and Backspace deletes, so check the page state with browser_snapshot first.", + "description": "Press Enter, Tab, Escape or Backspace in one of the agent's tabs, for example Enter to submit a form after browser_type. Only these four keys are supported; use browser_type for characters. Tab moves focus to the next field and says which element has it; it fails rather than leave focus where it was. Enter can submit forms and Backspace deletes, so check the page state with browser_snapshot first.", "inputSchema": { "type": "object", "properties": { @@ -1086,7 +1086,7 @@ fn all_tool_defs() -> Value { }, { "name": "browser_close_all_tabs", - "description": "Close every tab the agent opened and remove its tab group. Tabs taken over with browser_use_tab are released back to the user, not closed. Call this when finished with the browser so no empty group is left in the user's tab strip. The MCP process also runs this automatically when the Computer Use session ends. Unsaved state in the agent's tabs is lost.", + "description": "Close every tab the agent opened, including tabs its clicks opened, and remove its tab group. Tabs taken over with browser_use_tab are released back to the user, not closed. Call this when finished with the browser so no empty group is left in the user's tab strip. The MCP process also runs this automatically when the Computer Use session ends. Unsaved state in the agent's tabs is lost.", "inputSchema": { "type": "object", "properties": {