fix(bsk): download bug fix

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<NonNullable<CdpRunner["onEvent"]>>[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<void>((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<NonNullable<CdpRunner["onEvent"]>>[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<void>((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>();

apps/extension/src/tools/download-capture.ts

@@ -108,13 +108,37 @@ function captureError(
   });
 }
 
+async function cleanupClaimedDownload(downloads: DownloadsApi, downloadId: number): Promise<void> {
+  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<DownloadCaptureResult | RpcError> {
   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";
       }