From fd5aeee84af997f24b76e067dcfa6e3def8e0074 Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 16 Sep 2026 14:13:38 +0800 Subject: [PATCH 1/2] fix(extension): capture target-blank attachment downloads --- .../__tests__/download-trigger-intent.test.ts | 39 +++++++++++ .../src/tools/__tests__/file-transfer.test.ts | 67 ++++++++++++++++++- apps/extension/src/tools/download-capture.ts | 30 ++++++--- .../src/tools/download-trigger-intent.ts | 48 +++++++++++++ apps/extension/src/tools/download.ts | 3 + docs/architecture.md | 2 +- 6 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 apps/extension/src/tools/__tests__/download-trigger-intent.test.ts create mode 100644 apps/extension/src/tools/download-trigger-intent.ts diff --git a/apps/extension/src/tools/__tests__/download-trigger-intent.test.ts b/apps/extension/src/tools/__tests__/download-trigger-intent.test.ts new file mode 100644 index 00000000..7e61b091 --- /dev/null +++ b/apps/extension/src/tools/__tests__/download-trigger-intent.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it, vi } from "vitest"; +import { resolveDownloadTriggerUrl } from "../download-trigger-intent"; +import type { ResolvedActionTarget } from "../interaction"; +import type { CdpRunner } from "../shared"; + +const target: ResolvedActionTarget = { + tab: { tabId: 4, windowId: 100, active: true }, + backendNodeId: 123, + cdpTarget: { tabId: 4 }, +}; + +describe("download trigger intent", () => { + it("returns a validated HTTP URL from a target-blank anchor", async () => { + const send = vi.fn(async (_tabId: number, method: string) => { + if (method === "DOM.resolveNode") return { object: { objectId: "anchor" } }; + if (method === "Runtime.callFunctionOn") { + return { result: { value: "https://example.test/report.csv" } }; + } + return {}; + }) as unknown as CdpRunner["send"]; + + await expect(resolveDownloadTriggerUrl({ send }, target)).resolves.toBe( + "https://example.test/report.csv", + ); + expect(send).toHaveBeenCalledWith(4, "Runtime.releaseObject", { objectId: "anchor" }); + }); + + it("rejects non-HTTP results", async () => { + const send = vi.fn(async (_tabId: number, method: string) => { + if (method === "DOM.resolveNode") return { object: { objectId: "anchor" } }; + if (method === "Runtime.callFunctionOn") { + return { result: { value: "javascript:download()" } }; + } + return {}; + }) as unknown as CdpRunner["send"]; + + await expect(resolveDownloadTriggerUrl({ send }, target)).resolves.toBeUndefined(); + }); +}); diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 740630ee..20909bcb 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -626,7 +626,11 @@ describe("file transfer tools", () => { const result = await handleDownload( manager, { session_id: "s1", ref: "@e3", browser_relative_dir: "BrowserSkill/tr_1" }, - { cdp, tabsApi: tabsApi(), downloads }, + { + cdp, + tabsApi: tabsApi(), + downloads, + }, ); expect(suggested).toEqual({ @@ -702,6 +706,67 @@ describe("file transfer tools", () => { }); }); + it("captures a unique download matching a trigger-authorized URL", async () => { + const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const initial = { + id: 18, + url: "https://example.test/popup.bin", + finalUrl: "https://example.test/popup.bin", + filename: "popup.bin", + state: "in_progress", + fileSize: -1, + totalBytes: 4, + mime: "application/octet-stream", + danger: "safe", + } as chrome.downloads.DownloadItem; + const complete = { + ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_2/popup.bin", + state: "complete", + fileSize: 4, + } as chrome.downloads.DownloadItem; + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => [complete]), + cancel: vi.fn(async () => {}), + removeFile: vi.fn(async () => {}), + }; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as unknown as CdpRunner["send"], + onEvent: () => ({ dispose: vi.fn() }), + }; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4 }, + expectedUrl: initial.url, + downloads, + browserRelativeDir: "BrowserSkill/tr_2", + timeoutMs: 200, + trigger: async () => { + await new Promise((resolve) => { + onDeterminingFilename.emit(initial, () => resolve()); + }); + onCreated.emit(complete); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ + item: { id: 18, filename: complete.filename, state: "complete" }, + }); + }); + it("correlates a filename candidate that arrives before the CDP intent", async () => { const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); diff --git a/apps/extension/src/tools/download-capture.ts b/apps/extension/src/tools/download-capture.ts index 6e75bff4..3477818c 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -54,6 +54,7 @@ export interface DownloadCaptureOptions { maxByteSize?: number; timeoutMs: number; signal?: AbortSignal; + expectedUrl?: string; trigger(): Promise; } @@ -64,7 +65,8 @@ export interface DownloadCaptureResult { interface DownloadIntent { url: string; - suggestedFilename: string; + cdpUrl?: string; + suggestedFilename?: string; frameId?: string; } @@ -85,8 +87,13 @@ function sameTarget(source: { tabId?: number; sessionId?: string }, target: CdpT } function matchesIntent(item: chrome.downloads.DownloadItem, intent: DownloadIntent): boolean { - const urlMatches = item.url === intent.url || item.finalUrl === intent.url; - return urlMatches && safeBasename(item.filename) === safeBasename(intent.suggestedFilename); + const urls = [intent.url, intent.cdpUrl].filter((url): url is string => Boolean(url)); + const urlMatches = urls.some((url) => item.url === url || item.finalUrl === url); + return ( + urlMatches && + (!intent.suggestedFilename || + safeBasename(item.filename) === safeBasename(intent.suggestedFilename)) + ); } function knownSize(item: chrome.downloads.DownloadItem): number | undefined { @@ -137,7 +144,10 @@ export async function captureBrowserDownload( options: DownloadCaptureOptions, ): Promise { let click: ClickResult | undefined; - let intent: DownloadIntent | undefined; + let intent: DownloadIntent | undefined = options.expectedUrl + ? { url: options.expectedUrl } + : undefined; + let cdpIntentSeen = false; let capturedId: number | undefined; let settled = false; let succeeded = false; @@ -199,7 +209,9 @@ export async function captureBrowserDownload( clearTimeout(candidate.graceTimer); capturedId = candidate.item.id; candidate.suggest({ - filename: `${options.browserRelativeDir}/${safeBasename(intent.suggestedFilename)}`, + filename: `${options.browserRelativeDir}/${safeBasename( + intent.suggestedFilename ?? candidate.item.filename, + )}`, conflictAction: "overwrite", }); const size = knownSize(candidate.item); @@ -278,12 +290,14 @@ export async function captureBrowserDownload( fail(new Error("download originated from a different frame")); return; } - if (intent) { + if (cdpIntentSeen) { fail(new Error("download trigger produced more than one browser download intent")); return; } + cdpIntentSeen = true; intent = { - url: event.url, + url: intent?.url ?? event.url, + ...(intent ? { cdpUrl: event.url } : {}), suggestedFilename: event.suggestedFilename, ...(typeof event.frameId === "string" ? { frameId: event.frameId } : {}), }; @@ -319,7 +333,7 @@ export async function captureBrowserDownload( if (isRpcError(triggered)) { void completion.catch(() => undefined); const effect: TransferEffectState = - capturedId !== undefined ? "committed" : intent ? "unknown" : "none"; + capturedId !== undefined ? "committed" : cdpIntentSeen ? "unknown" : "none"; failureResult = { ...triggered, data: { ...triggered.data, effect_state: effect, phase: "trigger" }, diff --git a/apps/extension/src/tools/download-trigger-intent.ts b/apps/extension/src/tools/download-trigger-intent.ts new file mode 100644 index 00000000..fd9a269b --- /dev/null +++ b/apps/extension/src/tools/download-trigger-intent.ts @@ -0,0 +1,48 @@ +import type { ResolvedActionTarget } from "./interaction"; +import { type CdpRunner, sendToCdpTarget } from "./shared"; + +const READ_ANCHOR_INTENT = `function () { + const anchor = this instanceof Element ? this.closest("a[href]") : null; + if (!(anchor instanceof HTMLAnchorElement) || anchor.target.toLowerCase() !== "_blank") return null; + const url = new URL(anchor.href, document.baseURI); + return url.protocol === "http:" || url.protocol === "https:" ? url.href : null; +}`; + +export async function resolveDownloadTriggerUrl( + cdp: CdpRunner, + target: ResolvedActionTarget, +): Promise { + let objectId: string | undefined; + try { + const resolved = await sendToCdpTarget<{ object?: { objectId?: string } }>( + cdp, + target.cdpTarget, + "DOM.resolveNode", + { backendNodeId: target.backendNodeId }, + ); + objectId = resolved.object?.objectId; + if (!objectId) return undefined; + const result = await sendToCdpTarget<{ result?: { value?: unknown } }>( + cdp, + target.cdpTarget, + "Runtime.callFunctionOn", + { + objectId, + functionDeclaration: READ_ANCHOR_INTENT, + returnByValue: true, + silent: true, + }, + ); + if (typeof result.result?.value !== "string") return undefined; + const url = new URL(result.result.value); + return url.protocol === "http:" || url.protocol === "https:" ? url.href : undefined; + } catch { + return undefined; + } finally { + if (objectId) { + await sendToCdpTarget(cdp, target.cdpTarget, "Runtime.releaseObject", { objectId }).catch( + () => {}, + ); + } + } +} diff --git a/apps/extension/src/tools/download.ts b/apps/extension/src/tools/download.ts index 047e263d..d4c0d6e1 100644 --- a/apps/extension/src/tools/download.ts +++ b/apps/extension/src/tools/download.ts @@ -4,6 +4,7 @@ import type { SessionManager } from "@/session-manager/manager"; import type { DownloadParams, DownloadResult, RpcError } from "@/transport/types"; import { captureBrowserDownload, chromeDownloadsApi, type DownloadsApi } from "./download-capture"; +import { resolveDownloadTriggerUrl } from "./download-trigger-intent"; import { clickResolvedTarget, type InteractionDeps, resolveActionTarget } from "./interaction"; import { enforceAgentWindow, isRpcError, lookupSession, resolveTargetTab } from "./shared"; @@ -35,6 +36,7 @@ export async function handleDownload( const address = await resolveActionTarget(deps.cdp, ctx, target, params, "download"); if (isRpcError(address)) return address; + const expectedUrl = await resolveDownloadTriggerUrl(deps.cdp, address); const capture = await captureBrowserDownload({ cdp: deps.cdp, target: address.cdpTarget, @@ -44,6 +46,7 @@ export async function handleDownload( timeoutMs: params.timeout_ms ?? 120_000, signal: deps.signal, expectedFrameId: address.frameId, + expectedUrl, trigger: () => clickResolvedTarget(ctx, address, {}, deps), }); if (isRpcError(capture)) return capture; diff --git a/docs/architecture.md b/docs/architecture.md index ac1a9321..d824e081 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -173,7 +173,7 @@ return `unsupported`; screenshots and other RPC content results remain available - The invoking agent/harness decides whether a transfer is authorized and supplies the task-local source or destination path. - The CLI is the only component that reads an upload source or writes the final download destination. Before browser dispatch it owns rollback of partially staged uploads; after dispatch, ownership moves to the session because a transport timeout cannot prove that Chrome did not attach the file. Download output becomes visible through one atomic commit, and replacement is opt-in without a pre-delete window. The extension never receives either agent-facing path. - The daemon is the authority for storage capabilities and limits. It issues opaque session-scoped transfer IDs, stages bounded chunks in a private runtime directory, and injects only private staged upload paths. For download it mints one relative Chrome directory capability. Only after validating the reported path, file type, symlink boundary, and authoritative byte limit does it take ownership of browser-file cleanup and import the bytes. -- The extension owns only the browser transaction. Every transfer resolves one `ResolvedActionTarget`. The default upload mechanism arms Chrome's chooser interception before clicking, then accepts either an exact `Page.fileChooserOpened` input node or an independent probe anchored in the trigger node's document; one verified input is committed with `DOM.setFileInputFiles`, while a non-input picker is rejected immediately. Explicit drop mode performs no click and never falls back to the chooser mechanism: after geometry resolution it temporarily excludes BrowserSkill's own overlay, verifies that the resolved drop zone still owns its local action point, and sends one native `dragEnter` / `dragOver` / `drop` transaction to that node's CDP target before restoring the overlay. OOPIF drops use target-local coordinates rather than top-level click coordinates. Download correlates exact-target CDP intent and `chrome.downloads` filename candidates in either arrival order, claims only one unique match, and never cancels an unclaimed candidate. +- The extension owns only the browser transaction. Every transfer resolves one `ResolvedActionTarget`. The default upload mechanism arms Chrome's chooser interception before clicking, then accepts either an exact `Page.fileChooserOpened` input node or an independent probe anchored in the trigger node's document; one verified input is committed with `DOM.setFileInputFiles`, while a non-input picker is rejected immediately. Explicit drop mode performs no click and never falls back to the chooser mechanism: after geometry resolution it temporarily excludes BrowserSkill's own overlay, verifies that the resolved drop zone still owns its local action point, and sends one native `dragEnter` / `dragOver` / `drop` transaction to that node's CDP target before restoring the overlay. OOPIF drops use target-local coordinates rather than top-level click coordinates. Download correlates `chrome.downloads` filename candidates with either exact-target CDP intent or the validated HTTP(S) URL of the resolved `_blank` anchor. The coordinator accepts only one unique match and never cancels an unclaimed candidate. - Browser-side operations report `effect_state` (`none`, `committed`, or `unknown`), `phase`, and `cleanup_state`. Confirmed success wins over a late cancel; an unknown effect is preserved across timeout or transport loss and must not be retried blindly. A transfer deadline sends cancellation to the extension and keeps the session queue occupied for bounded compensation rather than abandoning an in-flight browser effect. - Download staging is released after CLI commit. Upload staging remains until session teardown because the page may read an attached file only on a later form submission. Remaining staging is released on session stop/browser disconnect and on daemon startup after a crash. BrowserSkill does not inspect content or decide whether a transfer is appropriate. From 83cc415d85f43ddeb17ff4ed18f921ff2a6573c9 Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 16 Sep 2026 15:04:31 +0800 Subject: [PATCH 2/2] fix(extension): correlate downloads with click dispatch lifecycle --- .../src/tools/__tests__/file-transfer.test.ts | 155 +++++++++++++++- .../src/tools/__tests__/interaction.test.ts | 44 +++++ apps/extension/src/tools/download-capture.ts | 165 ++++++++++++------ apps/extension/src/tools/download.ts | 8 +- apps/extension/src/tools/interaction.ts | 13 ++ 5 files changed, 326 insertions(+), 59 deletions(-) diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 20909bcb..b0d81faf 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -686,7 +686,9 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_1", timeoutMs: 5, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; cdpEvent?.({ tabId: 4, sessionId: "other-child" }, "Page.downloadWillBegin", { url: unrelated.url, suggestedFilename: unrelated.filename, @@ -694,6 +696,7 @@ describe("file transfer tools", () => { onDeterminingFilename.emit(unrelated, (suggestion) => { defaultSuggestionCalled = suggestion === undefined; }); + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -753,11 +756,14 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_2", timeoutMs: 200, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; await new Promise((resolve) => { onDeterminingFilename.emit(initial, () => resolve()); }); onCreated.emit(complete); + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -767,6 +773,131 @@ describe("file transfer tools", () => { }); }); + it("does not expose URL attribution before mouse press dispatch", async () => { + const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const candidate = (id: number) => + ({ + id, + url: "https://example.test/popup.bin", + finalUrl: "https://example.test/popup.bin", + filename: "popup.bin", + state: "in_progress", + fileSize: -1, + totalBytes: 4, + }) as chrome.downloads.DownloadItem; + const completed = { + ...candidate(42), + filename: "/profile/Downloads/BrowserSkill/tr_42/popup.bin", + state: "complete", + fileSize: 4, + } as chrome.downloads.DownloadItem; + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => [completed]), + cancel: vi.fn(async () => {}), + removeFile: vi.fn(async () => {}), + }; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as CdpRunner["send"], + onEvent: () => ({ dispose: vi.fn() }), + }; + let prePressSuggestionCalled = false; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4 }, + expectedUrl: completed.url, + downloads, + browserRelativeDir: "BrowserSkill/tr_42", + timeoutMs: 1_000, + trigger: async (observer) => { + onDeterminingFilename.emit(candidate(41), () => { + prePressSuggestionCalled = true; + }); + const armError = observer.beforePressDispatch(); + if (armError) return armError; + onDeterminingFilename.emit(candidate(42), () => {}); + onCreated.emit(completed); + observer.afterPressDispatch(); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(prePressSuggestionCalled).toBe(false); + expect(result).toMatchObject({ item: { id: 42, state: "complete" } }); + expect(downloads.cancel).not.toHaveBeenCalled(); + }); + + it("finishes a URL-attributed download when mouse release fails", async () => { + const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const initial = { + id: 43, + url: "https://example.test/release-race.bin", + finalUrl: "https://example.test/release-race.bin", + filename: "release-race.bin", + state: "in_progress", + fileSize: -1, + totalBytes: 4, + } as chrome.downloads.DownloadItem; + const completed = { + ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_43/release-race.bin", + state: "complete", + fileSize: 4, + } as chrome.downloads.DownloadItem; + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => [completed]), + cancel: vi.fn(async () => {}), + removeFile: vi.fn(async () => {}), + }; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as CdpRunner["send"], + onEvent: () => ({ dispose: vi.fn() }), + }; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4 }, + expectedUrl: initial.url, + downloads, + browserRelativeDir: "BrowserSkill/tr_43", + timeoutMs: 1_000, + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; + onDeterminingFilename.emit(initial, () => {}); + onCreated.emit(completed); + observer.afterPressDispatch(); + return { code: "cdp_failed", message: "mouseReleased failed" }; + }, + }); + + expect(result).toMatchObject({ item: { id: 43, state: "complete" } }); + expect(downloads.cancel).not.toHaveBeenCalled(); + expect(downloads.removeFile).not.toHaveBeenCalled(); + }); + it("correlates a filename candidate that arrives before the CDP intent", async () => { const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); @@ -816,7 +947,9 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_21", timeoutMs: 1_000, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; const suggested = new Promise((resolve) => { onDeterminingFilename.emit(initial, (value) => { suggestion = value; @@ -829,6 +962,7 @@ describe("file transfer tools", () => { }); await suggested; onCreated.emit(complete); + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -892,7 +1026,9 @@ describe("file transfer tools", () => { browserRelativeDir: "BrowserSkill/tr_22", maxByteSize: 4, timeoutMs: 1_000, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: initial.url, suggestedFilename: initial.filename, @@ -902,6 +1038,7 @@ describe("file transfer tools", () => { }); onCreated.emit(initial); onChanged.emit({ id: initial.id, state: { current: "complete" } }); + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -967,7 +1104,9 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_23", timeoutMs: 80, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: initial.url, suggestedFilename: initial.filename, @@ -976,6 +1115,7 @@ describe("file transfer tools", () => { onDeterminingFilename.emit(initial, () => resolve()); }); onCreated.emit(initial); + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -1031,7 +1171,9 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_ambiguous", timeoutMs: 100, - trigger: async () => { + trigger: async (observer) => { + const armError = observer.beforePressDispatch(); + if (armError) return armError; cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: "https://example.test/same.bin", suggestedFilename: "same.bin", @@ -1041,6 +1183,7 @@ describe("file transfer tools", () => { if (value === undefined) defaults.push(id); }); } + observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 4ad2fd53..66cb9499 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -3,6 +3,7 @@ import { SessionManager } from "@/session-manager/manager"; import type { CdpRunner } from "@/tools/shared"; import { withInputReady } from "../input-readiness"; import { + clickResolvedTarget, handleBlur, handleClick, handleFill, @@ -181,6 +182,49 @@ describe("handleClick", () => { }); }); + it("notifies an optional observer immediately around mouse press dispatch", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + const lifecycle: string[] = []; + const fake = makeFakeCdp({ + "DOM.scrollIntoViewIfNeeded": () => ({}), + "DOM.getContentQuads": () => ({ quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }), + "Input.dispatchMouseEvent": (params) => { + lifecycle.push((params as { type: string }).type); + return {}; + }, + }); + + const result = await clickResolvedTarget( + ctx, + { + tab: { tabId: 4, windowId: 100, active: true }, + backendNodeId: 1234, + cdpTarget: { tabId: 4 }, + usedRef: "e3", + }, + {}, + { cdp: fake.cdp, tabsApi: fake.tabsApi }, + undefined, + { + beforePressDispatch: () => { + lifecycle.push("beforePressDispatch"); + return null; + }, + afterPressDispatch: () => lifecycle.push("afterPressDispatch"), + }, + ); + + expect(result).toMatchObject({ tab_id: 4, used_ref: "e3" }); + expect(lifecycle).toEqual([ + "mouseMoved", + "beforePressDispatch", + "mousePressed", + "afterPressDispatch", + "mouseReleased", + ]); + }); + 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/download-capture.ts b/apps/extension/src/tools/download-capture.ts index 3477818c..9e2377be 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -55,14 +55,18 @@ export interface DownloadCaptureOptions { timeoutMs: number; signal?: AbortSignal; expectedUrl?: string; - trigger(): Promise; + trigger(observer: DownloadTriggerObserver): Promise; } export interface DownloadCaptureResult { - click: ClickResult; item: chrome.downloads.DownloadItem; } +export interface DownloadTriggerObserver { + beforePressDispatch(): RpcError | null; + afterPressDispatch(): void; +} + interface DownloadIntent { url: string; cdpUrl?: string; @@ -77,6 +81,8 @@ interface DownloadCandidate { graceTimer: ReturnType; } +type DownloadPhase = "idle" | "dispatching" | "active" | "uncertain"; + function safeBasename(filename: string): string { const basename = filename.split(/[\\/]/).pop()?.trim(); return basename && basename !== "." && basename !== ".." ? basename : "download"; @@ -143,17 +149,18 @@ async function cleanupClaimedDownload(downloads: DownloadsApi, downloadId: numbe export async function captureBrowserDownload( options: DownloadCaptureOptions, ): Promise { - let click: ClickResult | undefined; - let intent: DownloadIntent | undefined = options.expectedUrl - ? { url: options.expectedUrl } - : undefined; + let phase: DownloadPhase = "idle"; + let intent: DownloadIntent | undefined; let cdpIntentSeen = false; let capturedId: number | undefined; let settled = false; let succeeded = false; + let armed = false; let failureResult: RpcError | undefined; + let cdpSubscription: { dispose(): void } | undefined; let uniquenessTimer: ReturnType | undefined; let operationTimer: ReturnType | undefined; + let attributionTimer: ReturnType | undefined; let sizePoll: ReturnType | undefined; const candidates = new Map(); const createdItems = new Map(); @@ -186,7 +193,7 @@ export async function captureBrowserDownload( }; const matchingCandidates = (): DownloadCandidate[] => { const currentIntent = intent; - return currentIntent + return currentIntent && phase !== "idle" ? [...candidates.values()].filter( (candidate) => !candidate.suggested && matchesIntent(candidate.item, currentIntent), ) @@ -195,7 +202,7 @@ export async function captureBrowserDownload( const claimUnique = () => { uniquenessTimer = undefined; - if (settled || capturedId !== undefined || !intent) return; + if (settled || capturedId !== undefined || !intent || phase === "idle") return; const matches = matchingCandidates(); if (matches.length !== 1) { if (matches.length > 1) { @@ -227,7 +234,7 @@ export async function captureBrowserDownload( } }; const reconcile = () => { - if (settled || capturedId !== undefined || !intent) return; + if (settled || capturedId !== undefined || !intent || phase === "idle") return; const matches = matchingCandidates(); if (matches.length > 1) { for (const candidate of matches) suggestDefault(candidate); @@ -247,7 +254,7 @@ export async function captureBrowserDownload( graceTimer: setTimeout(() => { suggestDefault(candidate); candidates.delete(item.id); - if (intent && matchesIntent(item, intent) && capturedId === undefined) { + if (intent && phase !== "idle" && matchesIntent(item, intent) && capturedId === undefined) { fail(new Error("download correlation grace elapsed before unique attribution")); } }, CORRELATION_GRACE_MS), @@ -282,7 +289,7 @@ export async function captureBrowserDownload( } }; const onAbort = () => fail(new DOMException("aborted", "AbortError")); - const cdpSubscription = options.cdp.onEvent?.((source, method, raw) => { + const cdpListener: Parameters>[0] = (source, method, raw) => { if (method !== "Page.downloadWillBegin" || !sameTarget(source, options.target)) return; const event = raw as { url?: unknown; suggestedFilename?: unknown; frameId?: unknown }; if (typeof event.url !== "string" || typeof event.suggestedFilename !== "string") return; @@ -296,57 +303,114 @@ export async function captureBrowserDownload( } cdpIntentSeen = true; intent = { - url: intent?.url ?? event.url, - ...(intent ? { cdpUrl: event.url } : {}), + url: options.expectedUrl ?? event.url, + ...(options.expectedUrl ? { cdpUrl: event.url } : {}), suggestedFilename: event.suggestedFilename, ...(typeof event.frameId === "string" ? { frameId: event.frameId } : {}), }; reconcile(); - }); - if (!cdpSubscription) { - return captureError("CDP download intent subscription unavailable", "none", "arm"); - } - - options.downloads.onDeterminingFilename.addListener(determiningListener); - options.downloads.onCreated.addListener(createdListener); - options.downloads.onChanged.addListener(changedListener); - options.signal?.addEventListener("abort", onAbort, { once: true }); - operationTimer = setTimeout( - () => fail(new Error("download did not complete before timeout")), - options.timeoutMs, - ); - sizePoll = setInterval(() => { - if (capturedId === undefined || settled || options.maxByteSize === undefined) return; - void options.downloads - .search({ id: capturedId }) - .then(([item]) => { - if (!item || settled) return; - if (item.bytesReceived > (options.maxByteSize as number)) { - fail(new Error(`download exceeds transfer limit ${options.maxByteSize}`)); - } - }) - .catch((err) => fail(err instanceof Error ? err : new Error(String(err)))); - }, SIZE_POLL_MS); + }; + const arm = (): RpcError | null => { + if (armed) return null; + if (options.signal?.aborted) return { code: "cancelled", message: "download aborted" }; + phase = "dispatching"; + cdpSubscription = options.cdp.onEvent?.(cdpListener); + if (!cdpSubscription) { + phase = "idle"; + return captureError("CDP download intent subscription unavailable", "none", "arm"); + } + options.downloads.onDeterminingFilename.addListener(determiningListener); + options.downloads.onCreated.addListener(createdListener); + options.downloads.onChanged.addListener(changedListener); + options.signal?.addEventListener("abort", onAbort, { once: true }); + operationTimer = setTimeout( + () => fail(new Error("download did not complete before timeout")), + options.timeoutMs, + ); + sizePoll = setInterval(() => { + if (capturedId === undefined || settled || options.maxByteSize === undefined) return; + void options.downloads + .search({ id: capturedId }) + .then(([item]) => { + if (!item || settled) return; + if (item.bytesReceived > (options.maxByteSize as number)) { + fail(new Error(`download exceeds transfer limit ${options.maxByteSize}`)); + } + }) + .catch((err) => fail(err instanceof Error ? err : new Error(String(err)))); + }, SIZE_POLL_MS); + armed = true; + if (options.expectedUrl) intent = { url: options.expectedUrl }; + return null; + }; + const activate = (nextPhase: "active" | "uncertain") => { + if (!armed) return; + phase = nextPhase; + if (!intent && options.expectedUrl) intent = { url: options.expectedUrl }; + reconcile(); + }; + const currentPhase = (): DownloadPhase => phase; + const observer: DownloadTriggerObserver = { + beforePressDispatch: arm, + afterPressDispatch: () => activate("active"), + }; try { - const triggered = await options.trigger(); + const triggered = await options.trigger(observer); if (isRpcError(triggered)) { + const triggerPhase = currentPhase(); + if (triggerPhase === "dispatching" || triggerPhase === "active") activate("uncertain"); + if (triggerPhase !== "idle") { + const attributed = await Promise.race([ + completion.then( + () => true, + () => true, + ), + new Promise((resolve) => { + attributionTimer = setTimeout( + () => resolve(false), + CORRELATION_GRACE_MS + UNIQUE_SETTLE_MS, + ); + }), + ]); + if (attributed || capturedId !== undefined) { + const item = await completion; + succeeded = true; + return { item }; + } + } void completion.catch(() => undefined); + const triggerEffect = triggered.data?.effect_state; const effect: TransferEffectState = - capturedId !== undefined ? "committed" : cdpIntentSeen ? "unknown" : "none"; + triggerEffect === "committed" + ? "committed" + : phase !== "idle" || triggerEffect === "unknown" + ? "unknown" + : "none"; failureResult = { ...triggered, - data: { ...triggered.data, effect_state: effect, phase: "trigger" }, + data: { + ...triggered.data, + effect_state: effect, + phase: triggered.data?.phase ?? "trigger", + }, }; return failureResult; } - click = triggered; + if (!armed) { + failureResult = captureError( + "download trigger completed without dispatch lifecycle", + "none", + "trigger", + ); + return failureResult; + } const item = await completion; succeeded = true; - return { click, item }; + return { item }; } catch (err) { const effect: TransferEffectState = - capturedId !== undefined ? "committed" : click ? "unknown" : "none"; + capturedId !== undefined ? "committed" : phase === "idle" ? "none" : "unknown"; failureResult = captureError( err instanceof Error ? err.message : String(err), effect, @@ -356,13 +420,16 @@ export async function captureBrowserDownload( } finally { settled = true; if (operationTimer) clearTimeout(operationTimer); + if (attributionTimer) clearTimeout(attributionTimer); if (uniquenessTimer) clearTimeout(uniquenessTimer); if (sizePoll) clearInterval(sizePoll); - options.signal?.removeEventListener("abort", onAbort); - options.downloads.onDeterminingFilename.removeListener(determiningListener); - options.downloads.onCreated.removeListener(createdListener); - options.downloads.onChanged.removeListener(changedListener); - cdpSubscription.dispose(); + if (armed) { + options.signal?.removeEventListener("abort", onAbort); + options.downloads.onDeterminingFilename.removeListener(determiningListener); + options.downloads.onCreated.removeListener(createdListener); + options.downloads.onChanged.removeListener(changedListener); + } + cdpSubscription?.dispose(); for (const candidate of candidates.values()) { clearTimeout(candidate.graceTimer); if (candidate.item.id !== capturedId) suggestDefault(candidate); diff --git a/apps/extension/src/tools/download.ts b/apps/extension/src/tools/download.ts index d4c0d6e1..60e15cda 100644 --- a/apps/extension/src/tools/download.ts +++ b/apps/extension/src/tools/download.ts @@ -47,14 +47,14 @@ export async function handleDownload( signal: deps.signal, expectedFrameId: address.frameId, expectedUrl, - trigger: () => clickResolvedTarget(ctx, address, {}, deps), + trigger: (observer) => clickResolvedTarget(ctx, address, {}, deps, undefined, observer), }); if (isRpcError(capture)) return capture; - const { click, item } = capture; + const { item } = capture; return { tab_id: target.tabId, - used_ref: click.used_ref, - used_selector: click.used_selector, + used_ref: address.usedRef, + used_selector: address.usedSelector, suggested_filename: item.filename.split(/[\\/]/).pop() ?? "download", byte_size: item.fileSize >= 0 ? item.fileSize : item.totalBytes, mime: item.mime || undefined, diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 86859a0b..a95e5659 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -73,6 +73,11 @@ export interface ResolvedActionTarget { usedSelector?: string; } +export interface ClickDispatchObserver { + beforePressDispatch(): RpcError | null; + afterPressDispatch(): void; +} + const DEFAULT_TIMEOUT_MS = 30_000; const DEFAULT_HOVER_SETTLE_MS = 200; @@ -485,6 +490,7 @@ export async function clickResolvedTarget( params: Pick, deps: InteractionDeps, markSent?: () => void, + observer?: ClickDispatchObserver, ): Promise { const { tab: target } = resolved; const dialogCursor = markDialogCursor(deps.cdp, target.tabId); @@ -534,6 +540,7 @@ export async function clickResolvedTarget( deps, undefined, markSent, + observer, ); if (error) return error; } finally { @@ -563,6 +570,7 @@ async function dispatchClickAtPoint( deps: InteractionDeps, beforePress?: () => Promise, markSent?: () => void, + observer?: ClickDispatchObserver, ): Promise { const button = params.button ?? "left", modifiers = modifiersBitfield(params.modifiers); @@ -611,6 +619,10 @@ async function dispatchClickAtPoint( } if (deps.signal?.aborted) return failure({ code: "cancelled", message: "click aborted" }); markSent?.(); + if (observer) { + const error = observer.beforePressDispatch(); + if (error) return failure(error); + } attempted = true; releaseNeeded = true; await deps.cdp.send(tabId, "Input.dispatchMouseEvent", { @@ -620,6 +632,7 @@ async function dispatchClickAtPoint( clickCount: count, modifiers, }); + observer?.afterPressDispatch(); await release(); releaseNeeded = false; }