diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d8e544f71..47f00ba7f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -126,6 +126,9 @@ ### Added - Added `PI_CONFIG_FILES`, a platform-delimited (`:` on Unix, `;` on Windows) environment path-list of settings overlays loaded before `--config` overlays, so wrapper scripts can inject settings without argv surgery ([#5685](https://github.com/can1357/oh-my-pi/issues/5685)). +### Fixed + +- Fixed browser `tab.click`/`type`/`fill`/`waitFor*`/`scrollIntoView` crashing with the opaque minified `A.trim is not a function` when handed the `ElementHandle` from `tab.id(n)`/`tab.ref(...)` (or an un-awaited `Promise` of one); the selector funnels now reject non-strings with a `ToolError` naming the recovery (`(await tab.id(n)).click()` or a string selector), and `browser.md` clarifies that handles are called directly rather than passed as selectors ([#5776](https://github.com/can1357/oh-my-pi/issues/5776)). ## [17.0.1] - 2026-07-16 diff --git a/packages/coding-agent/src/prompts/tools/browser.md b/packages/coding-agent/src/prompts/tools/browser.md index cbf57a5ab..76a917b33 100644 --- a/packages/coding-agent/src/prompts/tools/browser.md +++ b/packages/coding-agent/src/prompts/tools/browser.md @@ -6,7 +6,7 @@ Drives real Chromium tab; full puppeteer access via JS. - `run` scope: `page`, `browser`, `tab`, `display`, `assert`, `wait` available. `wait(fn)` polls until truthy — use instead of polling inside `tab.evaluate`. - `tab` helpers (drop to raw puppeteer `page` for anything uncovered): - Element handles: `tab.ref("e5")` / `tab.id(n)`. Snapshot refs work in any selector slot: `tab.click("e5")` ≡ `tab.click("aria-ref=e5")`. + Element handles: `tab.ref("e5")` / `tab.id(n)` return a handle you call methods on directly — `(await tab.id(n)).click()`. Handles are NOT selectors: `tab.click`/`type`/`fill`/`waitFor*` take STRING selectors only. Snapshot refs work in any selector slot: `tab.click("e5")` ≡ `tab.click("aria-ref=e5")`. Simple: `tab.goto`, `tab.click`, `tab.type`, `tab.fill`, `tab.press`, `tab.scroll`, `tab.scrollIntoView`, `tab.drag`, `tab.uploadFile`, `tab.select`, `tab.screenshot`, `tab.extract`, `tab.evaluate`. Waits: `tab.waitFor`, `tab.waitForSelector`, `tab.waitForUrl`, `tab.waitForResponse`, `tab.waitForNavigation`. Snapshots: `tab.observe()` → accessibility tree; `tab.ariaSnapshot()` → ARIA YAML with `[ref=eN]`. diff --git a/packages/coding-agent/src/tools/browser/aria/aria-snapshot.ts b/packages/coding-agent/src/tools/browser/aria/aria-snapshot.ts index bd25a12dd..693251281 100644 --- a/packages/coding-agent/src/tools/browser/aria/aria-snapshot.ts +++ b/packages/coding-agent/src/tools/browser/aria/aria-snapshot.ts @@ -1,4 +1,5 @@ import type { ElementHandle, JSHandle, Page } from "puppeteer-core"; +import { ToolError } from "../../tool-errors"; import ariaBundle from "./aria-snapshot.bundle.txt" with { type: "text" }; // `aria-snapshot.bundle.txt` is a generated, committed artifact: Playwright's // injected ARIA-snapshot sources (pinned, Apache-2.0) bundled to a CJS module. @@ -68,6 +69,29 @@ export async function resolveAriaRefHandle(page: Page, ref: string): Promise selector.startsWith(prefix)) && diff --git a/packages/coding-agent/test/tools/browser-aria-snapshot.test.ts b/packages/coding-agent/test/tools/browser-aria-snapshot.test.ts index 424acafda..cd54a850b 100644 --- a/packages/coding-agent/test/tools/browser-aria-snapshot.test.ts +++ b/packages/coding-agent/test/tools/browser-aria-snapshot.test.ts @@ -25,6 +25,22 @@ describe("parseAriaRefSelector", () => { expect(parseAriaRefSelector("e5x")).toBeNull(); // eN must be the whole selector expect(parseAriaRefSelector("section e5")).toBeNull(); // descendant CSS, not a ref }); + + it("rejects non-string selectors (handle/Promise) with a recovery-naming ToolError", () => { + // Regression: tab.click(await tab.id(n)) / tab.click(tab.id(n)) used to reach + // `selector.trim()` and throw the opaque minified `A.trim is not a function`. + const handle = { + click: async () => {}, + asElement() { + return this; + }, + }; + expect(() => parseAriaRefSelector(handle as never)).toThrow(/must be a string; got an ElementHandle/); + expect(() => parseAriaRefSelector(handle as never)).toThrow(/\(await tab\.id\(n\)\)\.click\(\)/); + const promise = Promise.resolve(handle); + expect(() => parseAriaRefSelector(promise as never)).toThrow(/got a Promise \(missing await\?\)/); + promise.catch(() => {}); + }); }); describe("buildAriaSnapshotScript", () => { diff --git a/packages/coding-agent/test/tools/browser-tab-timeouts.test.ts b/packages/coding-agent/test/tools/browser-tab-timeouts.test.ts index 96d8b0e0f..1c2858bf2 100644 --- a/packages/coding-agent/test/tools/browser-tab-timeouts.test.ts +++ b/packages/coding-agent/test/tools/browser-tab-timeouts.test.ts @@ -149,4 +149,17 @@ describe("browser selector guard", () => { it("still rewrites legacy p- prefixes", () => { expect(normalizeSelector("p-text/Continue")).toBe("text/Continue"); }); + + it("rejects non-string selectors (handle/number) instead of crashing on .startsWith", () => { + // Regression: passing the ElementHandle from tab.id()/tab.ref() reached + // `selector.startsWith(...)` and threw the opaque `A.trim is not a function`. + const handle = { + click: async () => {}, + asElement() { + return this; + }, + }; + expect(() => normalizeSelector(handle as never)).toThrow(/must be a string; got an ElementHandle/); + expect(() => normalizeSelector(23 as never)).toThrow(/must be a string; got a number/); + }); });