fix(browser): downloads complete over CDP connections (#89416)

* fix(browser): surface navigate downloads in CDP mode

* Validate navigation downloads before saving

* fix(browser): observe navigation download capture timeouts

* refactor(browser): unify managed download capture

* test(browser): satisfy download fixture lint

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>

Author: zhangguiping-xydt

docs/tools/browser.md

@@ -18,6 +18,7 @@ OpenClaw can run a **dedicated Chrome/Brave/Edge/Chromium profile** that the age
 - A separate browser profile named **openclaw** (orange accent by default).
 - Deterministic tab control (list/open/focus/close).
 - Agent actions (click/type/drag/select), snapshots, screenshots, PDFs.
+- Playwright-backed profiles save direct attachment navigations under the managed downloads directory and return `{ url, suggestedFilename, path }` metadata after final-URL policy validation.
 - A bundled `browser-automation` skill that teaches agents the snapshot,
   stable-tab, stale-ref, and manual-blocker recovery loop when the browser
   plugin is enabled.

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

@@ -1,6 +1,7 @@
 /**
  * Shared result types for browser client action helpers.
  */
+import type { BrowserDownloadResult } from "./download-types.js";
 import type { AnnotationItem } from "./screenshot-annotate.js";
 
 /** Generic success result for action endpoints. */
@@ -11,6 +12,7 @@ export type BrowserActionTabResult = {
   ok: true;
   targetId: string;
   url?: string;
+  download?: BrowserDownloadResult;
 };
 
 /** Success result carrying a filesystem output path. */

extensions/browser/src/browser/client.test.ts

@@ -193,6 +193,11 @@ describe("browser client", () => {
               ok: true,
               targetId: "t1",
               url: "https://y",
+              download: {
+                url: "https://y/report.csv",
+                suggestedFilename: "report.csv",
+                path: "/tmp/openclaw/downloads/report.csv",
+              },
             }),
           } as unknown as Response;
         }
@@ -331,6 +336,11 @@ describe("browser client", () => {
     });
     expect(navigation.ok).toBe(true);
     expect(navigation.targetId).toBe("t1");
+    expect(navigation.download).toEqual({
+      url: "https://y/report.csv",
+      suggestedFilename: "report.csv",
+      path: "/tmp/openclaw/downloads/report.csv",
+    });
 
     const act = await browserAct("http://127.0.0.1:18791", { kind: "click", ref: "1" });
     expect(act.ok).toBe(true);

extensions/browser/src/browser/download-types.ts

@@ -0,0 +1,9 @@
+/** Metadata for a browser download saved under the configured output root. */
+export type BrowserDownloadResult = {
+  url: string;
+  suggestedFilename: string;
+  path: string;
+};
+
+/** Download metadata available before any bytes are written. */
+export type BrowserDownloadCandidate = Omit<BrowserDownloadResult, "path">;

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

@@ -0,0 +1,139 @@
+/** Shared Playwright download capture and output handling. */
+import crypto from "node:crypto";
+import path from "node:path";
+import type { Page } from "playwright-core";
+import type { BrowserDownloadCandidate, BrowserDownloadResult } from "./download-types.js";
+import { writeExternalFileWithinOutputRoot } from "./output-files.js";
+import { DEFAULT_DOWNLOAD_DIR } from "./paths.js";
+import { sanitizeUntrustedFileName } from "./safe-filename.js";
+
+export type BrowserDownloadCaptureState = {
+  downloadWaiterDepth: number;
+};
+
+export type BrowserDownloadCaptureOptions = {
+  beforeSave?: (download: BrowserDownloadCandidate) => Promise<void> | void;
+  mode?: "passive" | "explicit";
+  outputPath?: string;
+  outputRoot?: string;
+  timeoutMessage?: string;
+};
+
+export type PlaywrightDownload = {
+  url?: () => string;
+  suggestedFilename?: () => string;
+  saveAs?: (outPath: string) => Promise<void>;
+};
+
+function buildManagedDownloadPath(rootDir: string, fileName: string): string {
+  const id = crypto.randomUUID();
+  const safeName = sanitizeUntrustedFileName(fileName, "download.bin");
+  return path.join(rootDir, `${id}-${safeName}`);
+}
+
+/** Validate metadata and atomically save one Playwright download. */
+export async function saveBrowserDownload(
+  download: PlaywrightDownload,
+  opts: BrowserDownloadCaptureOptions = {},
+): Promise<BrowserDownloadResult> {
+  const suggestedFilename = download.suggestedFilename?.() || "download.bin";
+  const candidate: BrowserDownloadCandidate = {
+    url: download.url?.() || "",
+    suggestedFilename,
+  };
+  await opts.beforeSave?.(candidate);
+  const saveAs = download.saveAs?.bind(download);
+  if (!saveAs) {
+    throw new Error("Download cannot be saved");
+  }
+  const requestedPath = opts.outputPath?.trim();
+  const implicitRoot = opts.outputRoot ?? DEFAULT_DOWNLOAD_DIR;
+  const managedPath = requestedPath || buildManagedDownloadPath(implicitRoot, suggestedFilename);
+  const savedPath = await writeExternalFileWithinOutputRoot({
+    rootDir: requestedPath ? opts.outputRoot : implicitRoot,
+    path: managedPath,
+    write: async (tempPath) => {
+      await saveAs(tempPath);
+    },
+  });
+  return { ...candidate, path: savedPath };
+}
+
+/** Arm one page download while maintaining explicit/passive ownership depth. */
+export function createDownloadCaptureForPage(
+  page: Page,
+  state: BrowserDownloadCaptureState,
+  timeoutMs: number,
+  opts: BrowserDownloadCaptureOptions = {},
+): {
+  armed: boolean;
+  promise: Promise<BrowserDownloadResult>;
+  cancel: () => void;
+} {
+  // Passive action capture yields to an explicit wait/download owner. Explicit
+  // waiters may overlap; their arm id decides which one is allowed to save.
+  if (opts.mode !== "explicit" && state.downloadWaiterDepth > 0) {
+    return {
+      armed: false,
+      promise: new Promise<BrowserDownloadResult>(() => {}),
+      cancel: () => {},
+    };
+  }
+
+  state.downloadWaiterDepth += 1;
+  let done = false;
+  let depthReleased = false;
+  let timer: NodeJS.Timeout | undefined;
+  let handler: ((download: unknown) => void) | undefined;
+
+  const cleanup = () => {
+    if (!depthReleased) {
+      depthReleased = true;
+      state.downloadWaiterDepth = Math.max(0, state.downloadWaiterDepth - 1);
+    }
+    if (timer) {
+      clearTimeout(timer);
+      timer = undefined;
+    }
+    if (handler) {
+      page.off("download", handler as never);
+      handler = undefined;
+    }
+  };
+
+  const promise = new Promise<BrowserDownloadResult>((resolve, reject) => {
+    handler = (download: unknown) => {
+      if (done) {
+        return;
+      }
+      done = true;
+      cleanup();
+      void saveBrowserDownload(download as PlaywrightDownload, opts).then(resolve, reject);
+    };
+    page.on("download", handler as never);
+    timer = setTimeout(
+      () => {
+        if (done) {
+          return;
+        }
+        done = true;
+        cleanup();
+        reject(new Error(opts.timeoutMessage ?? "Timeout waiting for download"));
+      },
+      Math.max(1, timeoutMs),
+    );
+    timer.unref?.();
+  });
+
+  return {
+    armed: true,
+    promise,
+    cancel: () => {
+      if (done) {
+        return;
+      }
+      done = true;
+      cleanup();
+    },
+  };
+}

extensions/browser/src/browser/pw-session.test.ts

@@ -4,8 +4,10 @@ import path from "node:path";
 import type { Page } from "playwright-core";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { DEFAULT_DOWNLOAD_DIR } from "./paths.js";
+import { createDownloadCaptureForPage } from "./pw-download-capture.js";
 import {
   ensurePageState,
+  isDownloadStartingNavigationError,
   refLocator,
   rememberRoleRefsForTarget,
   restoreRoleRefsForTarget,
@@ -39,6 +41,14 @@ function fakePage(): {
     handlers.set(event, list);
     return undefined as unknown;
   });
+  const off = vi.fn((event: string, cb: (...args: unknown[]) => void) => {
+    const list = handlers.get(event) ?? [];
+    handlers.set(
+      event,
+      list.filter((handler) => handler !== cb),
+    );
+    return undefined as unknown;
+  });
   const getByRole = vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) }));
   const frameLocator = vi.fn(() => ({
     getByRole: vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })),
@@ -48,6 +58,7 @@ function fakePage(): {
 
   const page = {
     on,
+    off,
     getByRole,
     frameLocator,
     locator,
@@ -239,6 +250,96 @@ describe("pw-session ensurePageState", () => {
     expect(download.saveAs).not.toHaveBeenCalled();
   });
 
+  it("captures navigation downloads under managed paths", async () => {
+    const { page, handlers } = fakePage();
+    const state = ensurePageState(page);
+    const capture = createDownloadCaptureForPage(page, state, 1_000);
+    const saveAs = vi.fn(async (outPath: string) => {
+      await fs.writeFile(outPath, "attachment", "utf8");
+    });
+    const download = {
+      url: () => "https://example.com/export.csv",
+      suggestedFilename: () => "export.csv",
+      saveAs,
+    };
+
+    for (const handler of handlers.get("download") ?? []) {
+      handler(download);
+    }
+
+    const result = await capture.promise;
+    expect(result.url).toBe("https://example.com/export.csv");
+    expect(result.suggestedFilename).toBe("export.csv");
+    expect(path.dirname(result.path)).toBe(DEFAULT_DOWNLOAD_DIR);
+    expect(path.basename(result.path)).toMatch(/-export\.csv$/);
+    expect(firstSavePath(saveAs)).not.toBe(result.path);
+    await expect(fs.readFile(result.path, "utf8")).resolves.toBe("attachment");
+  });
+
+  it("validates captured navigation downloads before saving managed bytes", async () => {
+    const { page, handlers } = fakePage();
+    const state = ensurePageState(page);
+    const blocked = new Error("blocked download");
+    const beforeSave = vi.fn(async () => {
+      throw blocked;
+    });
+    const capture = createDownloadCaptureForPage(page, state, 1_000, { beforeSave });
+    const saveAs = vi.fn(async (outPath: string) => {
+      await fs.writeFile(outPath, "blocked", "utf8");
+    });
+    const download = {
+      url: () => "http://127.0.0.1:18080/export.csv",
+      suggestedFilename: () => "export.csv",
+      saveAs,
+    };
+
+    for (const handler of handlers.get("download") ?? []) {
+      handler(download);
+    }
+
+    await expect(capture.promise).rejects.toBe(blocked);
+    expect(beforeSave).toHaveBeenCalledWith({
+      url: "http://127.0.0.1:18080/export.csv",
+      suggestedFilename: "export.csv",
+    });
+    expect(saveAs).not.toHaveBeenCalled();
+  });
+
+  it("lets explicit download owners arm while passive capture yields", () => {
+    const { page } = fakePage();
+    const state = ensurePageState(page);
+    state.downloadWaiterDepth = 1;
+
+    const passive = createDownloadCaptureForPage(page, state, 1_000);
+    const explicit = createDownloadCaptureForPage(page, state, 1_000, { mode: "explicit" });
+
+    expect(passive.armed).toBe(false);
+    expect(explicit.armed).toBe(true);
+    expect(state.downloadWaiterDepth).toBe(2);
+    explicit.cancel();
+    expect(state.downloadWaiterDepth).toBe(1);
+  });
+
+  it("recognizes Playwright download-starting navigation aborts", () => {
+    expect(isDownloadStartingNavigationError(new Error("page.goto: Download is starting"))).toBe(
+      true,
+    );
+    expect(isDownloadStartingNavigationError(new Error("page.goto: net::ERR_ABORTED"))).toBe(false);
+    expect(
+      isDownloadStartingNavigationError(
+        new Error("page.goto: net::ERR_ABORTED at http://127.0.0.1:3333/download"),
+        "http://127.0.0.1:3333/download",
+      ),
+    ).toBe(true);
+    expect(
+      isDownloadStartingNavigationError(
+        new Error("page.goto: net::ERR_ABORTED at http://127.0.0.1:3333/other"),
+        "http://127.0.0.1:3333/download",
+      ),
+    ).toBe(false);
+    expect(isDownloadStartingNavigationError(new Error("Navigation failed"))).toBe(false);
+  });
+
   it("tracks page errors and network requests (best-effort)", () => {
     const { page, handlers } = fakePage();
     const state = ensurePageState(page);