fix(browser): cover cdp attach fallbacks

This commit is contained in:
danzaio
2026-06-10 14:37:58 -03:00
parent b73c9bce94
commit e6b0e76341
4 changed files with 47 additions and 15 deletions
+4 -6
View File
@@ -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.
@@ -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<Page> {
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);
}),
);
@@ -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<BrowserHandle> {
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({
@@ -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");
});
});