From f15ec269946140fe6879100ec2eadd66d6fe7cd9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 02:14:50 -0700 Subject: [PATCH] fix(browser): honor inherited timeouts across browser commands (#129176) * fix(browser): honor inherited timeouts across browser commands * test(browser): keep resize timeout regression type-safe --- .../register.navigation.test.ts | 57 +++++++++------- .../register.navigation.ts | 21 +++--- .../cli/browser-cli-actions-observe.test.ts | 18 +++++ .../src/cli/browser-cli-actions-observe.ts | 36 ++++------ .../browser/src/cli/browser-cli-debug.test.ts | 52 +++++++++++++++ .../browser/src/cli/browser-cli-debug.ts | 21 ++---- .../src/cli/browser-cli-inspect.test.ts | 23 ++++++- .../browser/src/cli/browser-cli-inspect.ts | 42 +++++------- .../browser-cli-manage.timeout-option.test.ts | 15 +++++ .../browser/src/cli/browser-cli-manage.ts | 50 ++++++-------- .../browser/src/cli/browser-cli-resize.ts | 19 +++--- .../browser/src/cli/browser-cli-shared.ts | 24 ------- .../cli/browser-cli-state.cookies-storage.ts | 40 +++++------- ...rowser-cli-state.option-collisions.test.ts | 65 +++++++++++++++++++ .../browser/src/cli/browser-cli-state.ts | 37 ++++------- .../browser/src/cli/browser-cli.lazy.test.ts | 30 +++++++++ 16 files changed, 339 insertions(+), 211 deletions(-) create mode 100644 extensions/browser/src/cli/browser-cli-debug.test.ts diff --git a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.test.ts b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.test.ts index b29e99084196..17f6eab78a84 100644 --- a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.test.ts +++ b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.test.ts @@ -37,6 +37,7 @@ const { registerBrowserNavigationCommands } = await import("./register.navigatio function createNavigationProgram(): Command { const { program, browser, parentOpts } = createBrowserProgram(); + browser.option("--timeout ", "Timeout in ms", "30000"); registerBrowserNavigationCommands(browser, parentOpts); return program; } @@ -48,44 +49,54 @@ describe("browser navigation commands", () => { getBrowserCliRuntimeCapture().resetRuntimeCapture(); }); - it("sends navigate requests with the URL and target id", async () => { - const program = createNavigationProgram(); + it.each(["30000", "60000"])( + "sends navigate requests with the URL, target id, and inherited %s ms timeout", + async (timeout) => { + const program = createNavigationProgram(); + const parentArgs = timeout === "30000" ? [] : ["--timeout", timeout]; - await program.parseAsync( - ["browser", "navigate", "https://example.test/page", "--target-id", "tab-1"], - { from: "user" }, - ); + await program.parseAsync( + ["browser", ...parentArgs, "navigate", "https://example.test/page", "--target-id", "tab-1"], + { from: "user" }, + ); - const request = mocks.callBrowserRequest.mock.calls.at(-1)?.[1] as - | { method?: string; path?: string; body?: Record } - | undefined; - const options = mocks.callBrowserRequest.mock.calls.at(-1)?.[2] as - | { timeoutMs?: number } - | undefined; - expect(request).toMatchObject({ - method: "POST", - path: "/navigate", - body: { url: "https://example.test/page", targetId: "tab-1" }, - }); - expect(options?.timeoutMs).toBe(20000); - }); + const request = mocks.callBrowserRequest.mock.calls.at(-1)?.[1] as + | { method?: string; path?: string; body?: Record } + | undefined; + expect(request).toMatchObject({ + method: "POST", + path: "/navigate", + body: { url: "https://example.test/page", targetId: "tab-1" }, + }); + expect(mocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout }), + expect.objectContaining({ path: "/navigate" }), + ); + }, + ); it("passes normalized resize dimensions and target id to the resize helper", async () => { const program = createNavigationProgram(); - await program.parseAsync(["browser", "resize", "1024", "768", "--target-id", "tab-2"], { - from: "user", - }); + await program.parseAsync( + ["browser", "--timeout", "60000", "resize", "1024", "768", "--target-id", "tab-2"], + { + from: "user", + }, + ); expect(mocks.runBrowserResizeWithOutput).toHaveBeenCalledWith( expect.objectContaining({ width: 1024, height: 768, targetId: "tab-2", - timeoutMs: 20000, + parent: expect.objectContaining({ timeout: "60000" }), successMessage: "resized to 1024x768", }), ); + expect(mocks.runBrowserResizeWithOutput).not.toHaveBeenCalledWith( + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); }); it("rejects non-decimal resize dimensions before dispatch", async () => { diff --git a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts index 0fe65f91b842..169ab1d77e26 100644 --- a/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts +++ b/extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts @@ -28,19 +28,15 @@ export function registerBrowserNavigationCommands( .action(async (url: string, opts, cmd) => { const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); try { - const result = await callBrowserRequest<{ url?: string }>( - parent, - { - method: "POST", - path: "/navigate", - query: profile ? { profile } : undefined, - body: { - url, - targetId: normalizeOptionalString(opts.targetId), - }, + const result = await callBrowserRequest<{ url?: string }>(parent, { + method: "POST", + path: "/navigate", + query: profile ? { profile } : undefined, + body: { + url, + targetId: normalizeOptionalString(opts.targetId), }, - { timeoutMs: 20000 }, - ); + }); if (parent?.json) { defaultRuntime.writeJson(result); return; @@ -72,7 +68,6 @@ export function registerBrowserNavigationCommands( width: normalizedWidth, height: normalizedHeight, targetId: opts.targetId, - timeoutMs: 20000, successMessage: `resized to ${normalizedWidth}x${normalizedHeight}`, }); } catch (err) { diff --git a/extensions/browser/src/cli/browser-cli-actions-observe.test.ts b/extensions/browser/src/cli/browser-cli-actions-observe.test.ts index 5cf4348b13ee..76481ef9817f 100644 --- a/extensions/browser/src/cli/browser-cli-actions-observe.test.ts +++ b/extensions/browser/src/cli/browser-cli-actions-observe.test.ts @@ -32,6 +32,7 @@ const { registerBrowserActionObserveCommands } = await import("./browser-cli-act function createActionObserveProgram(): Command { const { program, browser, parentOpts } = createBrowserProgram(); + browser.option("--timeout ", "Timeout in ms", "30000"); registerBrowserActionObserveCommands(browser, parentOpts); return program; } @@ -42,6 +43,23 @@ describe("browser action observe commands", () => { getBrowserCliRuntimeCapture().resetRuntimeCapture(); }); + it.each([ + { command: "console", path: "/console", timeout: "30000" }, + { command: "console", path: "/console", timeout: "60000" }, + { command: "pdf", path: "/pdf", timeout: "30000" }, + { command: "pdf", path: "/pdf", timeout: "60000" }, + ])("inherits parent $timeout ms timeout for $command", async ({ command, path, timeout }) => { + const program = createActionObserveProgram(); + const parentArgs = timeout === "30000" ? ["--json"] : ["--json", "--timeout", timeout]; + + await program.parseAsync(["browser", ...parentArgs, command], { from: "user" }); + + expect(mocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout }), + expect.objectContaining({ path }), + ); + }); + it("rejects non-decimal responsebody numeric flags before dispatch", async () => { const program = createActionObserveProgram(); diff --git a/extensions/browser/src/cli/browser-cli-actions-observe.ts b/extensions/browser/src/cli/browser-cli-actions-observe.ts index 8d69e3f5f7f2..6922304dc80c 100644 --- a/extensions/browser/src/cli/browser-cli-actions-observe.ts +++ b/extensions/browser/src/cli/browser-cli-actions-observe.ts @@ -43,19 +43,15 @@ export function registerBrowserActionObserveCommands( const parent = parentOpts(cmd); const profile = parent?.browserProfile; await runBrowserObserve(async () => { - const result = await callBrowserRequest<{ messages: unknown[] }>( - parent, - { - method: "GET", - path: "/console", - query: { - level: normalizeOptionalString(opts.level), - targetId: normalizeOptionalString(opts.targetId), - profile, - }, + const result = await callBrowserRequest<{ messages: unknown[] }>(parent, { + method: "GET", + path: "/console", + query: { + level: normalizeOptionalString(opts.level), + targetId: normalizeOptionalString(opts.targetId), + profile, }, - { timeoutMs: 20000 }, - ); + }); if (printBrowserJsonResult(parent, result)) { return; } @@ -71,16 +67,12 @@ export function registerBrowserActionObserveCommands( const parent = parentOpts(cmd); const profile = parent?.browserProfile; await runBrowserObserve(async () => { - const result = await callBrowserRequest<{ path: string }>( - parent, - { - method: "POST", - path: "/pdf", - query: profile ? { profile } : undefined, - body: { targetId: normalizeOptionalString(opts.targetId) }, - }, - { timeoutMs: 20000 }, - ); + const result = await callBrowserRequest<{ path: string }>(parent, { + method: "POST", + path: "/pdf", + query: profile ? { profile } : undefined, + body: { targetId: normalizeOptionalString(opts.targetId) }, + }); if (printBrowserJsonResult(parent, result)) { return; } diff --git a/extensions/browser/src/cli/browser-cli-debug.test.ts b/extensions/browser/src/cli/browser-cli-debug.test.ts new file mode 100644 index 000000000000..98a9da27dd33 --- /dev/null +++ b/extensions/browser/src/cli/browser-cli-debug.test.ts @@ -0,0 +1,52 @@ +// Browser tests cover browser cli debug plugin behavior. +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as browserCliSharedModule from "./browser-cli-shared.js"; +import { + createBrowserProgram, + getBrowserCliRuntime, + getBrowserCliRuntimeCapture, +} from "./browser-cli.test-support.js"; +import * as cliCoreApiModule from "./core-api.js"; + +const mocks = vi.hoisted(() => ({ + callBrowserRequest: vi.fn(async (..._args: unknown[]) => ({ ok: true })), +})); + +vi.spyOn(browserCliSharedModule, "callBrowserRequest").mockImplementation(mocks.callBrowserRequest); +const browserCliRuntime = getBrowserCliRuntime(); +vi.spyOn(cliCoreApiModule.defaultRuntime, "writeJson").mockImplementation( + browserCliRuntime.writeJson, +); +vi.spyOn(cliCoreApiModule.defaultRuntime, "error").mockImplementation(browserCliRuntime.error); +vi.spyOn(cliCoreApiModule.defaultRuntime, "exit").mockImplementation(browserCliRuntime.exit); + +const { registerBrowserDebugCommands } = await import("./browser-cli-debug.js"); + +describe("browser debug command timeouts", () => { + beforeEach(() => { + mocks.callBrowserRequest.mockClear(); + getBrowserCliRuntimeCapture().resetRuntimeCapture(); + }); + + it.each([ + { args: ["highlight", "e1"], path: "/highlight" }, + { args: ["errors"], path: "/errors" }, + { args: ["requests"], path: "/requests" }, + { args: ["trace", "start"], path: "/trace/start" }, + { args: ["trace", "stop"], path: "/trace/stop" }, + ])("inherits the parent timeout for $path", async ({ args, path }) => { + for (const timeout of ["30000", "60000"]) { + const { program, browser, parentOpts } = createBrowserProgram(); + browser.option("--timeout ", "Timeout in ms", "30000"); + registerBrowserDebugCommands(browser, parentOpts); + const parentArgs = timeout === "30000" ? ["--json"] : ["--json", "--timeout", timeout]; + + await program.parseAsync(["browser", ...parentArgs, ...args], { from: "user" }); + + expect(mocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout }), + expect.objectContaining({ path }), + ); + } + }); +}); diff --git a/extensions/browser/src/cli/browser-cli-debug.ts b/extensions/browser/src/cli/browser-cli-debug.ts index 3d5ad0228bed..11a9b514e86f 100644 --- a/extensions/browser/src/cli/browser-cli-debug.ts +++ b/extensions/browser/src/cli/browser-cli-debug.ts @@ -13,10 +13,6 @@ import { } from "./browser-cli-shared.js"; import { defaultRuntime, shortenHomePath } from "./core-api.js"; -const BROWSER_DEBUG_TIMEOUT_MS = 20000; - -type BrowserRequestParams = Parameters[1]; - type DebugContext = { parent: BrowserParentOpts; profile?: string; @@ -36,13 +32,6 @@ async function withDebugContext( ); } -async function callDebugRequest( - parent: BrowserParentOpts, - params: BrowserRequestParams, -): Promise { - return callBrowserRequest(parent, params, { timeoutMs: BROWSER_DEBUG_TIMEOUT_MS }); -} - function resolveDebugQuery(params: { targetId?: unknown; clear?: unknown; @@ -69,7 +58,7 @@ export function registerBrowserDebugCommands( .option("--target-id ", BROWSER_TAB_REFERENCE_HELP) .action(async (ref: string, opts, cmd) => { await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { - const result = await callDebugRequest(parent, { + const result = await callBrowserRequest(parent, { method: "POST", path: "/highlight", query: resolveProfileQuery(profile), @@ -92,7 +81,7 @@ export function registerBrowserDebugCommands( .option("--target-id ", BROWSER_TAB_REFERENCE_HELP) .action(async (opts, cmd) => { await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { - const result = await callDebugRequest<{ + const result = await callBrowserRequest<{ errors: Array<{ timestamp: string; name?: string; message: string }>; }>(parent, { method: "GET", @@ -126,7 +115,7 @@ export function registerBrowserDebugCommands( .option("--target-id ", BROWSER_TAB_REFERENCE_HELP) .action(async (opts, cmd) => { await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { - const result = await callDebugRequest<{ + const result = await callBrowserRequest<{ requests: Array<{ timestamp: string; method: string; @@ -176,7 +165,7 @@ export function registerBrowserDebugCommands( .option("--sources", "Include sources (bigger traces)", false) .action(async (opts, cmd) => { await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { - const result = await callDebugRequest(parent, { + const result = await callBrowserRequest(parent, { method: "POST", path: "/trace/start", query: resolveProfileQuery(profile), @@ -204,7 +193,7 @@ export function registerBrowserDebugCommands( .option("--target-id ", BROWSER_TAB_REFERENCE_HELP) .action(async (opts, cmd) => { await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { - const result = await callDebugRequest<{ path: string }>(parent, { + const result = await callBrowserRequest<{ path: string }>(parent, { method: "POST", path: "/trace/stop", query: resolveProfileQuery(profile), diff --git a/extensions/browser/src/cli/browser-cli-inspect.test.ts b/extensions/browser/src/cli/browser-cli-inspect.test.ts index a0ff9b885ee3..d090a4af013c 100644 --- a/extensions/browser/src/cli/browser-cli-inspect.test.ts +++ b/extensions/browser/src/cli/browser-cli-inspect.test.ts @@ -95,7 +95,10 @@ function installInspectSpies() { describe("browser cli snapshot defaults", () => { const runBrowserInspect = async (args: string[], withJson = false) => { const program = new Command(); - const browser = program.command("browser").option("--json", "JSON output", false); + const browser = program + .command("browser") + .option("--json", "JSON output", false) + .option("--timeout ", "Timeout in ms", "30000"); registerBrowserInspectCommands(browser, (cmd) => cmd.parent?.opts() ?? {}); await program.parseAsync(withJson ? ["browser", "--json", ...args] : ["browser", ...args], { from: "user", @@ -123,6 +126,24 @@ describe("browser cli snapshot defaults", () => { configMocks.loadConfig.mockReturnValue({ browser: {} }); }); + it.each([ + { command: "screenshot", requestPath: "/screenshot", timeout: "30000" }, + { command: "screenshot", requestPath: "/screenshot", timeout: "60000" }, + { command: "snapshot", requestPath: "/snapshot", timeout: "30000" }, + { command: "snapshot", requestPath: "/snapshot", timeout: "60000" }, + ])( + "inherits parent $timeout ms timeout for $command", + async ({ command, requestPath, timeout }) => { + const args = timeout === "30000" ? [command] : ["--timeout", timeout, command]; + await runBrowserInspect(args, true); + + expect(sharedMocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout }), + expect.objectContaining({ path: requestPath }), + ); + }, + ); + it.each([ { label: "uses config snapshot defaults when mode is not provided", diff --git a/extensions/browser/src/cli/browser-cli-inspect.ts b/extensions/browser/src/cli/browser-cli-inspect.ts index 1ffcdec8d381..9c37095e981b 100644 --- a/extensions/browser/src/cli/browser-cli-inspect.ts +++ b/extensions/browser/src/cli/browser-cli-inspect.ts @@ -80,23 +80,19 @@ export function registerBrowserInspectCommands( return; } try { - const result = await callBrowserRequest<{ path: string }>( - parent, - { - method: "POST", - path: "/screenshot", - query: profile ? { profile } : undefined, - body: { - targetId: normalizeOptionalString(targetId), - fullPage: Boolean(opts.fullPage), - ref: normalizeOptionalString(opts.ref), - element: normalizeOptionalString(opts.element), - labels: Boolean(opts.labels), - type, - }, + const result = await callBrowserRequest<{ path: string }>(parent, { + method: "POST", + path: "/screenshot", + query: profile ? { profile } : undefined, + body: { + targetId: normalizeOptionalString(targetId), + fullPage: Boolean(opts.fullPage), + ref: normalizeOptionalString(opts.ref), + element: normalizeOptionalString(opts.element), + labels: Boolean(opts.labels), + type, }, - { timeoutMs: 20000 }, - ); + }); if (parent?.json) { defaultRuntime.writeJson(result); return; @@ -170,15 +166,11 @@ export function registerBrowserInspectCommands( mode, profile, }; - const result = await callBrowserRequest( - parent, - { - method: "GET", - path: "/snapshot", - query, - }, - { timeoutMs: 20000 }, - ); + const result = await callBrowserRequest(parent, { + method: "GET", + path: "/snapshot", + query, + }); if (opts.out) { const payload = diff --git a/extensions/browser/src/cli/browser-cli-manage.timeout-option.test.ts b/extensions/browser/src/cli/browser-cli-manage.timeout-option.test.ts index f6d137b9bf0b..2b46799fcac9 100644 --- a/extensions/browser/src/cli/browser-cli-manage.timeout-option.test.ts +++ b/extensions/browser/src/cli/browser-cli-manage.timeout-option.test.ts @@ -25,6 +25,21 @@ describe("browser manage start timeout option", () => { expect(startCall[2]).toBeUndefined(); }); + it.each([ + { args: ["reset-profile"], path: "/reset-profile" }, + { args: ["create-profile", "--name", "work"], path: "/profiles/create" }, + { args: ["delete-profile", "--name", "work"], path: "/profiles/work" }, + ])("inherits parent --timeout for $path", async ({ args, path }) => { + const program = createBrowserManageProgram({ withParentTimeout: true }); + await program.parseAsync(["browser", "--timeout", "60000", "--json", ...args], { + from: "user", + }); + + const request = findBrowserManageCall(path); + expect(request?.[0]).toEqual(expect.objectContaining({ timeout: "60000" })); + expect(request?.[2]).toBeUndefined(); + }); + it("passes headless=true for browser start --headless", async () => { const program = createBrowserManageProgram({ withParentTimeout: true }); await program.parseAsync(["browser", "start", "--headless"], { from: "user" }); diff --git a/extensions/browser/src/cli/browser-cli-manage.ts b/extensions/browser/src/cli/browser-cli-manage.ts index 99fcc1db4d58..e108f16dfe3a 100644 --- a/extensions/browser/src/cli/browser-cli-manage.ts +++ b/extensions/browser/src/cli/browser-cli-manage.ts @@ -429,15 +429,11 @@ export function registerBrowserManageCommands( const parent = parentOpts(cmd); const profile = parent?.browserProfile; await runBrowserCommand(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/reset-profile", - query: resolveProfileQuery(profile), - }, - { timeoutMs: 20000 }, - ); + const result = await callBrowserRequest(parent, { + method: "POST", + path: "/reset-profile", + query: resolveProfileQuery(profile), + }); if (printJsonResult(parent, result)) { return; } @@ -817,21 +813,17 @@ export function registerBrowserManageCommands( ) { throw new Error("--driver must be openclaw or existing-session"); } - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/profiles/create", - body: { - name: opts.name, - color: opts.color, - cdpUrl: opts.cdpUrl, - userDataDir: opts.userDataDir, - driver: opts.driver === "existing-session" ? "existing-session" : undefined, - }, + const result = await callBrowserRequest(parent, { + method: "POST", + path: "/profiles/create", + body: { + name: opts.name, + color: opts.color, + cdpUrl: opts.cdpUrl, + userDataDir: opts.userDataDir, + driver: opts.driver === "existing-session" ? "existing-session" : undefined, }, - { timeoutMs: 10_000 }, - ); + }); if (printJsonResult(parent, result)) { return; } @@ -854,14 +846,10 @@ export function registerBrowserManageCommands( .action(async (opts: { name: string }, cmd) => { const parent = parentOpts(cmd); await runBrowserCommand(async () => { - const result = await callBrowserRequest( - parent, - { - method: "DELETE", - path: `/profiles/${encodeURIComponent(opts.name)}`, - }, - { timeoutMs: 20_000 }, - ); + const result = await callBrowserRequest(parent, { + method: "DELETE", + path: `/profiles/${encodeURIComponent(opts.name)}`, + }); if (printJsonResult(parent, result)) { return; } diff --git a/extensions/browser/src/cli/browser-cli-resize.ts b/extensions/browser/src/cli/browser-cli-resize.ts index 2fdc0531dc80..9579b860a2cc 100644 --- a/extensions/browser/src/cli/browser-cli-resize.ts +++ b/extensions/browser/src/cli/browser-cli-resize.ts @@ -1,9 +1,10 @@ /** * Shared Browser CLI resize runner used by resize and set viewport commands. */ +import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { ACT_MAX_VIEWPORT_DIMENSION } from "../browser/act-policy.js"; import { - callBrowserResize, + callBrowserRequest, parseBrowserPositiveIntegerValue, type BrowserParentOpts, } from "./browser-cli-shared.js"; @@ -31,7 +32,6 @@ export async function runBrowserResizeWithOutput(params: { width: number; height: number; targetId?: string; - timeoutMs?: number; successMessage: string; }): Promise { const { width, height } = params; @@ -46,16 +46,17 @@ export async function runBrowserResizeWithOutput(params: { return; } - const result = await callBrowserResize( - params.parent, - { - profile: params.profile, + const result = await callBrowserRequest(params.parent, { + method: "POST", + path: "/act", + query: params.profile ? { profile: params.profile } : undefined, + body: { + kind: "resize", width, height, - targetId: params.targetId, + targetId: normalizeOptionalString(params.targetId), }, - { timeoutMs: params.timeoutMs ?? 20000 }, - ); + }); if (params.parent?.json) { defaultRuntime.writeJson(result); diff --git a/extensions/browser/src/cli/browser-cli-shared.ts b/extensions/browser/src/cli/browser-cli-shared.ts index 9b9b66412e49..f519555eaed5 100644 --- a/extensions/browser/src/cli/browser-cli-shared.ts +++ b/extensions/browser/src/cli/browser-cli-shared.ts @@ -5,7 +5,6 @@ import { parseStrictNonNegativeInteger, parseStrictPositiveInteger, } from "openclaw/plugin-sdk/number-runtime"; -import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { BROWSER_REQUEST_GATEWAY_METHOD, BROWSER_REQUEST_GATEWAY_SCOPES, @@ -129,26 +128,3 @@ export async function callBrowserRequest( } return payload as T; } - -/** Sends a Browser resize action through the shared request helper. */ -export async function callBrowserResize( - opts: BrowserParentOpts, - params: { profile?: string; width: number; height: number; targetId?: string }, - extra?: { timeoutMs?: number }, -): Promise { - return callBrowserRequest( - opts, - { - method: "POST", - path: "/act", - query: params.profile ? { profile: params.profile } : undefined, - body: { - kind: "resize", - width: params.width, - height: params.height, - targetId: normalizeOptionalString(params.targetId), - }, - }, - extra, - ); -} diff --git a/extensions/browser/src/cli/browser-cli-state.cookies-storage.ts b/extensions/browser/src/cli/browser-cli-state.cookies-storage.ts index 7866dea3ff12..0f90df6fda7a 100644 --- a/extensions/browser/src/cli/browser-cli-state.cookies-storage.ts +++ b/extensions/browser/src/cli/browser-cli-state.cookies-storage.ts @@ -27,7 +27,7 @@ async function runMutationRequest(params: { successMessage: string; }) { try { - const result = await callBrowserRequest(params.parent, params.request, { timeoutMs: 20000 }); + const result = await callBrowserRequest(params.parent, params.request); if (params.parent?.json) { defaultRuntime.writeJson(result); return; @@ -51,18 +51,14 @@ export function registerBrowserCookiesAndStorageCommands( const profile = parent?.browserProfile; const targetId = resolveTargetId(opts.targetId, cmd); try { - const result = await callBrowserRequest<{ cookies?: unknown[] }>( - parent, - { - method: "GET", - path: "/cookies", - query: { - targetId, - profile, - }, + const result = await callBrowserRequest<{ cookies?: unknown[] }>(parent, { + method: "GET", + path: "/cookies", + query: { + targetId, + profile, }, - { timeoutMs: 20000 }, - ); + }); if (parent?.json) { defaultRuntime.writeJson(result); return; @@ -143,19 +139,15 @@ export function registerBrowserCookiesAndStorageCommands( const profile = parent?.browserProfile; const targetId = resolveTargetId(opts.targetId, cmd2); try { - const result = await callBrowserRequest<{ values?: Record }>( - parent, - { - method: "GET", - path: `/storage/${kind}`, - query: { - key: normalizeOptionalString(key), - targetId, - profile, - }, + const result = await callBrowserRequest<{ values?: Record }>(parent, { + method: "GET", + path: `/storage/${kind}`, + query: { + key: normalizeOptionalString(key), + targetId, + profile, }, - { timeoutMs: 20000 }, - ); + }); if (parent?.json) { defaultRuntime.writeJson(result); return; diff --git a/extensions/browser/src/cli/browser-cli-state.option-collisions.test.ts b/extensions/browser/src/cli/browser-cli-state.option-collisions.test.ts index abe44d4307d1..23dc4e105990 100644 --- a/extensions/browser/src/cli/browser-cli-state.option-collisions.test.ts +++ b/extensions/browser/src/cli/browser-cli-state.option-collisions.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ runBrowserResizeWithOutput: vi.fn(async (_params: unknown) => {}), })); +const runActualBrowserResizeWithOutput = browserCliResizeModule.runBrowserResizeWithOutput; vi.spyOn(browserCliSharedModule, "callBrowserRequest").mockImplementation(mocks.callBrowserRequest); vi.spyOn(browserCliResizeModule, "runBrowserResizeWithOutput").mockImplementation( mocks.runBrowserResizeWithOutput, @@ -44,6 +45,7 @@ describe("browser state option collisions", () => { const createStateProgram = ({ withGatewayUrl = false } = {}) => { const { program, browser, parentOpts } = createBrowserProgramShared({ withGatewayUrl }); + browser.option("--timeout ", "Timeout in ms", "30000"); registerBrowserStateCommands(browser, parentOpts); return program; }; @@ -80,6 +82,69 @@ describe("browser state option collisions", () => { getBrowserCliRuntime().exit.mockImplementation(() => {}); }); + it.each([ + { args: ["cookies"], path: "/cookies" }, + { + args: ["cookies", "set", "session", "value", "--url", "https://example.com"], + path: "/cookies/set", + }, + { args: ["cookies", "clear"], path: "/cookies/clear" }, + { args: ["storage", "local", "get"], path: "/storage/local" }, + { args: ["storage", "local", "set", "key", "value"], path: "/storage/local/set" }, + { args: ["storage", "local", "clear"], path: "/storage/local/clear" }, + { args: ["storage", "session", "get"], path: "/storage/session" }, + { args: ["storage", "session", "set", "key", "value"], path: "/storage/session/set" }, + { args: ["storage", "session", "clear"], path: "/storage/session/clear" }, + { args: ["set", "offline", "on"], path: "/set/offline" }, + { args: ["set", "headers", "{}"], path: "/set/headers" }, + { args: ["set", "credentials", "name", "value"], path: "/set/credentials" }, + { args: ["set", "geo", "48", "16"], path: "/set/geolocation" }, + { args: ["set", "media", "dark"], path: "/set/media" }, + { args: ["set", "timezone", "UTC"], path: "/set/timezone" }, + { args: ["set", "locale", "en-US"], path: "/set/locale" }, + { args: ["set", "device", "iPhone 14"], path: "/set/device" }, + ])("inherits parent timeout for $path", async ({ args, path }) => { + await runBrowserCommand(["--timeout", "60000", "--json", ...args]); + + expect(mocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout: "60000" }), + expect.objectContaining({ path }), + ); + }); + + it("inherits the parent timeout for the viewport resize alias", async () => { + await runBrowserCommand(["--timeout", "60000", "set", "viewport", "1024", "768"]); + + expect(mocks.runBrowserResizeWithOutput).toHaveBeenCalledWith( + expect.objectContaining({ + parent: expect.objectContaining({ timeout: "60000" }), + width: 1024, + height: 768, + }), + ); + expect(mocks.runBrowserResizeWithOutput.mock.calls.at(-1)?.[0]).not.toHaveProperty("timeoutMs"); + }); + + it("keeps the parent timeout and normalized target at the shared resize request boundary", async () => { + await runActualBrowserResizeWithOutput({ + parent: { timeout: "60000", json: true }, + profile: "work", + width: 1024, + height: 768, + targetId: " tab-1 ", + successMessage: "unused", + }); + + expect(mocks.callBrowserRequest).toHaveBeenLastCalledWith( + expect.objectContaining({ timeout: "60000" }), + expect.objectContaining({ + path: "/act", + query: { profile: "work" }, + body: { kind: "resize", width: 1024, height: 768, targetId: "tab-1" }, + }), + ); + }); + it("forwards parent-captured --target-id on `browser cookies set`", async () => { const request = await runBrowserCommandAndGetRequest([ "cookies", diff --git a/extensions/browser/src/cli/browser-cli-state.ts b/extensions/browser/src/cli/browser-cli-state.ts index 6f670183523f..812033ae199a 100644 --- a/extensions/browser/src/cli/browser-cli-state.ts +++ b/extensions/browser/src/cli/browser-cli-state.ts @@ -45,16 +45,12 @@ async function runBrowserSetRequest(params: { }) { await runBrowserCommand(async () => { const profile = params.parent?.browserProfile; - const result = await callBrowserRequest( - params.parent, - { - method: "POST", - path: params.path, - query: profile ? { profile } : undefined, - body: params.body, - }, - { timeoutMs: 20000 }, - ); + const result = await callBrowserRequest(params.parent, { + method: "POST", + path: params.path, + query: profile ? { profile } : undefined, + body: params.body, + }); if (printBrowserJsonResult(params.parent, result)) { return; } @@ -92,7 +88,6 @@ export function registerBrowserStateCommands( width, height, targetId: opts.targetId, - timeoutMs: 20000, successMessage: `viewport set: ${width}x${height}`, }); }); @@ -147,19 +142,15 @@ export function registerBrowserStateCommands( } } const profile = parent?.browserProfile; - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/set/headers", - query: profile ? { profile } : undefined, - body: { - headers, - targetId: normalizeOptionalString(opts.targetId), - }, + const result = await callBrowserRequest(parent, { + method: "POST", + path: "/set/headers", + query: profile ? { profile } : undefined, + body: { + headers, + targetId: normalizeOptionalString(opts.targetId), }, - { timeoutMs: 20000 }, - ); + }); if (printBrowserJsonResult(parent, result)) { return; } diff --git a/extensions/browser/src/cli/browser-cli.lazy.test.ts b/extensions/browser/src/cli/browser-cli.lazy.test.ts index 9865ca2a92e4..c59e12fc20da 100644 --- a/extensions/browser/src/cli/browser-cli.lazy.test.ts +++ b/extensions/browser/src/cli/browser-cli.lazy.test.ts @@ -373,6 +373,36 @@ describe("registerBrowserCli lazy browser subcommands", () => { expect(tabsCommand.parent?.opts().browserProfile).toBe("remote"); }); + it.each([ + ["before", ["browser", "--timeout", "60000", "status"]], + ["after", ["browser", "status", "--timeout", "60000"]], + ])( + "preserves parent timeout %s a lazily loaded leaf in positional mode", + async (_place, args) => { + const program = new Command().name("openclaw").enablePositionalOptions(); + registerBrowserCli(program, ["node", "openclaw", ...args]); + + await program.parseAsync(args, { from: "user" }); + + const command = requireTrailingCommand( + requireFirstCall(manageMocks.statusAction, "status action call"), + "status action", + ); + expect(command.parent?.opts().timeout).toBe("60000"); + }, + ); + + it("preserves parent timeout before a nested lazily loaded storage-family leaf", async () => { + const program = new Command().name("openclaw").enablePositionalOptions(); + const args = ["browser", "--timeout", "60000", "cookies", "set", "session", "abc"]; + registerBrowserCli(program, ["node", "openclaw", ...args]); + + await program.parseAsync(args, { from: "user" }); + + const cookieCall = requireFirstCall(stateMocks.cookieSetAction, "cookie set action call"); + expect(cookieCall[3]).toMatchObject({ timeout: "60000" }); + }); + it("skips browser option values when selecting the lazy command group", async () => { const program = new Command(); program.name("openclaw");