feat(vom): actions refactor

apps/extension/src/tools/__tests__/interaction.test.ts

@@ -11,6 +11,7 @@ import {
   parseKeySpec,
   resolveKeyDescriptor,
 } from "../interaction";
+import { captureSurfaceEnvironment } from "../surface-target";
 
 function fakeAgentWindow(ids: number[]) {
   let i = 0;
@@ -54,6 +55,61 @@ function makeFakeCdp(handlers: Record<string, (params: unknown) => unknown>) {
   return { cdp, tabsApi, sent };
 }
 
+async function makeSurfaceClickFixture(onMouse?: (params: Record<string, unknown>) => void) {
+  const manager = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
+  const context = await manager.start("aa11");
+  context.refStore.set("e3", 99, {
+    tabId: 4,
+    kind: "surface",
+    visibleRect: { x: 10, y: 20, w: 100, h: 40 },
+  });
+  const fake = makeFakeCdp({
+    "Page.getFrameTree": () => ({
+      frameTree: {
+        frame: { id: "main", loaderId: "loader-1", url: "https://fixture.test/" },
+      },
+    }),
+    "Page.getLayoutMetrics": () => ({
+      cssLayoutViewport: { clientWidth: 800, clientHeight: 600, pageX: 0, pageY: 0 },
+      cssVisualViewport: {
+        clientWidth: 800,
+        clientHeight: 600,
+        pageX: 0,
+        pageY: 0,
+        scale: 1,
+      },
+    }),
+    "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }),
+    "Runtime.evaluate": () => ({
+      result: { value: { overlayHostPresent: false, overlayHostConnected: false } },
+    }),
+    "Input.dispatchMouseEvent": (params) => {
+      onMouse?.(params as Record<string, unknown>);
+      return {};
+    },
+  });
+  const environment = await captureSurfaceEnvironment(fake.cdp, 4, undefined);
+  if ("code" in environment) throw new Error(environment.message);
+  const entry = context.refStore.resolveEntry("e3");
+  if (!entry) throw new Error("missing Surface ref");
+  const capture = context.surfaceCaptures.create({
+    sessionId: "aa11",
+    tabId: 4,
+    navigationIdentity: environment.navigationIdentity,
+    surface: {
+      ref: "e3",
+      backendNodeId: 99,
+      observationGeneration: entry.generation,
+    },
+    topViewportRect: { x: 10, y: 20, w: 100, h: 40 },
+    imageWidth: 200,
+    imageHeight: 80,
+    viewportSignature: environment.viewportSignature,
+    frameProjectionSignature: environment.frameProjectionSignature,
+  });
+  return { manager, context, fake, capture };
+}
+
 describe("modifiersBitfield", () => {
   it("matches CDP's expected bit layout", () => {
     expect(modifiersBitfield([])).toBe(0);
@@ -190,6 +246,74 @@ describe("handleClick", () => {
     });
   });
 
+  it.each([
+    "left",
+    "middle",
+    "right",
+  ] as const)("uses the common click executor for a %s-button Surface click", async (button) => {
+    const { manager, fake, capture } = await makeSurfaceClickFixture();
+    const result = await handleClick(
+      manager,
+      {
+        session_id: "aa11",
+        ref: "@e3",
+        capture_id: capture.id,
+        image_x: 50,
+        image_y: 20,
+        button,
+        click_count: 2,
+        modifiers: ["ctrl", "shift"],
+      },
+      { cdp: fake.cdp, tabsApi: fake.tabsApi },
+    );
+
+    if ("code" in result) throw new Error(JSON.stringify(result));
+    expect(result).toMatchObject({
+      used_ref: "e3",
+      capture_id: capture.id,
+      image_x: 50,
+      image_y: 20,
+      x: 35,
+      y: 30,
+    });
+    const mouse = fake.sent.filter((call) => call.method === "Input.dispatchMouseEvent");
+    expect(mouse).toHaveLength(3);
+    expect(mouse[1].params).toMatchObject({
+      type: "mousePressed",
+      button,
+      clickCount: 2,
+      modifiers: 2 | 8,
+    });
+    expect(mouse[2].params).toMatchObject({
+      type: "mouseReleased",
+      button,
+      clickCount: 2,
+    });
+  });
+
+  it("consumes a Surface capture when the common click executor fails", async () => {
+    let mouseCalls = 0;
+    const { manager, fake, capture } = await makeSurfaceClickFixture((params) => {
+      mouseCalls += 1;
+      if (params.type === "mousePressed") throw new Error("input failed");
+    });
+    const params = {
+      session_id: "aa11",
+      ref: "@e3",
+      capture_id: capture.id,
+      image_x: 50,
+      image_y: 20,
+    };
+
+    expect(
+      await handleClick(manager, params, { cdp: fake.cdp, tabsApi: fake.tabsApi }),
+    ).toMatchObject({ code: "cdp_failed" });
+    expect(mouseCalls).toBe(2);
+    expect(
+      await handleClick(manager, params, { cdp: fake.cdp, tabsApi: fake.tabsApi }),
+    ).toMatchObject({ data: { reason: "surface_capture_consumed" } });
+  });
+
   it("resolves frame refs in their CDP session and dispatches input in top coordinates", async () => {
     const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
     const ctx = await sm.start("aa11");

apps/extension/src/tools/__tests__/surface-target.test.ts

@@ -1,7 +1,11 @@
 import { describe, expect, it, vi } from "vitest";
 import { SessionManager } from "@/session-manager/manager";
 import type { CdpRunner } from "../shared";
-import { captureSurfaceEnvironment, handleSurfacePointClick } from "../surface-point-action";
+import {
+  captureSurfaceEnvironment,
+  resolveSurfacePointerTarget,
+  SURFACE_POINTER_MOVE_SETTLE_MS,
+} from "../surface-target";
 
 function fakeAgentWindow() {
   return {
@@ -15,7 +19,6 @@ function fakeBrowser() {
   let loaderId = "loader-1";
   let pageY = 0;
   let quad = [10, 20, 110, 20, 110, 60, 10, 60];
-  let dispatchFails = false;
   const sent: Array<{ method: string; params?: object }> = [];
   const cdp: CdpRunner = {
     send: vi.fn(async (_tabId: number, method: string, params?: object) => {
@@ -38,23 +41,12 @@ function fakeBrowser() {
         } as never;
       }
       if (method === "DOM.getContentQuads") return { quads: [quad] } as never;
-      if (method === "Input.dispatchMouseEvent") {
-        if (dispatchFails) throw new Error("input failed");
-        return {} as never;
-      }
       throw new Error(`unexpected CDP call ${method}`);
     }) as CdpRunner["send"],
     trackSessionTab: vi.fn(),
   };
-  const tabsApi = {
-    get: vi.fn(
-      async (tabId: number) => ({ id: tabId, windowId: 100, active: true }) as chrome.tabs.Tab,
-    ),
-    query: vi.fn(async () => [{ id: 4, windowId: 100, active: true } as chrome.tabs.Tab]),
-  };
   return {
     cdp,
-    tabsApi,
     sent,
     setLoaderId: (value: string) => {
       loaderId = value;
@@ -65,9 +57,6 @@ function fakeBrowser() {
     setQuad: (value: number[]) => {
       quad = value;
     },
-    failDispatch: () => {
-      dispatchFails = true;
-    },
   };
 }
 
@@ -104,33 +93,38 @@ async function setupCapture() {
 
 function pointParams(captureId: string) {
   return {
-    session_id: "aa11",
     ref: "@e3",
-    capture_id: captureId,
+    captureId,
-    image_x: 50,
+    imageX: 50,
-    image_y: 20,
+    imageY: 20,
   };
 }
 
-describe("Surface screenshot-bound point click", () => {
+describe("Surface screenshot-bound pointer target", () => {
-  it("maps one fresh capture coordinate and dispatches exactly one trusted click", async () => {
+  it("maps one fresh capture coordinate without executing an action", async () => {
-    const { manager, browser, capture } = await setupCapture();
+    const { context, browser, capture } = await setupCapture();
-    const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser);
+    const result = await resolveSurfacePointerTarget(
+      context,
+      { tabId: 4 },
+      pointParams(capture.id),
+      { cdp: browser.cdp },
+    );
 
     if ("code" in result) throw new Error(JSON.stringify(result));
     expect(result).toMatchObject({
-      used_ref: "e3",
+      usedRef: "e3",
-      capture_id: capture.id,
+      point: { x: 35, y: 30 },
-      image_x: 50,
+      moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS,
-      image_y: 20,
+      capture: { id: capture.id, imageX: 50, imageY: 20 },
-      x: 35,
-      y: 30,
     });
-    expect(browser.sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength(
+    expect(browser.sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false);
-      3,
-    );
 
-    const repeated = await handleSurfacePointClick(manager, pointParams(capture.id), browser);
+    const repeated = await resolveSurfacePointerTarget(
+      context,
+      { tabId: 4 },
+      pointParams(capture.id),
+      { cdp: browser.cdp },
+    );
     expect(repeated).toMatchObject({
       code: "permission_denied",
       data: { reason: "surface_capture_consumed" },
@@ -138,7 +132,7 @@ describe("Surface screenshot-bound point click", () => {
   });
 
   it("rejects a new observation generation before sending input", async () => {
-    const { manager, context, browser, capture } = await setupCapture();
+    const { context, browser, capture } = await setupCapture();
     context.refStore.replace([
       [
         "e3",
@@ -151,7 +145,12 @@ describe("Surface screenshot-bound point click", () => {
       ],
     ]);
 
-    const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser);
+    const result = await resolveSurfacePointerTarget(
+      context,
+      { tabId: 4 },
+      pointParams(capture.id),
+      { cdp: browser.cdp },
+    );
     expect(result).toMatchObject({
       code: "permission_denied",
       data: { reason: "surface_capture_stale" },
@@ -166,9 +165,14 @@ describe("Surface screenshot-bound point click", () => {
       (browser: ReturnType<typeof fakeBrowser>) =>
         browser.setQuad([11, 20, 111, 20, 111, 60, 11, 60]),
     ]) {
-      const { manager, browser, capture } = await setupCapture();
+      const { context, browser, capture } = await setupCapture();
       mutate(browser);
-      const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser);
+      const result = await resolveSurfacePointerTarget(
+        context,
+        { tabId: 4 },
+        pointParams(capture.id),
+        { cdp: browser.cdp },
+      );
       expect(result).toMatchObject({
         code: "permission_denied",
         data: { reason: "surface_capture_stale" },
@@ -177,33 +181,26 @@ describe("Surface screenshot-bound point click", () => {
     }
   });
 
-  it("consumes the capture when coordinates or input dispatch fail", async () => {
+  it("consumes the capture when coordinate validation fails", async () => {
     const invalid = await setupCapture();
-    const outside = await handleSurfacePointClick(
+    const outside = await resolveSurfacePointerTarget(
-      invalid.manager,
+      invalid.context,
-      { ...pointParams(invalid.capture.id), image_x: 200 },
+      { tabId: 4 },
-      invalid.browser,
+      { ...pointParams(invalid.capture.id), imageX: 200 },
+      { cdp: invalid.browser.cdp },
     );
     expect(outside).toMatchObject({
       code: "invalid_params",
       data: { reason: "surface_coordinate_invalid" },
     });
     expect(
-      await handleSurfacePointClick(
+      await resolveSurfacePointerTarget(
-        invalid.manager,
+        invalid.context,
+        { tabId: 4 },
         pointParams(invalid.capture.id),
-        invalid.browser,
+        { cdp: invalid.browser.cdp },
       ),
     ).toMatchObject({ data: { reason: "surface_capture_consumed" } });
-
-    const failed = await setupCapture();
-    failed.browser.failDispatch();
-    expect(
-      await handleSurfacePointClick(failed.manager, pointParams(failed.capture.id), failed.browser),
-    ).toMatchObject({ code: "cdp_failed" });
-    expect(
-      await handleSurfacePointClick(failed.manager, pointParams(failed.capture.id), failed.browser),
-    ).toMatchObject({ data: { reason: "surface_capture_consumed" } });
   });
 
   it("maps image coordinates through an OOPIF frame projection", async () => {
@@ -269,10 +266,6 @@ describe("Surface screenshot-bound point click", () => {
       getFrameGraph: vi.fn(async () => graph),
       trackSessionTab: vi.fn(),
     };
-    const tabsApi = {
-      get: vi.fn(async () => ({ id: 4, windowId: 100, active: true }) as chrome.tabs.Tab),
-      query: vi.fn(async () => [{ id: 4, windowId: 100, active: true } as chrome.tabs.Tab]),
-    };
     const environment = await captureSurfaceEnvironment(cdp, 4, "child");
     if ("code" in environment) throw new Error(environment.message);
     const entry = context.refStore.resolveEntry("e3");
@@ -294,20 +287,20 @@ describe("Surface screenshot-bound point click", () => {
       frameProjectionSignature: environment.frameProjectionSignature,
     });
 
-    const result = await handleSurfacePointClick(
+    const result = await resolveSurfacePointerTarget(
-      manager,
+      context,
+      { tabId: 4 },
       {
-        session_id: "aa11",
         ref: "@e3",
-        capture_id: capture.id,
+        captureId: capture.id,
-        image_x: 100,
+        imageX: 100,
-        image_y: 40,
+        imageY: 40,
       },
-      { cdp, tabsApi },
+      { cdp },
     );
 
     if ("code" in result) throw new Error(JSON.stringify(result));
-    expect(result).toMatchObject({ x: 170, y: 130 });
+    expect(result).toMatchObject({ point: { x: 170, y: 130 } });
-    expect(sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength(3);
+    expect(sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false);
   });
 });

apps/extension/src/tools/interaction.ts

@@ -44,7 +44,7 @@ import {
   resolveTargetTab,
 } from "./shared";
 import { resolveSnapshotRef } from "./snapshot-ref";
-import { handleSurfacePointClick } from "./surface-point-action";
+import { resolveSurfacePointerTarget } from "./surface-target";
 
 export interface InteractionDeps {
   cdp: CdpRunner;
@@ -221,6 +221,18 @@ async function resolveBackendNode(
 // tool.click
 // ---------------------------------------------------------------------------
 
+interface ResolvedClickPointerTarget {
+  point: { x: number; y: number };
+  usedRef?: string;
+  usedSelector?: string;
+  moveSettleMs: number;
+  capture?: {
+    id: string;
+    imageX: number;
+    imageY: number;
+  };
+}
+
 export async function handleClick(
   manager: SessionManager,
   params: ClickParams,
@@ -228,9 +240,6 @@ export async function handleClick(
 ): Promise<ClickResult | RpcError> {
   const hasSurfacePointParams =
     params.capture_id !== undefined || params.image_x !== undefined || params.image_y !== undefined;
-  if (hasSurfacePointParams) {
-    return handleSurfacePointClick(manager, params, deps);
-  }
   const ctxOrErr = lookupSession(manager, params, "click");
   if (isRpcError(ctxOrErr)) return ctxOrErr;
   const ctx = ctxOrErr;
@@ -242,39 +251,67 @@ export async function handleClick(
   if (denied) return denied;
   const dialogCursor = markDialogCursor(deps.cdp, target.tabId);
 
-  const node = await resolveBackendNode(deps.cdp, ctx, target, params, "click");
+  const button: MouseButton = params.button ?? "left";
-  if (isRpcError(node)) return node;
+  const clickCount = params.click_count ?? 1;
+  if (!Number.isSafeInteger(clickCount) || clickCount < 1) {
+    return { code: "invalid_params", message: "click_count must be a positive integer" };
+  }
+  const modifiers = modifiersBitfield(params.modifiers);
 
   if (throwIfAborted(deps.signal)) {
     return { code: "cancelled", message: "click aborted" };
   }
 
   deps.cdp.trackSessionTab?.(ctx.sessionId, target.tabId);
-  const geometry = await resolveNodeGeometry(
+  const pointerTarget: ResolvedClickPointerTarget | RpcError = hasSurfacePointParams
-    deps.cdp,
+    ? await resolveSurfacePointerTarget(
-    target.tabId,
+        ctx,
-    {
+        target,
-      target: node.cdpTarget,
+        {
-      backendNodeId: node.backendNodeId,
+          ref: params.ref,
-      ...(node.frameId ? { frameId: node.frameId } : {}),
+          selector: params.selector,
-    },
+          captureId: params.capture_id,
-    { scrollIntoView: true },
+          imageX: params.image_x,
-  );
+          imageY: params.image_y,
-  if (isRpcError(geometry)) return geometry;
+        },
-  const centre = geometry.actionPoint;
+        {
+          cdp: deps.cdp,
+          signal: deps.signal,
+        },
+      )
+    : await (async () => {
+        const node = await resolveBackendNode(deps.cdp, ctx, target, params, "click");
+        if (isRpcError(node)) return node;
+        const geometry = await resolveNodeGeometry(
+          deps.cdp,
+          target.tabId,
+          {
+            target: node.cdpTarget,
+            backendNodeId: node.backendNodeId,
+            ...(node.frameId ? { frameId: node.frameId } : {}),
+          },
+          { scrollIntoView: true },
+        );
+        if (isRpcError(geometry)) return geometry;
+        return {
+          point: geometry.actionPoint,
+          usedRef: node.usedRef,
+          usedSelector: node.usedSelector,
+          moveSettleMs: 0,
+        };
+      })();
+  if (isRpcError(pointerTarget)) return pointerTarget;
 
   if (throwIfAborted(deps.signal)) {
     return { code: "cancelled", message: "click aborted" };
   }
 
-  const button: MouseButton = params.button ?? "left";
+  const overlayBlocking = await checkOverlayAtPoint(
-  const clickCount = params.click_count ?? 1;
+    deps.cdp,
-  if (clickCount < 1) {
+    target.tabId,
-    return { code: "invalid_params", message: "click_count must be greater than zero" };
+    pointerTarget.point.x,
-  }
+    pointerTarget.point.y,
-  const modifiers = modifiersBitfield(params.modifiers);
+  );
-
-  const overlayBlocking = await checkOverlayAtPoint(deps.cdp, target.tabId, centre.x, centre.y);
   let automationBypassEnabled = false;
   if (overlayBlocking && deps.bypassOverlay) {
     try {
@@ -286,11 +323,12 @@ export async function handleClick(
   }
 
   try {
-    const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, centre, {
+    const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, pointerTarget.point, {
       button,
       clickCount,
       modifiers,
       signal: deps.signal,
+      moveSettleMs: pointerTarget.moveSettleMs,
     });
     if (dispatch === "cancelled") {
       return { code: "cancelled", message: "click aborted" };
@@ -312,10 +350,17 @@ export async function handleClick(
 
   return attachDialogs(deps.cdp, target.tabId, dialogCursor, {
     tab_id: target.tabId,
-    used_ref: node.usedRef,
+    used_ref: pointerTarget.usedRef,
-    used_selector: node.usedSelector,
+    used_selector: pointerTarget.usedSelector,
-    x: centre.x,
+    x: pointerTarget.point.x,
-    y: centre.y,
+    y: pointerTarget.point.y,
+    ...(pointerTarget.capture
+      ? {
+          capture_id: pointerTarget.capture.id,
+          image_x: pointerTarget.capture.imageX,
+          image_y: pointerTarget.capture.imageY,
+        }
+      : {}),
   });
 }
 

apps/extension/src/tools/observation.ts

@@ -49,7 +49,7 @@ import {
 } from "./shared";
 import { resolveSnapshotRef } from "./snapshot-ref";
 import { surfaceVisibleRect } from "./surface-coordinate";
-import { captureSurfaceEnvironment } from "./surface-point-action";
+import { captureSurfaceEnvironment } from "./surface-target";
 import {
   type CapturedNode,
   type CapturedViewModel,

apps/extension/src/tools/surface-target.ts

@@ -1,19 +1,9 @@
 import type { CdpFrameGraph } from "@/browser-driver/frame-graph";
-import type { SessionManager } from "@/session-manager/manager";
+import type { SessionContext } from "@/session-manager/manager";
-import type { ClickParams, ClickResult, RpcError } from "@/transport/types";
+import type { RpcError } from "@/transport/types";
-import { attachDialogs, markDialogCursor } from "./dialogs";
 import { rpcError } from "./errors";
 import { resolveNodeGeometry } from "./frame-geometry";
-import { dispatchMouseClick } from "./mouse-input";
+import { type CdpRunner, isRpcError, normaliseRef } from "./shared";
-import {
-  type CdpRunner,
-  type ChromeTabsApi,
-  enforceAgentWindow,
-  isRpcError,
-  lookupSession,
-  normaliseRef,
-  resolveTargetTab,
-} from "./shared";
 import { resolveSnapshotRef } from "./snapshot-ref";
 import {
   mapImagePointToViewport,
@@ -28,16 +18,33 @@ export interface SurfaceCaptureEnvironment {
   frameProjectionSignature: string;
 }
 
-export interface SurfacePointActionDeps {
+export interface SurfaceTargetResolverDeps {
   cdp: CdpRunner;
-  tabsApi: ChromeTabsApi;
   signal?: AbortSignal;
-  bypassOverlay?: (tabId: number, enabled: boolean) => Promise<void>;
+}
+
+export interface SurfacePointerTargetInput {
+  ref?: string;
+  selector?: string;
+  captureId?: string;
+  imageX?: number;
+  imageY?: number;
+}
+
+export interface ResolvedSurfacePointerTarget {
+  point: { x: number; y: number };
+  usedRef: string;
+  moveSettleMs: number;
+  capture: {
+    id: string;
+    imageX: number;
+    imageY: number;
+  };
 }
 
 // Canvas-style controls commonly update their pointer hit-test on the next
 // animation frame. Leave a scheduling boundary after moving and before pressing.
-const SURFACE_POINTER_MOVE_SETTLE_MS = 32;
+export const SURFACE_POINTER_MOVE_SETTLE_MS = 32;
 
 function stale(message: string): RpcError {
   return rpcError("permission_denied", "surface_capture_stale", message);
@@ -126,31 +133,22 @@ export async function captureSurfaceEnvironment(
   }
 }
 
-function validatePointParams(params: ClickParams): RpcError | null {
+function validatePointParams(input: SurfacePointerTargetInput): RpcError | null {
-  if (!params.capture_id || typeof params.capture_id !== "string") {
+  if (!input.captureId || typeof input.captureId !== "string") {
     return { code: "invalid_params", message: "surface point click requires capture_id" };
   }
-  if (typeof params.image_x !== "number" || !Number.isFinite(params.image_x)) {
+  if (typeof input.imageX !== "number" || !Number.isFinite(input.imageX)) {
     return { code: "invalid_params", message: "image_x must be a finite number" };
   }
-  if (typeof params.image_y !== "number" || !Number.isFinite(params.image_y)) {
+  if (typeof input.imageY !== "number" || !Number.isFinite(input.imageY)) {
     return { code: "invalid_params", message: "image_y must be a finite number" };
   }
-  if (!params.ref || params.selector) {
+  if (!input.ref || input.selector) {
     return {
       code: "invalid_params",
       message: "surface point click requires one Surface ref and does not accept a selector",
     };
   }
-  if ((params.button ?? "left") !== "left" || (params.click_count ?? 1) !== 1) {
-    return {
-      code: "invalid_params",
-      message: "surface point click currently supports one left click only",
-    };
-  }
-  if (params.modifiers && params.modifiers.length > 0) {
-    return { code: "invalid_params", message: "surface point click does not accept modifiers" };
-  }
   return null;
 }
 
@@ -160,27 +158,24 @@ function captureConsumeError(reason: "not_found" | "expired" | "consumed"): RpcE
 }
 
 function aborted(signal: AbortSignal | undefined): RpcError | null {
-  return signal?.aborted ? { code: "cancelled", message: "surface point click aborted" } : null;
+  return signal?.aborted
+    ? { code: "cancelled", message: "surface target resolution aborted" }
+    : null;
 }
 
-export async function handleSurfacePointClick(
+/** Resolve one fresh Surface capture coordinate into a validated top-viewport pointer target. */
-  manager: SessionManager,
+export async function resolveSurfacePointerTarget(
-  params: ClickParams,
+  ctx: SessionContext,
-  deps: SurfacePointActionDeps,
+  target: { tabId: number; url?: string },
-): Promise<ClickResult | RpcError> {
+  input: SurfacePointerTargetInput,
-  const invalid = validatePointParams(params);
+  deps: SurfaceTargetResolverDeps,
+): Promise<ResolvedSurfacePointerTarget | RpcError> {
+  const invalid = validatePointParams(input);
   if (invalid) return invalid;
-  const ctxOrError = lookupSession(manager, params, "click");
-  if (isRpcError(ctxOrError)) return ctxOrError;
-  const ctx = ctxOrError;
   const earlyAbort = aborted(deps.signal);
   if (earlyAbort) return earlyAbort;
-  const target = await resolveTargetTab(manager, ctx, params.tab_id, deps.tabsApi);
-  if (isRpcError(target)) return target;
-  const denied = enforceAgentWindow(ctx, target, "click");
-  if (denied) return denied;
 
-  const consumed = ctx.surfaceCaptures.consume(params.capture_id as string);
+  const consumed = ctx.surfaceCaptures.consume(input.captureId as string);
   if (!consumed.ok) return captureConsumeError(consumed.reason);
   const capture = consumed.capture;
   const postConsumeAbort = aborted(deps.signal);
@@ -189,10 +184,10 @@ export async function handleSurfacePointClick(
   if (capture.sessionId !== ctx.sessionId || capture.tabId !== target.tabId) {
     return stale("surface capture belongs to a different session or tab");
   }
-  if (normaliseRef(params.ref as string) !== capture.surface.ref) {
+  if (normaliseRef(input.ref as string) !== capture.surface.ref) {
     return stale("surface ref does not match the capture");
   }
-  const node = resolveSnapshotRef(ctx, params.ref as string, target.tabId, "screenshot");
+  const node = resolveSnapshotRef(ctx, input.ref as string, target.tabId, "screenshot");
   if (isRpcError(node)) return stale(node.message);
   if (
     node.kind !== "surface" ||
@@ -241,8 +236,8 @@ export async function handleSurfacePointClick(
     capture.topViewportRect,
     capture.imageWidth,
     capture.imageHeight,
-    params.image_x as number,
+    input.imageX as number,
-    params.image_y as number,
+    input.imageY as number,
   );
   if (!point || !pointInRegion(point, geometry.topVisibleRegions)) {
     return rpcError(
@@ -252,42 +247,14 @@ export async function handleSurfacePointClick(
     );
   }
 
-  const dialogCursor = markDialogCursor(deps.cdp, target.tabId);
+  return {
-  let bypassEnabled = false;
+    point,
-  try {
+    usedRef: capture.surface.ref,
-    if (deps.bypassOverlay) {
+    moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS,
-      await deps.bypassOverlay(target.tabId, true);
+    capture: {
-      bypassEnabled = true;
+      id: capture.id,
-    }
+      imageX: input.imageX as number,
-    const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, point, {
+      imageY: input.imageY as number,
-      button: "left",
+    },
-      clickCount: 1,
+  };
-      modifiers: 0,
-      signal: deps.signal,
-      moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS,
-    });
-    if (dispatch === "cancelled") {
-      return { code: "cancelled", message: "surface point click aborted" };
-    }
-  } catch (error) {
-    return { code: "cdp_failed", message: error instanceof Error ? error.message : String(error) };
-  } finally {
-    if (bypassEnabled && deps.bypassOverlay) {
-      try {
-        await deps.bypassOverlay(target.tabId, false);
-      } catch (error) {
-        console.debug("[bsk surface-point] overlay bypass disable failed", error);
-      }
-    }
-  }
-
-  return attachDialogs(deps.cdp, target.tabId, dialogCursor, {
-    tab_id: target.tabId,
-    used_ref: capture.surface.ref,
-    x: point.x,
-    y: point.y,
-    capture_id: capture.id,
-    image_x: params.image_x,
-    image_y: params.image_y,
-  });
 }

packages/dsh-plugin-browserskill/src/tools.ts

@@ -581,9 +581,15 @@ function defineBrowserOperations(deps: ToolDeps, register: DefinitionRegistrar):
           type: "integer",
           description: "Number of consecutive presses (double-click = 2).",
         },
+        modifiers: {
+          type: "array",
+          items: { type: "string", enum: ["alt", "ctrl", "meta", "shift"] },
+          description: "Keyboard modifiers held during the click.",
+        },
         captureId: {
           type: "string",
-          description: "Short-lived Surface capture id returned by browser_screenshot.",
+          description:
+            "Short-lived Surface capture id returned by browser_inspect action=screenshot.",
         },
         imageX: {
           type: "number",
@@ -626,6 +632,9 @@ function defineBrowserOperations(deps: ToolDeps, register: DefinitionRegistrar):
         const cmdArgs = ["click", "--session", sessionId];
         if (args.button !== undefined) cmdArgs.push("--button", args.button);
         if (args.clickCount !== undefined) cmdArgs.push("--click-count", String(args.clickCount));
+        if (args.modifiers !== undefined && args.modifiers.length > 0) {
+          cmdArgs.push("--modifiers", args.modifiers.join(","));
+        }
         if (args.captureId !== undefined) {
           cmdArgs.push(
             "--capture",

packages/dsh-plugin-browserskill/tests/tools.test.ts

@@ -478,12 +478,12 @@ describe("interaction tools", () => {
     emulate: { tab_id: 7, cleared: false },
   };
 
-  it("interact.click maps target, button, and click count", async () => {
+  it("interact.click maps target, button, click count, and modifiers", async () => {
     const { tools, calls } = setup(responses);
     await startSession(tools);
     const click = tools.get("interact.click");
     const value = (await click?.execute(
-      { target: "@e1", button: "right", clickCount: 2 },
+      { target: "@e1", button: "right", clickCount: 2, modifiers: ["ctrl", "shift"] },
       makeExec(),
     )) as { session: string; x: number; y: number };
     expect(value).toMatchObject({ session: "s1", x: 10, y: 20 });
@@ -495,6 +495,8 @@ describe("interaction tools", () => {
       "right",
       "--click-count",
       "2",
+      "--modifiers",
+      "ctrl,shift",
       "@e1",
     ]);
   });
@@ -973,7 +975,7 @@ describe("inspect.screenshot", () => {
       },
     });
     await startSession(tools);
-    const screenshot = tools.get("browser_screenshot");
+    const screenshot = tools.get("inspect.screenshot");
     const value = await screenshot?.execute({ ref: "@e1" }, makeExec());
 
     expect(value).toMatchObject({