diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 1e30091..f3f365c 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -493,6 +493,155 @@ describe("file transfer tools", () => { expect(result).toMatchObject({ item: { id: 21, state: "complete" } }); }); + it("removes a completed download when the final size exceeds the limit", 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: 22, + url: "https://example.test/oversized.bin", + finalUrl: "https://example.test/oversized.bin", + filename: "oversized.bin", + state: "in_progress", + fileSize: -1, + totalBytes: -1, + bytesReceived: 0, + } as chrome.downloads.DownloadItem; + const complete = { + ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_22/oversized.bin", + state: "complete", + fileSize: 8, + totalBytes: 8, + bytesReceived: 8, + } as chrome.downloads.DownloadItem; + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => [complete]), + cancel: vi.fn(async () => {}), + removeFile: vi.fn(async () => {}), + }; + let cdpEvent: Parameters>[0] | undefined; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as CdpRunner["send"], + onEvent: (handler) => { + cdpEvent = handler; + return { dispose: vi.fn() }; + }, + }; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4 }, + downloads, + browserRelativeDir: "BrowserSkill/tr_22", + maxByteSize: 4, + timeoutMs: 1_000, + trigger: async () => { + cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { + url: initial.url, + suggestedFilename: initial.filename, + }); + await new Promise((resolve) => { + onDeterminingFilename.emit(initial, () => resolve()); + }); + onCreated.emit(initial); + onChanged.emit({ id: initial.id, state: { current: "complete" } }); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_capture_failed", effect_state: "committed" }, + }); + expect(downloads.removeFile).toHaveBeenCalledWith(initial.id); + expect(downloads.cancel).not.toHaveBeenCalled(); + }); + + it("reconciles a download that completes while cancellation is being requested", 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: 23, + url: "https://example.test/racing.bin", + finalUrl: "https://example.test/racing.bin", + filename: "racing.bin", + state: "in_progress", + fileSize: -1, + totalBytes: -1, + bytesReceived: 0, + } as chrome.downloads.DownloadItem; + const complete = { + ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_23/racing.bin", + state: "complete", + fileSize: 4, + totalBytes: 4, + bytesReceived: 4, + } as chrome.downloads.DownloadItem; + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn().mockResolvedValueOnce([initial]).mockResolvedValueOnce([complete]), + cancel: vi.fn(async () => { + throw new Error("download already complete"); + }), + removeFile: vi.fn(async () => {}), + }; + let cdpEvent: Parameters>[0] | undefined; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as CdpRunner["send"], + onEvent: (handler) => { + cdpEvent = handler; + return { dispose: vi.fn() }; + }, + }; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4 }, + downloads, + browserRelativeDir: "BrowserSkill/tr_23", + timeoutMs: 80, + trigger: async () => { + cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { + url: initial.url, + suggestedFilename: initial.filename, + }); + await new Promise((resolve) => { + onDeterminingFilename.emit(initial, () => resolve()); + }); + onCreated.emit(initial); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_capture_failed", effect_state: "committed" }, + }); + expect(downloads.cancel).toHaveBeenCalledWith(initial.id); + expect(downloads.removeFile).toHaveBeenCalledWith(initial.id); + expect(result).not.toMatchObject({ data: { cleanup_state: "failed" } }); + }); + it("rejects ambiguous attribution without cancelling either unclaimed download", 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 9a6ca9d..6e75bff 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -108,13 +108,37 @@ function captureError( }); } +async function cleanupClaimedDownload(downloads: DownloadsApi, downloadId: number): Promise { + const lookup = async () => (await downloads.search({ id: downloadId }))[0]; + const item = await lookup(); + if (!item || item.state === "interrupted") return; + if (item.state === "complete") { + await downloads.removeFile(downloadId); + return; + } + + try { + await downloads.cancel(downloadId); + } catch (cancelError) { + // Completion can win the race after the lookup but before cancellation. + // Reconcile against Chrome's authoritative state before declaring cleanup + // failed so every terminal state has one explicit cleanup path. + const reconciled = await lookup(); + if (!reconciled || reconciled.state === "interrupted") return; + if (reconciled.state === "complete") { + await downloads.removeFile(downloadId); + return; + } + throw cancelError; + } +} + export async function captureBrowserDownload( options: DownloadCaptureOptions, ): Promise { let click: ClickResult | undefined; let intent: DownloadIntent | undefined; let capturedId: number | undefined; - let capturedItem: chrome.downloads.DownloadItem | undefined; let settled = false; let succeeded = false; let failureResult: RpcError | undefined; @@ -143,7 +167,6 @@ export async function captureBrowserDownload( return; } settled = true; - capturedItem = item; resolveCompletion(item); }; const suggestDefault = (candidate: DownloadCandidate) => { @@ -175,7 +198,6 @@ export async function captureBrowserDownload( candidate.suggested = true; clearTimeout(candidate.graceTimer); capturedId = candidate.item.id; - capturedItem = candidate.item; candidate.suggest({ filename: `${options.browserRelativeDir}/${safeBasename(intent.suggestedFilename)}`, conflictAction: "overwrite", @@ -225,7 +247,6 @@ export async function captureBrowserDownload( const createdListener = (item: chrome.downloads.DownloadItem) => { createdItems.set(item.id, item); if (capturedId !== item.id) return; - capturedItem = item; if (item.state === "interrupted") { fail(new Error(item.error ?? "download interrupted")); } else if (item.state === "complete") { @@ -286,7 +307,6 @@ export async function captureBrowserDownload( .search({ id: capturedId }) .then(([item]) => { if (!item || settled) return; - capturedItem = item; if (item.bytesReceived > (options.maxByteSize as number)) { fail(new Error(`download exceeds transfer limit ${options.maxByteSize}`)); } @@ -334,13 +354,8 @@ export async function captureBrowserDownload( if (candidate.item.id !== capturedId) suggestDefault(candidate); } if (!succeeded && capturedId !== undefined) { - const item = capturedItem ?? createdItems.get(capturedId); try { - if (item?.state === "complete") { - await options.downloads.removeFile(capturedId); - } else { - await options.downloads.cancel(capturedId); - } + await cleanupClaimedDownload(options.downloads, capturedId); } catch { if (failureResult?.data) failureResult.data.cleanup_state = "failed"; }