Browser: scope nested batch failures in switch

src/browser/pw-tools-core.interactions.batch.test.ts

@@ -0,0 +1,131 @@
+import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
+
+let page: { evaluate: ReturnType<typeof vi.fn> } | null = null;
+
+const getPageForTargetId = vi.fn(async () => {
+  if (!page) {
+    throw new Error("test: page not set");
+  }
+  return page;
+});
+const ensurePageState = vi.fn(() => {});
+const forceDisconnectPlaywrightForTarget = vi.fn(async () => {});
+const refLocator = vi.fn(() => {
+  throw new Error("test: refLocator should not be called");
+});
+const restoreRoleRefsForTarget = vi.fn(() => {});
+
+const closePageViaPlaywright = vi.fn(async () => {});
+const resizeViewportViaPlaywright = vi.fn(async () => {});
+
+vi.mock("./pw-session.js", () => ({
+  ensurePageState,
+  forceDisconnectPlaywrightForTarget,
+  getPageForTargetId,
+  refLocator,
+  restoreRoleRefsForTarget,
+}));
+
+vi.mock("./pw-tools-core.snapshot.js", () => ({
+  closePageViaPlaywright,
+  resizeViewportViaPlaywright,
+}));
+
+let batchViaPlaywright: typeof import("./pw-tools-core.interactions.js").batchViaPlaywright;
+
+describe("batchViaPlaywright", () => {
+  beforeAll(async () => {
+    ({ batchViaPlaywright } = await import("./pw-tools-core.interactions.js"));
+  });
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    page = {
+      evaluate: vi.fn(async () => "ok"),
+    };
+  });
+
+  it("propagates evaluate timeouts through batched execution", async () => {
+    const result = await batchViaPlaywright({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+      evaluateEnabled: true,
+      actions: [{ kind: "evaluate", fn: "() => 1", timeoutMs: 5000 }],
+    });
+
+    expect(result).toEqual({ results: [{ ok: true }] });
+    expect(page?.evaluate).toHaveBeenCalledWith(
+      expect.any(Function),
+      expect.objectContaining({
+        fnBody: "() => 1",
+        timeoutMs: 4500,
+      }),
+    );
+  });
+
+  it("supports resize and close inside a batch", async () => {
+    const result = await batchViaPlaywright({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+      actions: [{ kind: "resize", width: 800, height: 600 }, { kind: "close" }],
+    });
+
+    expect(result).toEqual({ results: [{ ok: true }, { ok: true }] });
+    expect(resizeViewportViaPlaywright).toHaveBeenCalledWith({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+      width: 800,
+      height: 600,
+    });
+    expect(closePageViaPlaywright).toHaveBeenCalledWith({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+    });
+  });
+
+  it("propagates nested batch failures to the parent batch result", async () => {
+    const result = await batchViaPlaywright({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+      actions: [
+        {
+          kind: "batch",
+          actions: [{ kind: "evaluate", fn: "() => 1" }],
+        },
+      ],
+    });
+
+    expect(result).toEqual({
+      results: [
+        { ok: false, error: "act:evaluate is disabled by config (browser.evaluateEnabled=false)" },
+      ],
+    });
+  });
+
+  it("includes all nested batch failures when stopOnError is false", async () => {
+    const result = await batchViaPlaywright({
+      cdpUrl: "http://127.0.0.1:9222",
+      targetId: "tab-1",
+      actions: [
+        {
+          kind: "batch",
+          stopOnError: false,
+          actions: [
+            { kind: "evaluate", fn: "() => 1" },
+            { kind: "evaluate", fn: "() => 2" },
+          ],
+        },
+      ],
+    });
+
+    expect(result).toEqual({
+      results: [
+        {
+          ok: false,
+          error:
+            "act:evaluate is disabled by config (browser.evaluateEnabled=false); act:evaluate is disabled by config (browser.evaluateEnabled=false)",
+        },
+      ],
+    });
+  });
+});

src/browser/pw-tools-core.interactions.ts

@@ -1,4 +1,4 @@
-import type { BrowserFormField } from "./client-actions-core.js";
+import type { BrowserActRequest, BrowserFormField } from "./client-actions-core.js";
 import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js";
 import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js";
 import {
@@ -8,12 +8,32 @@ import {
   refLocator,
   restoreRoleRefsForTarget,
 } from "./pw-session.js";
-import { normalizeTimeoutMs, requireRef, toAIFriendlyError } from "./pw-tools-core.shared.js";
+import {
+  normalizeTimeoutMs,
+  requireRef,
+  requireRefOrSelector,
+  toAIFriendlyError,
+} from "./pw-tools-core.shared.js";
+import { closePageViaPlaywright, resizeViewportViaPlaywright } from "./pw-tools-core.snapshot.js";
 
 type TargetOpts = {
   cdpUrl: string;
   targetId?: string;
 };
+const MAX_CLICK_DELAY_MS = 5_000;
+const MAX_WAIT_TIME_MS = 30_000;
+const MAX_BATCH_ACTIONS = 100;
+
+function resolveBoundedDelayMs(value: number | undefined, label: string, maxMs: number): number {
+  const normalized = Math.floor(value ?? 0);
+  if (!Number.isFinite(normalized) || normalized < 0) {
+    throw new Error(`${label} must be >= 0`);
+  }
+  if (normalized > maxMs) {
+    throw new Error(`${label} exceeds maximum of ${maxMs}ms`);
+  }
+  return normalized;
+}
 
 async function getRestoredPageForTarget(opts: TargetOpts) {
   const page = await getPageForTargetId(opts);
@@ -59,17 +79,27 @@ export async function highlightViaPlaywright(opts: {
 export async function clickViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  ref: string;
+  ref?: string;
+  selector?: string;
   doubleClick?: boolean;
   button?: "left" | "right" | "middle";
   modifiers?: Array<"Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift">;
+  delayMs?: number;
   timeoutMs?: number;
 }): Promise<void> {
+  const resolved = requireRefOrSelector(opts.ref, opts.selector);
   const page = await getRestoredPageForTarget(opts);
-  const ref = requireRef(opts.ref);
+  const label = resolved.ref ?? resolved.selector!;
-  const locator = refLocator(page, ref);
+  const locator = resolved.ref
+    ? refLocator(page, requireRef(resolved.ref))
+    : page.locator(resolved.selector);
   const timeout = resolveInteractionTimeoutMs(opts.timeoutMs);
   try {
+    const delayMs = resolveBoundedDelayMs(opts.delayMs, "click delayMs", MAX_CLICK_DELAY_MS);
+    if (delayMs > 0) {
+      await locator.hover({ timeout });
+      await new Promise((r) => setTimeout(r, delayMs));
+    }
     if (opts.doubleClick) {
       await locator.dblclick({
         timeout,
@@ -84,67 +114,84 @@ export async function clickViaPlaywright(opts: {
       });
     }
   } catch (err) {
-    throw toAIFriendlyError(err, ref);
+    throw toAIFriendlyError(err, label);
   }
 }
 
 export async function hoverViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  ref: string;
+  ref?: string;
+  selector?: string;
   timeoutMs?: number;
 }): Promise<void> {
-  const ref = requireRef(opts.ref);
+  const resolved = requireRefOrSelector(opts.ref, opts.selector);
   const page = await getRestoredPageForTarget(opts);
+  const label = resolved.ref ?? resolved.selector!;
+  const locator = resolved.ref
+    ? refLocator(page, requireRef(resolved.ref))
+    : page.locator(resolved.selector);
   try {
-    await refLocator(page, ref).hover({
+    await locator.hover({
       timeout: resolveInteractionTimeoutMs(opts.timeoutMs),
     });
   } catch (err) {
-    throw toAIFriendlyError(err, ref);
+    throw toAIFriendlyError(err, label);
   }
 }
 
 export async function dragViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  startRef: string;
+  startRef?: string;
-  endRef: string;
+  startSelector?: string;
+  endRef?: string;
+  endSelector?: string;
   timeoutMs?: number;
 }): Promise<void> {
-  const startRef = requireRef(opts.startRef);
+  const resolvedStart = requireRefOrSelector(opts.startRef, opts.startSelector);
-  const endRef = requireRef(opts.endRef);
+  const resolvedEnd = requireRefOrSelector(opts.endRef, opts.endSelector);
-  if (!startRef || !endRef) {
-    throw new Error("startRef and endRef are required");
-  }
   const page = await getRestoredPageForTarget(opts);
+  const startLocator = resolvedStart.ref
+    ? refLocator(page, requireRef(resolvedStart.ref))
+    : page.locator(resolvedStart.selector);
+  const endLocator = resolvedEnd.ref
+    ? refLocator(page, requireRef(resolvedEnd.ref))
+    : page.locator(resolvedEnd.selector);
+  const startLabel = resolvedStart.ref ?? resolvedStart.selector!;
+  const endLabel = resolvedEnd.ref ?? resolvedEnd.selector!;
   try {
-    await refLocator(page, startRef).dragTo(refLocator(page, endRef), {
+    await startLocator.dragTo(endLocator, {
       timeout: resolveInteractionTimeoutMs(opts.timeoutMs),
     });
   } catch (err) {
-    throw toAIFriendlyError(err, `${startRef} -> ${endRef}`);
+    throw toAIFriendlyError(err, `${startLabel} -> ${endLabel}`);
   }
 }
 
 export async function selectOptionViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  ref: string;
+  ref?: string;
+  selector?: string;
   values: string[];
   timeoutMs?: number;
 }): Promise<void> {
-  const ref = requireRef(opts.ref);
+  const resolved = requireRefOrSelector(opts.ref, opts.selector);
   if (!opts.values?.length) {
     throw new Error("values are required");
   }
   const page = await getRestoredPageForTarget(opts);
+  const label = resolved.ref ?? resolved.selector!;
+  const locator = resolved.ref
+    ? refLocator(page, requireRef(resolved.ref))
+    : page.locator(resolved.selector);
   try {
-    await refLocator(page, ref).selectOption(opts.values, {
+    await locator.selectOption(opts.values, {
       timeout: resolveInteractionTimeoutMs(opts.timeoutMs),
     });
   } catch (err) {
-    throw toAIFriendlyError(err, ref);
+    throw toAIFriendlyError(err, label);
   }
 }
 
@@ -168,16 +215,20 @@ export async function pressKeyViaPlaywright(opts: {
 export async function typeViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  ref: string;
+  ref?: string;
+  selector?: string;
   text: string;
   submit?: boolean;
   slowly?: boolean;
   timeoutMs?: number;
 }): Promise<void> {
+  const resolved = requireRefOrSelector(opts.ref, opts.selector);
   const text = String(opts.text ?? "");
   const page = await getRestoredPageForTarget(opts);
-  const ref = requireRef(opts.ref);
+  const label = resolved.ref ?? resolved.selector!;
-  const locator = refLocator(page, ref);
+  const locator = resolved.ref
+    ? refLocator(page, requireRef(resolved.ref))
+    : page.locator(resolved.selector);
   const timeout = resolveInteractionTimeoutMs(opts.timeoutMs);
   try {
     if (opts.slowly) {
@@ -190,7 +241,7 @@ export async function typeViaPlaywright(opts: {
       await locator.press("Enter", { timeout });
     }
   } catch (err) {
-    throw toAIFriendlyError(err, ref);
+    throw toAIFriendlyError(err, label);
   }
 }
 
@@ -367,18 +418,22 @@ export async function evaluateViaPlaywright(opts: {
 export async function scrollIntoViewViaPlaywright(opts: {
   cdpUrl: string;
   targetId?: string;
-  ref: string;
+  ref?: string;
+  selector?: string;
   timeoutMs?: number;
 }): Promise<void> {
+  const resolved = requireRefOrSelector(opts.ref, opts.selector);
   const page = await getRestoredPageForTarget(opts);
   const timeout = normalizeTimeoutMs(opts.timeoutMs, 20_000);
 
-  const ref = requireRef(opts.ref);
+  const label = resolved.ref ?? resolved.selector!;
-  const locator = refLocator(page, ref);
+  const locator = resolved.ref
+    ? refLocator(page, requireRef(resolved.ref))
+    : page.locator(resolved.selector);
   try {
     await locator.scrollIntoViewIfNeeded({ timeout });
   } catch (err) {
-    throw toAIFriendlyError(err, ref);
+    throw toAIFriendlyError(err, label);
   }
 }
 
@@ -399,7 +454,7 @@ export async function waitForViaPlaywright(opts: {
   const timeout = normalizeTimeoutMs(opts.timeoutMs, 20_000);
 
   if (typeof opts.timeMs === "number" && Number.isFinite(opts.timeMs)) {
-    await page.waitForTimeout(Math.max(0, opts.timeMs));
+    await page.waitForTimeout(resolveBoundedDelayMs(opts.timeMs, "wait timeMs", MAX_WAIT_TIME_MS));
   }
   if (opts.text) {
     await page.getByText(opts.text).first().waitFor({
@@ -648,3 +703,197 @@ export async function setInputFilesViaPlaywright(opts: {
     // Best-effort for sites that don't react to setInputFiles alone.
   }
 }
+
+const MAX_BATCH_DEPTH = 5;
+
+async function executeSingleAction(
+  action: BrowserActRequest,
+  cdpUrl: string,
+  targetId?: string,
+  evaluateEnabled?: boolean,
+  depth = 0,
+): Promise<void> {
+  if (depth > MAX_BATCH_DEPTH) {
+    throw new Error(`Batch nesting depth exceeds maximum of ${MAX_BATCH_DEPTH}`);
+  }
+  const effectiveTargetId = action.targetId ?? targetId;
+  switch (action.kind) {
+    case "click":
+      await clickViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        ref: action.ref,
+        selector: action.selector,
+        doubleClick: action.doubleClick,
+        button: action.button as "left" | "right" | "middle" | undefined,
+        modifiers: action.modifiers as Array<
+          "Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift"
+        >,
+        delayMs: action.delayMs,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "type":
+      await typeViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        ref: action.ref,
+        selector: action.selector,
+        text: action.text,
+        submit: action.submit,
+        slowly: action.slowly,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "press":
+      await pressKeyViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        key: action.key,
+        delayMs: action.delayMs,
+      });
+      break;
+    case "hover":
+      await hoverViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        ref: action.ref,
+        selector: action.selector,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "scrollIntoView":
+      await scrollIntoViewViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        ref: action.ref,
+        selector: action.selector,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "drag":
+      await dragViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        startRef: action.startRef,
+        startSelector: action.startSelector,
+        endRef: action.endRef,
+        endSelector: action.endSelector,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "select":
+      await selectOptionViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        ref: action.ref,
+        selector: action.selector,
+        values: action.values,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "fill":
+      await fillFormViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        fields: action.fields,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "resize":
+      await resizeViewportViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        width: action.width,
+        height: action.height,
+      });
+      break;
+    case "wait":
+      if (action.fn && !evaluateEnabled) {
+        throw new Error("wait --fn is disabled by config (browser.evaluateEnabled=false)");
+      }
+      await waitForViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        timeMs: action.timeMs,
+        text: action.text,
+        textGone: action.textGone,
+        selector: action.selector,
+        url: action.url,
+        loadState: action.loadState,
+        fn: action.fn,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "evaluate":
+      if (!evaluateEnabled) {
+        throw new Error("act:evaluate is disabled by config (browser.evaluateEnabled=false)");
+      }
+      await evaluateViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+        fn: action.fn,
+        ref: action.ref,
+        timeoutMs: action.timeoutMs,
+      });
+      break;
+    case "close":
+      await closePageViaPlaywright({
+        cdpUrl,
+        targetId: effectiveTargetId,
+      });
+      break;
+    case "batch": {
+      // Nested batches: delegate recursively
+      const nestedFailures = (
+        await batchViaPlaywright({
+          cdpUrl,
+          targetId: effectiveTargetId,
+          actions: action.actions,
+          stopOnError: action.stopOnError,
+          evaluateEnabled,
+          depth: depth + 1,
+        })
+      ).results.filter((result) => !result.ok);
+      if (nestedFailures.length > 0) {
+        throw new Error(
+          nestedFailures.map((result) => result.error ?? "Nested batch action failed").join("; "),
+        );
+      }
+      break;
+    }
+    default:
+      throw new Error(`Unsupported batch action kind: ${(action as { kind: string }).kind}`);
+  }
+}
+
+export async function batchViaPlaywright(opts: {
+  cdpUrl: string;
+  targetId?: string;
+  actions: BrowserActRequest[];
+  stopOnError?: boolean;
+  evaluateEnabled?: boolean;
+  depth?: number;
+}): Promise<{ results: Array<{ ok: boolean; error?: string }> }> {
+  const depth = opts.depth ?? 0;
+  if (depth > MAX_BATCH_DEPTH) {
+    throw new Error(`Batch nesting depth exceeds maximum of ${MAX_BATCH_DEPTH}`);
+  }
+  if (opts.actions.length > MAX_BATCH_ACTIONS) {
+    throw new Error(`Batch exceeds maximum of ${MAX_BATCH_ACTIONS} actions`);
+  }
+  const results: Array<{ ok: boolean; error?: string }> = [];
+  for (const action of opts.actions) {
+    try {
+      await executeSingleAction(action, opts.cdpUrl, opts.targetId, opts.evaluateEnabled, depth);
+      results.push({ ok: true });
+    } catch (err) {
+      const message = err instanceof Error ? err.message : String(err);
+      results.push({ ok: false, error: message });
+      if (opts.stopOnError !== false) {
+        break;
+      }
+    }
+  }
+  return { results };
+}