diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d97a1f633..5531273f6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed browser automation disrupting a browser it attached to over `app.cdp_url`: the tool now adopts the tab the user actually has in the foreground and no longer raises its own tab when taking a screenshot. Owned and headless browsers keep activating the target before capture. + ## [17.1.8] - 2026-07-28 ### Breaking Changes diff --git a/packages/coding-agent/src/tools/browser/attach.ts b/packages/coding-agent/src/tools/browser/attach.ts index dc75e8919..fdfb5eeca 100644 --- a/packages/coding-agent/src/tools/browser/attach.ts +++ b/packages/coding-agent/src/tools/browser/attach.ts @@ -122,8 +122,12 @@ export async function findReusableCdp( * Pick the best page target on an attached browser. Prefer discoverable page * targets first so Chromium/Edge attach flows that hide pages from * `browser.pages()` can still return a usable tab. + * + * `preferVisible` is for attaching to a browser a human is using: among equally + * usable tabs, take the one that is actually foregrounded rather than whichever + * target CDP happens to enumerate first. */ -export async function pickElectronTarget(browser: Browser, matcher?: string): Promise { +export async function pickElectronTarget(browser: Browser, matcher?: string, preferVisible = false): Promise { const discoveredPages = await Promise.all( browser.targets().map(async target => { if (String(target.type()) !== "page") return null; @@ -132,14 +136,14 @@ export async function pickElectronTarget(browser: Browser, matcher?: string): Pr ); const usablePages = discoveredPages.filter((page): page is Page => page !== null); if (usablePages.length > 0) { - return pickPageFromList(usablePages, matcher); + return pickPageFromList(usablePages, matcher, preferVisible); } const fallbackPages = await browser.pages(); if (!fallbackPages.length) { throw new ToolError("No page targets available on the attached browser"); } - return pickPageFromList(fallbackPages, matcher); + return pickPageFromList(fallbackPages, matcher, preferVisible); } async function enrichPages(pages: Page[]): Promise> { @@ -152,7 +156,7 @@ async function enrichPages(pages: Page[]): Promise { +async function pickPageFromList(pages: Page[], matcher?: string, preferVisible = false): Promise { const enriched = await enrichPages(pages); if (matcher) { const needle = matcher.toLowerCase(); @@ -161,10 +165,24 @@ async function pickPageFromList(pages: Page[], matcher?: string): Promise const summary = enriched.map(p => `- ${p.title || "(untitled)"} ${p.url}`).join("\n"); throw new ToolError(`No page target matched ${JSON.stringify(matcher)}. Available pages:\n${summary}`); } - return ( - enriched.find(p => !ATTACH_TARGET_SKIP_PATTERN.test(p.url) && !ATTACH_TARGET_SKIP_PATTERN.test(p.title))?.page ?? - enriched[0]!.page + const usable = enriched.filter( + p => !ATTACH_TARGET_SKIP_PATTERN.test(p.url) && !ATTACH_TARGET_SKIP_PATTERN.test(p.title), ); + if (preferVisible && usable.length > 1) { + // Best-effort foreground probe; a tab that cannot answer counts as hidden. + const visibility = await Promise.all( + usable.map(async p => { + try { + return (await p.page.evaluate(() => document.visibilityState === "visible")) === true; + } catch { + return false; + } + }), + ); + const foreground = visibility.indexOf(true); + if (foreground >= 0) return usable[foreground]!.page; + } + return usable[0]?.page ?? enriched[0]!.page; } /** diff --git a/packages/coding-agent/src/tools/browser/tab-protocol.ts b/packages/coding-agent/src/tools/browser/tab-protocol.ts index cd0d6de9a..3b3903697 100644 --- a/packages/coding-agent/src/tools/browser/tab-protocol.ts +++ b/packages/coding-agent/src/tools/browser/tab-protocol.ts @@ -65,6 +65,11 @@ export type WorkerInitPayload = * previously force-killed the tab). Never set for first-time Electron attach. */ recover?: boolean; + /** + * Whether the worker may raise this tab before capturing a screenshot. Unset + * behaves as `true`; the supervisor clears it for browsers we did not launch. + */ + activateForScreenshot?: boolean; }; export type ToolReply = { ok: true; value: unknown } | { ok: false; error: RunErrorPayload }; diff --git a/packages/coding-agent/src/tools/browser/tab-supervisor.ts b/packages/coding-agent/src/tools/browser/tab-supervisor.ts index dd9ff45af..2dc0b955b 100644 --- a/packages/coding-agent/src/tools/browser/tab-supervisor.ts +++ b/packages/coding-agent/src/tools/browser/tab-supervisor.ts @@ -684,7 +684,11 @@ async function buildInitPayload(browser: PuppeteerBrowserHandle, opts: AcquireTa timeoutMs: opts.timeoutMs, }; } - const page = await pickElectronTarget(browser.browser, opts.target); + // A "connected" browser is one the user is driving themselves (cdp attach), so the + // tab we adopt and the focus we take are visible to them; owned Electron/spawned + // browsers keep the previous behavior. + const userDriven = browser.kind.kind === "connected"; + const page = await pickElectronTarget(browser.browser, opts.target, userDriven); const targetId = await targetIdForPage(page); return { mode: "attach", @@ -692,6 +696,7 @@ async function buildInitPayload(browser: PuppeteerBrowserHandle, opts: AcquireTa safeDir, targetId, dialogs: opts.dialogs, + activateForScreenshot: !userDriven, }; } @@ -793,6 +798,7 @@ async function recycleTimedOutWorkerTab(tab: WorkerTabSession, timeoutMs: number // Unblock a wedged page (open JS dialog, hung navigation) before adopting it — // otherwise init stalls, times out, and the tab gets force-killed. recover: true, + activateForScreenshot: tab.kindTag !== "connected", }; let worker = await spawnTabWorker(); try { diff --git a/packages/coding-agent/src/tools/browser/tab-worker.ts b/packages/coding-agent/src/tools/browser/tab-worker.ts index af2d56d58..9f84dd1ef 100644 --- a/packages/coding-agent/src/tools/browser/tab-worker.ts +++ b/packages/coding-agent/src/tools/browser/tab-worker.ts @@ -72,6 +72,7 @@ declare global { var innerHeight: number; var document: { elementFromPoint(x: number, y: number): Element | null; + readonly visibilityState: "visible" | "hidden"; }; } @@ -732,6 +733,7 @@ export class WorkerCore { #runtime: JsRuntime | null = null; #unsub: () => void; #mode?: WorkerInitPayload["mode"]; + #activateForScreenshot = true; #dialogPolicy?: DialogPolicy; #dialogHandler?: (dialog: Dialog) => void; #openDialog?: OpenDialogInfo; @@ -780,6 +782,7 @@ export class WorkerCore { async #init(payload: WorkerInitPayload): Promise { try { this.#mode = payload.mode; + this.#activateForScreenshot = payload.mode === "headless" || payload.activateForScreenshot !== false; const puppeteer = await loadPuppeteerInWorker(payload.safeDir); this.#browser = await puppeteer.connect({ browserWSEndpoint: payload.browserWSEndpoint, @@ -1521,11 +1524,17 @@ export class WorkerCore { const page = this.#requirePage(); // Multiple tabs can share one Chromium (sibling headless tabs on a shared // endpoint, cdp/app attach). CDP `Page.captureScreenshot` reads the - // compositor surface, which follows the *active* target — a backgrounded + // compositor surface, which follows the *active* target: a backgrounded // page can stall waiting for a fresh frame (the 20s screenshot timeouts) // or hand back a sibling tab's pixels. Activate first; best-effort so an // already-active or freshly-closed target never fails the capture. - await untilAborted(signal, () => page.bringToFront()).catch(() => undefined); + // + // Browsers we do not own are the exception: raising a tab in the human's + // running Chrome switches their visible tab and pulls window focus, so a + // connected browser stays backgrounded and takes the stall risk instead. + if (this.#activateForScreenshot) { + await untilAborted(signal, () => page.bringToFront()).catch(() => undefined); + } const fullPage = opts.selector ? false : (opts.fullPage ?? false); const captureType = "png"; const captureMime = "image/png" as const; diff --git a/packages/coding-agent/test/tools/browser-attach.test.ts b/packages/coding-agent/test/tools/browser-attach.test.ts index b063514f7..2356f053f 100644 --- a/packages/coding-agent/test/tools/browser-attach.test.ts +++ b/packages/coding-agent/test/tools/browser-attach.test.ts @@ -6,12 +6,14 @@ import type { Browser, Page, Target } from "puppeteer-core"; interface FakePageOptions { url: string; title: string; + visible?: boolean; } function fakePage(options: FakePageOptions): Page { return { url: () => options.url, title: async () => options.title, + evaluate: async () => options.visible === true, } as unknown as Page; } @@ -60,6 +62,29 @@ describe("pickElectronTarget", () => { ); }); + test("prefers the foreground tab when asked to, without disturbing default order", async () => { + const background = fakePage({ url: "https://example.com/", title: "Example" }); + const foreground = fakePage({ url: "https://example.org/", title: "Example Org", visible: true }); + const browser = { + targets: () => [fakeTarget("page", background), fakeTarget("page", foreground)], + pages: async () => [], + } as unknown as Browser; + + await expect(pickElectronTarget(browser, undefined, true)).resolves.toBe(foreground); + await expect(pickElectronTarget(browser)).resolves.toBe(background); + }); + + test("falls back to the first usable tab when no tab reports itself visible", async () => { + const first = fakePage({ url: "https://example.com/", title: "Example" }); + const second = fakePage({ url: "https://example.org/", title: "Example Org" }); + const browser = { + targets: () => [fakeTarget("page", first), fakeTarget("page", second)], + pages: async () => [], + } as unknown as Browser; + + await expect(pickElectronTarget(browser, undefined, true)).resolves.toBe(first); + }); + test("rejects websocket cdp_url values with an actionable diagnostic", () => { expect(() => normalizeConnectedCdpUrl("ws://127.0.0.1:9222/devtools/browser/id")).toThrow( "browser app.cdp_url must be the HTTP CDP discovery endpoint",