From fd5aeee84af997f24b76e067dcfa6e3def8e0074 Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 16 Sep 2026 14:13:38 +0800 Subject: [PATCH 1/3] 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 0000000..7e61b09 --- /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 740630e..20909bc 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 6e75bff..3477818 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 0000000..fd9a269 --- /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 047e263..d4c0d6e 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 ac1a932..d824e08 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/3] 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 20909bc..b0d81fa 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 4ad2fd5..66cb949 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 3477818..9e2377b 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 d4c0d6e..60e15cd 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 86859a0..a95e565 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; } From 03f8561e0368e969478771053e85e021b3eefa15 Mon Sep 17 00:00:00 2001 From: drakezhang Date: Wed, 23 Sep 2026 21:06:22 +0800 Subject: [PATCH 3/3] fix(extension): attribute new-tab downloads through the clicked tab A target="_blank" attachment link downloads from a transient new tab, so the clicked target never receives Page.downloadWillBegin and bsk download timed out. Instead of matching the anchor href against every browser download, accept the URL of a webNavigation navigation target whose source is the clicked tab and that appears after the mouse press, and match it only against download candidates observed after the press. The dispatch point reuses clickResolvedTarget's existing markSent hook, so the click pipeline and the exact-target CDP path keep their behavior. --- .../__tests__/download-trigger-intent.test.ts | 39 -- .../src/tools/__tests__/file-transfer.test.ts | 482 ++++++++++-------- .../src/tools/__tests__/interaction.test.ts | 44 -- apps/extension/src/tools/download-capture.ts | 235 ++++----- .../src/tools/download-trigger-intent.ts | 48 -- apps/extension/src/tools/download.ts | 23 +- apps/extension/src/tools/interaction.ts | 13 - docs/architecture.md | 2 +- 8 files changed, 384 insertions(+), 502 deletions(-) delete mode 100644 apps/extension/src/tools/__tests__/download-trigger-intent.test.ts delete 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 deleted file mode 100644 index 7e61b09..0000000 --- a/apps/extension/src/tools/__tests__/download-trigger-intent.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -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 b0d81fa..9c79747 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -38,6 +38,76 @@ function fakeEvent unknown>() { }; } +function popupDownloadFakes() { + const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const onCreatedNavigationTarget = + fakeEvent<(details: chrome.webNavigation.WebNavigationSourceCallbackDetails) => void>(); + const completed = new Map(); + const downloads: DownloadsApi = { + onCreated, + onChanged: fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(), + onDeterminingFilename, + search: vi.fn(async ({ id }: chrome.downloads.DownloadQuery) => { + const item = id === undefined ? undefined : completed.get(id); + return item ? [item] : []; + }), + cancel: vi.fn(async () => {}), + removeFile: vi.fn(async () => {}), + }; + return { + downloads, + navigationTargets: { onCreatedNavigationTarget }, + item: (id: number, url: string, finalUrl = url) => + ({ + id, + url, + finalUrl, + filename: `report-${id}.csv`, + state: "in_progress", + fileSize: -1, + totalBytes: 4, + }) as chrome.downloads.DownloadItem, + /** Resolves with the suggestion Chrome would receive for this candidate. */ + offer: (item: chrome.downloads.DownloadItem) => + new Promise((resolve) => { + onDeterminingFilename.emit(item, resolve); + }), + finish: (item: chrome.downloads.DownloadItem) => { + const done = { + ...item, + filename: `/profile/Downloads/${item.filename}`, + state: "complete", + fileSize: 4, + } as chrome.downloads.DownloadItem; + completed.set(item.id, done); + onCreated.emit(done); + }, + popup: (sourceTabId: number, url: string) => + onCreatedNavigationTarget.emit({ + sourceTabId, + sourceFrameId: 0, + sourceProcessId: 1, + tabId: 99, + url, + timeStamp: 0, + } as chrome.webNavigation.WebNavigationSourceCallbackDetails), + }; +} + +function silentCdp(): CdpRunner { + return { + send: vi.fn(async () => ({})) as CdpRunner["send"], + onEvent: () => ({ dispose: vi.fn() }), + }; +} + function actionTarget(frameId?: string, sessionId?: string): ResolvedActionTarget { return { tab: { tabId: 4, windowId: 100, active: true }, @@ -630,6 +700,7 @@ describe("file transfer tools", () => { cdp, tabsApi: tabsApi(), downloads, + navigationTargets: popupDownloadFakes().navigationTargets, }, ); @@ -686,9 +757,7 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_1", timeoutMs: 5, - trigger: async (observer) => { - const armError = observer.beforePressDispatch(); - if (armError) return armError; + trigger: async () => { cdpEvent?.({ tabId: 4, sessionId: "other-child" }, "Page.downloadWillBegin", { url: unrelated.url, suggestedFilename: unrelated.filename, @@ -696,7 +765,6 @@ describe("file transfer tools", () => { onDeterminingFilename.emit(unrelated, (suggestion) => { defaultSuggestionCalled = suggestion === undefined; }); - observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -709,195 +777,6 @@ 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 (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 }; - }, - }); - - expect(result).toMatchObject({ - item: { id: 18, filename: complete.filename, state: "complete" }, - }); - }); - - 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>(); @@ -947,9 +826,7 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_21", timeoutMs: 1_000, - trigger: async (observer) => { - const armError = observer.beforePressDispatch(); - if (armError) return armError; + trigger: async () => { const suggested = new Promise((resolve) => { onDeterminingFilename.emit(initial, (value) => { suggestion = value; @@ -962,7 +839,6 @@ describe("file transfer tools", () => { }); await suggested; onCreated.emit(complete); - observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -1026,9 +902,7 @@ describe("file transfer tools", () => { browserRelativeDir: "BrowserSkill/tr_22", maxByteSize: 4, timeoutMs: 1_000, - trigger: async (observer) => { - const armError = observer.beforePressDispatch(); - if (armError) return armError; + trigger: async () => { cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: initial.url, suggestedFilename: initial.filename, @@ -1038,7 +912,6 @@ 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 }; }, }); @@ -1104,9 +977,7 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_23", timeoutMs: 80, - trigger: async (observer) => { - const armError = observer.beforePressDispatch(); - if (armError) return armError; + trigger: async () => { cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: initial.url, suggestedFilename: initial.filename, @@ -1115,7 +986,6 @@ describe("file transfer tools", () => { onDeterminingFilename.emit(initial, () => resolve()); }); onCreated.emit(initial); - observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -1171,9 +1041,7 @@ describe("file transfer tools", () => { downloads, browserRelativeDir: "BrowserSkill/tr_ambiguous", timeoutMs: 100, - trigger: async (observer) => { - const armError = observer.beforePressDispatch(); - if (armError) return armError; + trigger: async () => { cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { url: "https://example.test/same.bin", suggestedFilename: "same.bin", @@ -1183,7 +1051,6 @@ describe("file transfer tools", () => { if (value === undefined) defaults.push(id); }); } - observer.afterPressDispatch(); return { tab_id: 4, x: 10, y: 10 }; }, }); @@ -1195,4 +1062,201 @@ describe("file transfer tools", () => { data: { effect_state: "unknown", phase: "attribution" }, }); }); + + it("routes an attachment that the clicked link opens in a new tab", async () => { + const manager = sessions(); + const ctx = await manager.start("s1"); + ctx.refStore.set("e3", 123, { tabId: 4 }); + const fakes = popupDownloadFakes(); + const download = fakes.item(50, "https://example.test/export?id=50"); + let suggested: Promise | undefined; + const send = vi.fn(async (_tabId: number, method: string, params?: object) => { + if (method === "Page.getLayoutMetrics") + return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; + if (method === "DOM.getContentQuads") return { quads: [[0, 0, 20, 0, 20, 20, 0, 20]] }; + if ( + method === "Input.dispatchMouseEvent" && + (params as { type?: string }).type === "mousePressed" + ) { + fakes.popup(4, download.url); + suggested = fakes.offer(download); + fakes.finish(download); + } + return {}; + }); + + const result = await handleDownload( + manager, + { + session_id: "s1", + ref: "@e3", + browser_relative_dir: "BrowserSkill/tr_50", + timeout_ms: 1_000, + }, + { + cdp: { ...silentCdp(), send: send as unknown as CdpRunner["send"] }, + tabsApi: tabsApi(), + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + }, + ); + + await expect(suggested).resolves.toEqual({ + filename: "BrowserSkill/tr_50/report-50.csv", + conflictAction: "overwrite", + }); + expect(result).toMatchObject({ + tab_id: 4, + suggested_filename: "report-50.csv", + browser_path: "/profile/Downloads/report-50.csv", + }); + }); + + it.each([ + "navigation-first", + "candidate-first", + ])("correlates a new-tab download in either arrival order (%s)", async (order) => { + const fakes = popupDownloadFakes(); + const download = fakes.item( + 51, + "https://example.test/export?id=51", + "https://cdn.example.test/report.csv", + ); + let suggested: Promise | undefined; + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_51", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + markDispatched(); + if (order === "navigation-first") fakes.popup(4, download.url); + suggested = fakes.offer(download); + if (order === "candidate-first") fakes.popup(4, download.url); + fakes.finish(download); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + await expect(suggested).resolves.toEqual({ + filename: "BrowserSkill/tr_51/report-51.csv", + conflictAction: "overwrite", + }); + expect(result).toMatchObject({ item: { id: 51, state: "complete" } }); + }); + + it("ignores new tabs opened before mouse press or by another tab", async () => { + const fakes = popupDownloadFakes(); + const url = "https://example.test/export?id=52"; + let suggested: Promise | undefined; + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_52", + timeoutMs: 100, + trigger: async (markDispatched) => { + fakes.popup(4, url); + markDispatched(); + fakes.popup(5, url); + suggested = fakes.offer(fakes.item(52, url)); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + await expect(suggested).resolves.toBeUndefined(); + expect(fakes.downloads.cancel).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_capture_failed", phase: "attribution" }, + }); + }); + + it("does not attribute a same-URL download observed before mouse press", async () => { + const fakes = popupDownloadFakes(); + const url = "https://example.test/export?id=53"; + const own = fakes.item(54, url); + let earlier: Promise | undefined; + let suggested: Promise | undefined; + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_54", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + earlier = fakes.offer(fakes.item(53, url)); + markDispatched(); + fakes.popup(4, url); + suggested = fakes.offer(own); + fakes.finish(own); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ item: { id: 54, state: "complete" } }); + await expect(suggested).resolves.toMatchObject({ + filename: "BrowserSkill/tr_54/report-54.csv", + }); + await expect(earlier).resolves.toBeUndefined(); + expect(fakes.downloads.cancel).not.toHaveBeenCalled(); + }); + + it("rejects ambiguous new-tab attribution without cancelling either download", async () => { + const fakes = popupDownloadFakes(); + const url = "https://example.test/export?id=55"; + const suggestions: Array> = []; + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_55", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + markDispatched(); + fakes.popup(4, url); + suggestions.push(fakes.offer(fakes.item(55, url)), fakes.offer(fakes.item(56, url))); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + await expect(Promise.all(suggestions)).resolves.toEqual([undefined, undefined]); + expect(fakes.downloads.cancel).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + code: "cdp_failed", + data: { effect_state: "unknown", phase: "attribution" }, + }); + }); + + it("reports an unknown effect when the click fails after opening a new tab", async () => { + const fakes = popupDownloadFakes(); + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_57", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + markDispatched(); + fakes.popup(4, "https://example.test/export?id=57"); + return { code: "cdp_failed", message: "mouseReleased failed" }; + }, + }); + + expect(result).toMatchObject({ + code: "cdp_failed", + data: { effect_state: "unknown", phase: "trigger" }, + }); + }); }); diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 66cb949..4ad2fd5 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -3,7 +3,6 @@ import { SessionManager } from "@/session-manager/manager"; import type { CdpRunner } from "@/tools/shared"; import { withInputReady } from "../input-readiness"; import { - clickResolvedTarget, handleBlur, handleClick, handleFill, @@ -182,49 +181,6 @@ 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 9e2377b..f6d32bd 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -1,6 +1,9 @@ // Order-independent coordinator for one browser download. CDP supplies the // exact target/frame intent while chrome.downloads supplies the download id -// and filename routing hook; neither event is assumed to arrive first. +// and filename routing hook; neither event is assumed to arrive first. A click +// that opens a new tab (for example `target="_blank"` to an attachment) emits +// no CDP intent on the clicked target, so a navigation target that the clicked +// tab opens after mouse press supplies the intent URL instead. import type { CdpTarget } from "@/browser-driver/frame-graph"; import type { ClickResult, RpcError, TransferEffectState } from "@/transport/types"; @@ -45,32 +48,40 @@ export const chromeDownloadsApi: DownloadsApi = { removeFile: (id) => chrome.downloads.removeFile(id), }; +export interface NavigationTargetsApi { + onCreatedNavigationTarget: ListenerEvent< + (details: chrome.webNavigation.WebNavigationSourceCallbackDetails) => void + >; +} + +export const chromeNavigationTargetsApi: NavigationTargetsApi = { + get onCreatedNavigationTarget() { + return chrome.webNavigation.onCreatedNavigationTarget; + }, +}; + export interface DownloadCaptureOptions { cdp: CdpRunner; target: CdpTarget; expectedFrameId?: string; downloads: DownloadsApi; + navigationTargets?: NavigationTargetsApi; browserRelativeDir: string; maxByteSize?: number; timeoutMs: number; signal?: AbortSignal; - expectedUrl?: string; - trigger(observer: DownloadTriggerObserver): Promise; + /** `markDispatched` must be called immediately before the mouse press is sent. */ + trigger(markDispatched: () => void): Promise; } export interface DownloadCaptureResult { + click: ClickResult; item: chrome.downloads.DownloadItem; } -export interface DownloadTriggerObserver { - beforePressDispatch(): RpcError | null; - afterPressDispatch(): void; -} - interface DownloadIntent { url: string; - cdpUrl?: string; - suggestedFilename?: string; + suggestedFilename: string; frameId?: string; } @@ -78,11 +89,10 @@ interface DownloadCandidate { item: chrome.downloads.DownloadItem; suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void; suggested: boolean; + afterDispatch: boolean; 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"; @@ -93,13 +103,12 @@ function sameTarget(source: { tabId?: number; sessionId?: string }, target: CdpT } function matchesIntent(item: chrome.downloads.DownloadItem, intent: DownloadIntent): boolean { - 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)) - ); + const urlMatches = item.url === intent.url || item.finalUrl === intent.url; + return urlMatches && safeBasename(item.filename) === safeBasename(intent.suggestedFilename); +} + +function matchesPopupUrl(item: chrome.downloads.DownloadItem, popupUrls: Set): boolean { + return popupUrls.has(item.url) || popupUrls.has(item.finalUrl); } function knownSize(item: chrome.downloads.DownloadItem): number | undefined { @@ -149,18 +158,16 @@ async function cleanupClaimedDownload(downloads: DownloadsApi, downloadId: numbe export async function captureBrowserDownload( options: DownloadCaptureOptions, ): Promise { - let phase: DownloadPhase = "idle"; + let click: ClickResult | undefined; let intent: DownloadIntent | undefined; - let cdpIntentSeen = false; + let dispatched = false; + const popupUrls = new Set(); 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(); @@ -191,18 +198,18 @@ export async function captureBrowserDownload( candidate.suggested = true; candidate.suggest(); }; - const matchingCandidates = (): DownloadCandidate[] => { - const currentIntent = intent; - return currentIntent && phase !== "idle" - ? [...candidates.values()].filter( - (candidate) => !candidate.suggested && matchesIntent(candidate.item, currentIntent), - ) - : []; - }; + const matchesCdpIntent = (candidate: DownloadCandidate): boolean => + intent !== undefined && matchesIntent(candidate.item, intent); + // Pre-dispatch candidates cannot come from the popup opened by this click. + const attributable = (candidate: DownloadCandidate): boolean => + matchesCdpIntent(candidate) || + (candidate.afterDispatch && matchesPopupUrl(candidate.item, popupUrls)); + const matchingCandidates = (): DownloadCandidate[] => + [...candidates.values()].filter((candidate) => !candidate.suggested && attributable(candidate)); const claimUnique = () => { uniquenessTimer = undefined; - if (settled || capturedId !== undefined || !intent || phase === "idle") return; + if (settled || capturedId !== undefined) return; const matches = matchingCandidates(); if (matches.length !== 1) { if (matches.length > 1) { @@ -215,10 +222,10 @@ export async function captureBrowserDownload( candidate.suggested = true; clearTimeout(candidate.graceTimer); capturedId = candidate.item.id; + const filename = + intent && matchesCdpIntent(candidate) ? intent.suggestedFilename : candidate.item.filename; candidate.suggest({ - filename: `${options.browserRelativeDir}/${safeBasename( - intent.suggestedFilename ?? candidate.item.filename, - )}`, + filename: `${options.browserRelativeDir}/${safeBasename(filename)}`, conflictAction: "overwrite", }); const size = knownSize(candidate.item); @@ -234,7 +241,7 @@ export async function captureBrowserDownload( } }; const reconcile = () => { - if (settled || capturedId !== undefined || !intent || phase === "idle") return; + if (settled || capturedId !== undefined) return; const matches = matchingCandidates(); if (matches.length > 1) { for (const candidate of matches) suggestDefault(candidate); @@ -251,10 +258,11 @@ export async function captureBrowserDownload( item, suggest, suggested: false, + afterDispatch: dispatched, graceTimer: setTimeout(() => { suggestDefault(candidate); candidates.delete(item.id); - if (intent && phase !== "idle" && matchesIntent(item, intent) && capturedId === undefined) { + if (attributable(candidate) && capturedId === undefined) { fail(new Error("download correlation grace elapsed before unique attribution")); } }, CORRELATION_GRACE_MS), @@ -289,7 +297,14 @@ export async function captureBrowserDownload( } }; const onAbort = () => fail(new DOMException("aborted", "AbortError")); - const cdpListener: Parameters>[0] = (source, method, raw) => { + const navigationTargetListener = ( + details: chrome.webNavigation.WebNavigationSourceCallbackDetails, + ) => { + if (!dispatched || details.sourceTabId !== options.target.tabId) return; + popupUrls.add(details.url); + reconcile(); + }; + const cdpSubscription = options.cdp.onEvent?.((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; @@ -297,120 +312,64 @@ export async function captureBrowserDownload( fail(new Error("download originated from a different frame")); return; } - if (cdpIntentSeen) { + if (intent) { fail(new Error("download trigger produced more than one browser download intent")); return; } - cdpIntentSeen = true; intent = { - url: options.expectedUrl ?? event.url, - ...(options.expectedUrl ? { cdpUrl: event.url } : {}), + url: event.url, suggestedFilename: event.suggestedFilename, ...(typeof event.frameId === "string" ? { frameId: event.frameId } : {}), }; reconcile(); - }; - 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"), - }; + }); + 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.navigationTargets?.onCreatedNavigationTarget.addListener(navigationTargetListener); + 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); try { - const triggered = await options.trigger(observer); + const triggered = await options.trigger(() => { + dispatched = true; + }); 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 = - triggerEffect === "committed" - ? "committed" - : phase !== "idle" || triggerEffect === "unknown" - ? "unknown" - : "none"; + capturedId !== undefined ? "committed" : intent || popupUrls.size > 0 ? "unknown" : "none"; failureResult = { ...triggered, - data: { - ...triggered.data, - effect_state: effect, - phase: triggered.data?.phase ?? "trigger", - }, + data: { ...triggered.data, effect_state: effect, phase: "trigger" }, }; return failureResult; } - if (!armed) { - failureResult = captureError( - "download trigger completed without dispatch lifecycle", - "none", - "trigger", - ); - return failureResult; - } + click = triggered; const item = await completion; succeeded = true; - return { item }; + return { click, item }; } catch (err) { const effect: TransferEffectState = - capturedId !== undefined ? "committed" : phase === "idle" ? "none" : "unknown"; + capturedId !== undefined ? "committed" : click ? "unknown" : "none"; failureResult = captureError( err instanceof Error ? err.message : String(err), effect, @@ -420,16 +379,14 @@ export async function captureBrowserDownload( } finally { settled = true; if (operationTimer) clearTimeout(operationTimer); - if (attributionTimer) clearTimeout(attributionTimer); if (uniquenessTimer) clearTimeout(uniquenessTimer); if (sizePoll) clearInterval(sizePoll); - if (armed) { - options.signal?.removeEventListener("abort", onAbort); - options.downloads.onDeterminingFilename.removeListener(determiningListener); - options.downloads.onCreated.removeListener(createdListener); - options.downloads.onChanged.removeListener(changedListener); - } - cdpSubscription?.dispose(); + options.signal?.removeEventListener("abort", onAbort); + options.downloads.onDeterminingFilename.removeListener(determiningListener); + options.downloads.onCreated.removeListener(createdListener); + options.downloads.onChanged.removeListener(changedListener); + options.navigationTargets?.onCreatedNavigationTarget.removeListener(navigationTargetListener); + 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-trigger-intent.ts b/apps/extension/src/tools/download-trigger-intent.ts deleted file mode 100644 index fd9a269..0000000 --- a/apps/extension/src/tools/download-trigger-intent.ts +++ /dev/null @@ -1,48 +0,0 @@ -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 60e15cd..546ae8d 100644 --- a/apps/extension/src/tools/download.ts +++ b/apps/extension/src/tools/download.ts @@ -3,17 +3,23 @@ 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 { + captureBrowserDownload, + chromeDownloadsApi, + chromeNavigationTargetsApi, + type DownloadsApi, + type NavigationTargetsApi, +} from "./download-capture"; import { clickResolvedTarget, type InteractionDeps, resolveActionTarget } from "./interaction"; import { enforceAgentWindow, isRpcError, lookupSession, resolveTargetTab } from "./shared"; let downloadActive = false; -export type { DownloadsApi } from "./download-capture"; +export type { DownloadsApi, NavigationTargetsApi } from "./download-capture"; export interface DownloadDeps extends InteractionDeps { downloads?: DownloadsApi; + navigationTargets?: NavigationTargetsApi; } export async function handleDownload( @@ -36,25 +42,24 @@ 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, downloads: deps.downloads ?? chromeDownloadsApi, + navigationTargets: deps.navigationTargets ?? chromeNavigationTargetsApi, browserRelativeDir: params.browser_relative_dir, maxByteSize: params.max_byte_size, timeoutMs: params.timeout_ms ?? 120_000, signal: deps.signal, expectedFrameId: address.frameId, - expectedUrl, - trigger: (observer) => clickResolvedTarget(ctx, address, {}, deps, undefined, observer), + trigger: (markDispatched) => clickResolvedTarget(ctx, address, {}, deps, markDispatched), }); if (isRpcError(capture)) return capture; - const { item } = capture; + const { click, item } = capture; return { tab_id: target.tabId, - used_ref: address.usedRef, - used_selector: address.usedSelector, + used_ref: click.used_ref, + used_selector: click.used_selector, 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 a95e565..86859a0 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -73,11 +73,6 @@ 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; @@ -490,7 +485,6 @@ export async function clickResolvedTarget( params: Pick, deps: InteractionDeps, markSent?: () => void, - observer?: ClickDispatchObserver, ): Promise { const { tab: target } = resolved; const dialogCursor = markDialogCursor(deps.cdp, target.tabId); @@ -540,7 +534,6 @@ export async function clickResolvedTarget( deps, undefined, markSent, - observer, ); if (error) return error; } finally { @@ -570,7 +563,6 @@ async function dispatchClickAtPoint( deps: InteractionDeps, beforePress?: () => Promise, markSent?: () => void, - observer?: ClickDispatchObserver, ): Promise { const button = params.button ?? "left", modifiers = modifiersBitfield(params.modifiers); @@ -619,10 +611,6 @@ 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", { @@ -632,7 +620,6 @@ async function dispatchClickAtPoint( clickCount: count, modifiers, }); - observer?.afterPressDispatch(); await release(); releaseNeeded = false; } diff --git a/docs/architecture.md b/docs/architecture.md index d824e08..41adeec 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 `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. +- 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. When the click opens a new tab instead (for example a `target="_blank"` attachment link), the clicked target receives no CDP intent; the URL of a `webNavigation` navigation target whose source is the clicked tab and that appears after the mouse press is then the intent, and only candidates observed after the press can match it. - 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.