diff --git a/docs/cli/browser.md b/docs/cli/browser.md index dd67897fc8ed..3434b23bd341 100644 --- a/docs/cli/browser.md +++ b/docs/cli/browser.md @@ -275,6 +275,8 @@ openclaw browser dialog --dismiss --dialog-id d1 Managed Chrome profiles save ordinary click-triggered downloads into the OpenClaw downloads directory (`/tmp/openclaw/downloads` by default, or the configured temp root). Use `waitfordownload` or `download` when the agent needs to wait for a specific file and return its path; those explicit waiters own the next download. Uploads accept files from the OpenClaw temp uploads root and OpenClaw-managed inbound media, including `media://inbound/` and sandbox-relative `media/inbound/` references. Nested media refs, traversal, and arbitrary local paths are rejected. +If saving a download fails, OpenClaw requests cancellation of the transfer and reports the original save error. Correct the output path or filesystem problem before starting a new download. + When an action opens a modal dialog, the action response returns `blockedByDialog` with `browserState.dialogs.pending`; pass `--dialog-id` to answer it directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`. Batch actions: diff --git a/extensions/browser/src/browser/pw-download-cancel.chromium.test.ts b/extensions/browser/src/browser/pw-download-cancel.chromium.test.ts index 88a75048507a..295d26bbeb3a 100644 --- a/extensions/browser/src/browser/pw-download-cancel.chromium.test.ts +++ b/extensions/browser/src/browser/pw-download-cancel.chromium.test.ts @@ -43,93 +43,117 @@ describe.runIf(runChromiumProof)("managed Chromium download cancellation", () => const cleanup: Array<() => Promise> = []; afterEach(async () => { + const errors: unknown[] = []; for (const dispose of cleanup.splice(0).toReversed()) { - await dispose(); + await dispose().catch((error: unknown) => errors.push(error)); + } + if (errors.length) { + throw new AggregateError(errors, "Chromium download fixture cleanup failed"); } }); - it("cancels a download already being streamed without publishing its output", async () => { - const rootDir = tempDirs.make("openclaw-download-stream-cancel-"); - cleanup.push(async () => await fs.rm(rootDir, { recursive: true, force: true })); - let closeDownloadResponse: (() => void) | undefined; - const downloadServer = createServer((request, response) => { - if (request.url === "/stream.bin") { - response.writeHead(200, { - "content-disposition": 'attachment; filename="stream.bin"', - "content-type": "application/octet-stream", - }); - response.write("partially downloaded bytes"); - closeDownloadResponse = () => response.destroy(); - return; + it.each(["caller abort", "invalid output directory"])( + "cancels a streaming download after %s without publishing output", + async (failure) => { + const rootDir = tempDirs.make("openclaw-download-stream-cancel-"); + cleanup.push(async () => await fs.rm(rootDir, { recursive: true, force: true })); + let closeDownloadResponse: (() => void) | undefined; + let responseClosed = false; + const downloadServer = createServer((request, response) => { + if (request.url === "/stream.bin") { + response.writeHead(200, { + "content-disposition": 'attachment; filename="stream.bin"', + "content-type": "application/octet-stream", + }); + response.write("partially downloaded bytes"); + response.once("close", () => { + responseClosed = true; + }); + closeDownloadResponse = () => response.destroy(); + return; + } + response.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + response.end('Download'); + }); + const downloadPort = await listen(downloadServer); + cleanup.push(async () => { + closeDownloadResponse?.(); + await closeServer(downloadServer); + }); + + const cdpPort = await getFreePort(); + const context = await getPlaywrightCore().chromium.launchPersistentContext( + path.join(rootDir, "profile"), + { + 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 })); + cleanup.push(async () => closeDownloadResponse?.()); + + const controlledPage = await getPageForTargetId({ cdpUrl, targetId }); + const saveStarted = createDeferred(); + let cancellationCount = 0; + controlledPage.once("download", (download) => { + const saveAs = download.saveAs.bind(download); + const cancel = download.cancel.bind(download); + download.saveAs = async (outputPath) => { + saveStarted.resolve(); + await saveAs(outputPath); + }; + download.cancel = async () => { + cancellationCount += 1; + await cancel(); + }; + }); + + const outputRoot = path.join(rootDir, "downloads"); + if (failure === "invalid output directory") { + await fs.writeFile(outputRoot, "not a directory"); + } + const outputPath = path.join(outputRoot, "cancelled.bin"); + const controller = new AbortController(); + const reason = new Error("streaming download aborted"); + const capture = waitForDownloadViaPlaywright({ + cdpUrl, + targetId, + path: outputPath, + rootDir: outputRoot, + timeoutMs: 5_000, + signal: controller.signal, + }); + const outcome = capture.then( + () => "resolved" as const, + (error: unknown) => error, + ); + await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1); + await page.locator("#download").click(); + if (failure === "caller abort") { + await Promise.race([saveStarted.promise, capture]); + controller.abort(reason); + await expect(outcome).resolves.toBe(reason); + await expect.poll(async () => await fs.readdir(outputRoot)).toEqual([]); + await expect(fs.access(outputPath)).rejects.toMatchObject({ code: "ENOENT" }); + } else { + await expect(outcome).resolves.toMatchObject({ + message: "Invalid path: must stay within output directory", + }); + await expect(fs.readFile(outputRoot, "utf8")).resolves.toBe("not a directory"); } - response.writeHead(200, { "content-type": "text/html; charset=utf-8" }); - response.end('Download'); - }); - const downloadPort = await listen(downloadServer); - cleanup.push(async () => { - closeDownloadResponse?.(); - await closeServer(downloadServer); - }); - const cdpPort = await getFreePort(); - const context = await getPlaywrightCore().chromium.launchPersistentContext( - path.join(rootDir, "profile"), - { - 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 controlledPage = await getPageForTargetId({ cdpUrl, targetId }); - const saveStarted = createDeferred(); - let cancellationCount = 0; - controlledPage.once("download", (download) => { - const saveAs = download.saveAs.bind(download); - const cancel = download.cancel.bind(download); - download.saveAs = async (outputPath) => { - saveStarted.resolve(); - await saveAs(outputPath); - }; - download.cancel = async () => { - cancellationCount += 1; - await cancel(); - }; - }); - - const outputRoot = path.join(rootDir, "downloads"); - const outputPath = path.join(outputRoot, "cancelled.bin"); - const controller = new AbortController(); - const reason = new Error("streaming download aborted"); - const capture = waitForDownloadViaPlaywright({ - cdpUrl, - targetId, - path: outputPath, - rootDir: outputRoot, - timeoutMs: 5_000, - signal: controller.signal, - }); - const outcome = capture.then( - () => "resolved" as const, - (error: unknown) => error, - ); - await expect.poll(() => ensurePageState(controlledPage).downloadWaiterDepth).toBe(1); - await page.locator("#download").click(); - await saveStarted.promise; - controller.abort(reason); - - expect(cancellationCount).toBe(1); - await expect(outcome).resolves.toBe(reason); - await expect.poll(async () => await fs.readdir(outputRoot)).toEqual([]); - await expect(fs.access(outputPath)).rejects.toMatchObject({ code: "ENOENT" }); - expect(ensurePageState(controlledPage).downloadWaiterDepth).toBe(0); - }, 20_000); + expect.soft(cancellationCount).toBe(1); + await expect.poll(() => responseClosed).toBe(true); + expect(ensurePageState(controlledPage).downloadWaiterDepth).toBe(0); + }, + 20_000, + ); it("does not let a cancelled waiter capture and write a later download", async () => { const rootDir = tempDirs.make("openclaw-download-cancel-"); diff --git a/extensions/browser/src/browser/pw-download-capture.ts b/extensions/browser/src/browser/pw-download-capture.ts index ade46b3c8534..25311f76bc40 100644 --- a/extensions/browser/src/browser/pw-download-capture.ts +++ b/extensions/browser/src/browser/pw-download-capture.ts @@ -65,6 +65,13 @@ export async function saveBrowserDownload( opts.signal?.throwIfAborted(); onReadyToPublish?.(); }, + }).catch((error: unknown) => { + // Admission failures can belong to a superseded waiter. Only failed saves + // cancel here; an aborted capture already owns its cancellation. + if (!opts.signal?.aborted) { + void download.cancel?.().catch(() => {}); + } + throw error; }); return { ...candidate, path: savedPath }; } diff --git a/extensions/browser/src/browser/pw-session.test.ts b/extensions/browser/src/browser/pw-session.test.ts index 4ce20dd6fb26..fda0aac1a432 100644 --- a/extensions/browser/src/browser/pw-session.test.ts +++ b/extensions/browser/src/browser/pw-session.test.ts @@ -657,15 +657,20 @@ describe("pw-session ensurePageState", () => { ensurePageState(page); const capture = beginActionDownloadCaptureOnPage(page); const error = new Error("action download save failed"); + const cancel = vi.fn(async () => { + throw new Error("browser disconnected during cancellation"); + }); handlers.get("download")?.[0]?.({ suggestedFilename: () => "failed.txt", saveAs: vi.fn(async () => { throw error; }), + cancel, }); await expect(capture.drain()).rejects.toBe(error); + expect(cancel).toHaveBeenCalledOnce(); capture.dispose(); }); diff --git a/extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts b/extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts index 3ed838d2111d..95d7b94ddacb 100644 --- a/extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts @@ -358,6 +358,7 @@ describe("pw-tools-core", () => { it("lets only the latest overlapping explicit waiter save the download", async () => { const harness = createDownloadEventHarness(); const state = sessionMocks.ensurePageState(); + const cancel = vi.fn(async () => {}); const saveAs = vi.fn(async (outPath: string) => { await fs.writeFile(outPath, "latest-content", "utf8"); }); @@ -380,10 +381,12 @@ describe("pw-tools-core", () => { url: () => "https://example.com/latest.bin", suggestedFilename: () => "latest.bin", saveAs, + cancel, }); await expect(first).rejects.toThrow("superseded by another waiter"); await expect(latest).resolves.toMatchObject({ suggestedFilename: "latest.bin" }); + expect(cancel).not.toHaveBeenCalled(); expect(saveAs).toHaveBeenCalledOnce(); expect(state.downloadWaiterDepth).toBe(0); expect(harness.activeHandlerCount()).toBe(0);