From e6b0e763414dc7bf3f43bb6d71cf05a440565f64 Mon Sep 17 00:00:00 2001 From: danzaio <213864024+danzaio@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:37:58 -0300 Subject: [PATCH] fix(browser): cover cdp attach fallbacks --- packages/coding-agent/CHANGELOG.md | 10 +++---- .../coding-agent/src/tools/browser/attach.ts | 5 ++-- .../src/tools/browser/registry.ts | 17 +++++++---- .../test/tools/browser-attach.test.ts | 30 +++++++++++++++++++ 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1a5498817..24ac9cd7d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed CDP attach target selection for Chromium/Edge endpoints that expose page targets through discovery before `browser.pages()` is populated, and improved `ws://` `app.cdp_url` diagnostics ([#2246](https://github.com/can1357/oh-my-pi/issues/2246)). + ## [15.10.12] - 2026-06-10 ### Added @@ -17,12 +21,6 @@ - Added support for Git repositories using the `reftable` storage format by detecting `extensions.refStorage = reftable` in the repository configuration and falling back to shelling out to Git commands (`git symbolic-ref`, `git rev-parse`) for reference and HEAD resolution. - Added `/setup providers` (also available as `/setup` or `/providers`) to reopen the interactive provider setup scene from an active TUI session, letting users sign in and choose a web search provider without rerunning the full onboarding flow. -### Fixed - -- Fixed Windows-style backslash paths and globs being parsed as literal POSIX path segments by search/find tools ([#2245](https://github.com/can1357/oh-my-pi/issues/2245)). - -- Fixed CDP attach target selection for Chromium/Edge endpoints that expose page targets through discovery before `browser.pages()` is populated, and improved `ws://` `app.cdp_url` diagnostics ([#2246](https://github.com/can1357/oh-my-pi/issues/2246)). - ### Changed - Bash execution now preserves minimized shell output inline while saving the untouched capture as an `artifact://…` footer when shell minimization rewrites a command's output. diff --git a/packages/coding-agent/src/tools/browser/attach.ts b/packages/coding-agent/src/tools/browser/attach.ts index 4533047a0..df51d34fa 100644 --- a/packages/coding-agent/src/tools/browser/attach.ts +++ b/packages/coding-agent/src/tools/browser/attach.ts @@ -1,6 +1,6 @@ import * as net from "node:net"; import { Process, ProcessStatus } from "@oh-my-pi/pi-natives"; -import type { Browser, Page } from "puppeteer-core"; +import { type Browser, type Page, TargetType } from "puppeteer-core"; import { ToolError, throwIfAborted } from "../tool-errors"; const ATTACH_TARGET_SKIP_PATTERN = @@ -126,8 +126,7 @@ export async function findReusableCdp( export async function pickElectronTarget(browser: Browser, matcher?: string): Promise { const discoveredPages = await Promise.all( browser.targets().map(async target => { - const targetType = target.type() as string; - if (targetType !== "page" && targetType !== "tab") return null; + if (target.type() !== TargetType.PAGE) return null; return await target.page().catch(() => null); }), ); diff --git a/packages/coding-agent/src/tools/browser/registry.ts b/packages/coding-agent/src/tools/browser/registry.ts index 2382edc7b..06f7a8ab2 100644 --- a/packages/coding-agent/src/tools/browser/registry.ts +++ b/packages/coding-agent/src/tools/browser/registry.ts @@ -58,6 +58,16 @@ export async function acquireBrowser(kind: BrowserKind, opts: AcquireBrowserOpti return handle; } +export function normalizeConnectedCdpUrl(rawCdpUrl: string): string { + const cdpUrl = rawCdpUrl.replace(/\/+$/, ""); + if (/^wss?:\/\//i.test(cdpUrl)) { + throw new ToolError( + "browser app.cdp_url must be the HTTP CDP discovery endpoint (for example http://127.0.0.1:9222), not a ws:// browser websocket URL.", + ); + } + return cdpUrl; +} + async function openBrowserHandle(kind: BrowserKind, opts: AcquireBrowserOptions): Promise { if (kind.kind === "headless") { const browser = await launchHeadlessBrowser({ headless: kind.headless, viewport: opts.viewport }); @@ -70,12 +80,7 @@ async function openBrowserHandle(kind: BrowserKind, opts: AcquireBrowserOptions) }; } if (kind.kind === "connected") { - const cdpUrl = kind.cdpUrl.replace(/\/+$/, ""); - if (/^wss?:\/\//i.test(cdpUrl)) { - throw new ToolError( - "browser app.cdp_url must be the HTTP CDP discovery endpoint (for example http://127.0.0.1:9222), not a ws:// browser websocket URL.", - ); - } + const cdpUrl = normalizeConnectedCdpUrl(kind.cdpUrl); await waitForCdp(cdpUrl, 5_000, opts.signal); const puppeteer = await loadPuppeteer(); const browser = await puppeteer.connect({ diff --git a/packages/coding-agent/test/tools/browser-attach.test.ts b/packages/coding-agent/test/tools/browser-attach.test.ts index 9fe6a0306..b063514f7 100644 --- a/packages/coding-agent/test/tools/browser-attach.test.ts +++ b/packages/coding-agent/test/tools/browser-attach.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { pickElectronTarget } from "@oh-my-pi/pi-coding-agent/tools/browser/attach"; +import { normalizeConnectedCdpUrl } from "@oh-my-pi/pi-coding-agent/tools/browser/registry"; import type { Browser, Page, Target } from "puppeteer-core"; interface FakePageOptions { @@ -36,4 +37,33 @@ describe("pickElectronTarget", () => { await expect(pickElectronTarget(browser, "google")).resolves.toBe(page); expect(pagesCalled).toBe(false); }); + + test("falls back to browser.pages when discovered targets have no usable page", async () => { + const page = fakePage({ url: "https://example.com/", title: "Example" }); + const browser = { + targets: () => [fakeTarget("browser", null), fakeTarget("service_worker", null)], + pages: async () => [page], + } as unknown as Browser; + + await expect(pickElectronTarget(browser)).resolves.toBe(page); + }); + + test("reports available pages when the matcher misses", async () => { + const page = fakePage({ url: "https://example.com/", title: "Example" }); + const browser = { + targets: () => [fakeTarget("page", page)], + pages: async () => [], + } as unknown as Browser; + + await expect(pickElectronTarget(browser, "missing")).rejects.toThrow( + 'No page target matched "missing". Available pages:\n- Example https://example.com/', + ); + }); + + 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", + ); + expect(normalizeConnectedCdpUrl("http://127.0.0.1:9222/")).toBe("http://127.0.0.1:9222"); + }); });