From f5dab955e2ba946a394b8fb5bb074a8ba62814ea Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Fri, 28 Aug 2026 15:50:32 +0800 Subject: [PATCH] feat(vom): actions refactor --- .../src/tools/__tests__/interaction.test.ts | 124 +++++++++++++++ ...-action.test.ts => surface-target.test.ts} | 129 ++++++++-------- apps/extension/src/tools/interaction.ts | 107 +++++++++---- apps/extension/src/tools/observation.ts | 2 +- ...face-point-action.ts => surface-target.ts} | 143 +++++++----------- packages/dsh-plugin-browserskill/src/tools.ts | 11 +- .../tests/tools.test.ts | 8 +- 7 files changed, 332 insertions(+), 192 deletions(-) rename apps/extension/src/tools/__tests__/{surface-point-action.test.ts => surface-target.test.ts} (72%) rename apps/extension/src/tools/{surface-point-action.ts => surface-target.ts} (67%) diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 6f716bb..dffb491 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/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 unknown>) { return { cdp, tabsApi, sent }; } +async function makeSurfaceClickFixture(onMouse?: (params: Record) => 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); + 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"); diff --git a/apps/extension/src/tools/__tests__/surface-point-action.test.ts b/apps/extension/src/tools/__tests__/surface-target.test.ts similarity index 72% rename from apps/extension/src/tools/__tests__/surface-point-action.test.ts rename to apps/extension/src/tools/__tests__/surface-target.test.ts index ce5bc80..b065c10 100644 --- a/apps/extension/src/tools/__tests__/surface-point-action.test.ts +++ b/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, - image_x: 50, - image_y: 20, + captureId, + imageX: 50, + imageY: 20, }; } -describe("Surface screenshot-bound point click", () => { - it("maps one fresh capture coordinate and dispatches exactly one trusted click", async () => { - const { manager, browser, capture } = await setupCapture(); - const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser); +describe("Surface screenshot-bound pointer target", () => { + it("maps one fresh capture coordinate without executing an action", async () => { + const { context, browser, capture } = await setupCapture(); + 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", - capture_id: capture.id, - image_x: 50, - image_y: 20, - x: 35, - y: 30, + usedRef: "e3", + point: { x: 35, y: 30 }, + moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS, + capture: { id: capture.id, imageX: 50, imageY: 20 }, }); - expect(browser.sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength( - 3, - ); + expect(browser.sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false); - 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) => 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( - invalid.manager, - { ...pointParams(invalid.capture.id), image_x: 200 }, - invalid.browser, + const outside = await resolveSurfacePointerTarget( + invalid.context, + { tabId: 4 }, + { ...pointParams(invalid.capture.id), imageX: 200 }, + { cdp: invalid.browser.cdp }, ); expect(outside).toMatchObject({ code: "invalid_params", data: { reason: "surface_coordinate_invalid" }, }); expect( - await handleSurfacePointClick( - invalid.manager, + await resolveSurfacePointerTarget( + 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( - manager, + const result = await resolveSurfacePointerTarget( + context, + { tabId: 4 }, { - session_id: "aa11", ref: "@e3", - capture_id: capture.id, - image_x: 100, - image_y: 40, + captureId: capture.id, + imageX: 100, + imageY: 40, }, - { cdp, tabsApi }, + { cdp }, ); if ("code" in result) throw new Error(JSON.stringify(result)); - expect(result).toMatchObject({ x: 170, y: 130 }); - expect(sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength(3); + expect(result).toMatchObject({ point: { x: 170, y: 130 } }); + expect(sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false); }); }); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index fb1dc9b..a196f19 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/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 { 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"); - if (isRpcError(node)) return node; + const button: MouseButton = params.button ?? "left"; + 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( - deps.cdp, - target.tabId, - { - target: node.cdpTarget, - backendNodeId: node.backendNodeId, - ...(node.frameId ? { frameId: node.frameId } : {}), - }, - { scrollIntoView: true }, - ); - if (isRpcError(geometry)) return geometry; - const centre = geometry.actionPoint; + const pointerTarget: ResolvedClickPointerTarget | RpcError = hasSurfacePointParams + ? await resolveSurfacePointerTarget( + ctx, + target, + { + ref: params.ref, + selector: params.selector, + captureId: params.capture_id, + imageX: params.image_x, + imageY: params.image_y, + }, + { + 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 clickCount = params.click_count ?? 1; - if (clickCount < 1) { - return { code: "invalid_params", message: "click_count must be greater than zero" }; - } - const modifiers = modifiersBitfield(params.modifiers); - - const overlayBlocking = await checkOverlayAtPoint(deps.cdp, target.tabId, centre.x, centre.y); + const overlayBlocking = await checkOverlayAtPoint( + deps.cdp, + target.tabId, + pointerTarget.point.x, + pointerTarget.point.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_selector: node.usedSelector, - x: centre.x, - y: centre.y, + used_ref: pointerTarget.usedRef, + used_selector: pointerTarget.usedSelector, + x: pointerTarget.point.x, + y: pointerTarget.point.y, + ...(pointerTarget.capture + ? { + capture_id: pointerTarget.capture.id, + image_x: pointerTarget.capture.imageX, + image_y: pointerTarget.capture.imageY, + } + : {}), }); } diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index 728e2ba..966f445 100644 --- a/apps/extension/src/tools/observation.ts +++ b/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, diff --git a/apps/extension/src/tools/surface-point-action.ts b/apps/extension/src/tools/surface-target.ts similarity index 67% rename from apps/extension/src/tools/surface-point-action.ts rename to apps/extension/src/tools/surface-target.ts index 79fc8c0..7529b54 100644 --- a/apps/extension/src/tools/surface-point-action.ts +++ b/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 { ClickParams, ClickResult, RpcError } from "@/transport/types"; -import { attachDialogs, markDialogCursor } from "./dialogs"; +import type { SessionContext } from "@/session-manager/manager"; +import type { RpcError } from "@/transport/types"; import { rpcError } from "./errors"; import { resolveNodeGeometry } from "./frame-geometry"; -import { dispatchMouseClick } from "./mouse-input"; -import { - type CdpRunner, - type ChromeTabsApi, - enforceAgentWindow, - isRpcError, - lookupSession, - normaliseRef, - resolveTargetTab, -} from "./shared"; +import { type CdpRunner, isRpcError, normaliseRef } 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; +} + +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 { - if (!params.capture_id || typeof params.capture_id !== "string") { +function validatePointParams(input: SurfacePointerTargetInput): RpcError | null { + 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( - manager: SessionManager, - params: ClickParams, - deps: SurfacePointActionDeps, -): Promise { - const invalid = validatePointParams(params); +/** Resolve one fresh Surface capture coordinate into a validated top-viewport pointer target. */ +export async function resolveSurfacePointerTarget( + ctx: SessionContext, + target: { tabId: number; url?: string }, + input: SurfacePointerTargetInput, + deps: SurfaceTargetResolverDeps, +): Promise { + 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, - params.image_y as number, + input.imageX 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); - let bypassEnabled = false; - try { - if (deps.bypassOverlay) { - await deps.bypassOverlay(target.tabId, true); - bypassEnabled = true; - } - const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, point, { - 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, - }); + return { + point, + usedRef: capture.surface.ref, + moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS, + capture: { + id: capture.id, + imageX: input.imageX as number, + imageY: input.imageY as number, + }, + }; } diff --git a/packages/dsh-plugin-browserskill/src/tools.ts b/packages/dsh-plugin-browserskill/src/tools.ts index 0d05515..db1315b 100644 --- a/packages/dsh-plugin-browserskill/src/tools.ts +++ b/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", diff --git a/packages/dsh-plugin-browserskill/tests/tools.test.ts b/packages/dsh-plugin-browserskill/tests/tools.test.ts index e023ad6..88bf1b2 100644 --- a/packages/dsh-plugin-browserskill/tests/tools.test.ts +++ b/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({