From 8c0d8a00f19a1ff0e1e19d183b319e7b303b0eeb Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Fri, 28 Aug 2026 11:27:39 +0530 Subject: [PATCH] Follow the window a site opens, and come back when it closes --- CHANGELOG.md | 16 +++++ agent-computer/src/index.ts | 10 ++- agent-computer/src/live-page.ts | 16 +++++ agent-computer/src/profiles.ts | 52 ++++++++++---- agent-computer/tests/follows-popup.test.ts | 84 ++++++++++++++++++++++ agent-computer/tests/live-page.test.ts | 36 ++++++++++ 6 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 agent-computer/src/live-page.ts create mode 100644 agent-computer/tests/follows-popup.test.ts create mode 100644 agent-computer/tests/live-page.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eb5d41b..f62c6d7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A sign-in a site opens in a new window is shown, and can be clicked + +A Bot's browser was bound to the page it launched with, and to nothing the site opened afterwards. +Anything arriving in a new window or tab was invisible on the live screen and unreachable by input, +so the popup sign-ins that a person takes the wheel to complete were exactly the ones they could not +complete. Worse than invisible: a click at the place the popup's button was drawn went to the page +underneath it, so a person trying to finish a sign-in could navigate the page the Bot was working on +without seeing either result. + +The browser now follows the window the site opens, and returns to the opener when it closes, which is +what a sign-in popup does when it succeeds. A snapshot taken before the change of page is refused +afterwards with the same "take a new snapshot" it already gives after a navigation, so a stale ref +cannot act on the wrong document. + +Nothing to configure. + ## 0.0.5 ### One Bot can hand work to another, and reach a person when no Bot will do diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 5d7177a6..6e6d4ea5 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -112,6 +112,8 @@ type BotSession = { control: Control; /** This Bot's snapshot generation. See the note above on staleness. */ snapshotId: number; + /** The page this Bot was last handed, so a change of page can retire its refs. */ + livePage?: Page; /** The one live screen viewer for this Bot, if a person is watching. */ viewer?: { socket: unknown; @@ -210,7 +212,13 @@ const DEFAULT_BOT_ID = (() => { })(); async function currentPage(botId: string): Promise { - return profiles.page(botId); + const session = sessionFor(botId); + const page = await profiles.page(botId); + // A ref names an element on the page it was taken from, so moving to a window the site opened has + // to retire the outstanding ones exactly as a navigation does, or a click lands on the wrong document. + if (session.livePage && session.livePage !== page) session.snapshotId += 1; + session.livePage = page; + return page; } /** diff --git a/agent-computer/src/live-page.ts b/agent-computer/src/live-page.ts new file mode 100644 index 00000000..ee18064b --- /dev/null +++ b/agent-computer/src/live-page.ts @@ -0,0 +1,16 @@ +// Its own file so a test can reach it without the Playwright import `profiles.ts` carries. + +/** Enough of a page for the choice. Keeps this file independent of what else is on one. */ +export type OpenPage = { isClosed(): boolean }; + +// The newest open page wins, so a window the site opens becomes the one being watched and driven, +// and closing it falls back to whatever is still open rather than to a page that has gone. +export function chooseLivePage( + opened: readonly T[], +): T | undefined { + for (let index = opened.length - 1; index >= 0; index -= 1) { + const page = opened[index]; + if (page && !page.isClosed()) return page; + } + return undefined; +} diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index 28f6547f..f4dc8cfc 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -40,6 +40,7 @@ import { profileDirectoryFor } from "./bot-id"; import { chooseEvictions, chooseIdle } from "./browser-eviction"; import { egressFor, egressLabel } from "./egress"; import { numberFromEnv } from "./env"; +import { chooseLivePage } from "./live-page"; import { botIdsIn } from "./profile-listing"; // Re-exported so callers that already import it from here do not change, while the test imports it @@ -180,17 +181,17 @@ const IDLE_TIMEOUT_MS = numberFromEnv("COMPUTER_BROWSER_IDLE_MS", 30 * 60_000); const IDLE_SWEEP_MS = 60_000; export function createProfiles(root: string) { + type LiveBrowser = { + context: BrowserContext; + page: Page; + startedAt: string; + /** When this Bot last asked for its page. Decides what the cap and the sweep close. */ + usedAt: number; + /** Point `page` at whatever is open now. Called on every page opening and closing. */ + retarget: () => void; + }; /** One running browser per Bot, up to {@link MAX_LIVE_BROWSERS}. */ - const live = new Map< - string, - { - context: BrowserContext; - page: Page; - startedAt: string; - /** When this Bot last asked for its page. Decides what the cap and the sweep close. */ - usedAt: number; - } - >(); + const live = new Map(); /** Launches in flight, so a cold computer is started once however many callers ask at once. */ const starting = new Map>(); @@ -274,6 +275,9 @@ export function createProfiles(root: string) { if (launching) return launching; const existing = live.get(botId); + // Asked before the page is judged, so a close that has not been delivered yet does not read as + // a browser that has gone. + existing?.retarget(); if ( existing?.context.browser()?.isConnected() && !existing.page.isClosed() @@ -309,16 +313,40 @@ export function createProfiles(root: string) { }); // Persistent contexts open with a page already; reuse it rather than leaving an extra blank tab. const page = context.pages()[0] ?? (await context.newPage()); - live.set(botId, { + const record: LiveBrowser = { context, page, startedAt: new Date().toISOString(), usedAt: Date.now(), + retarget: () => {}, + }; + record.retarget = () => { + const next = chooseLivePage(context.pages()); + if (!next || next === record.page) return; + record.page = next; + console.info( + JSON.stringify({ + type: "computer-page-changed", + botId, + url: next.url(), + }), + ); + }; + // Without this the Bot stays pinned to the page it launched with, so a sign-in the site opens + // in a new window is neither shown to the person taking the wheel nor reachable by input. + context.on("page", (opened) => { + record.retarget(); + // A popup closes itself when it succeeds, and the record must move back to the opener + // rather than leave a closed page to be read as a dead browser. + opened.on("close", () => record.retarget()); }); + page.on("close", () => record.retarget()); + live.set(botId, record); // After the new one is in the map, so the cap counts what is really running and the Bot that // just asked is the most recently used and therefore never the one closed. await enforceCap(); - return page; + // Not `page`: a window opened while the browser was starting is already the live one. + return record.page; })(); starting.set(botId, launch); diff --git a/agent-computer/tests/follows-popup.test.ts b/agent-computer/tests/follows-popup.test.ts new file mode 100644 index 00000000..245757da --- /dev/null +++ b/agent-computer/tests/follows-popup.test.ts @@ -0,0 +1,84 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, describe, expect, test } from "bun:test"; + +/** + * Asked for by name, like the deployment journey at the repository root: it launches a real Chromium, + * which the machine running `bun test` is not required to have. The import is inside the test rather + * than at the top of the file for the same reason. + * + * cd agent-computer && bunx playwright install chromium + * OPENBOT_COMPUTER_BROWSER=1 bun test tests/follows-popup.test.ts + */ +const asked = process.env.OPENBOT_COMPUTER_BROWSER === "1"; +const LAUNCH_TIMEOUT_MS = 120_000; + +const html = (title: string, body: string) => + new Response(`${title}${body}`, { + headers: { "content-type": "text/html; charset=utf-8" }, + }); + +const site = Bun.serve({ + port: 0, + fetch(request) { + const { pathname } = new URL(request.url); + if (pathname === "/popup") return html("POPUP", "

the popup

"); + return html( + "OPENER", + ``, + ); + }, +}); +const origin = `http://127.0.0.1:${site.port}`; + +afterAll(() => { + site.stop(true); +}); + +/** Playwright delivers the opening and the closing as events, so the record moves a tick later. */ +async function settle(read: () => Promise, wanted: string) { + for (let attempt = 0; attempt < 50; attempt += 1) { + if ((await read()).includes(wanted)) return; + await new Promise((resolve) => setTimeout(resolve, 100)); + } +} + +describe.skipIf(!asked)("the page a Bot is on", () => { + test( + "follows a window the site opens, and comes back when it closes", + async () => { + const { createProfiles } = await import("../src/profiles"); + const root = await mkdtemp(join(tmpdir(), "openbot-profiles-")); + const profiles = createProfiles(root); + const bot = "popup-test"; + const url = async () => (await profiles.page(bot)).url(); + + try { + const opener = await profiles.page(bot); + await opener.goto(`${origin}/opener`, { + waitUntil: "domcontentloaded", + }); + expect(await url()).toContain("/opener"); + + await opener.click("#open"); + await settle(url, "/popup"); + // The defect this covers: the popup is open and rendering, and everything the Bot and the + // person taking the wheel can reach is still pointed at the page it launched with. + expect(await url()).toContain("/popup"); + + const popup = await profiles.page(bot); + await popup.close(); + await settle(url, "/opener"); + expect(await url()).toContain("/opener"); + // A sign-in popup closes itself when it succeeds, and the profile that just received it must + // still be there. + expect(opener.isClosed()).toBeFalse(); + } finally { + await profiles.stop(bot).catch(() => undefined); + await rm(root, { recursive: true, force: true }).catch(() => undefined); + } + }, + LAUNCH_TIMEOUT_MS, + ); +}); diff --git a/agent-computer/tests/live-page.test.ts b/agent-computer/tests/live-page.test.ts new file mode 100644 index 00000000..27cf5c94 --- /dev/null +++ b/agent-computer/tests/live-page.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, test } from "bun:test"; +import { chooseLivePage } from "../src/live-page"; + +// A popup OAuth sign-in is the case this exists for: the page a Bot was pinned to at launch is not +// the page the person taking the wheel needs to see, and the popup closes itself when it succeeds. +const page = (closed = false) => ({ isClosed: () => closed }); + +describe("choosing the page a Bot is on", () => { + test("one page is that page", () => { + const only = page(); + expect(chooseLivePage([only])).toBe(only); + }); + + test("a window the site opens becomes the live one", () => { + const opener = page(); + const popup = page(); + expect(chooseLivePage([opener, popup])).toBe(popup); + }); + + test("closing that window falls back to the opener", () => { + const opener = page(); + const popup = page(true); + expect(chooseLivePage([opener, popup])).toBe(opener); + }); + + test("a closed opener is not chosen while another page is open", () => { + const opener = page(true); + const popup = page(); + expect(chooseLivePage([opener, popup])).toBe(popup); + }); + + test("nothing open is nothing to choose", () => { + expect(chooseLivePage([page(true), page(true)])).toBeUndefined(); + expect(chooseLivePage([])).toBeUndefined(); + }); +});