diff --git a/docs/tools/browser.md b/docs/tools/browser.md index b8254c889..54dd7e98f 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -65,8 +65,8 @@ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `all` | `boolean` | No | Close every known tab. Omitted closes only `name`. | -| `kill` | `boolean` | No | When a tab release drops a spawned-app browser handle to refcount 0, also terminate its process tree. Has no effect on headless shutdown and only disconnects connected CDP browsers. | +| `all` | `boolean` | No | Release every known managed tab. Omitted releases only `name`. Tool-owned headless pages and owned cmux surfaces close; spawned, connected, and relay pages remain open. | +| `kill` | `boolean` | No | When a tab release drops a spawned-app browser handle to refcount 0, also terminate its process tree. Has no effect on headless shutdown; connected and relay browsers are only disconnected. | ### `action: "run"` @@ -160,7 +160,7 @@ The tool returns one result per call; no streaming partial output is emitted fro 18. `tab.click()` uses a custom retry loop for `text/...` selectors to find an actionable visible match; other selectors use `page.locator(...).click()`. Interactive actions (`click`/`fill`/`type`/`press`/`scroll`/`drag`/`scrollIntoView`/`select`/`uploadFile`) and the `waitFor*` helpers run under a per-op deadline (`min(cellBudget − slack, ceiling)`) threaded into both the puppeteer `signal` and `.setTimeout()`, so a stalled helper aborts the CDP action and rejects with a named `tab. timed out after ms` that leaves cell budget — never the opaque whole-cell timeout. `goto`/`evaluate` stay uncapped. 19. `tab.screenshot()` captures the page or selected element as PNG, resizes a model copy, saves under `browser.screenshotDir` or the OS temp directory, returns that path, records metadata, and optionally emits text plus image content. 20. `display()` calls accumulate in an array. After code finishes, the worker posts `{ displays, returnValue, screenshots }`; `BrowserTool.#run()` appends the return value as trailing text content when not `undefined`. -21. `close` releases one tab or all tabs via `releaseTab()` / `releaseAllTabs()`. Each tab aborts pending runs, asks the worker to close, waits up to `750` ms for a `closed` ack, terminates the worker, decrements browser refcount, and disposes the browser handle when refcount reaches zero. +21. `close` releases one managed tab handle or all handles via `releaseTab()` / `releaseAllTabs()`. Each tab aborts pending runs, asks the worker to clean up, waits up to `750` ms for a `closed` ack, terminates the worker, decrements browser refcount, and disposes the browser handle when refcount reaches zero. Headless workers close their tool-owned page; attach workers disconnect without closing spawned, connected, or relay pages. ## Modes / Variants - **Action dispatch** @@ -258,7 +258,7 @@ The tool returns one result per call; no streaming partial output is emitted fro - Relay mode drives an existing user browser and receives no stealth patches. Anything that can reach the relay endpoint can drive logged-in tabs; the built-in server binds loopback, and an optional shared token gates the extension connection. - `applyStealthPatches()` also strips Puppeteer's `//# sourceURL=__puppeteer_evaluation_script__` suffix from CDP `Runtime.evaluate` / `Runtime.callFunctionOn` payloads. - `tab.extract()` reads `page.content()`, runs Readability first, then falls back to the first non-empty of `[data-pagefind-body]`/`main article`/`article`/`main`/`[role='main']`/`body`, and returns `null` if neither extraction path yields content. -- `close(all: true, kill: false)` disconnects from spawned, connected, and relay browsers when the last tab closes but leaves spawned app processes and the user's Chrome running. +- `close(all: true, kill: false)` disconnects from spawned, connected, and relay browsers when the last managed tab is released but leaves their pages, spawned app processes, and the user's Chrome running. `kill: true` additionally terminates spawned-app processes; it never closes or kills connected or relay browsers. - Headless orphan cleanup is best-effort: if a worker dies before closing its page, the supervisor searches browser targets by `targetId` and closes that page. - Console methods inside `run` do not appear in tool output; they are forwarded as debug/warn/error logs through the worker transport. - Raw page request interception is run-scoped. At run end the worker removes user `request` handlers, disables interception, and releases held requests; cleanup failure marks the tab for recovery. \ No newline at end of file diff --git a/packages/coding-agent/src/prompts/tools/browser.md b/packages/coding-agent/src/prompts/tools/browser.md index 08daf0085..a6805d943 100644 --- a/packages/coding-agent/src/prompts/tools/browser.md +++ b/packages/coding-agent/src/prompts/tools/browser.md @@ -21,6 +21,7 @@ Drives real Chromium tab; full puppeteer access via JS. - `app.path` → NEVER tamper with a real desktop app (no stealth patches). - `app.relay: true` → drive the user's own Chrome tabs via the omp browser relay (auto-started; needs the OMP Browser Relay extension installed). `app.target` picks a tab by URL/title substring; without it the visible tab is adopted without stealing focus. +- `close` releases the named tool session. It closes tool-owned headless pages and owned cmux surfaces, but NEVER closes pages in spawned, CDP-connected, or relay browsers; those user-owned pages remain open. - Selectors: CSS + puppeteer `aria/…`, `text/…`, `xpath/…`, `pierce/…`. Playwright-only pseudos (`:has-text()`, `:visible`) are REJECTED. diff --git a/packages/coding-agent/src/tools/browser.ts b/packages/coding-agent/src/tools/browser.ts index 188c2b9bc..b9f77464b 100644 --- a/packages/coding-agent/src/tools/browser.ts +++ b/packages/coding-agent/src/tools/browser.ts @@ -69,7 +69,7 @@ const browserSchema = type({ "dialogs?": type("'accept' | 'dismiss'").describe("auto-handle dialogs"), "code?": type("string").describe("js body to run in tab"), "timeout?": type("number").describe("timeout in seconds"), - "all?": type("boolean").describe("close every tab"), + "all?": type("boolean").describe("release every managed tab"), "kill?": type("boolean").describe("also kill spawned-app browsers"), }); @@ -133,7 +133,7 @@ function resolveBrowserKind(params: BrowserParams, session: ToolSession): Browse /** * Browser tool: stateful, multi-tab. Three actions: * - `open` → acquire/create a named tab on a browser kind (headless | spawned | connected) and optionally goto a url. - * - `close` → release a named tab (or all tabs); dispose browser when refcount hits 0. + * - `close` → release a named tab handle (or all handles); attached/spawned/relay pages remain open. * - `run` → execute JS code against an existing tab with `page`/`browser`/`tab` helpers in scope. */ export class BrowserTool implements AgentTool { @@ -204,7 +204,7 @@ export class BrowserTool implements AgentTool releaseAllTabs({ kill, timeoutMs })); - details.result = `Closed ${count} tab(s)`; + details.result = `Released ${count} managed tab${count === 1 ? "" : "s"}`; return toolResult(details).text(details.result).done(); } const closed = await untilAborted(signal, () => releaseTab(name, { kill, timeoutMs })); - details.result = closed ? `Closed tab ${JSON.stringify(name)}` : `No tab named ${JSON.stringify(name)}`; + details.result = closed ? `Released managed tab ${JSON.stringify(name)}` : `No tab named ${JSON.stringify(name)}`; return toolResult(details).text(details.result).done(); } diff --git a/packages/coding-agent/src/tools/browser/render.ts b/packages/coding-agent/src/tools/browser/render.ts index d37c5f34a..9eb2d1890 100644 --- a/packages/coding-agent/src/tools/browser/render.ts +++ b/packages/coding-agent/src/tools/browser/render.ts @@ -162,7 +162,7 @@ function renderOpenOrCloseLine( let title: string; if (action === "close") { const all = args.all === true || (args.name === undefined && details?.name === undefined); - title = all ? "Close all tabs" : `Close ${tabLabel(args, details)}`; + title = all ? "Release all tabs" : `Release ${tabLabel(args, details)}`; if (args.kill) title += " (kill)"; } else { title = `Open ${tabLabel(args, details)}`; diff --git a/packages/coding-agent/test/tools/browser-attach.test.ts b/packages/coding-agent/test/tools/browser-attach.test.ts index 2a59cf5cb..ba8c3e293 100644 --- a/packages/coding-agent/test/tools/browser-attach.test.ts +++ b/packages/coding-agent/test/tools/browser-attach.test.ts @@ -1,4 +1,7 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { BrowserTool } from "@oh-my-pi/pi-coding-agent/tools/browser"; import { pickElectronTarget, shouldPreserveConnectedBrowserFocus, @@ -9,13 +12,23 @@ import { normalizeConnectedCdpUrl, releaseBrowser, } from "@oh-my-pi/pi-coding-agent/tools/browser/registry"; -import { acquireTab, releaseTab } from "@oh-my-pi/pi-coding-agent/tools/browser/tab-supervisor"; +import { acquireTab } from "@oh-my-pi/pi-coding-agent/tools/browser/tab-supervisor"; import type { Browser, Page, Target } from "puppeteer-core"; import { chromiumAvailable } from "./chromium-probe"; const CHROMIUM_AVAILABLE = await chromiumAvailable(); let sharedHeadless: BrowserHandle | undefined; +function makeSession(): ToolSession { + return { + cwd: process.cwd(), + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated({ "browser.headless": true }), + }; +} + interface FakePageOptions { url: string; title: string; @@ -121,32 +134,34 @@ describe("pickElectronTarget", () => { // Launches real headless Chromium; skipped where Chrome's system libraries are absent. test.skipIf(!CHROMIUM_AVAILABLE)( - "navigates a fresh attached tab to the requested URL", + "navigates a fresh attached tab and releases its handle without closing the target", async () => { const launched = sharedHeadless; if (!launched || !("browser" in launched)) throw new Error("Expected a shared Puppeteer browser"); const endpoint = new URL(launched.browser.wsEndpoint()); - let attached: BrowserHandle | undefined; + const tool = new BrowserTool(makeSession()); let opened = false; const tabName = `attach-navigation-${process.pid}-${Math.random().toString(36).slice(2)}`; const requested = "data:text/html,attached-navigation-target"; + const targetPage = (await launched.browser.pages())[0]; + if (!targetPage) throw new Error("Expected the launched browser to expose a page target"); try { - attached = await acquireBrowser( - { kind: "connected", cdpUrl: `http://${endpoint.host}` }, - { cwd: process.cwd() }, - ); - const { tab } = await acquireTab(tabName, attached, { + await tool.execute("open", { + action: "open", + name: tabName, url: requested, - waitUntil: "domcontentloaded", - timeoutMs: 10_000, + app: { cdp_url: `http://${endpoint.host}` }, }); opened = true; - expect(tab.info.url).toBe(requested); + const closeResult = await tool.execute("close", { action: "close", name: tabName }); + opened = false; + expect(closeResult.content).toEqual([{ type: "text", text: `Released managed tab "${tabName}"` }]); + expect(targetPage.isClosed()).toBe(false); + expect(targetPage.url()).toBe(requested); } finally { - if (opened) await releaseTab(tabName, { kill: false }); - else if (attached) await releaseBrowser(attached, { kill: false }); + if (opened) await tool.execute("close", { action: "close", name: tabName }); } }, 30_000,