Skip to content

Commit b16eb37

Browse files
steipetesteipete
committed
fix(browser): keep upload errors specific
1 parent 988f30e commit b16eb37

5 files changed

Lines changed: 84 additions & 13 deletions

File tree

‎docs/tools/browser-control.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ openclaw browser select 9 OptionA OptionB
223223
openclaw browser download e12 report.pdf
224224
openclaw browser waitfordownload report.pdf
225225
openclaw browser upload /tmp/openclaw/uploads/file.pdf
226+
openclaw browser upload /tmp/openclaw/uploads/file.pdf --ref e12
226227
openclaw browser upload media://inbound/file.pdf
227228
openclaw browser fill --fields '[{"ref":"1","type":"text","value":"Ada"}]'
228229
openclaw browser dialog --accept
@@ -269,7 +270,7 @@ Notes:
269270
download URL, suggested filename, and guarded local path. Explicit download
270271
interception is available for managed Playwright profiles; existing-session
271272
profiles return an unsupported-operation error.
272-
- `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`.
273+
- 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`.
273274
- `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.
274275
- Download and trace paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads}` (fallback: `${os.tmpdir()}/openclaw/...`).
275276
- `upload` accepts files from the OpenClaw temp uploads root and

‎extensions/browser/src/browser-tool.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,8 @@ describe("browser tool description", () => {
507507
expect(tool.description).toContain("omit timeoutMs on act:type");
508508
expect(tool.description).toContain("existing-session profiles");
509509
expect(tool.description).toContain("browser-automation skill");
510+
expect(tool.description).toContain("trigger ref with paths in the same upload call");
511+
expect(tool.description).toContain("paths-only arming");
510512
});
511513
});
512514

‎extensions/browser/src/browser-tool.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ export function createBrowserTool(opts?: {
524524
"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.",
525525
'For stable, self-resolving refs across calls, use snapshot with refs="aria" (Playwright aria-ref ids). Default refs="role" are role+name-based.',
526526
"Use snapshot+act for UI automation. Avoid act:wait by default; use only in exceptional cases when no reliable UI state exists.",
527+
"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.",
527528
`target selects browser location (sandbox|host|node). Default: ${targetDefault}.`,
528529
hostHint,
529530
].join(" "),

‎extensions/browser/src/browser/client-actions-core.hooks.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const clientFetchMocks = vi.hoisted(() => ({
4+
fetchBrowserJson: vi.fn(async (..._args: unknown[]) => ({ ok: true })),
5+
}));
6+
7+
vi.mock("./client-fetch.js", () => clientFetchMocks);
8+
9+
import { browserArmDialog, browserArmFileChooser } from "./client-actions-core.js";
10+
11+
function lastFetchCall(): { url: string; options: { body?: string; timeoutMs?: number } } {
12+
const call = clientFetchMocks.fetchBrowserJson.mock.calls.at(-1);
13+
if (!call) {
14+
throw new Error("fetchBrowserJson was not called");
15+
}
16+
return {
17+
url: String(call[0]),
18+
options: call[1] as { body?: string; timeoutMs?: number },
19+
};
20+
}
21+
22+
describe("browser hook client actions", () => {
23+
beforeEach(() => vi.clearAllMocks());
24+
25+
it("adds transport slack to an atomic file chooser upload", async () => {
26+
await browserArmFileChooser(undefined, {
27+
paths: ["/tmp/openclaw/uploads/report.pdf"],
28+
ref: "e12",
29+
targetId: "tab-1",
30+
timeoutMs: 30_000,
31+
profile: "openclaw",
32+
});
33+
34+
const call = lastFetchCall();
35+
expect(call.url).toBe("/hooks/file-chooser?profile=openclaw");
36+
expect(call.options.timeoutMs).toBe(35_000);
37+
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
38+
paths: ["/tmp/openclaw/uploads/report.pdf"],
39+
ref: "e12",
40+
targetId: "tab-1",
41+
timeoutMs: 30_000,
42+
});
43+
});
44+
45+
it("preserves paths-only arming with the advertised 120 second default", async () => {
46+
await browserArmFileChooser(undefined, {
47+
paths: ["/tmp/openclaw/uploads/report.pdf"],
48+
});
49+
50+
const call = lastFetchCall();
51+
expect(call.url).toBe("/hooks/file-chooser");
52+
expect(call.options.timeoutMs).toBe(125_000);
53+
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
54+
paths: ["/tmp/openclaw/uploads/report.pdf"],
55+
});
56+
});
57+
58+
it("keeps dialog hook requests alive past their operation timeout", async () => {
59+
await browserArmDialog(undefined, { accept: true, timeoutMs: 45_000 });
60+
61+
const call = lastFetchCall();
62+
expect(call.url).toBe("/hooks/dialog");
63+
expect(call.options.timeoutMs).toBe(50_000);
64+
expect(JSON.parse(call.options.body ?? "{}")).toEqual({
65+
accept: true,
66+
timeoutMs: 45_000,
67+
});
68+
});
69+
});

‎extensions/browser/src/browser/client-actions-core.ts‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ type BrowserActResponse = {
3838
downloads?: BrowserDownloadResult[];
3939
};
4040

41-
const BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS = 5_000;
42-
const BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS = 5_000;
41+
const BROWSER_REQUEST_TIMEOUT_SLACK_MS = 5_000;
4342

4443
type BrowserDownloadActionResult = BrowserActionTabResult & { download: BrowserDownloadResult };
4544

@@ -52,24 +51,23 @@ function resolveBrowserActRequestTimeoutMs(req: BrowserActRequest): number {
5251
const candidateTimeouts =
5352
explicitTimeout === undefined
5453
? [DEFAULT_BROWSER_ACTION_TIMEOUT_MS]
55-
: [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
54+
: [addTimerTimeoutGraceMs(explicitTimeout, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1];
5655
if (req.kind === "wait") {
5756
const waitDuration = normalizePositiveTimeoutMs(req.timeMs);
5857
if (waitDuration !== undefined) {
5958
candidateTimeouts.push(
60-
addTimerTimeoutGraceMs(waitDuration, BROWSER_ACT_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
59+
addTimerTimeoutGraceMs(waitDuration, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1,
6160
);
6261
}
6362
}
6463
return Math.max(...candidateTimeouts);
6564
}
6665

67-
function resolveBrowserDownloadRequestTimeoutMs(timeoutMs: unknown): number {
68-
const waitTimeoutMs =
66+
function resolveBrowserOperationRequestTimeoutMs(timeoutMs: unknown): number {
67+
const operationTimeoutMs =
6968
normalizePositiveTimeoutMs(timeoutMs) ?? DEFAULT_BROWSER_DOWNLOAD_TIMEOUT_MS;
70-
// Keep the HTTP client alive after the Playwright waiter expires so its
71-
// timeout/error response reaches the caller instead of a transport abort.
72-
return addTimerTimeoutGraceMs(waitTimeoutMs, BROWSER_DOWNLOAD_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
69+
// Let the browser operation report its own timeout/error before the client watchdog fires.
70+
return addTimerTimeoutGraceMs(operationTimeoutMs, BROWSER_REQUEST_TIMEOUT_SLACK_MS) ?? 1;
7371
}
7472

7573
async function postDownloadRequest(
@@ -84,7 +82,7 @@ async function postDownloadRequest(
8482
method: "POST",
8583
headers: { "Content-Type": "application/json" },
8684
body: JSON.stringify(body),
87-
timeoutMs: resolveBrowserDownloadRequestTimeoutMs(timeoutMs),
85+
timeoutMs: resolveBrowserOperationRequestTimeoutMs(timeoutMs),
8886
});
8987
}
9088

@@ -129,7 +127,7 @@ export async function browserArmDialog(
129127
targetId: opts.targetId,
130128
timeoutMs: opts.timeoutMs,
131129
}),
132-
timeoutMs: 20000,
130+
timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
133131
});
134132
}
135133

@@ -158,7 +156,7 @@ export async function browserArmFileChooser(
158156
targetId: opts.targetId,
159157
timeoutMs: opts.timeoutMs,
160158
}),
161-
timeoutMs: 20000,
159+
timeoutMs: resolveBrowserOperationRequestTimeoutMs(opts.timeoutMs),
162160
});
163161
}
164162

0 commit comments

Comments
 (0)