fix(browser): stop cancelled downloads from capturing later files (#124382)

* fix(browser): cancel abandoned download waits

* fix(browser): cancel in-flight download clicks

extensions/browser/src/browser-tool.actions.ts

@@ -395,6 +395,7 @@ export async function executeDownloadAction(params: {
   baseUrl?: string;
   profile?: string;
   proxyRequest: BrowserProxyRequest | null;
+  signal?: AbortSignal;
   onTabActivity?: (targetId: string | undefined) => void;
 }): Promise<AgentToolResult<unknown>> {
   const { action, input, baseUrl, profile, proxyRequest } = params;
@@ -419,12 +420,14 @@ export async function executeDownloadAction(params: {
           targetId,
           timeoutMs,
           profile,
+          signal: params.signal,
         })
       : await browserToolActionDeps.browserWaitForDownload(baseUrl, {
           path: request.path,
           targetId,
           timeoutMs,
           profile,
+          signal: params.signal,
         });
   params.onTabActivity?.(readStringValue((result as { targetId?: unknown }).targetId) ?? targetId);
   return formatBrowserExternalToolResult({ kind: "download", payload: result });

extensions/browser/src/browser-tool.ts

@@ -932,6 +932,7 @@ export function createBrowserTool(opts?: {
             baseUrl,
             profile,
             proxyRequest,
+            signal,
             onTabActivity: sessionTabs.touch,
           });
         case "upload": {

extensions/browser/src/browser/client-actions-core.ts

@@ -64,6 +64,7 @@ async function postDownloadRequest(
   body: Record<string, unknown>,
   profile?: string,
   timeoutMs?: number,
+  signal?: AbortSignal,
 ): Promise<BrowserDownloadActionResult> {
   const q = buildProfileQuery(profile);
   return await fetchBrowserJson<BrowserDownloadActionResult>(withBaseUrl(baseUrl, `${route}${q}`), {
@@ -71,6 +72,7 @@ async function postDownloadRequest(
     headers: { "Content-Type": "application/json" },
     body: JSON.stringify(body),
     timeoutMs: resolveBrowserOperationRequestTimeoutMs(timeoutMs),
+    signal,
   });
 }
 
@@ -160,6 +162,7 @@ export async function browserWaitForDownload(
     targetId?: string;
     timeoutMs?: number;
     profile?: string;
+    signal?: AbortSignal;
   },
 ): Promise<BrowserDownloadActionResult> {
   return await postDownloadRequest(
@@ -172,6 +175,7 @@ export async function browserWaitForDownload(
     },
     opts.profile,
     opts.timeoutMs,
+    opts.signal,
   );
 }
 
@@ -184,6 +188,7 @@ export async function browserDownload(
     targetId?: string;
     timeoutMs?: number;
     profile?: string;
+    signal?: AbortSignal;
   },
 ): Promise<BrowserDownloadActionResult> {
   return await postDownloadRequest(
@@ -197,6 +202,7 @@ export async function browserDownload(
     },
     opts.profile,
     opts.timeoutMs,
+    opts.signal,
   );
 }
 

extensions/browser/src/browser/pw-download-cancel.chromium.test.ts

@@ -0,0 +1,271 @@
+import fs from "node:fs/promises";
+import { createServer, type Server } from "node:http";
+import type { AddressInfo } from "node:net";
+import path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import { useAutoCleanupTempDirTracker } from "../../test-support.js";
+import { playwrightCore } from "./playwright-core.runtime.js";
+import { ensurePageState } from "./pw-session-state.js";
+import { closePlaywrightBrowserConnection, getPageForTargetId } from "./pw-session.js";
+import { downloadViaPlaywright, waitForDownloadViaPlaywright } from "./pw-tools-core.downloads.js";
+import { getFreePort } from "./test-port.js";
+
+const runChromiumProof = process.env.OPENCLAW_BROWSER_DOWNLOAD_E2E === "1";
+const tempDirs = useAutoCleanupTempDirTracker(afterEach);
+
+function listen(server: Server): Promise<number> {
+  return new Promise((resolve, reject) => {
+    server.once("error", reject);
+    server.listen(0, "127.0.0.1", () => {
+      resolve((server.address() as AddressInfo).port);
+    });
+  });
+}
+
+function closeServer(server: Server): Promise<void> {
+  return new Promise((resolve, reject) => {
+    server.close((error) => (error ? reject(error) : resolve()));
+  });
+}
+
+async function readTargetId(page: import("playwright-core").Page): Promise<string> {
+  const session = await page.context().newCDPSession(page);
+  try {
+    const { targetInfo } = await session.send("Target.getTargetInfo");
+    return targetInfo.targetId;
+  } finally {
+    await session.detach();
+  }
+}
+
+describe.runIf(runChromiumProof)("managed Chromium download cancellation", () => {
+  const cleanup: Array<() => Promise<void>> = [];
+
+  afterEach(async () => {
+    for (const dispose of cleanup.splice(0).toReversed()) {
+      await dispose();
+    }
+  });
+
+  it("does not let a cancelled waiter capture and write a later download", async () => {
+    const rootDir = tempDirs.make("openclaw-download-cancel-");
+    cleanup.push(async () => await fs.rm(rootDir, { recursive: true, force: true }));
+
+    const abandonedPayload = Buffer.from("abandoned-click-download\n");
+    const successorPayload = Buffer.from("successor-download\n");
+    const downloadServer = createServer((request, response) => {
+      if (request.url === "/late.txt") {
+        response.writeHead(200, {
+          "content-disposition": 'attachment; filename="duplicate.txt"',
+          "content-length": String(abandonedPayload.byteLength),
+          "content-type": "text/plain",
+        });
+        response.end(abandonedPayload);
+        return;
+      }
+      if (request.url === "/successor.txt") {
+        response.writeHead(200, {
+          "content-disposition": 'attachment; filename="duplicate.txt"',
+          "content-length": String(successorPayload.byteLength),
+          "content-type": "text/plain",
+        });
+        response.end(successorPayload);
+        return;
+      }
+      response.writeHead(200, { "content-type": "text/html; charset=utf-8" });
+      response.end(
+        '<button id="delayed" disabled onclick="location.href=\'/late.txt\'">Delayed Download</button>' +
+          '<a id="download" href="/successor.txt" download>Download</a>',
+      );
+    });
+    const downloadPort = await listen(downloadServer);
+    cleanup.push(async () => await closeServer(downloadServer));
+
+    const cdpPort = await getFreePort();
+    const profileDir = path.join(rootDir, "profile");
+    const context = await playwrightCore.chromium.launchPersistentContext(profileDir, {
+      headless: true,
+      executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH,
+      args: [`--remote-debugging-port=${cdpPort}`],
+    });
+    cleanup.push(async () => await context.close());
+    const page = context.pages()[0] ?? (await context.newPage());
+    await page.goto(`http://127.0.0.1:${downloadPort}/`);
+
+    const cdpUrl = `http://127.0.0.1:${cdpPort}`;
+    const targetId = await readTargetId(page);
+    cleanup.push(async () => await closePlaywrightBrowserConnection({ cdpUrl }));
+    const outputRoot = path.join(rootDir, "downloads");
+    const outputPath = path.join(outputRoot, "cancelled.txt");
+    const controller = new AbortController();
+    const wait = waitForDownloadViaPlaywright({
+      cdpUrl,
+      targetId,
+      path: outputPath,
+      rootDir: outputRoot,
+      timeoutMs: 5_000,
+      signal: controller.signal,
+    });
+    const outcome = wait.then(
+      (download) => ({ kind: "resolved" as const, download }),
+      (error: unknown) => ({
+        kind: "rejected" as const,
+        message: error instanceof Error ? error.message : String(error),
+      }),
+    );
+
+    const controlledPage = await getPageForTargetId({ cdpUrl, targetId });
+    await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1);
+    controller.abort(new Error("request aborted"));
+    const afterAbort = await Promise.race([
+      outcome,
+      new Promise<{ kind: "pending" }>((resolve) => {
+        setTimeout(() => resolve({ kind: "pending" }), 200);
+      }),
+    ]);
+
+    const successorPath = path.join(outputRoot, "successor.txt");
+    const successor = waitForDownloadViaPlaywright({
+      cdpUrl,
+      targetId,
+      path: successorPath,
+      rootDir: outputRoot,
+      timeoutMs: 5_000,
+    });
+    await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1);
+    await page.locator("#download").click();
+    const finalOutcome = await outcome;
+    const written = await fs.readFile(outputPath).catch(() => undefined);
+    const successorResult = await successor;
+
+    expect({
+      afterAbort,
+      finalOutcome,
+      written: written?.toString("utf8"),
+      successor: {
+        bytes: await fs.readFile(successorResult.path, "utf8"),
+        suggestedFilename: successorResult.suggestedFilename,
+      },
+    }).toEqual({
+      afterAbort: { kind: "rejected", message: "request aborted" },
+      finalOutcome: { kind: "rejected", message: "request aborted" },
+      written: undefined,
+      successor: {
+        bytes: successorPayload.toString("utf8"),
+        suggestedFilename: "duplicate.txt",
+      },
+    });
+
+    const pageState = ensurePageState(controlledPage);
+    pageState.roleRefs = { e1: { role: "button", name: "Delayed Download" } };
+    pageState.roleRefsMode = "role";
+    await page.evaluate(() => {
+      setTimeout(() => {
+        const delayed = document.querySelector<HTMLButtonElement>("#delayed");
+        if (delayed) {
+          delayed.disabled = false;
+        }
+      }, 500);
+    });
+    const sharedPath = path.join(outputRoot, "shared.txt");
+    const clickController = new AbortController();
+    const cancelledClick = downloadViaPlaywright({
+      cdpUrl,
+      targetId,
+      ref: "e1",
+      path: sharedPath,
+      rootDir: outputRoot,
+      timeoutMs: 5_000,
+      signal: clickController.signal,
+    });
+    const clickOutcome = cancelledClick.then(
+      () => ({ kind: "resolved" as const }),
+      (error: unknown) => ({
+        kind: "rejected" as const,
+        message: error instanceof Error ? error.message : String(error),
+      }),
+    );
+    await expect.poll(() => pageState.downloadWaiterDepth).toBe(1);
+    clickController.abort(new Error("click request aborted"));
+    const clickAfterAbort = await Promise.race([
+      clickOutcome,
+      new Promise<{ kind: "pending" }>((resolve) => {
+        setTimeout(() => resolve({ kind: "pending" }), 200);
+      }),
+    ]);
+    expect(clickAfterAbort).toEqual({ kind: "rejected", message: "click request aborted" });
+    expect(pageState.downloadWaiterDepth).toBe(0);
+
+    const clickSuccessor = waitForDownloadViaPlaywright({
+      cdpUrl,
+      targetId,
+      path: sharedPath,
+      rootDir: outputRoot,
+      timeoutMs: 5_000,
+    });
+    await expect.poll(() => pageState.downloadWaiterDepth).toBe(1);
+    const beforeSuccessorClick = await Promise.race([
+      clickSuccessor.then(() => "resolved" as const),
+      new Promise<"pending">((resolve) => {
+        setTimeout(() => resolve("pending"), 700);
+      }),
+    ]);
+    expect(beforeSuccessorClick).toBe("pending");
+    await expect(fs.access(sharedPath)).rejects.toMatchObject({ code: "ENOENT" });
+    await page.locator("#download").click();
+    const clickSuccessorResult = await clickSuccessor;
+    await expect(fs.readFile(clickSuccessorResult.path, "utf8")).resolves.toBe(
+      successorPayload.toString("utf8"),
+    );
+
+    await closePlaywrightBrowserConnection({ cdpUrl });
+    await context.close();
+
+    const restartedCdpPort = await getFreePort();
+    const restartedContext = await playwrightCore.chromium.launchPersistentContext(profileDir, {
+      headless: true,
+      executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH,
+      args: [`--remote-debugging-port=${restartedCdpPort}`],
+    });
+    cleanup.push(async () => await restartedContext.close());
+    const restartedPage = restartedContext.pages()[0] ?? (await restartedContext.newPage());
+    await restartedPage.goto(`http://127.0.0.1:${downloadPort}/`);
+    const restartedCdpUrl = `http://127.0.0.1:${restartedCdpPort}`;
+    const restartedTargetId = await readTargetId(restartedPage);
+    cleanup.push(async () => await closePlaywrightBrowserConnection({ cdpUrl: restartedCdpUrl }));
+
+    const downloadAfterRestart = async () => {
+      const pending = waitForDownloadViaPlaywright({
+        cdpUrl: restartedCdpUrl,
+        targetId: restartedTargetId,
+        rootDir: outputRoot,
+        timeoutMs: 5_000,
+      });
+      const connected = await getPageForTargetId({
+        cdpUrl: restartedCdpUrl,
+        targetId: restartedTargetId,
+      });
+      await expect.poll(() => ensurePageState(connected).downloadWaiterDepth).toBe(1);
+      await restartedPage.locator("#download").click();
+      return await pending;
+    };
+    const firstDuplicate = await downloadAfterRestart();
+    const secondDuplicate = await downloadAfterRestart();
+
+    expect(firstDuplicate.suggestedFilename).toBe("duplicate.txt");
+    expect(secondDuplicate.suggestedFilename).toBe("duplicate.txt");
+    expect(firstDuplicate.path).not.toBe(secondDuplicate.path);
+    await expect(fs.readFile(firstDuplicate.path, "utf8")).resolves.toBe(
+      successorPayload.toString("utf8"),
+    );
+    await expect(fs.readFile(secondDuplicate.path, "utf8")).resolves.toBe(
+      successorPayload.toString("utf8"),
+    );
+    expect((await fs.readdir(outputRoot)).some((name) => name.endsWith(".part"))).toBe(false);
+
+    await closePlaywrightBrowserConnection({ cdpUrl: restartedCdpUrl });
+    await restartedContext.close();
+    await fs.rm(rootDir, { recursive: true, force: true });
+    await expect(fs.access(rootDir)).rejects.toMatchObject({ code: "ENOENT" });
+  }, 30_000);
+});

extensions/browser/src/browser/pw-download-capture.ts

@@ -16,6 +16,7 @@ export type BrowserDownloadCaptureOptions = {
   mode?: "passive" | "explicit";
   outputPath?: string;
   outputRoot?: string;
+  signal?: AbortSignal;
   timeoutMessage?: string;
 };
 
@@ -85,6 +86,7 @@ export function createDownloadCaptureForPage(
   let depthReleased = false;
   let timer: NodeJS.Timeout | undefined;
   let handler: ((download: unknown) => void) | undefined;
+  let abort = () => {};
 
   const cleanup = () => {
     if (!depthReleased) {
@@ -99,6 +101,7 @@ export function createDownloadCaptureForPage(
       page.off("download", handler as never);
       handler = undefined;
     }
+    opts.signal?.removeEventListener("abort", abort);
   };
 
   const promise = new Promise<BrowserDownloadResult>((resolve, reject) => {
@@ -123,6 +126,19 @@ export function createDownloadCaptureForPage(
       Math.max(1, timeoutMs),
     );
     timer.unref?.();
+    abort = () => {
+      if (done) {
+        return;
+      }
+      done = true;
+      cleanup();
+      const reason = opts.signal?.reason;
+      reject(reason instanceof Error ? reason : new Error("Download wait was cancelled"));
+    };
+    opts.signal?.addEventListener("abort", abort, { once: true });
+    if (opts.signal?.aborted) {
+      abort();
+    }
   });
 
   return {

extensions/browser/src/browser/pw-tools-core.downloads.ts

@@ -49,6 +49,7 @@ function createExplicitDownloadCapture(params: {
   timeoutMs: number;
   outPath?: string;
   rootDir?: string;
+  signal?: AbortSignal;
 }) {
   params.state.armIdDownload = bumpDownloadArmId();
   const armId = params.state.armIdDownload;
@@ -56,6 +57,7 @@ function createExplicitDownloadCapture(params: {
     mode: "explicit",
     outputPath: params.outPath,
     outputRoot: params.rootDir,
+    signal: params.signal,
     beforeSave: () => {
       if (params.state.armIdDownload !== armId) {
         throw new Error("Download was superseded by another waiter");
@@ -374,6 +376,7 @@ export async function waitForDownloadViaPlaywright(opts: {
   targetId?: string;
   path?: string;
   rootDir?: string;
+  signal?: AbortSignal;
   timeoutMs?: number;
 }): Promise<BrowserDownloadResult> {
   const page = await getPageForTargetId(opts);
@@ -386,13 +389,9 @@ export async function waitForDownloadViaPlaywright(opts: {
     timeoutMs: timeout,
     outPath: opts.path,
     rootDir: opts.path?.trim() ? opts.rootDir : (opts.rootDir ?? resolveImplicitDownloadRoot()),
+    signal: opts.signal,
   });
-  try {
-    return await capture.promise;
-  } catch (err) {
-    capture.cancel();
-    throw err;
-  }
+  return await capture.promise;
 }
 
 /** Clicks an element ref and saves the download triggered by that click. */
@@ -402,6 +401,7 @@ export async function downloadViaPlaywright(opts: {
   ref: string;
   path: string;
   rootDir?: string;
+  signal?: AbortSignal;
   timeoutMs?: number;
 }): Promise<BrowserDownloadResult> {
   const page = await getPageForTargetId(opts);
@@ -421,17 +421,17 @@ export async function downloadViaPlaywright(opts: {
     timeoutMs: timeout,
     outPath,
     rootDir: opts.rootDir,
+    signal: opts.signal,
   });
+  void capture.promise.catch(() => {});
   try {
     const locator = refLocator(page, ref);
-    try {
-      await locator.click({ timeout });
-    } catch (err) {
-      throw toAIFriendlyError(err, ref);
-    }
-    return await capture.promise;
+    await locator.click({ timeout, signal: opts.signal });
   } catch (err) {
     capture.cancel();
-    throw err;
+    throw opts.signal?.aborted && opts.signal.reason instanceof Error
+      ? opts.signal.reason
+      : toAIFriendlyError(err, ref);
   }
+  return await capture.promise;
 }

extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts

@@ -318,6 +318,43 @@ describe("pw-tools-core", () => {
     expect(harness.activeHandlerCount()).toBe(0);
   });
 
+  it("releases a cancelled waiter before the next download", async () => {
+    const harness = createDownloadEventHarness();
+    const state = sessionMocks.ensurePageState();
+    const controller = new AbortController();
+    const cancelled = mod.waitForDownloadViaPlaywright({
+      cdpUrl: "http://127.0.0.1:18792",
+      targetId: "T1",
+      timeoutMs: 1000,
+      signal: controller.signal,
+    });
+
+    await Promise.resolve();
+    expect(state.downloadWaiterDepth).toBe(1);
+    controller.abort(new Error("request aborted"));
+    await expect(cancelled).rejects.toThrow("request aborted");
+    expect(state.downloadWaiterDepth).toBe(0);
+    expect(harness.activeHandlerCount()).toBe(0);
+
+    const successor = mod.waitForDownloadViaPlaywright({
+      cdpUrl: "http://127.0.0.1:18792",
+      targetId: "T1",
+      timeoutMs: 1000,
+    });
+    const saveAs = vi.fn(async (outPath: string) => {
+      await fs.writeFile(outPath, "successor-content", "utf8");
+    });
+    await Promise.resolve();
+    harness.trigger({
+      url: () => "https://example.com/successor.bin",
+      suggestedFilename: () => "successor.bin",
+      saveAs,
+    });
+
+    await expect(successor).resolves.toMatchObject({ suggestedFilename: "successor.bin" });
+    expect(saveAs).toHaveBeenCalledOnce();
+  });
+
   it("lets only the latest overlapping explicit waiter save the download", async () => {
     const harness = createDownloadEventHarness();
     const state = sessionMocks.ensurePageState();

extensions/browser/src/browser/routes/agent.act.download.ts

@@ -50,7 +50,7 @@ export function registerBrowserAgentActDownloadRoutes(
       ctx,
       targetId,
       enforceCurrentUrlAllowed: true,
-      run: async ({ profileCtx, cdpUrl, tab }) => {
+      run: async ({ profileCtx, cdpUrl, tab, signal }) => {
         if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) {
           return jsonError(res, 501, EXISTING_SESSION_LIMITS.download.waitUnsupported);
         }
@@ -77,6 +77,7 @@ export function registerBrowserAgentActDownloadRoutes(
           ...requestBase,
           path: downloadPath,
           rootDir: DEFAULT_DOWNLOAD_DIR,
+          signal,
         });
         res.json({ ok: true, targetId: tab.targetId, download: result });
       },
@@ -107,7 +108,7 @@ export function registerBrowserAgentActDownloadRoutes(
       ctx,
       targetId,
       enforceCurrentUrlAllowed: true,
-      run: async ({ profileCtx, cdpUrl, tab }) => {
+      run: async ({ profileCtx, cdpUrl, tab, signal }) => {
         if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) {
           return jsonError(res, 501, EXISTING_SESSION_LIMITS.download.downloadUnsupported);
         }
@@ -131,6 +132,7 @@ export function registerBrowserAgentActDownloadRoutes(
           ref,
           path: downloadPath,
           rootDir: DEFAULT_DOWNLOAD_DIR,
+          signal,
         });
         res.json({ ok: true, targetId: tab.targetId, download: result });
       },

extensions/browser/src/browser/server.agent-contract-form-layout-act-commands.test.ts

@@ -841,9 +841,42 @@ describe("browser control server", () => {
     expectRecordFields(waitCall, "wait download call", {
       targetId: "abcd1234",
     });
+    expect(waitCall.signal).toBeInstanceOf(AbortSignal);
     expect(String(waitCall.path)).toContain("safe-wait.pdf");
   });
 
+  it("cancels wait/download when its HTTP caller disconnects", async () => {
+    const base = await startServerAndBase();
+    let operationSignal: AbortSignal | undefined;
+    requirePwMock("waitForDownloadViaPlaywright").mockImplementationOnce(async (value) => {
+      const options = value as { signal?: AbortSignal };
+      operationSignal = options.signal;
+      await new Promise<void>((_resolve, reject) => {
+        options.signal?.addEventListener(
+          "abort",
+          () => {
+            const reason = options.signal?.reason;
+            reject(reason instanceof Error ? reason : new Error("request aborted"));
+          },
+          { once: true },
+        );
+      });
+      throw new Error("unreachable");
+    });
+    const controller = new AbortController();
+    const response = realFetch(`${base}/wait/download`, {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({ path: "cancelled-wait.pdf" }),
+      signal: controller.signal,
+    });
+
+    await vi.waitFor(() => expect(operationSignal).toBeInstanceOf(AbortSignal));
+    controller.abort(new Error("caller disconnected"));
+    await expect(response).rejects.toThrow();
+    await vi.waitFor(() => expect(operationSignal?.aborted).toBe(true));
+  });
+
   it("download accepts in-root relative output path", async () => {
     const base = await startServerAndBase();
     const res = await postJson<{ ok?: boolean; download?: { path?: string } }>(`${base}/download`, {
@@ -857,6 +890,7 @@ describe("browser control server", () => {
       targetId: "abcd1234",
       ref: "e12",
     });
+    expect(downloadCall.signal).toBeInstanceOf(AbortSignal);
     expect(String(downloadCall.path)).toContain("safe-download.pdf");
   });
 });