From 22ca308342dbb5f3a8f0bd5a6f8357c57c0fcf42 Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 26 Aug 2026 14:09:02 +0800 Subject: [PATCH 1/3] feat(vom): canvas position calculation and projection --- .../__tests__/surface-capture-store.test.ts | 67 ++++ apps/extension/src/session-manager/manager.ts | 3 + .../session-manager/surface-capture-store.ts | 100 ++++++ .../src/tools/__tests__/observation.test.ts | 9 + .../src/tools/__tests__/session.test.ts | 14 +- .../src/tools/__tests__/snapshot-ref.test.ts | 4 + .../__tests__/surface-coordinate.test.ts | 52 +++ .../__tests__/surface-point-action.test.ts | 313 +++++++++++++++++ apps/extension/src/tools/interaction.ts | 6 + apps/extension/src/tools/observation.ts | 73 ++-- apps/extension/src/tools/session.ts | 1 + apps/extension/src/tools/snapshot-ref.ts | 2 + .../extension/src/tools/surface-coordinate.ts | 88 +++++ .../src/tools/surface-point-action.ts | 320 ++++++++++++++++++ apps/extension/src/transport/types.ts | 19 ++ crates/bsk-cli/skill/SKILL.md | 14 +- crates/bsk-cli/src/cli/interaction.rs | 31 ++ crates/bsk-cli/src/cli/render_error.rs | 63 +++- crates/bsk-cli/src/cli/screenshot.rs | 12 +- crates/bsk-cli/tests/cli_parse.rs | 23 ++ crates/bsk-cli/tests/tools_ipc.rs | 2 + crates/bsk-cli/tests/tools_m7_ipc.rs | 9 + .../schema/tool_click_params.json | 23 ++ .../schema/tool_click_result.json | 20 ++ .../schema/tool_screenshot_result.json | 35 ++ crates/bsk-protocol/src/tools/interaction.rs | 18 + crates/bsk-protocol/src/tools/observation.rs | 16 + packages/dsh-plugin-browserskill/src/tools.ts | 54 ++- .../tests/tools.test.ts | 61 ++++ skill/SKILL.md | 14 +- 30 files changed, 1433 insertions(+), 33 deletions(-) create mode 100644 apps/extension/src/session-manager/__tests__/surface-capture-store.test.ts create mode 100644 apps/extension/src/session-manager/surface-capture-store.ts create mode 100644 apps/extension/src/tools/__tests__/surface-coordinate.test.ts create mode 100644 apps/extension/src/tools/__tests__/surface-point-action.test.ts create mode 100644 apps/extension/src/tools/surface-coordinate.ts create mode 100644 apps/extension/src/tools/surface-point-action.ts diff --git a/apps/extension/src/session-manager/__tests__/surface-capture-store.test.ts b/apps/extension/src/session-manager/__tests__/surface-capture-store.test.ts new file mode 100644 index 00000000..fdd33641 --- /dev/null +++ b/apps/extension/src/session-manager/__tests__/surface-capture-store.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { type SurfaceCaptureInput, SurfaceCaptureStore } from "../surface-capture-store"; + +function input(): SurfaceCaptureInput { + return { + sessionId: "aa11", + tabId: 7, + navigationIdentity: "navigation", + surface: { + ref: "e3", + frameId: "main", + backendNodeId: 99, + observationGeneration: 2, + }, + topViewportRect: { x: 10, y: 20, w: 100, h: 50 }, + imageWidth: 200, + imageHeight: 100, + viewportSignature: "viewport", + frameProjectionSignature: "frame", + }; +} + +describe("SurfaceCaptureStore", () => { + it("creates metadata-only, short-lived captures and consumes them once", () => { + let now = 1_000; + const store = new SurfaceCaptureStore({ + now: () => now, + ttlMs: 500, + createId: () => "sc_test", + }); + const capture = store.create(input()); + + expect(capture).toMatchObject({ + id: "sc_test", + createdAt: 1_000, + expiresAt: 1_500, + consumed: false, + }); + expect(capture).not.toHaveProperty("imageBase64"); + expect(store.consume("sc_test")).toMatchObject({ ok: true }); + expect(store.consume("sc_test")).toEqual({ ok: false, reason: "consumed" }); + + now = 1_600; + expect(store.size()).toBe(0); + }); + + it("rejects expired and unknown captures", () => { + let now = 1_000; + const store = new SurfaceCaptureStore({ + now: () => now, + ttlMs: 10, + createId: () => "sc_expired", + }); + store.create(input()); + now = 1_010; + + expect(store.consume("sc_expired")).toEqual({ ok: false, reason: "expired" }); + expect(store.consume("sc_missing")).toEqual({ ok: false, reason: "not_found" }); + }); + + it("clears every capture on session cleanup", () => { + const store = new SurfaceCaptureStore({ createId: () => "sc_clear" }); + store.create(input()); + store.clear(); + expect(store.size()).toBe(0); + }); +}); diff --git a/apps/extension/src/session-manager/manager.ts b/apps/extension/src/session-manager/manager.ts index 2fef0d76..e7c6d9f8 100644 --- a/apps/extension/src/session-manager/manager.ts +++ b/apps/extension/src/session-manager/manager.ts @@ -1,10 +1,12 @@ import { AGENT_WINDOW_HOME, type AgentWindowApi, chromeAgentWindowApi } from "./agent-window"; import { RefStore } from "./ref-store"; +import { SurfaceCaptureStore } from "./surface-capture-store"; export interface SessionContext { sessionId: string; agentWindowId: number; refStore: RefStore; + surfaceCaptures: SurfaceCaptureStore; borrowedTabs: Map; createdAtMs: number; } @@ -186,6 +188,7 @@ export class SessionManager { sessionId, agentWindowId: windowId, refStore: new RefStore(), + surfaceCaptures: new SurfaceCaptureStore({ now: this.now }), borrowedTabs: new Map(), createdAtMs: this.now(), }; diff --git a/apps/extension/src/session-manager/surface-capture-store.ts b/apps/extension/src/session-manager/surface-capture-store.ts new file mode 100644 index 00000000..10985e03 --- /dev/null +++ b/apps/extension/src/session-manager/surface-capture-store.ts @@ -0,0 +1,100 @@ +import type { Rect } from "@browser-skill/vom"; + +export const SURFACE_CAPTURE_TTL_MS = 30_000; + +export interface SurfaceCapture { + id: string; + sessionId: string; + tabId: number; + navigationIdentity: string; + surface: { + ref: string; + frameId?: string; + backendNodeId: number; + observationGeneration: number; + }; + topViewportRect: Rect; + imageWidth: number; + imageHeight: number; + viewportSignature: string; + frameProjectionSignature: string; + createdAt: number; + expiresAt: number; + consumed: boolean; +} + +export type SurfaceCaptureInput = Omit< + SurfaceCapture, + "id" | "createdAt" | "expiresAt" | "consumed" +>; + +export type SurfaceCaptureConsumeResult = + | { ok: true; capture: SurfaceCapture } + | { ok: false; reason: "not_found" | "expired" | "consumed" }; + +export interface SurfaceCaptureStoreOptions { + now?: () => number; + ttlMs?: number; + createId?: () => string; +} + +function randomCaptureId(): string { + const bytes = crypto.getRandomValues(new Uint8Array(12)); + return `sc_${Array.from(bytes, (byte) => byte.toString(16).padStart(2, "0")).join("")}`; +} + +/** Per-session, metadata-only store for short-lived screenshot coordinate transactions. */ +export class SurfaceCaptureStore { + private readonly captures = new Map(); + private readonly now: () => number; + private readonly ttlMs: number; + private readonly createId: () => string; + + constructor(options: SurfaceCaptureStoreOptions = {}) { + this.now = options.now ?? Date.now; + this.ttlMs = options.ttlMs ?? SURFACE_CAPTURE_TTL_MS; + this.createId = options.createId ?? randomCaptureId; + } + + create(input: SurfaceCaptureInput): SurfaceCapture { + this.purgeExpired(); + const createdAt = this.now(); + const capture: SurfaceCapture = { + ...input, + id: this.createId(), + createdAt, + expiresAt: createdAt + this.ttlMs, + consumed: false, + }; + this.captures.set(capture.id, capture); + return capture; + } + + consume(id: string): SurfaceCaptureConsumeResult { + const capture = this.captures.get(id); + if (!capture) return { ok: false, reason: "not_found" }; + if (capture.expiresAt <= this.now()) { + this.captures.delete(id); + return { ok: false, reason: "expired" }; + } + if (capture.consumed) return { ok: false, reason: "consumed" }; + capture.consumed = true; + return { ok: true, capture }; + } + + clear(): void { + this.captures.clear(); + } + + size(): number { + this.purgeExpired(); + return this.captures.size; + } + + private purgeExpired(): void { + const now = this.now(); + for (const [id, capture] of this.captures) { + if (capture.expiresAt <= now) this.captures.delete(id); + } + } +} diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index cc3b0708..0e3a5a2a 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -67,6 +67,9 @@ function makeFakeCdp(handlers: Record unknown>) { if (!handler && method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; } + if (!handler && method === "Page.getFrameTree") { + return { frameTree: { frame: { id: "main", loaderId: "loader-1", url: "https://test/" } } }; + } if (!handler) throw new Error(`unexpected CDP call ${method}`); return handler(params); }); @@ -334,6 +337,11 @@ describe("handleScreenshot", () => { }; expect(clip.clip).toMatchObject({ width: 4096, height: 4096 }); expect(clip.clip?.scale).toBeCloseTo(Math.sqrt(4_000_000 / (4096 * 4096))); + expect(res.capture).toMatchObject({ + surface_ref: "@e5", + coordinate_space: "capture-image-pixel", + }); + expect(ctx.surfaceCaptures.size()).toBe(1); }); it("crops a visual surface screenshot to its observation-time visible region", async () => { @@ -368,6 +376,7 @@ describe("handleScreenshot", () => { clip?: { x: number; y: number; width: number; height: number }; }; expect(clip.clip).toMatchObject({ x: 25, y: 30, width: 50, height: 40 }); + expect(res.capture?.id).toMatch(/^sc_/); }); it("returns not_found for unknown ref", async () => { diff --git a/apps/extension/src/tools/__tests__/session.test.ts b/apps/extension/src/tools/__tests__/session.test.ts index 118c18e7..14fafddf 100644 --- a/apps/extension/src/tools/__tests__/session.test.ts +++ b/apps/extension/src/tools/__tests__/session.test.ts @@ -252,12 +252,23 @@ describe("handleSessionStop with auto-return", () => { expect(aw.remove).not.toHaveBeenCalled(); }); - it("clears the RefStore before window teardown", async () => { + it("clears refs and Surface captures before window teardown", async () => { const aw = fakeAgentWindow([100]); const sm = new SessionManager({ agentWindow: aw }); const ctx = await sm.start("aa11"); // Insert a fake ref so we can verify clear() ran. ctx.refStore.set("e1", 123, { tabId: 7 }); + ctx.surfaceCaptures.create({ + sessionId: "aa11", + tabId: 7, + navigationIdentity: "navigation", + surface: { ref: "e1", backendNodeId: 123, observationGeneration: 0 }, + topViewportRect: { x: 0, y: 0, w: 100, h: 50 }, + imageWidth: 100, + imageHeight: 50, + viewportSignature: "viewport", + frameProjectionSignature: "frame", + }); const state: FakeState = { tabs: new Map(), windowsClosed: new Set(), @@ -266,5 +277,6 @@ describe("handleSessionStop with auto-return", () => { const { tabs, windows } = makeApis(state); await handleSessionStop(sm, { session_id: "aa11" }, { tabManagement: { tabs, windows } }); expect(ctx.refStore.isEmpty()).toBe(true); + expect(ctx.surfaceCaptures.size()).toBe(0); }); }); diff --git a/apps/extension/src/tools/__tests__/snapshot-ref.test.ts b/apps/extension/src/tools/__tests__/snapshot-ref.test.ts index 5c7282ff..32b21dda 100644 --- a/apps/extension/src/tools/__tests__/snapshot-ref.test.ts +++ b/apps/extension/src/tools/__tests__/snapshot-ref.test.ts @@ -26,12 +26,14 @@ describe("lookupSnapshotRef", () => { refKey: "e3", kind: "dom", capabilities: ["interact", "screenshot"], + generation: 0, }); expect(lookupSnapshotRef(ctx, "e3", 4)).toEqual({ backendNodeId: 1234, refKey: "e3", kind: "dom", capabilities: ["interact", "screenshot"], + generation: 0, }); }); @@ -68,6 +70,7 @@ describe("resolveSnapshotRef", () => { cdpSessionId: "child-session", kind: "dom" as const, capabilities: ["interact", "screenshot"] as const, + generation: 0, }; expect(lookupSnapshotRef(ctx, "@e3", 4)).toEqual(expected); expect(resolveSnapshotRef(ctx, "@e3", 4)).toEqual(expected); @@ -115,6 +118,7 @@ describe("resolveSnapshotRef", () => { refKey: "e3", kind: "dom", capabilities: ["interact", "screenshot"], + generation: 0, }); }); diff --git a/apps/extension/src/tools/__tests__/surface-coordinate.test.ts b/apps/extension/src/tools/__tests__/surface-coordinate.test.ts new file mode 100644 index 00000000..609ea8fb --- /dev/null +++ b/apps/extension/src/tools/__tests__/surface-coordinate.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { + mapImagePointToViewport, + pointInRegion, + sameRect, + surfaceVisibleRect, +} from "../surface-coordinate"; + +describe("surface coordinate mapping", () => { + it("maps capture image pixels through the actual screenshot dimensions", () => { + expect(mapImagePointToViewport({ x: 100, y: 50, w: 400, h: 200 }, 800, 400, 200, 100)).toEqual({ + x: 200, + y: 100, + }); + }); + + it("rejects invalid image coordinates and dimensions", () => { + const rect = { x: 0, y: 0, w: 100, h: 50 }; + expect(mapImagePointToViewport(rect, 100, 50, -1, 0)).toBeNull(); + expect(mapImagePointToViewport(rect, 100, 50, 100, 0)).toBeNull(); + expect(mapImagePointToViewport(rect, 100, 50, 0, 50)).toBeNull(); + expect(mapImagePointToViewport(rect, 100, 50, Number.NaN, 0)).toBeNull(); + expect(mapImagePointToViewport(rect, 100, 50, Number.POSITIVE_INFINITY, 0)).toBeNull(); + expect(mapImagePointToViewport(rect, 0, 50, 0, 0)).toBeNull(); + }); + + it("intersects live and observed visible regions and compares them strictly", () => { + const clipped = surfaceVisibleRect( + { x: 0, y: 0, width: 200, height: 100 }, + { x: 25, y: 30, w: 50, h: 40 }, + ); + expect(clipped).toEqual({ x: 25, y: 30, w: 50, h: 40 }); + expect(sameRect(clipped as NonNullable, { x: 25, y: 30, w: 50, h: 40 })).toBe( + true, + ); + expect(sameRect(clipped as NonNullable, { x: 26, y: 30, w: 50, h: 40 })).toBe( + false, + ); + }); + + it("checks the projected visible region rather than only its bounding box", () => { + const triangle = [ + [ + { x: 0, y: 0 }, + { x: 100, y: 0 }, + { x: 0, y: 100 }, + ], + ]; + expect(pointInRegion({ x: 20, y: 20 }, triangle)).toBe(true); + expect(pointInRegion({ x: 90, y: 90 }, triangle)).toBe(false); + }); +}); diff --git a/apps/extension/src/tools/__tests__/surface-point-action.test.ts b/apps/extension/src/tools/__tests__/surface-point-action.test.ts new file mode 100644 index 00000000..ce5bc809 --- /dev/null +++ b/apps/extension/src/tools/__tests__/surface-point-action.test.ts @@ -0,0 +1,313 @@ +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"; + +function fakeAgentWindow() { + return { + create: vi.fn(async () => 100), + remove: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => {}), + }; +} + +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) => { + sent.push({ method, params }); + if (method === "Page.getFrameTree") { + return { + frameTree: { frame: { id: "main", loaderId, url: "https://fixture.test/" } }, + } as never; + } + if (method === "Page.getLayoutMetrics") { + return { + cssLayoutViewport: { clientWidth: 800, clientHeight: 600, pageX: 0, pageY }, + cssVisualViewport: { + clientWidth: 800, + clientHeight: 600, + pageX: 0, + pageY, + scale: 1, + }, + } 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; + }, + setPageY: (value: number) => { + pageY = value; + }, + setQuad: (value: number[]) => { + quad = value; + }, + failDispatch: () => { + dispatchFails = true; + }, + }; +} + +async function setupCapture() { + const manager = new SessionManager({ agentWindow: fakeAgentWindow() }); + 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 browser = fakeBrowser(); + const environment = await captureSurfaceEnvironment(browser.cdp, 4, undefined); + if ("code" in environment) throw new Error(environment.message); + const entry = context.refStore.resolveEntry("e3"); + if (!entry) throw new Error("missing 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, browser, capture }; +} + +function pointParams(captureId: string) { + return { + session_id: "aa11", + ref: "@e3", + capture_id: captureId, + image_x: 50, + image_y: 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); + + 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, + }); + expect(browser.sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength( + 3, + ); + + const repeated = await handleSurfacePointClick(manager, pointParams(capture.id), browser); + expect(repeated).toMatchObject({ + code: "permission_denied", + data: { reason: "surface_capture_consumed" }, + }); + }); + + it("rejects a new observation generation before sending input", async () => { + const { manager, context, browser, capture } = await setupCapture(); + context.refStore.replace([ + [ + "e3", + { + backendNodeId: 99, + tabId: 4, + kind: "surface" as const, + visibleRect: { x: 10, y: 20, w: 100, h: 40 }, + }, + ], + ]); + + const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser); + expect(result).toMatchObject({ + code: "permission_denied", + data: { reason: "surface_capture_stale" }, + }); + expect(browser.sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false); + }); + + it("rejects navigation, scroll, and visible geometry changes", async () => { + for (const mutate of [ + (browser: ReturnType) => browser.setLoaderId("loader-2"), + (browser: ReturnType) => browser.setPageY(20), + (browser: ReturnType) => + browser.setQuad([11, 20, 111, 20, 111, 60, 11, 60]), + ]) { + const { manager, browser, capture } = await setupCapture(); + mutate(browser); + const result = await handleSurfacePointClick(manager, pointParams(capture.id), browser); + expect(result).toMatchObject({ + code: "permission_denied", + data: { reason: "surface_capture_stale" }, + }); + expect(browser.sent.some((call) => call.method === "Input.dispatchMouseEvent")).toBe(false); + } + }); + + it("consumes the capture when coordinates or input dispatch fail", async () => { + const invalid = await setupCapture(); + const outside = await handleSurfacePointClick( + invalid.manager, + { ...pointParams(invalid.capture.id), image_x: 200 }, + invalid.browser, + ); + expect(outside).toMatchObject({ + code: "invalid_params", + data: { reason: "surface_coordinate_invalid" }, + }); + expect( + await handleSurfacePointClick( + invalid.manager, + pointParams(invalid.capture.id), + invalid.browser, + ), + ).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 () => { + const manager = new SessionManager({ agentWindow: fakeAgentWindow() }); + const context = await manager.start("aa11"); + context.refStore.set("e3", 99, { + tabId: 4, + frameId: "child", + cdpSessionId: "child-session", + kind: "surface", + visibleRect: { x: 120, y: 110, w: 100, h: 40 }, + }); + const graph = { + rootFrameId: "main", + frames: [ + { frameId: "main", target: { tabId: 4 } }, + { + frameId: "child", + parentFrameId: "main", + ownerBackendNodeId: 77, + target: { tabId: 4, sessionId: "child-session" }, + }, + ], + }; + const sent: Array<{ method: string; params?: object }> = []; + const cdp: CdpRunner = { + send: vi.fn(async (_tabId: number, method: string, params?: object) => { + sent.push({ method, params }); + if (method === "Page.getFrameTree") { + return { + frameTree: { + frame: { id: "main", loaderId: "loader-1", url: "https://fixture.test/" }, + }, + } as never; + } + if (method === "Page.getLayoutMetrics") { + return { + cssLayoutViewport: { clientWidth: 800, clientHeight: 600, pageX: 0, pageY: 0 }, + cssVisualViewport: { + clientWidth: 800, + clientHeight: 600, + pageX: 0, + pageY: 0, + scale: 1, + }, + } as never; + } + if (method === "DOM.getBoxModel") { + return { model: { content: [100, 100, 300, 100, 300, 200, 100, 200] } } as never; + } + if (method === "Input.dispatchMouseEvent") return {} as never; + throw new Error(`unexpected root CDP call ${method}`); + }) as CdpRunner["send"], + sendToTarget: vi.fn(async (_target, method) => { + if (method === "DOM.getContentQuads") { + return { quads: [[20, 10, 120, 10, 120, 50, 20, 50]] } as never; + } + if (method === "Page.getLayoutMetrics") { + return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } } as never; + } + throw new Error(`unexpected child CDP call ${method}`); + }) as CdpRunner["sendToTarget"], + 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"); + if (!entry) throw new Error("missing ref"); + const capture = context.surfaceCaptures.create({ + sessionId: "aa11", + tabId: 4, + navigationIdentity: environment.navigationIdentity, + surface: { + ref: "e3", + frameId: "child", + backendNodeId: 99, + observationGeneration: entry.generation, + }, + topViewportRect: { x: 120, y: 110, w: 100, h: 40 }, + imageWidth: 200, + imageHeight: 80, + viewportSignature: environment.viewportSignature, + frameProjectionSignature: environment.frameProjectionSignature, + }); + + const result = await handleSurfacePointClick( + manager, + { + session_id: "aa11", + ref: "@e3", + capture_id: capture.id, + image_x: 100, + image_y: 40, + }, + { cdp, tabsApi }, + ); + + 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); + }); +}); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 350dc957..0f29ec95 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -43,6 +43,7 @@ import { resolveTargetTab, } from "./shared"; import { resolveSnapshotRef } from "./snapshot-ref"; +import { handleSurfacePointClick } from "./surface-point-action"; export interface InteractionDeps { cdp: CdpRunner; @@ -224,6 +225,11 @@ export async function handleClick( params: ClickParams, deps: InteractionDeps = getDefaultDeps(), ): 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; diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index f5290486..728e2baa 100644 --- a/apps/extension/src/tools/observation.ts +++ b/apps/extension/src/tools/observation.ts @@ -48,6 +48,8 @@ import { type ToolEffect, } from "./shared"; import { resolveSnapshotRef } from "./snapshot-ref"; +import { surfaceVisibleRect } from "./surface-coordinate"; +import { captureSurfaceEnvironment } from "./surface-point-action"; import { type CapturedNode, type CapturedViewModel, @@ -194,7 +196,9 @@ async function captureElementScreenshot( visualSurface = false, observedVisibleRect?: Rect, signal?: AbortSignal, -): Promise<{ image_base64: string; width: number; height: number } | RpcError> { +): Promise< + { image_base64: string; width: number; height: number; topViewportRect: Rect } | RpcError +> { if (signal?.aborted) return cancelled("screenshot"); const geometry = await resolveNodeGeometry( cdp, @@ -204,24 +208,19 @@ async function captureElementScreenshot( ); if (isRpcError(geometry)) return geometry; if (signal?.aborted) return cancelled("screenshot"); - const liveRect = geometry.topBounds; - const rect = - visualSurface && observedVisibleRect - ? (() => { - const x = Math.max(liveRect.x, observedVisibleRect.x); - const y = Math.max(liveRect.y, observedVisibleRect.y); - const right = Math.min( - liveRect.x + liveRect.width, - observedVisibleRect.x + observedVisibleRect.w, - ); - const bottom = Math.min( - liveRect.y + liveRect.height, - observedVisibleRect.y + observedVisibleRect.h, - ); - return right > x && bottom > y ? { x, y, width: right - x, height: bottom - y } : null; - })() - : liveRect; - if (!rect) return { code: "permission_denied", message: "surface is no longer visible" }; + const topViewportRect = surfaceVisibleRect( + geometry.topBounds, + visualSurface ? observedVisibleRect : undefined, + ); + if (!topViewportRect) { + return { code: "permission_denied", message: "surface is no longer visible" }; + } + const rect = { + x: topViewportRect.x, + y: topViewportRect.y, + width: topViewportRect.w, + height: topViewportRect.h, + }; const scale = visualSurface ? Math.min( 1, @@ -251,7 +250,7 @@ async function captureElementScreenshot( width: Math.round(rect.width), height: Math.round(rect.height), }; - return { image_base64, width: dims.width, height: dims.height }; + return { image_base64, width: dims.width, height: dims.height, topViewportRect }; } catch (err) { return { code: "cdp_failed", @@ -374,12 +373,46 @@ export async function handleScreenshot( ); if (isRpcError(captured)) return captured; if (signal?.aborted) return cancelled("screenshot"); + const capture = + node.kind === "surface" + ? await (async () => { + const environment = await captureSurfaceEnvironment(cdp, target.tabId, node.frameId); + if (isRpcError(environment)) return environment; + return ctx.surfaceCaptures.create({ + sessionId: ctx.sessionId, + tabId: target.tabId, + navigationIdentity: environment.navigationIdentity, + surface: { + ref: node.refKey, + ...(node.frameId ? { frameId: node.frameId } : {}), + backendNodeId: node.backendNodeId, + observationGeneration: node.generation, + }, + topViewportRect: captured.topViewportRect, + imageWidth: captured.width, + imageHeight: captured.height, + viewportSignature: environment.viewportSignature, + frameProjectionSignature: environment.frameProjectionSignature, + }); + })() + : null; + if (capture && isRpcError(capture)) return capture; return withShotDialogs({ image_base64: captured.image_base64, width: captured.width, height: captured.height, format: "png", tab_id: target.tabId, + ...(capture + ? { + capture: { + id: capture.id, + surface_ref: `@${capture.surface.ref}`, + coordinate_space: "capture-image-pixel" as const, + expires_at: capture.expiresAt, + }, + } + : {}), }); } diff --git a/apps/extension/src/tools/session.ts b/apps/extension/src/tools/session.ts index 9bd1b277..b4a9e9ba 100644 --- a/apps/extension/src/tools/session.ts +++ b/apps/extension/src/tools/session.ts @@ -243,6 +243,7 @@ export async function handleSessionStop( // Step 2: clear the per-session RefStore (review M6/M7 parity). ctx.refStore.clear(); + ctx.surfaceCaptures.clear(); clearRecordingForSession(params.session_id); diff --git a/apps/extension/src/tools/snapshot-ref.ts b/apps/extension/src/tools/snapshot-ref.ts index 19e3a25e..a1c5a109 100644 --- a/apps/extension/src/tools/snapshot-ref.ts +++ b/apps/extension/src/tools/snapshot-ref.ts @@ -16,6 +16,7 @@ export interface SnapshotRefLookup { visibleRect?: Rect; kind: RefTargetKind; capabilities: RefCapability[]; + generation: number; } function refEntryForTab(ctx: SessionContext, refKey: string, tabId: number) { @@ -44,6 +45,7 @@ export function lookupSnapshotRef( ...(entry.visibleRect ? { visibleRect: entry.visibleRect } : {}), kind: entry.kind, capabilities: entry.capabilities, + generation: entry.generation, }; } diff --git a/apps/extension/src/tools/surface-coordinate.ts b/apps/extension/src/tools/surface-coordinate.ts new file mode 100644 index 00000000..ced1a907 --- /dev/null +++ b/apps/extension/src/tools/surface-coordinate.ts @@ -0,0 +1,88 @@ +import type { Rect } from "@browser-skill/vom"; +import type { Point, Region, ViewportRect } from "./geometry"; + +const RECT_EPSILON = 0.01; + +export function surfaceVisibleRect( + liveRect: ViewportRect, + observedRect: Rect | undefined, +): Rect | null { + const observed = observedRect ?? { + x: liveRect.x, + y: liveRect.y, + w: liveRect.width, + h: liveRect.height, + }; + const x = Math.max(liveRect.x, observed.x); + const y = Math.max(liveRect.y, observed.y); + const right = Math.min(liveRect.x + liveRect.width, observed.x + observed.w); + const bottom = Math.min(liveRect.y + liveRect.height, observed.y + observed.h); + return right > x && bottom > y ? { x, y, w: right - x, h: bottom - y } : null; +} + +export function sameRect(a: Rect, b: Rect): boolean { + return ( + Math.abs(a.x - b.x) <= RECT_EPSILON && + Math.abs(a.y - b.y) <= RECT_EPSILON && + Math.abs(a.w - b.w) <= RECT_EPSILON && + Math.abs(a.h - b.h) <= RECT_EPSILON + ); +} + +export function mapImagePointToViewport( + rect: Rect, + imageWidth: number, + imageHeight: number, + imageX: number, + imageY: number, +): Point | null { + if ( + !Number.isFinite(imageX) || + !Number.isFinite(imageY) || + !Number.isSafeInteger(imageWidth) || + !Number.isSafeInteger(imageHeight) || + imageWidth <= 0 || + imageHeight <= 0 || + imageX < 0 || + imageY < 0 || + imageX >= imageWidth || + imageY >= imageHeight || + !Number.isFinite(rect.x) || + !Number.isFinite(rect.y) || + !Number.isFinite(rect.w) || + !Number.isFinite(rect.h) || + rect.w <= 0 || + rect.h <= 0 + ) { + return null; + } + return { + x: rect.x + (imageX / imageWidth) * rect.w, + y: rect.y + (imageY / imageHeight) * rect.h, + }; +} + +function pointInPolygon(point: Point, polygon: Point[]): boolean { + for (let index = 0; index < polygon.length; index += 1) { + const a = polygon[index]; + const b = polygon[(index + 1) % polygon.length]; + const cross = (point.x - a.x) * (b.y - a.y) - (point.y - a.y) * (b.x - a.x); + const withinX = point.x >= Math.min(a.x, b.x) - 1e-9 && point.x <= Math.max(a.x, b.x) + 1e-9; + const withinY = point.y >= Math.min(a.y, b.y) - 1e-9 && point.y <= Math.max(a.y, b.y) + 1e-9; + if (Math.abs(cross) <= 1e-9 && withinX && withinY) return true; + } + let inside = false; + for (let index = 0, previous = polygon.length - 1; index < polygon.length; previous = index++) { + const a = polygon[index]; + const b = polygon[previous]; + const intersects = + a.y > point.y !== b.y > point.y && + point.x < ((b.x - a.x) * (point.y - a.y)) / (b.y - a.y) + a.x; + if (intersects) inside = !inside; + } + return inside; +} + +export function pointInRegion(point: Point, region: Region): boolean { + return region.some((polygon) => polygon.length >= 3 && pointInPolygon(point, polygon)); +} diff --git a/apps/extension/src/tools/surface-point-action.ts b/apps/extension/src/tools/surface-point-action.ts new file mode 100644 index 00000000..e61b6615 --- /dev/null +++ b/apps/extension/src/tools/surface-point-action.ts @@ -0,0 +1,320 @@ +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 { rpcError } from "./errors"; +import { resolveNodeGeometry } from "./frame-geometry"; +import { + type CdpRunner, + type ChromeTabsApi, + enforceAgentWindow, + isRpcError, + lookupSession, + normaliseRef, + resolveTargetTab, +} from "./shared"; +import { resolveSnapshotRef } from "./snapshot-ref"; +import { + mapImagePointToViewport, + pointInRegion, + sameRect, + surfaceVisibleRect, +} from "./surface-coordinate"; + +export interface SurfaceCaptureEnvironment { + navigationIdentity: string; + viewportSignature: string; + frameProjectionSignature: string; +} + +export interface SurfacePointActionDeps { + cdp: CdpRunner; + tabsApi: ChromeTabsApi; + signal?: AbortSignal; + bypassOverlay?: (tabId: number, enabled: boolean) => Promise; +} + +function stale(message: string): RpcError { + return rpcError("permission_denied", "surface_capture_stale", message); +} + +function finiteObject(value: unknown): unknown { + if (Array.isArray(value)) return value.map(finiteObject); + if (typeof value !== "object" || value === null) return value; + return Object.fromEntries( + Object.entries(value) + .filter( + ([, entry]) => entry !== undefined && (typeof entry !== "number" || Number.isFinite(entry)), + ) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, entry]) => [key, finiteObject(entry)]), + ); +} + +function framePathSignature(graph: CdpFrameGraph, frameId: string | undefined): string | null { + if (!frameId) return "top"; + const frames = new Map(graph.frames.map((frame) => [frame.frameId, frame])); + const path: Array<{ + frameId: string; + parentFrameId?: string; + ownerBackendNodeId?: number; + targetSessionId?: string; + }> = []; + const seen = new Set(); + let current = frames.get(frameId); + while (current) { + if (seen.has(current.frameId)) return null; + seen.add(current.frameId); + path.push({ + frameId: current.frameId, + ...(current.parentFrameId ? { parentFrameId: current.parentFrameId } : {}), + ...(current.ownerBackendNodeId !== undefined + ? { ownerBackendNodeId: current.ownerBackendNodeId } + : {}), + ...(current.target.sessionId ? { targetSessionId: current.target.sessionId } : {}), + }); + if (!current.parentFrameId) return JSON.stringify(path); + const parent = frames.get(current.parentFrameId); + if (!parent) return null; + current = parent; + } + return null; +} + +export async function captureSurfaceEnvironment( + cdp: CdpRunner, + tabId: number, + frameId: string | undefined, +): Promise { + try { + const [tree, metrics, graph] = await Promise.all([ + cdp.send<{ + frameTree?: { frame?: { id?: string; loaderId?: string; url?: string } }; + }>(tabId, "Page.getFrameTree", {}), + cdp.send>(tabId, "Page.getLayoutMetrics", {}), + frameId && cdp.getFrameGraph ? cdp.getFrameGraph(tabId) : Promise.resolve(null), + ]); + const root = tree.frameTree?.frame; + if (!root?.id || !root.loaderId) { + return { code: "cdp_failed", message: "could not identify the current navigation" }; + } + const frameProjectionSignature = frameId + ? graph + ? framePathSignature(graph, frameId) + : null + : "top"; + if (!frameProjectionSignature) { + return { code: "cdp_failed", message: `could not identify frame projection for ${frameId}` }; + } + return { + navigationIdentity: JSON.stringify({ id: root.id, loaderId: root.loaderId, url: root.url }), + viewportSignature: JSON.stringify( + finiteObject({ + cssLayoutViewport: metrics.cssLayoutViewport, + cssVisualViewport: metrics.cssVisualViewport, + }), + ), + frameProjectionSignature, + }; + } catch (error) { + return { code: "cdp_failed", message: error instanceof Error ? error.message : String(error) }; + } +} + +function validatePointParams(params: ClickParams): RpcError | null { + if (!params.capture_id || typeof params.capture_id !== "string") { + return { code: "invalid_params", message: "surface point click requires capture_id" }; + } + if (typeof params.image_x !== "number" || !Number.isFinite(params.image_x)) { + return { code: "invalid_params", message: "image_x must be a finite number" }; + } + if (typeof params.image_y !== "number" || !Number.isFinite(params.image_y)) { + return { code: "invalid_params", message: "image_y must be a finite number" }; + } + if (!params.ref || params.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; +} + +function captureConsumeError(reason: "not_found" | "expired" | "consumed"): RpcError { + const code = reason === "not_found" ? "not_found" : "permission_denied"; + return rpcError(code, `surface_capture_${reason}`, `surface capture is ${reason}`); +} + +function aborted(signal: AbortSignal | undefined): RpcError | null { + return signal?.aborted ? { code: "cancelled", message: "surface point click aborted" } : null; +} + +export async function handleSurfacePointClick( + manager: SessionManager, + params: ClickParams, + deps: SurfacePointActionDeps, +): Promise { + const invalid = validatePointParams(params); + 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); + if (!consumed.ok) return captureConsumeError(consumed.reason); + const capture = consumed.capture; + const postConsumeAbort = aborted(deps.signal); + if (postConsumeAbort) return postConsumeAbort; + + 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) { + return stale("surface ref does not match the capture"); + } + const node = resolveSnapshotRef(ctx, params.ref as string, target.tabId, "screenshot"); + if (isRpcError(node)) return stale(node.message); + if ( + node.kind !== "surface" || + node.backendNodeId !== capture.surface.backendNodeId || + node.frameId !== capture.surface.frameId || + node.generation !== capture.surface.observationGeneration + ) { + return stale("surface observation generation or node identity changed"); + } + + deps.cdp.trackSessionTab?.(ctx.sessionId, target.tabId); + try { + await deps.cdp.ensureAttachedToUrl?.(target.tabId, target.url); + } catch (error) { + return { code: "cdp_failed", message: error instanceof Error ? error.message : String(error) }; + } + const environment = await captureSurfaceEnvironment(deps.cdp, target.tabId, node.frameId); + if (isRpcError(environment)) return environment; + if ( + environment.navigationIdentity !== capture.navigationIdentity || + environment.viewportSignature !== capture.viewportSignature || + environment.frameProjectionSignature !== capture.frameProjectionSignature + ) { + return stale("navigation, viewport, zoom, scroll, or frame projection changed"); + } + + const geometry = await resolveNodeGeometry( + deps.cdp, + target.tabId, + { + target: { + tabId: target.tabId, + ...(node.cdpSessionId ? { sessionId: node.cdpSessionId } : {}), + }, + backendNodeId: node.backendNodeId, + ...(node.frameId ? { frameId: node.frameId } : {}), + }, + { scrollIntoView: false }, + ); + if (isRpcError(geometry)) return stale(geometry.message); + const currentRect = surfaceVisibleRect(geometry.topBounds, node.visibleRect); + if (!currentRect || !sameRect(currentRect, capture.topViewportRect)) { + return stale("surface visible region changed"); + } + const point = mapImagePointToViewport( + capture.topViewportRect, + capture.imageWidth, + capture.imageHeight, + params.image_x as number, + params.image_y as number, + ); + if (!point || !pointInRegion(point, geometry.topVisibleRegions)) { + return rpcError( + "invalid_params", + "surface_coordinate_invalid", + "image coordinate is outside the captured Surface", + ); + } + + const dialogCursor = markDialogCursor(deps.cdp, target.tabId); + let bypassEnabled = false; + let pressed = false; + const release = () => + deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { + type: "mouseReleased", + x: point.x, + y: point.y, + button: "left", + clickCount: 1, + modifiers: 0, + }); + try { + if (deps.bypassOverlay) { + await deps.bypassOverlay(target.tabId, true); + bypassEnabled = true; + } + await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { + type: "mouseMoved", + x: point.x, + y: point.y, + modifiers: 0, + }); + const beforePressAbort = aborted(deps.signal); + if (beforePressAbort) return beforePressAbort; + await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { + type: "mousePressed", + x: point.x, + y: point.y, + button: "left", + clickCount: 1, + modifiers: 0, + }); + pressed = true; + const afterPressAbort = aborted(deps.signal); + if (afterPressAbort) { + await release(); + pressed = false; + return afterPressAbort; + } + await release(); + pressed = false; + } catch (error) { + if (pressed) { + try { + await release(); + } catch (releaseError) { + console.debug("[bsk surface-point] best-effort mouse release failed", releaseError); + } + } + 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, + }); +} diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index f9e5ffb4..9ec464a9 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -34,6 +34,11 @@ export type RpcErrorReason = | "restricted_tab_url" | "borrow_conflict" | "screenshot_capture_failed" + | "surface_capture_not_found" + | "surface_capture_expired" + | "surface_capture_consumed" + | "surface_capture_stale" + | "surface_coordinate_invalid" | "cleanup_failed"; export interface RpcErrorData { @@ -299,9 +304,17 @@ export interface ScreenshotResult { height: number; format: string; tab_id: number; + capture?: SurfaceCaptureInfo; dialogs?: JavaScriptDialogInfo[]; } +export interface SurfaceCaptureInfo { + id: string; + surface_ref: string; + coordinate_space: "capture-image-pixel"; + expires_at: number; +} + export interface SnapshotParams { session_id: string; tab_id?: number; @@ -415,6 +428,9 @@ export interface ClickParams { click_count?: number; modifiers?: KeyModifier[]; timeout_ms?: number; + capture_id?: string; + image_x?: number; + image_y?: number; } export interface ClickResult { @@ -423,6 +439,9 @@ export interface ClickResult { used_selector?: string; x: number; y: number; + capture_id?: string; + image_x?: number; + image_y?: number; dialogs?: JavaScriptDialogInfo[]; } diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index e0987301..4d5ce56f 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -80,7 +80,15 @@ Prefer `@eN` refs from the latest snapshot over raw CSS selectors. Use `--ref` / When VOM renders `[hover first: …]` on an element, the listed items are not currently clickable refs. Run `bsk hover --session `, then immediately run `bsk snapshot` or `bsk observe` again and click the newly visible menu item ref. Do not click the trigger itself unless the user explicitly wants the trigger action. -When `bsk observe` renders `@eN surface ... [visual-only; requires=image-understanding; ...]`, the ref identifies rendered canvas content that is not represented by the text observation, not an interactable DOM control. Keep using any reliable DOM/AX text and controls that appear alongside it. If you can actually inspect image output, use `bsk screenshot --ref @eN --session ` to obtain the visible crop. If you lack multimodal or image-reading capability, do not take a screenshot and pretend to know its contents, and never guess coordinates; tell the user that they need to switch to a model with image-understanding capability. Do not pass a visual surface ref to click, fill, hover, or select; those commands intentionally reject screenshot-only refs. +When `bsk observe` renders `@eN surface ... [visual-only; requires=image-understanding; ...]`, the ref identifies rendered canvas content that is not represented by the text observation, not an interactable DOM control. Keep using any reliable DOM/AX text and controls that appear alongside it. If you lack multimodal or image-reading capability, do not take a screenshot and pretend to know its contents, and never guess coordinates; tell the user that they need to switch to a model with image-understanding capability. + +If you can actually inspect image output, use `bsk screenshot --ref @eN --session ` to obtain the visible crop and its short-lived, single-use Surface capture id. Only when the task requires a point action and the image provides a clear target, bind that exact screenshot to the same general click command: + +```bash +bsk click @eN --capture --image-x --image-y --session +``` + +Coordinates are pixels in the returned capture image, not viewport CSS coordinates. Re-observing, navigating, scrolling, resizing, zooming, changing Frame projection, expiry, or reusing the capture makes the action fail; take a new Surface screenshot instead of adjusting or retrying coordinates. A point click does not reveal Canvas semantics and must not be followed by an assumed fill/press workflow. Without all three capture arguments, visual Surface refs remain screenshot-only and click/fill/hover/select reject them. ## Observation priority @@ -176,7 +184,7 @@ bsk emulate --session --off | `bsk snapshot` | First-choice static page understanding: accessibility tree with `@eN` element refs | | `bsk observe` | Semantic VOM observation with bounded perception probes for conditional surfaces | | `bsk get-html` | Raw HTML dump after snapshot is insufficient (high token cost) | -| `bsk screenshot` | PNG capture after snapshot is insufficient: full visible tab, or `--ref @eN` to crop to one element (`--out` path optional) | +| `bsk screenshot` | PNG capture after snapshot is insufficient: full visible tab, or `--ref @eN` to crop to one element (`--out` path optional); Surface crops return a short-lived capture id | ### Console & network debugging (read-only; require `--session`) @@ -202,7 +210,7 @@ Both capture from the moment the tab is attached and read a bounded per-tab buff | Command | Summary | |---------|---------| -| `bsk click ` | Click element (`--button`, `--click-count`, `--modifiers`) | +| `bsk click ` | Click a DOM element (`--button`, `--click-count`, `--modifiers`), or perform one screenshot-bound Surface point click with `--capture`, `--image-x`, and `--image-y` | | `bsk hover ` | Move the mouse to an element and wait for hover UI to settle (`--settle`, `--modifiers`) | | `bsk fill --value ` | Clear and type into input | | `bsk select --value ` | Set `` option(s) by `value` (repeat `--value` for multi-select) | From 71f7e6d4b6e5c8297164d236902cb943f020368e Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 26 Aug 2026 14:51:28 +0800 Subject: [PATCH 2/3] fix(vom): canvas bug fix --- .../src/tools/__tests__/mouse-input.test.ts | 119 ++++++++++++++++++ apps/extension/src/tools/interaction.ts | 39 +----- apps/extension/src/tools/mouse-input.ts | 96 ++++++++++++++ .../src/tools/surface-point-action.ts | 47 ++----- .../test-fixtures/rendered-surfaces/README.md | 8 ++ .../rendered-surfaces/frame.html | 28 ++++- .../rendered-surfaces/frames.html | 4 +- 7 files changed, 265 insertions(+), 76 deletions(-) create mode 100644 apps/extension/src/tools/__tests__/mouse-input.test.ts create mode 100644 apps/extension/src/tools/mouse-input.ts diff --git a/apps/extension/src/tools/__tests__/mouse-input.test.ts b/apps/extension/src/tools/__tests__/mouse-input.test.ts new file mode 100644 index 00000000..51e4c26c --- /dev/null +++ b/apps/extension/src/tools/__tests__/mouse-input.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it, vi } from "vitest"; +import { dispatchMouseClick } from "../mouse-input"; +import type { CdpRunner } from "../shared"; + +function fakeCdp(onSend?: (method: string, params: Record) => void): { + cdp: CdpRunner; + events: Record[]; +} { + const events: Record[] = []; + return { + events, + cdp: { + send: vi.fn(async (_tabId, method, params) => { + const event = params as Record; + events.push(event); + onSend?.(method, event); + return {} as never; + }), + }, + }; +} + +describe("dispatchMouseClick", () => { + it("emits one move, press, and release sequence", async () => { + const { cdp, events } = fakeCdp(); + + expect( + await dispatchMouseClick( + cdp, + 4, + { x: 12, y: 34 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + moveSettleMs: 0, + }, + ), + ).toBe("completed"); + + expect(events).toEqual([ + { + type: "mouseMoved", + x: 12, + y: 34, + modifiers: 0, + }, + { + type: "mousePressed", + x: 12, + y: 34, + button: "left", + clickCount: 1, + modifiers: 0, + }, + { + type: "mouseReleased", + x: 12, + y: 34, + button: "left", + clickCount: 1, + modifiers: 0, + }, + ]); + }); + + it("releases a pressed button when cancellation arrives", async () => { + const controller = new AbortController(); + const { cdp, events } = fakeCdp((_method, event) => { + if (event.type === "mousePressed") controller.abort(); + }); + + expect( + await dispatchMouseClick( + cdp, + 4, + { x: 1, y: 2 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + signal: controller.signal, + moveSettleMs: 0, + }, + ), + ).toBe("cancelled"); + expect(events.at(-1)).toMatchObject({ type: "mouseReleased" }); + }); + + it("settles pointer-move hit testing before pressing", async () => { + vi.useFakeTimers(); + try { + const { cdp, events } = fakeCdp(); + const click = dispatchMouseClick( + cdp, + 4, + { x: 1, y: 2 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + moveSettleMs: 32, + }, + ); + + await vi.advanceTimersByTimeAsync(31); + expect(events.map((event) => event.type)).toEqual(["mouseMoved"]); + await vi.advanceTimersByTimeAsync(1); + await click; + expect(events.map((event) => event.type)).toEqual([ + "mouseMoved", + "mousePressed", + "mouseReleased", + ]); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 0f29ec95..fb1dc9b2 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -32,6 +32,7 @@ import { attachDialogs, markDialogCursor } from "./dialogs"; import { backendNodeToObject } from "./element-geometry"; import { rpcError } from "./errors"; import { resolveNodeGeometry, scrollElementAndFramesIntoView } from "./frame-geometry"; +import { dispatchMouseClick } from "./mouse-input"; import { type CdpRunner, type ChromeTabsApi, @@ -285,47 +286,15 @@ export async function handleClick( } try { - // Move first so hover state activates, then press → release. - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseMoved", - x: centre.x, - y: centre.y, - modifiers, - }); - if (throwIfAborted(deps.signal)) { - return { code: "cancelled", message: "click aborted" }; - } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mousePressed", - x: centre.x, - y: centre.y, + const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, centre, { button, clickCount, modifiers, + signal: deps.signal, }); - if (throwIfAborted(deps.signal)) { - try { - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: centre.x, - y: centre.y, - button, - clickCount, - modifiers, - }); - } catch (err) { - console.debug("[bsk interaction] best-effort mouseReleased after abort failed", err); - } + if (dispatch === "cancelled") { return { code: "cancelled", message: "click aborted" }; } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: centre.x, - y: centre.y, - button, - clickCount, - modifiers, - }); } catch (err) { return { code: "cdp_failed", diff --git a/apps/extension/src/tools/mouse-input.ts b/apps/extension/src/tools/mouse-input.ts new file mode 100644 index 00000000..c0100b31 --- /dev/null +++ b/apps/extension/src/tools/mouse-input.ts @@ -0,0 +1,96 @@ +import type { MouseButton } from "@/transport/types"; +import type { Point } from "./geometry"; +import type { CdpRunner } from "./shared"; + +interface MouseClickDispatchOptions { + button: MouseButton; + clickCount: number; + modifiers: number; + signal?: AbortSignal; + moveSettleMs?: number; +} + +type MouseClickDispatchResult = "completed" | "cancelled"; + +function waitForSettle(ms: number, signal: AbortSignal | undefined): Promise { + if (ms <= 0) return Promise.resolve(signal?.aborted !== true); + if (signal?.aborted) return Promise.resolve(false); + return new Promise((resolve) => { + let settled = false; + const finish = (completed: boolean) => { + if (settled) return; + settled = true; + clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + resolve(completed); + }; + const timer = setTimeout(() => finish(true), ms); + const onAbort = () => finish(false); + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} + +/** + * Dispatch one coherent CDP mouse click in top-viewport CSS coordinates. + * + * Keeping the sequence here prevents DOM and Surface interaction paths from + * duplicating cancellation and best-effort release behavior. Callers that + * target asynchronously hit-tested content may request a settle interval. + */ +export async function dispatchMouseClick( + cdp: CdpRunner, + tabId: number, + point: Point, + options: MouseClickDispatchOptions, +): Promise { + const { button, clickCount, modifiers, signal, moveSettleMs = 0 } = options; + let pressed = false; + const release = () => + cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mouseReleased", + x: point.x, + y: point.y, + button, + clickCount, + modifiers, + }); + + try { + await cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mouseMoved", + x: point.x, + y: point.y, + modifiers, + }); + if (!(await waitForSettle(moveSettleMs, signal))) return "cancelled"; + + await cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mousePressed", + x: point.x, + y: point.y, + button, + clickCount, + modifiers, + }); + pressed = true; + + if (signal?.aborted) { + await release(); + pressed = false; + return "cancelled"; + } + + await release(); + pressed = false; + return "completed"; + } catch (error) { + if (pressed) { + try { + await release(); + } catch (releaseError) { + console.debug("[bsk mouse-input] best-effort mouse release failed", releaseError); + } + } + throw error; + } +} diff --git a/apps/extension/src/tools/surface-point-action.ts b/apps/extension/src/tools/surface-point-action.ts index e61b6615..79fc8c06 100644 --- a/apps/extension/src/tools/surface-point-action.ts +++ b/apps/extension/src/tools/surface-point-action.ts @@ -4,6 +4,7 @@ import type { ClickParams, ClickResult, 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, type ChromeTabsApi, @@ -34,6 +35,10 @@ export interface SurfacePointActionDeps { bypassOverlay?: (tabId: number, enabled: boolean) => Promise; } +// 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; + function stale(message: string): RpcError { return rpcError("permission_denied", "surface_capture_stale", message); } @@ -249,54 +254,22 @@ export async function handleSurfacePointClick( const dialogCursor = markDialogCursor(deps.cdp, target.tabId); let bypassEnabled = false; - let pressed = false; - const release = () => - deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: point.x, - y: point.y, - button: "left", - clickCount: 1, - modifiers: 0, - }); try { if (deps.bypassOverlay) { await deps.bypassOverlay(target.tabId, true); bypassEnabled = true; } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseMoved", - x: point.x, - y: point.y, - modifiers: 0, - }); - const beforePressAbort = aborted(deps.signal); - if (beforePressAbort) return beforePressAbort; - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mousePressed", - x: point.x, - y: point.y, + const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, point, { button: "left", clickCount: 1, modifiers: 0, + signal: deps.signal, + moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS, }); - pressed = true; - const afterPressAbort = aborted(deps.signal); - if (afterPressAbort) { - await release(); - pressed = false; - return afterPressAbort; + if (dispatch === "cancelled") { + return { code: "cancelled", message: "surface point click aborted" }; } - await release(); - pressed = false; } catch (error) { - if (pressed) { - try { - await release(); - } catch (releaseError) { - console.debug("[bsk surface-point] best-effort mouse release failed", releaseError); - } - } return { code: "cdp_failed", message: error instanceof Error ? error.message : String(error) }; } finally { if (bypassEnabled && deps.bypassOverlay) { diff --git a/apps/extension/test-fixtures/rendered-surfaces/README.md b/apps/extension/test-fixtures/rendered-surfaces/README.md index d0f10a00..a377507d 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/README.md +++ b/apps/extension/test-fixtures/rendered-surfaces/README.md @@ -77,6 +77,14 @@ Expected Surface labels: The cross-origin frame uses `http://localhost:4173` while the parent uses `http://127.0.0.1:4173`. With Chromium site isolation this exercises the OOPIF path without an external service. +Each frame Canvas also exposes a `pointer status` live region. A successful +screenshot-bound point click must report +`down button=0 buttons=1 hoverReady=true; click received` and turn the Canvas +yellow. The hover flag is set on the animation frame after `pointermove`, so the +fixture verifies both child-document delivery and the scheduling boundary +required before `pointerdown`, rather than merely validating projected +coordinates or a successful CDP response. + ### `viewport.html` Run with a `900x700` Agent Window. diff --git a/apps/extension/test-fixtures/rendered-surfaces/frame.html b/apps/extension/test-fixtures/rendered-surfaces/frame.html index cc42cd42..da17b819 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/frame.html +++ b/apps/extension/test-fixtures/rendered-surfaces/frame.html @@ -8,6 +8,7 @@ body { padding: 12px; background: #fdfefe; } canvas { display: block; width: 220px; height: 90px; background: #d5f5e3; border: 2px solid #1e8449; } iframe { width: 280px; height: 150px; margin-top: 10px; border: 2px solid #85929e; } + #pointer-status { margin: 8px 0 0; min-height: 20px; } @@ -16,11 +17,34 @@ const params = new URLSearchParams(location.search); const canvas = document.querySelector("#frame-canvas"); if (params.get("canvas") === "0") canvas.remove(); - else canvas.setAttribute("aria-label", params.get("label") || "fixture-frame-default"); + else { + canvas.setAttribute("aria-label", params.get("label") || "fixture-frame-default"); + if (params.get("interactive") === "1") { + const status = document.createElement("p"); + status.id = "pointer-status"; + status.setAttribute("aria-live", "polite"); + status.textContent = "pointer status: idle"; + canvas.insertAdjacentElement("afterend", status); + let hoverReady = false; + canvas.addEventListener("pointermove", () => { + hoverReady = false; + requestAnimationFrame(() => { + hoverReady = true; + }); + }); + canvas.addEventListener("pointerdown", (event) => { + status.textContent = `pointer status: down button=${event.button} buttons=${event.buttons} hoverReady=${hoverReady}`; + if (event.buttons === 1 && hoverReady) canvas.style.background = "#f9e79f"; + }); + canvas.addEventListener("click", () => { + status.textContent += "; click received"; + }); + } + } if (params.get("nested") === "1") { const iframe = document.createElement("iframe"); iframe.title = "nested fixture frame"; - iframe.src = "frame.html?label=fixture-nested-frame"; + iframe.src = "frame.html?label=fixture-nested-frame&interactive=1"; document.body.append(iframe); } diff --git a/apps/extension/test-fixtures/rendered-surfaces/frames.html b/apps/extension/test-fixtures/rendered-surfaces/frames.html index 63550309..0f780d90 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/frames.html +++ b/apps/extension/test-fixtures/rendered-surfaces/frames.html @@ -14,8 +14,8 @@

Frame matrix

- - + +
From f5dab955e2ba946a394b8fb5bb074a8ba62814ea Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Fri, 28 Aug 2026 15:50:32 +0800 Subject: [PATCH 3/3] 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 6f716bba..dffb4918 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 ce5bc809..b065c106 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 fb1dc9b2..a196f199 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 728e2baa..966f445d 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 79fc8c06..7529b541 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 0d05515a..db1315b4 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 e023ad66..88bf1b24 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({