smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tools): stop browser automation from stealing focus in an attached browser

Browse Source 
Attaching over app.cdp_url points automation at a browser the user is driving,
so two behaviors that are correct for a browser we own are wrong there.

pickElectronTarget enumerated CDP targets and took the first usable one, which
is not necessarily the tab in front of the user, and #captureScreenshot always
called page.bringToFront(), which switches the user's visible tab and pulls
window focus on every screenshot.

Connected browsers now prefer a tab that reports document.visibilityState
"visible" and skip the pre-capture activation, accepting the compositor stall
risk that activation avoids. Headless and spawned browsers are unchanged.
This commit is contained in:
Nik Divjak
2026-07-29 11:26:13 +02:00
parent cc00ab161b
commit b8b0d342da
6 changed files with 77 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -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
packages/coding-agent/src/tools/browser/attach.ts
+25 -7
View File
@@ -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<Page> {
export async function pickElectronTarget(browser: Browser, matcher?: string, preferVisible = false): Promise<Page> {
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<Array<{ page: Page; url: string; title: string }>> {
@@ -152,7 +156,7 @@ async function enrichPages(pages: Page[]): Promise<Array<{ page: Page; url: stri
);
}
async function pickPageFromList(pages: Page[], matcher?: string): Promise<Page> {
async function pickPageFromList(pages: Page[], matcher?: string, preferVisible = false): Promise<Page> {
const enriched = await enrichPages(pages);
if (matcher) {
const needle = matcher.toLowerCase();
@@ -161,10 +165,24 @@ async function pickPageFromList(pages: Page[], matcher?: string): Promise<Page>
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;
}
/**
packages/coding-agent/src/tools/browser/tab-protocol.ts
+5
View File
@@ -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 };
packages/coding-agent/src/tools/browser/tab-supervisor.ts
+7 -1
View File
@@ -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 {
packages/coding-agent/src/tools/browser/tab-worker.ts
+11 -2
View File
@@ -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<void> {
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;
packages/coding-agent/test/tools/browser-attach.test.ts
+25
View File
@@ -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",