diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 193ba83f820..78a4b00f9b9 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -19,6 +19,7 @@ OpenClaw can run a **dedicated Chrome/Brave/Edge/Chromium profile** that the age - 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. +- Playwright-backed agent actions return a `downloads` array with the same managed metadata when the action immediately starts one or more downloads. - 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. diff --git a/extensions/browser/src/browser/client-actions-core.ts b/extensions/browser/src/browser/client-actions-core.ts index 85d6433faad..ea6151b10a0 100644 --- a/extensions/browser/src/browser/client-actions-core.ts +++ b/extensions/browser/src/browser/client-actions-core.ts @@ -21,6 +21,7 @@ import { DEFAULT_BROWSER_ACTION_TIMEOUT_MS, DEFAULT_BROWSER_SCREENSHOT_TIMEOUT_MS, } from "./constants.js"; +import type { BrowserDownloadResult } from "./download-types.js"; export type { BrowserFormField } from "./client-actions.types.js"; @@ -32,6 +33,8 @@ type BrowserActResponse = { results?: Array<{ ok: boolean; error?: string }>; blockedByDialog?: boolean; browserState?: unknown; + /** Download info when a click/batch/evaluate action triggers a browser download. */ + downloads?: BrowserDownloadResult[]; }; const BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS = 5_000; diff --git a/extensions/browser/src/browser/client.test.ts b/extensions/browser/src/browser/client.test.ts index 31e44a30bf6..13ce5de5b31 100644 --- a/extensions/browser/src/browser/client.test.ts +++ b/extensions/browser/src/browser/client.test.ts @@ -210,6 +210,13 @@ describe("browser client", () => { url: "https://x", result: 1, results: [{ ok: true }], + downloads: [ + { + url: "https://x/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ], }), } as unknown as Response; } @@ -346,6 +353,13 @@ describe("browser client", () => { expect(act.ok).toBe(true); expect(act.targetId).toBe("t1"); expect(act.results).toEqual([{ ok: true }]); + expect(act.downloads).toEqual([ + { + url: "https://x/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ]); const fileChooser = await browserArmFileChooser("http://127.0.0.1:18791", { paths: ["/tmp/a.txt"], diff --git a/extensions/browser/src/browser/pw-session.test.ts b/extensions/browser/src/browser/pw-session.test.ts index 28c86af48f6..4625dd61058 100644 --- a/extensions/browser/src/browser/pw-session.test.ts +++ b/extensions/browser/src/browser/pw-session.test.ts @@ -6,6 +6,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_DOWNLOAD_DIR } from "./paths.js"; import { createDownloadCaptureForPage } from "./pw-download-capture.js"; import { + beginActionDownloadCaptureOnPage, ensurePageState, isDownloadStartingNavigationError, refLocator, @@ -15,6 +16,7 @@ import { import { BROWSER_REF_MARKER_ATTRIBUTE } from "./pw-session.page-cdp.js"; type MutableDownload = { + url?: () => string; suggestedFilename: () => string; saveAs: ReturnType; path?: () => Promise; @@ -250,6 +252,312 @@ describe("pw-session ensurePageState", () => { expect(download.saveAs).not.toHaveBeenCalled(); }); + it("reports all downloads owned by the active action with managed metadata", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const capture = beginActionDownloadCaptureOnPage(page); + const firstSave = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "first-action-download", "utf8"); + }); + const secondSave = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "second-action-download", "utf8"); + }); + + for (const download of [ + { + url: () => "https://example.com/first.txt", + suggestedFilename: () => "first.txt", + saveAs: firstSave, + }, + { + url: () => "https://example.com/second.txt", + suggestedFilename: () => "second.txt", + saveAs: secondSave, + }, + ]) { + handlers.get("download")?.[0]?.(download); + } + + const result = await capture.drain(); + capture.dispose(); + + expect(result).toEqual([ + { + url: "https://example.com/first.txt", + suggestedFilename: "first.txt", + path: expect.stringMatching(/-first\.txt$/), + }, + { + url: "https://example.com/second.txt", + suggestedFilename: "second.txt", + path: expect.stringMatching(/-second\.txt$/), + }, + ]); + await expect(fs.readFile(result?.[0]?.path ?? "", "utf8")).resolves.toBe( + "first-action-download", + ); + await expect(fs.readFile(result?.[1]?.path ?? "", "utf8")).resolves.toBe( + "second-action-download", + ); + }); + + it("waits only the requested first-event grace for a just-late action download", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const capture = beginActionDownloadCaptureOnPage(page); + const saveAs = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "late-action-download", "utf8"); + }); + const drain = capture.drain({ firstEventGraceMs: 1_000 }); + + setImmediate(() => { + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/late.txt", + suggestedFilename: () => "late.txt", + saveAs, + }); + }); + + await expect(drain).resolves.toEqual([ + expect.objectContaining({ suggestedFilename: "late.txt" }), + ]); + capture.dispose(); + }); + + it("keeps a quiet window open for sibling downloads after the first event", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const capture = beginActionDownloadCaptureOnPage(page); + const save = (contents: string) => + vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, contents, "utf8"); + }); + + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/first.txt", + suggestedFilename: () => "first.txt", + saveAs: save("first"), + }); + setImmediate(() => { + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/second.txt", + suggestedFilename: () => "second.txt", + saveAs: save("second"), + }); + }); + + await expect(capture.drain({ quietMs: 1_000 })).resolves.toEqual([ + expect.objectContaining({ suggestedFilename: "first.txt" }), + expect.objectContaining({ suggestedFilename: "second.txt" }), + ]); + capture.dispose(); + }); + + it("detaches ownership before waiting for slow file saves", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + let releaseFirstSave: (() => void) | undefined; + const firstSaveGate = new Promise((resolve) => { + releaseFirstSave = resolve; + }); + const beforeSave = vi.fn(async () => {}); + const capture = beginActionDownloadCaptureOnPage(page, { beforeSave }); + const firstSave = vi.fn(async (outPath: string) => { + await firstSaveGate; + await fs.writeFile(outPath, "first", "utf8"); + }); + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/first.txt", + suggestedFilename: () => "first.txt", + saveAs: firstSave, + }); + + const drain = capture.drain(); + const lateSave = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "late", "utf8"); + }); + const lateDownload: MutableDownload = { + url: () => "https://example.com/late.txt", + suggestedFilename: () => "late.txt", + saveAs: lateSave, + }; + handlers.get("download")?.[0]?.(lateDownload); + releaseFirstSave?.(); + + await expect(drain).resolves.toEqual([ + expect.objectContaining({ suggestedFilename: "first.txt" }), + ]); + await expect(lateDownload.path?.()).resolves.toMatch(/-late\.txt$/); + expect(beforeSave).toHaveBeenCalledOnce(); + expect(lateSave).toHaveBeenCalledOnce(); + capture.dispose(); + }); + + it("keeps started saves with their owner and assigns future downloads to the latest action", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const first = beginActionDownloadCaptureOnPage(page); + const firstSaveAs = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "first-action-download", "utf8"); + }); + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/first.txt", + suggestedFilename: () => "first.txt", + saveAs: firstSaveAs, + }); + + const latest = beginActionDownloadCaptureOnPage(page); + first.dispose(); + const latestSaveAs = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "latest-action-download", "utf8"); + }); + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/latest.txt", + suggestedFilename: () => "latest.txt", + saveAs: latestSaveAs, + }); + + await expect(first.drain()).resolves.toEqual([ + expect.objectContaining({ suggestedFilename: "first.txt" }), + ]); + await expect(latest.drain()).resolves.toEqual([ + expect.objectContaining({ suggestedFilename: "latest.txt" }), + ]); + latest.dispose(); + expect(firstSaveAs).toHaveBeenCalledOnce(); + expect(latestSaveAs).toHaveBeenCalledOnce(); + }); + + it("leaves action capture empty while an explicit download owner is armed", async () => { + const { page, handlers } = fakePage(); + const state = ensurePageState(page); + state.downloadWaiterDepth = 1; + const capture = beginActionDownloadCaptureOnPage(page); + const download = { + suggestedFilename: () => "explicit.txt", + saveAs: vi.fn(async () => {}), + }; + + handlers.get("download")?.[0]?.(download); + + await expect(capture.drain()).resolves.toBeUndefined(); + capture.dispose(); + expect(download).not.toHaveProperty("path"); + expect(download.saveAs).not.toHaveBeenCalled(); + }); + + it("validates action-owned download URLs before saving bytes", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const blocked = new Error("blocked action download"); + const beforeSave = vi.fn(async () => { + throw blocked; + }); + const capture = beginActionDownloadCaptureOnPage(page, { beforeSave }); + const saveAs = vi.fn(async (outPath: string) => { + await fs.writeFile(outPath, "blocked-action-download", "utf8"); + }); + + handlers.get("download")?.[0]?.({ + url: () => "http://127.0.0.1/private.txt", + suggestedFilename: () => "private.txt", + saveAs, + }); + + await expect(capture.drain()).rejects.toBe(blocked); + capture.dispose(); + expect(beforeSave).toHaveBeenCalledWith({ + url: "http://127.0.0.1/private.txt", + suggestedFilename: "private.txt", + }); + expect(saveAs).not.toHaveBeenCalled(); + }); + + it("drains late siblings before surfacing the first download policy failure", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const blocked = new Error("blocked action download"); + const beforeSave = vi.fn(async () => { + throw blocked; + }); + const capture = beginActionDownloadCaptureOnPage(page, { beforeSave }); + const firstSave = vi.fn(async () => {}); + const secondSave = vi.fn(async () => {}); + + handlers.get("download")?.[0]?.({ + url: () => "http://127.0.0.1/first.txt", + suggestedFilename: () => "first.txt", + saveAs: firstSave, + }); + setImmediate(() => { + handlers.get("download")?.[0]?.({ + url: () => "http://127.0.0.1/second.txt", + suggestedFilename: () => "second.txt", + saveAs: secondSave, + }); + }); + + await expect(capture.drain({ quietMs: 1_000 })).rejects.toBe(blocked); + capture.dispose(); + expect(beforeSave).toHaveBeenCalledTimes(2); + expect(firstSave).not.toHaveBeenCalled(); + expect(secondSave).not.toHaveBeenCalled(); + }); + + it("surfaces a sibling policy denial without waiting for an allowed slow save", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const blocked = new Error("blocked sibling download"); + const beforeSave = vi.fn(async (candidate: { url: string }) => { + if (candidate.url.endsWith("/blocked.txt")) { + throw blocked; + } + }); + const capture = beginActionDownloadCaptureOnPage(page, { beforeSave }); + let releaseAllowedSave: (() => void) | undefined; + const allowedSaveGate = new Promise((resolve) => { + releaseAllowedSave = resolve; + }); + const allowedDownload: MutableDownload = { + url: () => "https://example.com/allowed.txt", + suggestedFilename: () => "allowed.txt", + saveAs: vi.fn(async (outPath: string) => { + await allowedSaveGate; + await fs.writeFile(outPath, "allowed", "utf8"); + }), + }; + handlers.get("download")?.[0]?.(allowedDownload); + setImmediate(() => { + handlers.get("download")?.[0]?.({ + url: () => "https://example.com/blocked.txt", + suggestedFilename: () => "blocked.txt", + saveAs: vi.fn(async () => {}), + }); + }); + + await expect(capture.drain({ quietMs: 100 })).rejects.toBe(blocked); + releaseAllowedSave?.(); + await expect(allowedDownload.path?.()).resolves.toMatch(/-allowed\.txt$/); + capture.dispose(); + }); + + it("surfaces action-owned download save failures without an unhandled rejection", async () => { + const { page, handlers } = fakePage(); + ensurePageState(page); + const capture = beginActionDownloadCaptureOnPage(page); + const error = new Error("action download save failed"); + + handlers.get("download")?.[0]?.({ + suggestedFilename: () => "failed.txt", + saveAs: vi.fn(async () => { + throw error; + }), + }); + + await expect(capture.drain()).rejects.toBe(error); + capture.dispose(); + }); + it("captures navigation downloads under managed paths", async () => { const { page, handlers } = fakePage(); const state = ensurePageState(page); diff --git a/extensions/browser/src/browser/pw-session.ts b/extensions/browser/src/browser/pw-session.ts index 1ce378d2360..229b458f92d 100644 --- a/extensions/browser/src/browser/pw-session.ts +++ b/extensions/browser/src/browser/pw-session.ts @@ -35,6 +35,7 @@ import { } from "./cdp.helpers.js"; import { AX_REF_PATTERN, normalizeCdpWsUrl } from "./cdp.js"; import { getChromeWebSocketUrl } from "./chrome.js"; +import type { BrowserDownloadCandidate, BrowserDownloadResult } from "./download-types.js"; import { BrowserTabNotFoundError } from "./errors.js"; import { assertBrowserNavigationAllowed, @@ -45,7 +46,11 @@ import { withBrowserNavigationPolicy, } from "./navigation-guard.js"; import { playwrightCore } from "./playwright-core.runtime.js"; -import { saveBrowserDownload, type PlaywrightDownload } from "./pw-download-capture.js"; +import { + saveBrowserDownload, + type BrowserDownloadCaptureOptions, + type PlaywrightDownload, +} from "./pw-download-capture.js"; import { BROWSER_REF_MARKER_ATTRIBUTE, withPageScopedCdpClient } from "./pw-session.page-cdp.js"; const { chromium } = playwrightCore; @@ -145,6 +150,14 @@ type DownloadPayload = PlaywrightDownload & { path?: () => Promise; }; +type ActionDownloadCapture = { + beforeSave?: (download: BrowserDownloadCandidate) => Promise | void; + lastEventAtMs?: number; + pending: Array>; + validations: Array>; + waiters: Array<() => void>; +}; + type PageState = { console: BrowserConsoleMessage[]; errors: BrowserPageError[]; @@ -154,6 +167,7 @@ type PageState = { armIdUpload: number; armIdDownload: number; downloadWaiterDepth: number; + actionDownloadCapture?: ActionDownloadCapture; nextObservedDialogId: number; pendingDialogs: PendingObservedDialog[]; recentDialogs: BrowserObservedDialogRecord[]; @@ -236,6 +250,84 @@ export function isDownloadStartingNavigationError(err: unknown, expectedUrl?: st ); } +/** Capture downloads started synchronously by one Browser action. */ +export function beginActionDownloadCaptureOnPage( + page: Page, + opts: { + beforeSave?: (download: BrowserDownloadCandidate) => Promise | void; + } = {}, +): { + drain: (opts?: { + firstEventGraceMs?: number; + maxWaitMs?: number; + quietMs?: number; + }) => Promise; + dispose: () => void; +} { + const state = ensurePageState(page); + const capture: ActionDownloadCapture = { + pending: [], + validations: [], + waiters: [], + ...(opts.beforeSave ? { beforeSave: opts.beforeSave } : {}), + }; + // One page event belongs to one action. A newer overlapping action owns + // future events; older captures may still drain saves they already started. + state.actionDownloadCapture = capture; + const detach = () => { + if (state.actionDownloadCapture === capture) { + state.actionDownloadCapture = undefined; + } + for (const finish of capture.waiters.splice(0)) { + finish(); + } + }; + + return { + drain: async (drainOpts = {}) => { + const waitForEvent = async (timeoutMs: number) => { + await new Promise((resolve) => { + const finish = () => { + clearTimeout(timer); + capture.waiters = capture.waiters.filter((waiter) => waiter !== finish); + resolve(); + }; + const timer = setTimeout(finish, timeoutMs); + capture.waiters.push(finish); + }); + }; + const firstEventGraceMs = Math.max(0, drainOpts.firstEventGraceMs ?? 0); + const maxWaitMs = Math.max(0, drainOpts.maxWaitMs ?? Number.POSITIVE_INFINITY); + const deadlineAtMs = Date.now() + maxWaitMs; + const remainingBudgetMs = () => Math.max(0, deadlineAtMs - Date.now()); + if (capture.pending.length === 0 && firstEventGraceMs > 0) { + await waitForEvent(Math.min(firstEventGraceMs, remainingBudgetMs())); + } + const quietMs = Math.max(0, drainOpts.quietMs ?? 0); + if (quietMs > 0) { + while (capture.lastEventAtMs !== undefined) { + const remainingQuietMs = Math.min( + quietMs - (Date.now() - capture.lastEventAtMs), + remainingBudgetMs(), + ); + if (remainingQuietMs <= 0) { + break; + } + await waitForEvent(remainingQuietMs); + } + } + // Establish event ownership before awaiting file I/O. Slow saves must not + // hold the action window open and absorb unrelated later downloads. + detach(); + const pending = capture.pending.slice(); + await Promise.all(capture.validations.slice()); + const downloads = await Promise.all(pending); + return downloads.length > 0 ? downloads : undefined; + }, + dispose: detach, + }; +} + function hasCachedPlaywrightBrowserConnection(cdpUrl: string): boolean { return cachedByCdpUrl.has(normalizeCdpUrl(cdpUrl)); } @@ -702,9 +794,28 @@ export function ensurePageState(page: Page): PageState { if (state.downloadWaiterDepth > 0) { return; } - const managedSave = saveBrowserDownload(download); + const actionCapture = state.actionDownloadCapture; + const beforeSave = actionCapture?.beforeSave; + const captureOptions: BrowserDownloadCaptureOptions | undefined = + actionCapture && beforeSave + ? { + beforeSave: (candidate) => { + const validation = Promise.resolve().then(() => beforeSave(candidate)); + actionCapture.validations.push(validation); + return validation; + }, + } + : undefined; + const managedSave = saveBrowserDownload(download, captureOptions); managedSave.catch(() => {}); download.path = async () => (await managedSave).path; + if (actionCapture) { + actionCapture.lastEventAtMs = Date.now(); + } + actionCapture?.pending.push(managedSave); + for (const finish of actionCapture?.waiters.splice(0) ?? []) { + finish(); + } }); page.on("close", () => { clearArmedDialogResponse(state); @@ -1279,7 +1390,7 @@ export function isPolicyDenyNavigationError(err: unknown): boolean { // page we have already proven is non-compliant. This is a pure bookkeeping // step; it does NOT close the tab. Read-only paths can call this safely on a // user-owned tab without losing the user's content. -async function quarantineBlockedTarget(opts: { +export async function quarantineBlockedNavigationTarget(opts: { cdpUrl: string; page: Page; targetId?: string; @@ -1304,7 +1415,7 @@ export async function closeBlockedNavigationTarget(opts: { page: Page; targetId?: string; }): Promise { - await quarantineBlockedTarget(opts); + await quarantineBlockedNavigationTarget(opts); await opts.page.close().catch(() => {}); } @@ -1333,7 +1444,7 @@ export async function assertPageNavigationCompletedSafely( }); } catch (err) { if (isPolicyDenyNavigationError(err)) { - await quarantineBlockedTarget({ + await quarantineBlockedNavigationTarget({ cdpUrl: opts.cdpUrl, page: opts.page, targetId: opts.targetId, diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts b/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts index 936f971f1af..323ba251cec 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts @@ -1296,4 +1296,115 @@ describe("pw-tools-core interaction navigation guard", () => { vi.useRealTimers(); } }); + + it("returns click downloads without adding a second policy grace", async () => { + const page = { url: vi.fn(() => "https://example.com") }; + const click = vi.fn(async () => {}); + const drain = vi.fn(async () => [ + { + url: "https://example.com/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ]); + const dispose = vi.fn(); + getPwToolsCoreSessionMocks().beginActionDownloadCaptureOnPage.mockReturnValueOnce({ + drain, + dispose, + }); + setPwToolsCoreCurrentPage(page); + setPwToolsCoreCurrentRefLocator({ click }); + + const result = await mod.executeActViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + action: { kind: "click", ref: "1" }, + ssrfPolicy: { allowPrivateNetwork: false }, + }); + + expect(result.downloads).toEqual([ + { + url: "https://example.com/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ]); + expect(drain).toHaveBeenCalledWith({ + firstEventGraceMs: 0, + maxWaitMs: 1_000, + quietMs: 250, + }); + expect(dispose).toHaveBeenCalledOnce(); + expect(getPwToolsCoreSessionMocks().beginActionDownloadCaptureOnPage).toHaveBeenCalledWith( + page, + { beforeSave: expect.any(Function) }, + ); + }); + + it("quarantines the target without closing it when an action download fails policy", async () => { + const page = { url: vi.fn(() => "https://example.com") }; + const click = vi.fn(async () => {}); + const blocked = new Error("blocked action download"); + blocked.name = "InvalidBrowserNavigationUrlError"; + const dispose = vi.fn(); + getPwToolsCoreSessionMocks().beginActionDownloadCaptureOnPage.mockReturnValueOnce({ + drain: vi.fn(async () => { + throw blocked; + }), + dispose, + }); + setPwToolsCoreCurrentPage(page); + setPwToolsCoreCurrentRefLocator({ click }); + + await expect( + mod.executeActViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + action: { kind: "click", ref: "1" }, + }), + ).rejects.toBe(blocked); + + expect(getPwToolsCoreSessionMocks().quarantineBlockedNavigationTarget).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:18792", + page, + targetId: "T1", + }); + expect(dispose).toHaveBeenCalledOnce(); + }); + + it("captures key-triggered downloads with a bounded event grace", async () => { + const page = { + keyboard: { press: vi.fn(async () => {}) }, + url: vi.fn(() => "https://example.com"), + }; + const drain = vi.fn(async () => [ + { + url: "https://example.com/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ]); + const dispose = vi.fn(); + getPwToolsCoreSessionMocks().beginActionDownloadCaptureOnPage.mockReturnValueOnce({ + drain, + dispose, + }); + setPwToolsCoreCurrentPage(page); + + const result = await mod.executeActViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + action: { kind: "press", key: "Enter" }, + }); + + expect(result.downloads).toEqual([ + expect.objectContaining({ suggestedFilename: "report.pdf" }), + ]); + expect(drain).toHaveBeenCalledWith({ + firstEventGraceMs: 250, + maxWaitMs: 1_000, + quietMs: 250, + }); + expect(dispose).toHaveBeenCalledOnce(); + }); }); diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.ts b/extensions/browser/src/browser/pw-tools-core.interactions.ts index 4856737469e..ee41cb78176 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.ts @@ -16,6 +16,7 @@ import { resolveActWaitTimeoutMs, } from "./act-policy.js"; import type { BrowserActRequest, BrowserFormField } from "./client-actions.types.js"; +import type { BrowserDownloadResult } from "./download-types.js"; import { normalizeBrowserEvaluateFunctionSource } from "./evaluate-source.js"; import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js"; import { @@ -25,12 +26,15 @@ import { import { resolveStrictExistingUploadPaths } from "./paths.js"; import { assertPageNavigationCompletedSafely, + beginActionDownloadCaptureOnPage, createObservedDialogAbortSignalForPage, ensurePageState, forceDisconnectPlaywrightForTarget, getPageForTargetId, isBrowserObservedDialogBlockedError, + isPolicyDenyNavigationError, markObservedDialogsHandledRemotelyForPage, + quarantineBlockedNavigationTarget, refLocator, restoreRoleRefsForTarget, } from "./pw-session.js"; @@ -57,6 +61,7 @@ type TargetOpts = { }; const INTERACTION_NAVIGATION_GRACE_MS = 250; +const ACT_DOWNLOAD_MAX_DRAIN_MS = 1_000; type NavigationObservablePage = Pick & { mainFrame?: () => Frame; @@ -1691,6 +1696,30 @@ async function executeSingleAction( return undefined; } +function actionNeedsStandaloneDownloadGrace( + action: BrowserActRequest, + ssrfPolicy?: SsrFPolicy, +): boolean { + switch (action.kind) { + case "close": + case "resize": + case "wait": + return false; + case "hover": + case "scrollIntoView": + case "drag": + return true; + case "batch": + return action.actions.some((nested) => + actionNeedsStandaloneDownloadGrace(nested, ssrfPolicy), + ); + default: + // Navigation-aware interactions already hold a 250 ms event window when + // policy is active. Policy-free internal callers need that window here. + return !ssrfPolicy; + } +} + /** Executes one high-level browser act request with bounded recursive actions. */ export async function executeActViaPlaywright(opts: { cdpUrl: string; @@ -1704,12 +1733,35 @@ export async function executeActViaPlaywright(opts: { results?: Array<{ ok: boolean; error?: string }>; blockedByDialog?: boolean; browserState?: unknown; + downloads?: BrowserDownloadResult[]; }> { const page = await getPageForTargetId({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, ssrfPolicy: opts.ssrfPolicy, }); + // Any DOM action can synchronously trigger a download. Capturing all actions + // keeps reporting and final-URL policy aligned with the actual file write. + const downloadCapture = beginActionDownloadCaptureOnPage(page, { + beforeSave: async (download) => { + if (!download.url) { + throw new Error("Action download URL is unavailable"); + } + await assertBrowserNavigationResultAllowed({ + url: download.url, + ...withBrowserNavigationPolicy(opts.ssrfPolicy), + }); + }, + }); + const downloadGraceMs = actionNeedsStandaloneDownloadGrace(opts.action, opts.ssrfPolicy) + ? INTERACTION_NAVIGATION_GRACE_MS + : 0; + const drainDownloads = async () => + await downloadCapture.drain({ + firstEventGraceMs: downloadGraceMs, + maxWaitMs: ACT_DOWNLOAD_MAX_DRAIN_MS, + quietMs: INTERACTION_NAVIGATION_GRACE_MS, + }); const dialogAbort = createObservedDialogAbortSignalForPage({ page, parentSignal: opts.signal, @@ -1725,7 +1777,11 @@ export async function executeActViaPlaywright(opts: { evaluateEnabled: opts.evaluateEnabled, signal: dialogAbort.signal, }); - return { results: batch.results }; + const newDownloads = await drainDownloads(); + return { + results: batch.results, + ...(newDownloads ? { downloads: newDownloads } : {}), + }; } const result = await executeSingleAction( opts.action, @@ -1736,16 +1792,33 @@ export async function executeActViaPlaywright(opts: { 0, dialogAbort.signal, ); + const newDownloads = await drainDownloads(); if (opts.action.kind === "evaluate") { - return { result }; + return { result, ...(newDownloads ? { downloads: newDownloads } : {}) }; } - return {}; + return newDownloads ? { downloads: newDownloads } : {}; } catch (err) { - if (isBrowserObservedDialogBlockedError(err)) { - return { blockedByDialog: true, browserState: err.browserState }; + let failure = err; + try { + await drainDownloads(); + } catch (downloadErr) { + // A download policy/save failure is the action's network-to-file result; + // preserve it even when the initiating interaction also failed. + failure = downloadErr; } - throw err; + if (isBrowserObservedDialogBlockedError(failure)) { + return { blockedByDialog: true, browserState: failure.browserState }; + } + if (isPolicyDenyNavigationError(failure)) { + await quarantineBlockedNavigationTarget({ + cdpUrl: opts.cdpUrl, + page, + targetId: opts.targetId, + }); + } + throw failure; } finally { + downloadCapture.dispose(); dialogAbort.cleanup(); } } diff --git a/extensions/browser/src/browser/pw-tools-core.test-harness.ts b/extensions/browser/src/browser/pw-tools-core.test-harness.ts index ed13a67c699..9f0b0e948f1 100644 --- a/extensions/browser/src/browser/pw-tools-core.test-harness.ts +++ b/extensions/browser/src/browser/pw-tools-core.test-harness.ts @@ -34,6 +34,10 @@ let pageState: { const sessionMocks = vi.hoisted(() => ({ assertPageNavigationCompletedSafely: vi.fn(async () => {}), + beginActionDownloadCaptureOnPage: vi.fn(() => ({ + drain: vi.fn(async (): Promise => undefined), + dispose: vi.fn(() => {}), + })), closeBlockedNavigationTarget: vi.fn(async () => {}), getPageForTargetId: vi.fn(async () => { if (!currentPage) { @@ -70,6 +74,7 @@ const sessionMocks = vi.hoisted(() => ({ } return err.name === "SsrFBlockedError" || err.name === "InvalidBrowserNavigationUrlError"; }), + quarantineBlockedNavigationTarget: vi.fn(async () => {}), restoreRoleRefsForTarget: vi.fn(() => {}), respondToObservedDialogOnPage: vi.fn(async () => { throw new Error("No dialog is pending."); diff --git a/extensions/browser/src/browser/routes/agent.act.ts b/extensions/browser/src/browser/routes/agent.act.ts index 5b30abe3bd2..b07d48a0831 100644 --- a/extensions/browser/src/browser/routes/agent.act.ts +++ b/extensions/browser/src/browser/routes/agent.act.ts @@ -673,21 +673,29 @@ export function registerBrowserAgentActRoutes( browserState: result.browserState, }); } + const downloads = result.downloads; switch (action.kind) { case "batch": return await jsonOk( - { results: result.results ?? [] }, + { results: result.results ?? [], ...(downloads ? { downloads } : {}) }, { resolveCurrentTarget: true }, ); case "evaluate": - return await jsonOk({ result: result.result }, { resolveCurrentTarget: true }); + return await jsonOk( + { result: result.result, ...(downloads ? { downloads } : {}) }, + { resolveCurrentTarget: true }, + ); case "click": case "clickCoords": - return await jsonOk(undefined, { resolveCurrentTarget: true }); + return await jsonOk(downloads ? { downloads } : undefined, { + resolveCurrentTarget: true, + }); case "resize": - return await jsonOk(); + return await jsonOk(downloads ? { downloads } : undefined); default: - return await jsonOk(undefined, { resolveCurrentTarget: true }); + return await jsonOk(downloads ? { downloads } : undefined, { + resolveCurrentTarget: true, + }); } }, }); diff --git a/extensions/browser/src/browser/server.agent-contract-core.test.ts b/extensions/browser/src/browser/server.agent-contract-core.test.ts index 500b108b334..2e810918d9b 100644 --- a/extensions/browser/src/browser/server.agent-contract-core.test.ts +++ b/extensions/browser/src/browser/server.agent-contract-core.test.ts @@ -313,6 +313,40 @@ describe("browser control server", () => { slowTimeoutMs, ); + it( + "returns action download metadata from /act responses", + async () => { + const base = await startServerAndBase(); + pwMocks.executeActViaPlaywright.mockResolvedValueOnce({ + downloads: [ + { + url: "https://example.com/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ], + }); + + const response = await postJson<{ + ok: boolean; + downloads?: Array<{ url?: string; suggestedFilename?: string; path?: string }>; + }>(`${base}/act`, { + kind: "click", + ref: "5", + }); + + expect(response.ok).toBe(true); + expect(response.downloads).toEqual([ + { + url: "https://example.com/report.pdf", + suggestedFilename: "report.pdf", + path: "/tmp/openclaw/downloads/report.pdf", + }, + ]); + }, + slowTimeoutMs, + ); + it( "returns ACT_SELECTOR_UNSUPPORTED for selector on unsupported action kinds", async () => {