fix(browser): keep upload errors specific (#101465)

steipete · web-flow · commit bd7da9decd7e · 2026-07-07T12:35:27.000+01:00
diff --git a/docs/tools/browser-control.md b/docs/tools/browser-control.md
@@ -223,6 +223,7 @@ openclaw browser select 9 OptionA OptionB
 openclaw browser download e12 report.pdf
 openclaw browser waitfordownload report.pdf
 openclaw browser upload /tmp/openclaw/uploads/file.pdf
+openclaw browser upload /tmp/openclaw/uploads/file.pdf --ref e12
 openclaw browser upload media://inbound/file.pdf
 openclaw browser fill --fields '[{"ref":"1","type":"text","value":"Ada"}]'
 openclaw browser dialog --accept
@@ -269,7 +270,7 @@ Notes:
   download URL, suggested filename, and guarded local path. Explicit download
   interception is available for managed Playwright profiles; existing-session
   profiles return an unsupported-operation error.
-- `upload` and `dialog` are **arming** calls; run them before the click/press that triggers the chooser/dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`.
+- Prefer atomic chooser uploads: pass the trigger `--ref` with the upload so OpenClaw arms and clicks in one request. Paths-only `upload` remains supported when a later trigger is intentional. Use `--input-ref` or `--element` to set a file input directly. `dialog` is an arming call; run it before the click/press that triggers the dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`.
 - `click`/`type`/etc require a `ref` from `snapshot` (numeric `12`, role ref `e12`, or actionable ARIA ref `ax12`). CSS selectors are intentionally not supported for actions. Use `click-coords` when the visible viewport position is the only reliable target.
 - Download and trace paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads}` (fallback: `${os.tmpdir()}/openclaw/...`).
 - `upload` accepts files from the OpenClaw temp uploads root and
diff --git a/extensions/browser/src/browser-tool.test.ts b/extensions/browser/src/browser-tool.test.ts
@@ -507,6 +507,8 @@ describe("browser tool description", () => {
     expect(tool.description).toContain("omit timeoutMs on act:type");
     expect(tool.description).toContain("existing-session profiles");
     expect(tool.description).toContain("browser-automation skill");
+    expect(tool.description).toContain("trigger ref with paths in the same upload call");
+    expect(tool.description).toContain("paths-only arming");
   });
 });
 
diff --git a/extensions/browser/src/browser-tool.ts b/extensions/browser/src/browser-tool.ts
@@ -524,6 +524,7 @@ export function createBrowserTool(opts?: {
       "For multi-step browser work, login checks, stale refs, duplicate tabs, or Google Meet flows, use the bundled browser-automation skill when it is available.",
       'For stable, self-resolving refs across calls, use snapshot with refs="aria" (Playwright aria-ref ids). Default refs="role" are role+name-based.',
       "Use snapshot+act for UI automation. Avoid act:wait by default; use only in exceptional cases when no reliable UI state exists.",
+      "For file chooser uploads, pass the trigger ref with paths in the same upload call when available; use paths-only arming only when a later trigger is intentional. Use inputRef or element to set a file input directly.",
       `target selects browser location (sandbox|host|node). Default: ${targetDefault}.`,
       hostHint,
     ].join(" "),
diff --git a/extensions/browser/src/browser/client-actions-core.hooks.test.ts b/extensions/browser/src/browser/client-actions-core.hooks.test.ts
@@ -0,0 +1,69 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const clientFetchMocks = vi.hoisted(() => ({
+  fetchBrowserJson: vi.fn(async (..._args: unknown[]) => ({ ok: true })),
+}));
+
+vi.mock("./client-fetch.js", () => clientFetchMocks);
+
+import { browserArmDialog, browserArmFileChooser } from "./client-actions-core.js";
+
+function lastFetchCall(): { url: string; options: { body?: string; timeoutMs?: number } } {
+  const call = clientFetchMocks.fetchBrowserJson.mock.calls.at(-1);
+  if (!call) {
+    throw new Error("fetchBrowserJson was not called");
+  }
+  return {
+    url: String(call[0]),
+    options: call[1] as { body?: string; timeoutMs?: number },
+  };
+}
+
+describe("browser hook client actions", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("adds transport slack to an atomic file chooser upload", async () => {
+    await browserArmFileChooser(undefined, {
+      paths: ["/tmp/openclaw/uploads/report.pdf"],
+      ref: "e12",
+      targetId: "tab-1",
+      timeoutMs: 30_000,
+      profile: "openclaw",
+    });
+
+    const call = lastFetchCall();
+    expect(call.url).toBe("/hooks/file-chooser?profile=openclaw");
+    expect(call.options.timeoutMs).toBe(35_000);
+    expect(JSON.parse(call.options.body ?? "{}")).toEqual({
+      paths: ["/tmp/openclaw/uploads/report.pdf"],
+      ref: "e12",
+      targetId: "tab-1",
+      timeoutMs: 30_000,
+    });
+  });
+
+  it("preserves paths-only arming with the advertised 120 second default", async () => {
+    await browserArmFileChooser(undefined, {
+      paths: ["/tmp/openclaw/uploads/report.pdf"],
+    });
+
+    const call = lastFetchCall();
+    expect(call.url).toBe("/hooks/file-chooser");
+    expect(call.options.timeoutMs).toBe(125_000);
+    expect(JSON.parse(call.options.body ?? "{}")).toEqual({
+      paths: ["/tmp/openclaw/uploads/report.pdf"],
+    });
+  });
+
+  it("keeps dialog hook requests alive past their operation timeout", async () => {
+    await browserArmDialog(undefined, { accept: true, timeoutMs: 45_000 });
+
+    const call = lastFetchCall();
+    expect(call.url).toBe("/hooks/dialog");
+    expect(call.options.timeoutMs).toBe(50_000);
+    expect(JSON.parse(call.options.body ?? "{}")).toEqual({
+      accept: true,
+      timeoutMs: 45_000,
+    });
+  });
+});
diff --git a/extensions/browser/src/browser/client-actions-core.ts b/extensions/browser/src/browser/client-actions-core.ts
@@ -38,8 +38,7 @@ type BrowserActResponse = {
   downloads?: BrowserDownloadResult[];
 };
 
-const BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS = 5_000;
-const BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS = 5_000;
+const BROWSER_REQUEST_TIMEOUT_SLACK_MS = 5_000;
 
 type BrowserDownloadActionResult = BrowserActionTabResult & { download: BrowserDownloadResult };
 
@@ -52,24 +51,23 @@ function resolveBrowserActRequestTimeoutMs(req: BrowserActRequest): number {
   const candidateTimeouts =
     explicitTimeout === undefined
       ? [DEFAULT_BROWSER_ACTION_TIMEOUT_MS]
-      : [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
+      : [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
   if (req.kind === "wait") {
     const waitDuration = normalizePositiveTimeoutMs(req.timeMs);
     if (waitDuration !== undefined) {
       candidateTimeouts.push(
-        addTimerTimeoutGraceMs(waitDuration, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
+        addTimerTimeoutGraceMs(waitDuration, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
       );
     }
   }
   return Math.max(...candidateTimeouts);
 }
 
-function resolveBrowserDownloadRequestTimeoutMs(timeoutMs: unknown): number {
-  const waitTimeoutMs =
+function resolveBrowserOperationRequestTimeoutMs(timeoutMs: unknown): number {
+  const operationTimeoutMs =
     normalizePositiveTimeoutMs(timeoutMs) ?? DEFAULT_BROWSER_DOWNLOAD_TIMEOUT_MS;
-  // Keep the HTTP client alive after the Playwright waiter expires so its
-  // timeout/error response reaches the caller instead of a transport abort.
-  return addTimerTimeoutGraceMs(waitTimeoutMs, BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
+  // Let the browser operation report its own timeout/error before the client watchdog fires.
+  return addTimerTimeoutGraceMs(operationTimeoutMs, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
 }
 
 async function postDownloadRequest(
@@ -84,7 +82,7 @@ async function postDownloadRequest(
     method: "POST",
     headers: { "Content-Type": "application/json" },
     body: JSON.stringify(body),
-    timeoutMs: resolveBrowserDownloadRequestTimeoutMs(timeoutMs),
+    timeoutMs: resolveBrowserOperationRequestTimeoutMs(timeoutMs),
   });
 }
 
@@ -129,7 +127,7 @@ export async function browserArmDialog(
       targetId: opts.targetId,
       timeoutMs: opts.timeoutMs,
     }),
-    timeoutMs: 20000,
+    timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
   });
 }
 
@@ -158,7 +156,7 @@ export async function browserArmFileChooser(
       targetId: opts.targetId,
       timeoutMs: opts.timeoutMs,
     }),
-    timeoutMs: 20000,
+    timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
   });
 }
 
