fix(browser): enforce download deadlines through atomic save (#129899)

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

@@ -7,6 +7,61 @@ import { describe, expect, it, vi } from "vitest";
 import { createDownloadCaptureForPage } from "./pw-download-capture.js";
 
 describe("Playwright download capture cancellation", () => {
+  it.each(["explicit", "passive"] as const)(
+    "enforces the %s download deadline after its event arrives",
+    async (mode) => {
+      vi.useFakeTimers();
+      const page = new EventEmitter();
+      const state = { downloadWaiterDepth: 0 };
+      const controller = new AbortController();
+      const validation = createDeferred<void>();
+      const saveAs = vi.fn(async () => {});
+      const cancel = vi.fn(async () => {});
+      const timeoutMessage =
+        mode === "passive"
+          ? "Timeout waiting for navigation download"
+          : "Timeout waiting for download";
+      const capture = createDownloadCaptureForPage(page, state, 25, {
+        mode,
+        signal: controller.signal,
+        timeoutMessage,
+        beforeSave: async () => {
+          await validation.promise;
+        },
+      });
+      const outcome = capture.promise.then(
+        () => "resolved" as const,
+        (error: unknown) => error,
+      );
+
+      try {
+        page.emit("download", {
+          url: () => "https://example.com/export.csv",
+          suggestedFilename: () => "export.csv",
+          saveAs,
+          cancel,
+        });
+        await vi.advanceTimersByTimeAsync(25);
+
+        await expect(Promise.race([outcome, Promise.resolve("pending")])).resolves.toMatchObject({
+          message: timeoutMessage,
+        });
+        expect(cancel).toHaveBeenCalledOnce();
+        validation.resolve();
+        await vi.advanceTimersByTimeAsync(0);
+
+        expect(saveAs).not.toHaveBeenCalled();
+        expect(state.downloadWaiterDepth).toBe(0);
+        expect(page.listenerCount("download")).toBe(0);
+      } finally {
+        controller.abort(new Error("download test cleanup"));
+        validation.resolve();
+        await outcome;
+        vi.useRealTimers();
+      }
+    },
+  );
+
   it("cancels a captured download before delayed validation can save bytes", async () => {
     const page = new EventEmitter();
     const state = { downloadWaiterDepth: 0 };
@@ -114,6 +169,58 @@ describe("Playwright download capture cancellation", () => {
     }
   });
 
+  it("cancels a timed-out in-progress download without publishing staged output", async () => {
+    const outputRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-download-timeout-"));
+    const outputPath = path.join(outputRoot, "timed-out.bin");
+    vi.useFakeTimers();
+    const page = new EventEmitter();
+    const state = { downloadWaiterDepth: 0 };
+    const saveGate = createDeferred<void>();
+    const saveStarted = createDeferred<string>();
+    const saveAs = vi.fn(async (tempPath: string) => {
+      await fs.writeFile(tempPath, "timed-out partial contents", "utf8");
+      saveStarted.resolve(tempPath);
+      await saveGate.promise;
+    });
+    const cancel = vi.fn(async () => {
+      saveGate.resolve();
+    });
+    const capture = createDownloadCaptureForPage(page, state, 25, {
+      mode: "explicit",
+      outputPath,
+      outputRoot,
+    });
+    const outcome = capture.promise.then(
+      () => "resolved" as const,
+      (error: unknown) => error,
+    );
+
+    try {
+      page.emit("download", {
+        url: () => "https://example.com/timed-out.bin",
+        suggestedFilename: () => "timed-out.bin",
+        saveAs,
+        cancel,
+      });
+      const partialPath = await saveStarted.promise;
+      await vi.advanceTimersByTimeAsync(25);
+
+      await expect(Promise.race([outcome, Promise.resolve("pending")])).resolves.toMatchObject({
+        message: "Timeout waiting for download",
+      });
+      expect(cancel).toHaveBeenCalledOnce();
+      await vi.waitFor(async () => {
+        await expect(fs.access(partialPath)).rejects.toMatchObject({ code: "ENOENT" });
+      });
+      await expect(fs.access(outputPath)).rejects.toMatchObject({ code: "ENOENT" });
+    } finally {
+      saveGate.resolve();
+      await outcome;
+      vi.useRealTimers();
+      await fs.rm(outputRoot, { recursive: true, force: true });
+    }
+  });
+
   it("finishes atomic publication when cancellation arrives after its commit boundary", async () => {
     const outputRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-download-publish-"));
     const outputPath = path.join(outputRoot, "published.bin");
@@ -178,4 +285,64 @@ describe("Playwright download capture cancellation", () => {
       await fs.rm(outputRoot, { recursive: true, force: true });
     }
   });
+
+  it("finishes atomic publication after its download deadline retires", async () => {
+    const outputRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-download-deadline-"));
+    const outputPath = path.join(outputRoot, "published.bin");
+    vi.useFakeTimers();
+    const renameStarted = createDeferred<void>();
+    const releaseRename = createDeferred<void>();
+    const renameFinished = createDeferred<void>();
+    const originalRename = fs.rename.bind(fs);
+    const rename = vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => {
+      if (String(destination).endsWith(`${path.sep}published.bin`)) {
+        renameStarted.resolve();
+        await releaseRename.promise;
+      }
+      try {
+        await originalRename(source, destination);
+      } finally {
+        renameFinished.resolve();
+      }
+    });
+    const page = new EventEmitter();
+    const state = { downloadWaiterDepth: 0 };
+    const cancel = vi.fn(async () => {});
+    const capture = createDownloadCaptureForPage(page, state, 25, {
+      mode: "explicit",
+      outputPath,
+      outputRoot,
+    });
+    const outcome = capture.promise.then(
+      (result) => result,
+      (error: unknown) => error,
+    );
+
+    try {
+      page.emit("download", {
+        url: () => "https://example.com/published.bin",
+        suggestedFilename: () => "published.bin",
+        saveAs: async (tempPath: string) => {
+          await fs.writeFile(tempPath, "completed download", "utf8");
+        },
+        cancel,
+      });
+      await renameStarted.promise;
+      await vi.advanceTimersByTimeAsync(25);
+
+      expect(await Promise.race([outcome, Promise.resolve("pending")])).toBe("pending");
+      expect(cancel).not.toHaveBeenCalled();
+      releaseRename.resolve();
+
+      await expect(capture.promise).resolves.toMatchObject({ path: outputPath });
+      await expect(fs.readFile(outputPath, "utf8")).resolves.toBe("completed download");
+    } finally {
+      releaseRename.resolve();
+      await renameFinished.promise;
+      await outcome;
+      rename.mockRestore();
+      vi.useRealTimers();
+      await fs.rm(outputRoot, { recursive: true, force: true });
+    }
+  });
 });

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

@@ -91,6 +91,7 @@ export function createDownloadCaptureForPage(
   }
 
   state.downloadWaiterDepth += 1;
+  const operation = new AbortController();
   let done = false;
   let timer: NodeJS.Timeout | undefined;
   let handler: ((download: unknown) => void) | undefined;
@@ -103,6 +104,9 @@ export function createDownloadCaptureForPage(
       page.off("download", handler);
       handler = undefined;
     }
+  };
+
+  const retireDeadline = () => {
     if (timer) {
       clearTimeout(timer);
       timer = undefined;
@@ -112,20 +116,31 @@ export function createDownloadCaptureForPage(
   const cleanup = () => {
     done = true;
     releaseWaiter();
+    retireDeadline();
     opts.signal?.removeEventListener("abort", abort);
   };
 
   const promise = new Promise<BrowserDownloadResult>((resolve, reject) => {
+    const rejectCapture = (reason: Error) => {
+      if (done) {
+        return;
+      }
+      operation.abort(reason);
+      cleanup();
+      void activeDownload?.cancel?.().catch(() => {});
+      reject(reason);
+    };
     handler = (download: unknown) => {
       if (done) {
         return;
       }
       activeDownload = download as PlaywrightDownload;
       releaseWaiter();
-      void saveBrowserDownload(activeDownload, opts, () => {
+      void saveBrowserDownload(activeDownload, { ...opts, signal: operation.signal }, () => {
         // Atomic publication cannot be revoked, so a later abort must not
         // report cancellation while its completed file is being published.
         opts.signal?.removeEventListener("abort", abort);
+        retireDeadline();
       })
         .finally(cleanup)
         .then(resolve, reject);
@@ -133,23 +148,14 @@ export function createDownloadCaptureForPage(
     page.on("download", handler);
     timer = setTimeout(
       () => {
-        if (done) {
-          return;
-        }
-        cleanup();
-        reject(new Error(opts.timeoutMessage ?? "Timeout waiting for download"));
+        rejectCapture(new Error(opts.timeoutMessage ?? "Timeout waiting for download"));
       },
       Math.max(1, timeoutMs),
     );
     timer.unref?.();
     abort = () => {
-      if (done) {
-        return;
-      }
-      cleanup();
-      void activeDownload?.cancel?.().catch(() => {});
       const reason = opts.signal?.reason;
-      reject(reason instanceof Error ? reason : new Error("Download wait was cancelled"));
+      rejectCapture(reason instanceof Error ? reason : new Error("Download wait was cancelled"));
     };
     opts.signal?.addEventListener("abort", abort, { once: true });
     if (opts.signal?.aborted) {