refactor: make browser actions ref-only
This commit is contained in:
@@ -14,8 +14,7 @@ export type BrowserFormField = {
|
||||
export type BrowserActRequest =
|
||||
| {
|
||||
kind: "click";
|
||||
ref?: string;
|
||||
selector?: string;
|
||||
ref: string;
|
||||
targetId?: string;
|
||||
doubleClick?: boolean;
|
||||
button?: string;
|
||||
@@ -23,8 +22,7 @@ export type BrowserActRequest =
|
||||
}
|
||||
| {
|
||||
kind: "type";
|
||||
ref?: string;
|
||||
selector?: string;
|
||||
ref: string;
|
||||
text: string;
|
||||
targetId?: string;
|
||||
submit?: boolean;
|
||||
|
||||
@@ -111,45 +111,6 @@ describe("pw-ai", () => {
|
||||
expect(p1.click).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("clicks a css selector when provided", async () => {
|
||||
const { chromium } = await import("playwright-core");
|
||||
const p1 = createPage({ targetId: "T1" });
|
||||
const browser = createBrowser([p1.page]);
|
||||
(
|
||||
chromium.connectOverCDP as unknown as ReturnType<typeof vi.fn>
|
||||
).mockResolvedValue(browser);
|
||||
|
||||
const mod = await importModule();
|
||||
await mod.clickViaPlaywright({
|
||||
cdpPort: 18792,
|
||||
targetId: "T1",
|
||||
selector: "button.save",
|
||||
});
|
||||
|
||||
expect(p1.locator).toHaveBeenCalledWith("button.save");
|
||||
expect(p1.click).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("types via css selector when provided", async () => {
|
||||
const { chromium } = await import("playwright-core");
|
||||
const p1 = createPage({ targetId: "T1" });
|
||||
const browser = createBrowser([p1.page]);
|
||||
(
|
||||
chromium.connectOverCDP as unknown as ReturnType<typeof vi.fn>
|
||||
).mockResolvedValue(browser);
|
||||
|
||||
const mod = await importModule();
|
||||
await mod.typeViaPlaywright({
|
||||
cdpPort: 18792,
|
||||
targetId: "T1",
|
||||
selector: "input[name=q]",
|
||||
text: "hello",
|
||||
});
|
||||
|
||||
expect(p1.locator).toHaveBeenCalledWith("input[name=q]");
|
||||
expect(p1.fill).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("fails with a clear error when _snapshotForAI is missing", async () => {
|
||||
const { chromium } = await import("playwright-core");
|
||||
const p1 = createPage({ targetId: "T1", hasSnapshotForAI: false });
|
||||
|
||||
@@ -10,18 +10,10 @@ import {
|
||||
let nextUploadArmId = 0;
|
||||
let nextDialogArmId = 0;
|
||||
|
||||
type LocatorPage = Parameters<typeof refLocator>[0];
|
||||
|
||||
function resolveLocator(
|
||||
page: LocatorPage,
|
||||
opts: { ref?: string; selector?: string },
|
||||
) {
|
||||
const selector =
|
||||
typeof opts.selector === "string" ? opts.selector.trim() : "";
|
||||
if (selector) return page.locator(selector);
|
||||
const ref = typeof opts.ref === "string" ? opts.ref.trim() : "";
|
||||
if (ref) return refLocator(page, ref);
|
||||
throw new Error("ref or selector is required");
|
||||
function requireRef(value: unknown): string {
|
||||
const ref = typeof value === "string" ? value.trim() : "";
|
||||
if (!ref) throw new Error("ref is required");
|
||||
return ref;
|
||||
}
|
||||
|
||||
export async function snapshotAiViaPlaywright(opts: {
|
||||
@@ -55,8 +47,7 @@ export async function snapshotAiViaPlaywright(opts: {
|
||||
export async function clickViaPlaywright(opts: {
|
||||
cdpPort: number;
|
||||
targetId?: string;
|
||||
ref?: string;
|
||||
selector?: string;
|
||||
ref: string;
|
||||
doubleClick?: boolean;
|
||||
button?: "left" | "right" | "middle";
|
||||
modifiers?: Array<"Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift">;
|
||||
@@ -67,10 +58,7 @@ export async function clickViaPlaywright(opts: {
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
ensurePageState(page);
|
||||
const locator = resolveLocator(page, {
|
||||
ref: opts.ref,
|
||||
selector: opts.selector,
|
||||
});
|
||||
const locator = refLocator(page, requireRef(opts.ref));
|
||||
const timeout = Math.max(
|
||||
500,
|
||||
Math.min(60_000, Math.floor(opts.timeoutMs ?? 8000)),
|
||||
@@ -157,8 +145,7 @@ export async function pressKeyViaPlaywright(opts: {
|
||||
export async function typeViaPlaywright(opts: {
|
||||
cdpPort: number;
|
||||
targetId?: string;
|
||||
ref?: string;
|
||||
selector?: string;
|
||||
ref: string;
|
||||
text: string;
|
||||
submit?: boolean;
|
||||
slowly?: boolean;
|
||||
@@ -167,10 +154,7 @@ export async function typeViaPlaywright(opts: {
|
||||
const text = String(opts.text ?? "");
|
||||
const page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
const locator = resolveLocator(page, {
|
||||
ref: opts.ref,
|
||||
selector: opts.selector,
|
||||
});
|
||||
const locator = refLocator(page, requireRef(opts.ref));
|
||||
const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000));
|
||||
if (opts.slowly) {
|
||||
await locator.click({ timeout });
|
||||
|
||||
@@ -35,6 +35,16 @@ type ActKind =
|
||||
type ClickButton = "left" | "right" | "middle";
|
||||
type ClickModifier = "Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift";
|
||||
|
||||
const SELECTOR_UNSUPPORTED_MESSAGE = [
|
||||
"Error: 'selector' is not supported. Use 'ref' from snapshot instead.",
|
||||
"",
|
||||
"Example workflow:",
|
||||
"1. snapshot action to get page state with refs",
|
||||
'2. act with ref: "e123" to interact with element',
|
||||
"",
|
||||
"This is more reliable for modern SPAs.",
|
||||
].join("\n");
|
||||
|
||||
function readBody(req: express.Request): Record<string, unknown> {
|
||||
const body = req.body as Record<string, unknown> | undefined;
|
||||
if (!body || typeof body !== "object" || Array.isArray(body)) return {};
|
||||
@@ -113,6 +123,9 @@ export function registerBrowserAgentRoutes(
|
||||
const body = readBody(req);
|
||||
const kind = toStringOrEmpty(body.kind) as ActKind;
|
||||
const targetId = toStringOrEmpty(body.targetId) || undefined;
|
||||
if (Object.prototype.hasOwnProperty.call(body, "selector")) {
|
||||
return jsonError(res, 400, SELECTOR_UNSUPPORTED_MESSAGE);
|
||||
}
|
||||
|
||||
if (
|
||||
kind !== "click" &&
|
||||
@@ -139,9 +152,7 @@ export function registerBrowserAgentRoutes(
|
||||
switch (kind) {
|
||||
case "click": {
|
||||
const ref = toStringOrEmpty(body.ref);
|
||||
const selector = toStringOrEmpty(body.selector);
|
||||
if (!ref && !selector)
|
||||
return jsonError(res, 400, "ref or selector is required");
|
||||
if (!ref) return jsonError(res, 400, "ref is required");
|
||||
const doubleClick = toBoolean(body.doubleClick) ?? false;
|
||||
const buttonRaw = toStringOrEmpty(body.button) || "";
|
||||
const button = buttonRaw ? parseClickButton(buttonRaw) : undefined;
|
||||
@@ -171,10 +182,9 @@ export function registerBrowserAgentRoutes(
|
||||
const clickRequest: Parameters<typeof pw.clickViaPlaywright>[0] = {
|
||||
cdpPort,
|
||||
targetId: tab.targetId,
|
||||
ref,
|
||||
doubleClick,
|
||||
};
|
||||
if (ref) clickRequest.ref = ref;
|
||||
if (selector) clickRequest.selector = selector;
|
||||
if (button) clickRequest.button = button;
|
||||
if (modifiers) clickRequest.modifiers = modifiers;
|
||||
await pw.clickViaPlaywright(clickRequest);
|
||||
@@ -182,9 +192,7 @@ export function registerBrowserAgentRoutes(
|
||||
}
|
||||
case "type": {
|
||||
const ref = toStringOrEmpty(body.ref);
|
||||
const selector = toStringOrEmpty(body.selector);
|
||||
if (!ref && !selector)
|
||||
return jsonError(res, 400, "ref or selector is required");
|
||||
if (!ref) return jsonError(res, 400, "ref is required");
|
||||
if (typeof body.text !== "string")
|
||||
return jsonError(res, 400, "text is required");
|
||||
const text = body.text;
|
||||
@@ -193,12 +201,11 @@ export function registerBrowserAgentRoutes(
|
||||
const typeRequest: Parameters<typeof pw.typeViaPlaywright>[0] = {
|
||||
cdpPort,
|
||||
targetId: tab.targetId,
|
||||
ref,
|
||||
text,
|
||||
submit,
|
||||
slowly,
|
||||
};
|
||||
if (ref) typeRequest.ref = ref;
|
||||
if (selector) typeRequest.selector = selector;
|
||||
await pw.typeViaPlaywright(typeRequest);
|
||||
return res.json({ ok: true, targetId: tab.targetId });
|
||||
}
|
||||
|
||||
@@ -327,21 +327,17 @@ describe("browser control server", () => {
|
||||
modifiers: ["Shift"],
|
||||
});
|
||||
|
||||
const clickSelector = (await realFetch(`${base}/act`, {
|
||||
const clickSelector = await realFetch(`${base}/act`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
kind: "click",
|
||||
selector: "button.save",
|
||||
}),
|
||||
}).then((r) => r.json())) as { ok: boolean };
|
||||
expect(clickSelector.ok).toBe(true);
|
||||
expect(pwMocks.clickViaPlaywright).toHaveBeenNthCalledWith(2, {
|
||||
cdpPort: testPort + 1,
|
||||
targetId: "abcd1234",
|
||||
selector: "button.save",
|
||||
doubleClick: false,
|
||||
});
|
||||
expect(clickSelector.status).toBe(400);
|
||||
const clickSelectorBody = (await clickSelector.json()) as { error?: string };
|
||||
expect(clickSelectorBody.error).toMatch(/selector is not supported/i);
|
||||
|
||||
const type = (await realFetch(`${base}/act`, {
|
||||
method: "POST",
|
||||
@@ -358,26 +354,6 @@ describe("browser control server", () => {
|
||||
slowly: false,
|
||||
});
|
||||
|
||||
const typeSelector = (await realFetch(`${base}/act`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
kind: "type",
|
||||
selector: "input[name=q]",
|
||||
text: "hello",
|
||||
submit: true,
|
||||
}),
|
||||
}).then((r) => r.json())) as { ok: boolean };
|
||||
expect(typeSelector.ok).toBe(true);
|
||||
expect(pwMocks.typeViaPlaywright).toHaveBeenNthCalledWith(2, {
|
||||
cdpPort: testPort + 1,
|
||||
targetId: "abcd1234",
|
||||
selector: "input[name=q]",
|
||||
text: "hello",
|
||||
submit: true,
|
||||
slowly: false,
|
||||
});
|
||||
|
||||
const press = (await realFetch(`${base}/act`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
Reference in New Issue
Block a user