fix(browser): persist role refs per targetId
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -70,6 +70,12 @@ type PageState = {
|
||||
roleRefsFrameSelector?: string;
|
||||
};
|
||||
|
||||
type RoleRefs = NonNullable<PageState["roleRefs"]>;
|
||||
type RoleRefsCacheEntry = {
|
||||
refs: RoleRefs;
|
||||
frameSelector?: string;
|
||||
};
|
||||
|
||||
type ContextState = {
|
||||
traceActive: boolean;
|
||||
};
|
||||
@@ -79,6 +85,11 @@ const contextStates = new WeakMap<BrowserContext, ContextState>();
|
||||
const observedContexts = new WeakSet<BrowserContext>();
|
||||
const observedPages = new WeakSet<Page>();
|
||||
|
||||
// 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<string, RoleRefsCacheEntry>();
|
||||
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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user