From d5d33d4848d7290ff627debf5cb5ed0f2c6ff8a7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 15 Jan 2026 09:56:19 +0000 Subject: [PATCH] fix(browser): persist role refs per targetId --- CHANGELOG.md | 1 + src/browser/pw-session.test.ts | 27 +++++++++- src/browser/pw-session.ts | 49 +++++++++++++++++++ ...re.clamps-timeoutms-scrollintoview.test.ts | 2 + src/browser/pw-tools-core.downloads.ts | 8 ++- src/browser/pw-tools-core.interactions.ts | 15 +++++- ...ls-core.last-file-chooser-arm-wins.test.ts | 2 + ...-core.screenshots-element-selector.test.ts | 2 + src/browser/pw-tools-core.snapshot.ts | 15 +++++- ...-core.waits-next-download-saves-it.test.ts | 2 + 10 files changed, 119 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d9e3fb7e..f8eb60804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ - Gateway/UI: ship session defaults in the hello snapshot so the Control UI canonicalizes main session keys (no bare `main` alias). - Agents: skip thinking/final tag stripping inside Markdown code spans. (#939) — thanks @ngutman. - Browser: add tests for snapshot labels/efficient query params and labeled image responses. +- Browser: persist role snapshot refs per CDP target so `snapshot` → `act` clicks work even if Playwright returns a different Page instance. - macOS: ensure launchd log directory exists with a test-only override. (#909) — thanks @roshanasingh4. - macOS: format ConnectionsStore config to satisfy SwiftFormat lint. (#852) — thanks @mneves75. - Packaging: run `pnpm build` on `prepack` so npm publishes include fresh `dist/` output. diff --git a/src/browser/pw-session.test.ts b/src/browser/pw-session.test.ts index 90fc2a8c2..ec5c75135 100644 --- a/src/browser/pw-session.test.ts +++ b/src/browser/pw-session.test.ts @@ -1,7 +1,12 @@ import type { Page } from "playwright-core"; import { describe, expect, it, vi } from "vitest"; -import { ensurePageState, refLocator } from "./pw-session.js"; +import { + ensurePageState, + refLocator, + rememberRoleRefsForTarget, + restoreRoleRefsForTarget, +} from "./pw-session.js"; function fakePage(): { page: Page; @@ -59,6 +64,26 @@ describe("pw-session refLocator", () => { }); }); +describe("pw-session role refs cache", () => { + it("restores refs for a different Page instance (same CDP targetId)", () => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = "t1"; + + rememberRoleRefsForTarget({ + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "OK" } }, + frameSelector: "iframe#main", + }); + + const { page, mocks } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page }); + + refLocator(page, "e1"); + expect(mocks.frameLocator).toHaveBeenCalledWith("iframe#main"); + }); +}); + describe("pw-session ensurePageState", () => { it("tracks page errors and network requests (best-effort)", () => { const { page, handlers } = fakePage(); diff --git a/src/browser/pw-session.ts b/src/browser/pw-session.ts index df081af8a..7997fe151 100644 --- a/src/browser/pw-session.ts +++ b/src/browser/pw-session.ts @@ -70,6 +70,12 @@ type PageState = { roleRefsFrameSelector?: string; }; +type RoleRefs = NonNullable; +type RoleRefsCacheEntry = { + refs: RoleRefs; + frameSelector?: string; +}; + type ContextState = { traceActive: boolean; }; @@ -79,6 +85,11 @@ const contextStates = new WeakMap(); const observedContexts = new WeakSet(); const observedPages = new WeakSet(); +// Best-effort cache to make role refs stable even if Playwright returns a different Page object +// for the same CDP target across requests. +const roleRefsByTarget = new Map(); +const MAX_ROLE_REFS_CACHE = 50; + const MAX_CONSOLE_MESSAGES = 500; const MAX_PAGE_ERRORS = 200; const MAX_NETWORK_REQUESTS = 500; @@ -90,6 +101,44 @@ function normalizeCdpUrl(raw: string) { return raw.replace(/\/$/, ""); } +function roleRefsKey(cdpUrl: string, targetId: string) { + return `${normalizeCdpUrl(cdpUrl)}::${targetId}`; +} + +export function rememberRoleRefsForTarget(opts: { + cdpUrl: string; + targetId: string; + refs: RoleRefs; + frameSelector?: string; +}): void { + const targetId = opts.targetId.trim(); + if (!targetId) return; + roleRefsByTarget.set(roleRefsKey(opts.cdpUrl, targetId), { + refs: opts.refs, + ...(opts.frameSelector ? { frameSelector: opts.frameSelector } : {}), + }); + while (roleRefsByTarget.size > MAX_ROLE_REFS_CACHE) { + const first = roleRefsByTarget.keys().next(); + if (first.done) break; + roleRefsByTarget.delete(first.value); + } +} + +export function restoreRoleRefsForTarget(opts: { + cdpUrl: string; + targetId?: string; + page: Page; +}): void { + const targetId = opts.targetId?.trim() || ""; + if (!targetId) return; + const cached = roleRefsByTarget.get(roleRefsKey(opts.cdpUrl, targetId)); + if (!cached) return; + const state = ensurePageState(opts.page); + if (state.roleRefs) return; + state.roleRefs = cached.refs; + state.roleRefsFrameSelector = cached.frameSelector; +} + export function ensurePageState(page: Page): PageState { const existing = pageStates.get(page); if (existing) return existing; diff --git a/src/browser/pw-tools-core.clamps-timeoutms-scrollintoview.test.ts b/src/browser/pw-tools-core.clamps-timeoutms-scrollintoview.test.ts index 60713a551..68586c2b6 100644 --- a/src/browser/pw-tools-core.clamps-timeoutms-scrollintoview.test.ts +++ b/src/browser/pw-tools-core.clamps-timeoutms-scrollintoview.test.ts @@ -15,10 +15,12 @@ const sessionMocks = vi.hoisted(() => ({ return currentPage; }), ensurePageState: vi.fn(() => pageState), + restoreRoleRefsForTarget: vi.fn(() => {}), refLocator: vi.fn(() => { if (!currentRefLocator) throw new Error("missing locator"); return currentRefLocator; }), + rememberRoleRefsForTarget: vi.fn(() => {}), })); vi.mock("./pw-session.js", () => sessionMocks); diff --git a/src/browser/pw-tools-core.downloads.ts b/src/browser/pw-tools-core.downloads.ts index bc2b3c534..3f8e89c26 100644 --- a/src/browser/pw-tools-core.downloads.ts +++ b/src/browser/pw-tools-core.downloads.ts @@ -4,7 +4,12 @@ import path from "node:path"; import type { Page } from "playwright-core"; -import { ensurePageState, getPageForTargetId, refLocator } from "./pw-session.js"; +import { + ensurePageState, + getPageForTargetId, + refLocator, + restoreRoleRefsForTarget, +} from "./pw-session.js"; import { bumpDialogArmId, bumpDownloadArmId, @@ -189,6 +194,7 @@ export async function downloadViaPlaywright(opts: { }> { const page = await getPageForTargetId(opts); const state = ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); const timeout = normalizeTimeoutMs(opts.timeoutMs, 120_000); const ref = requireRef(opts.ref); diff --git a/src/browser/pw-tools-core.interactions.ts b/src/browser/pw-tools-core.interactions.ts index eaa3d16d6..0f652be34 100644 --- a/src/browser/pw-tools-core.interactions.ts +++ b/src/browser/pw-tools-core.interactions.ts @@ -1,5 +1,10 @@ import type { BrowserFormField } from "./client-actions-core.js"; -import { ensurePageState, getPageForTargetId, refLocator } from "./pw-session.js"; +import { + ensurePageState, + getPageForTargetId, + refLocator, + restoreRoleRefsForTarget, +} from "./pw-session.js"; import { normalizeTimeoutMs, requireRef, toAIFriendlyError } from "./pw-tools-core.shared.js"; export async function highlightViaPlaywright(opts: { @@ -9,6 +14,7 @@ export async function highlightViaPlaywright(opts: { }): Promise { const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); const ref = requireRef(opts.ref); try { await refLocator(page, ref).highlight(); @@ -31,6 +37,7 @@ export async function clickViaPlaywright(opts: { targetId: opts.targetId, }); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); const ref = requireRef(opts.ref); const locator = refLocator(page, ref); const timeout = Math.max(500, Math.min(60_000, Math.floor(opts.timeoutMs ?? 8000))); @@ -62,6 +69,7 @@ export async function hoverViaPlaywright(opts: { const ref = requireRef(opts.ref); const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); try { await refLocator(page, ref).hover({ timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), @@ -83,6 +91,7 @@ export async function dragViaPlaywright(opts: { if (!startRef || !endRef) throw new Error("startRef and endRef are required"); const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); try { await refLocator(page, startRef).dragTo(refLocator(page, endRef), { timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), @@ -103,6 +112,7 @@ export async function selectOptionViaPlaywright(opts: { if (!opts.values?.length) throw new Error("values are required"); const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); try { await refLocator(page, ref).selectOption(opts.values, { timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), @@ -139,6 +149,7 @@ export async function typeViaPlaywright(opts: { const text = String(opts.text ?? ""); const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); const ref = requireRef(opts.ref); const locator = refLocator(page, ref); const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)); @@ -165,6 +176,7 @@ export async function fillFormViaPlaywright(opts: { }): Promise { const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)); for (const field of opts.fields) { const ref = field.ref.trim(); @@ -206,6 +218,7 @@ export async function evaluateViaPlaywright(opts: { if (!fnText) throw new Error("function is required"); const page = await getPageForTargetId(opts); ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); if (opts.ref) { const locator = refLocator(page, opts.ref); // Use Function constructor at runtime to avoid esbuild adding __name helper diff --git a/src/browser/pw-tools-core.last-file-chooser-arm-wins.test.ts b/src/browser/pw-tools-core.last-file-chooser-arm-wins.test.ts index 7684cb367..ffc371b62 100644 --- a/src/browser/pw-tools-core.last-file-chooser-arm-wins.test.ts +++ b/src/browser/pw-tools-core.last-file-chooser-arm-wins.test.ts @@ -15,10 +15,12 @@ const sessionMocks = vi.hoisted(() => ({ return currentPage; }), ensurePageState: vi.fn(() => pageState), + restoreRoleRefsForTarget: vi.fn(() => {}), refLocator: vi.fn(() => { if (!currentRefLocator) throw new Error("missing locator"); return currentRefLocator; }), + rememberRoleRefsForTarget: vi.fn(() => {}), })); vi.mock("./pw-session.js", () => sessionMocks); diff --git a/src/browser/pw-tools-core.screenshots-element-selector.test.ts b/src/browser/pw-tools-core.screenshots-element-selector.test.ts index 61e9ac849..a5ff14756 100644 --- a/src/browser/pw-tools-core.screenshots-element-selector.test.ts +++ b/src/browser/pw-tools-core.screenshots-element-selector.test.ts @@ -15,10 +15,12 @@ const sessionMocks = vi.hoisted(() => ({ return currentPage; }), ensurePageState: vi.fn(() => pageState), + restoreRoleRefsForTarget: vi.fn(() => {}), refLocator: vi.fn(() => { if (!currentRefLocator) throw new Error("missing locator"); return currentRefLocator; }), + rememberRoleRefsForTarget: vi.fn(() => {}), })); vi.mock("./pw-session.js", () => sessionMocks); diff --git a/src/browser/pw-tools-core.snapshot.ts b/src/browser/pw-tools-core.snapshot.ts index f7f4c3a08..92ab149aa 100644 --- a/src/browser/pw-tools-core.snapshot.ts +++ b/src/browser/pw-tools-core.snapshot.ts @@ -6,7 +6,12 @@ import { getRoleSnapshotStats, type RoleSnapshotOptions, } from "./pw-role-snapshot.js"; -import { ensurePageState, getPageForTargetId, type WithSnapshotForAI } from "./pw-session.js"; +import { + ensurePageState, + getPageForTargetId, + rememberRoleRefsForTarget, + type WithSnapshotForAI, +} from "./pw-session.js"; export async function snapshotAriaViaPlaywright(opts: { cdpUrl: string; @@ -97,6 +102,14 @@ export async function snapshotRoleViaPlaywright(opts: { const built = buildRoleSnapshotFromAriaSnapshot(String(ariaSnapshot ?? ""), opts.options); state.roleRefs = built.refs; state.roleRefsFrameSelector = frameSelector || undefined; + if (opts.targetId) { + rememberRoleRefsForTarget({ + cdpUrl: opts.cdpUrl, + targetId: opts.targetId, + refs: built.refs, + frameSelector: frameSelector || undefined, + }); + } return { snapshot: built.snapshot, refs: built.refs, diff --git a/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts b/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts index dd55a1a95..90151c48f 100644 --- a/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts +++ b/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts @@ -16,10 +16,12 @@ const sessionMocks = vi.hoisted(() => ({ return currentPage; }), ensurePageState: vi.fn(() => pageState), + restoreRoleRefsForTarget: vi.fn(() => {}), refLocator: vi.fn(() => { if (!currentRefLocator) throw new Error("missing locator"); return currentRefLocator; }), + rememberRoleRefsForTarget: vi.fn(() => {}), })); vi.mock("./pw-session.js", () => sessionMocks);