Merge PR #5778: fix(browser): reject non-string tab selectors with a named error (@roboomp)

# Conflicts:
#	packages/coding-agent/src/prompts/tools/browser.md
#	packages/coding-agent/src/tools/browser/aria/aria-snapshot.ts

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
 

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]`.

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<Ele
 
 const ARIA_REF_PREFIXES = ["aria-ref=", "aria-ref/", "ariaref/"];
 
+/**
+ * Guard the selector funnels: `tab.click`/`type`/`fill`/`waitFor*`/`scrollIntoView`
+ * take string selectors only, but user `run` code routinely passes the ElementHandle
+ * from `tab.id(n)`/`tab.ref(...)` (or an un-awaited Promise of one) straight in.
+ * Without this the value reaches `.trim()`/`.startsWith()` and throws the opaque,
+ * minified `A.trim is not a function` instead of a recovery-naming ToolError.
+ */
+export function assertSelectorString(selector: unknown): asserts selector is string {
+	if (typeof selector === "string") return;
+	let kind: string;
+	if (selector !== null && typeof selector === "object") {
+		kind =
+			"then" in selector && typeof selector.then === "function" ? "a Promise (missing await?)" : "an ElementHandle";
+	} else {
+		kind = `a ${typeof selector}`;
+	}
+	throw new ToolError(
+		`Browser selector must be a string; got ${kind}. ` +
+			"tab.click/type/fill/waitFor take string selectors only — " +
+			'call the handle method directly (e.g. (await tab.id(n)).click()) or pass a string like "aria-ref=eN".',
+	);
+}
+
 /**
  * Recognize a snapshot-ref selector and return the bare ref id, else null.
  * Accepts `aria-ref=e5` (Playwright-MCP style), `aria-ref/e5`, `ariaref/e5`,
@@ -80,6 +104,7 @@ const ARIA_REF_PREFIXES = ["aria-ref=", "aria-ref/", "ariaref/"];
  * observe ids; either way `eN` means "the id from the last page dump".)
  */
 export function parseAriaRefSelector(selector: string): string | null {
+	assertSelectorString(selector);
 	const trimmed = selector.trim();
 	for (const prefix of ARIA_REF_PREFIXES) {
 		if (trimmed.startsWith(prefix)) {

packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts

@@ -9,7 +9,7 @@ import type { ToolSession } from "../../index";
 import { resolveToCwd } from "../../path-utils";
 import { formatScreenshot } from "../../render-utils";
 import { ToolAbortError, ToolError, throwIfAborted } from "../../tool-errors";
-import { type AriaSnapshotOptions, buildAriaSnapshotScript } from "../aria/aria-snapshot";
+import { type AriaSnapshotOptions, assertSelectorString, buildAriaSnapshotScript } from "../aria/aria-snapshot";
 import { DEFAULT_VIEWPORT } from "../launch";
 import { extractReadableFromHtml, type ReadableFormat } from "../readable";
 import {
@@ -1015,6 +1015,7 @@ export class CmuxTab {
 	}
 
 	#selectorSpec(selector: string): SelectorSpec {
+		assertSelectorString(selector);
 		const raw = selector;
 		let normalized = selector;
 		if (normalized.startsWith("p-text/")) normalized = `text/${normalized.slice("p-text/".length)}`;

packages/coding-agent/src/tools/browser/tab-worker.ts

@@ -24,6 +24,7 @@ import { formatScreenshot } from "../render-utils";
 import { ToolAbortError, ToolError, throwIfAborted } from "../tool-errors";
 import {
 	type AriaSnapshotOptions,
+	assertSelectorString,
 	captureAriaSnapshot,
 	parseAriaRefSelector,
 	resolveAriaRefHandle,
@@ -265,6 +266,7 @@ interface TabApi {
 }
 
 export function normalizeSelector(selector: string): string {
+	assertSelectorString(selector);
 	if (!selector) return selector;
 	if (
 		!SELECTOR_HANDLER_PREFIXES.some(prefix => selector.startsWith(prefix)) &&

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", () => {

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/);
+	});
 });