From db28dda1206f257e0301c45801d359e48c1bcd72 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 1 Mar 2026 23:16:23 -0800 Subject: [PATCH] fix(cli): let browser start honor --timeout (#31365) * fix(cli): respect browser start timeout option * test(cli): cover browser start timeout propagation * changelog: note browser start timeout propagation fix --- CHANGELOG.md | 1 + .../browser-cli-manage.timeout-option.test.ts | 83 +++++++++++++++++++ src/cli/browser-cli-manage.ts | 14 ++-- 3 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 src/cli/browser-cli-manage.timeout-option.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5992a979a60..be9adf504a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai - Browser/Extension navigation reattach: preserve debugger re-attachment when relay is temporarily disconnected by deferring relay attach events until reconnect/re-announce, reducing post-navigation tab loss. (#28725) Thanks @stone-jin. - Browser/Profile defaults: prefer `openclaw` profile over `chrome` in headless/no-sandbox environments unless an explicit `defaultProfile` is configured. (#14944) Thanks @BenediktSchackenberg. - Browser/Remote CDP ownership checks: skip local-process ownership errors for non-loopback remote CDP profiles when HTTP is reachable but the websocket handshake fails, and surface the remote websocket attach/retry path instead. (#15582) Landed from contributor (#28780) Thanks @stubbi, @bsormagec, @unblockedgamesstudio and @vincentkoc. +- CLI/Browser start timeout: honor `openclaw browser --timeout start` and stop by removing the fixed 15000ms override so slower Chrome startups can use caller-provided timeouts. (#22412, #23427) Thanks @vincentkoc. - Browser/CDP startup diagnostics: include Chrome stderr output and a Linux no-sandbox hint in startup timeout errors so failed launches are easier to diagnose. (#29312) Thanks @veast. - Docker/Image health checks: add Dockerfile `HEALTHCHECK` that probes gateway `GET /healthz` so container runtimes can mark unhealthy instances without requiring auth credentials in the probe command. (#11478) Thanks @U-C4N and @vincentkoc. - Docker/Sandbox bootstrap hardening: make `OPENCLAW_SANDBOX` opt-in parsing explicit (`1|true|yes|on`), support custom Docker socket paths via `OPENCLAW_DOCKER_SOCKET`, defer docker.sock exposure until sandbox prerequisites pass, and reset/roll back persisted sandbox mode to `off` when setup is skipped or partially fails to avoid stale broken sandbox state. (#29974) Thanks @jamtujest and @vincentkoc. diff --git a/src/cli/browser-cli-manage.timeout-option.test.ts b/src/cli/browser-cli-manage.timeout-option.test.ts new file mode 100644 index 00000000000..87af6a24a79 --- /dev/null +++ b/src/cli/browser-cli-manage.timeout-option.test.ts @@ -0,0 +1,83 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { registerBrowserManageCommands } from "./browser-cli-manage.js"; +import type { BrowserParentOpts } from "./browser-cli-shared.js"; + +const mocks = vi.hoisted(() => ({ + callBrowserRequest: vi.fn(async (_opts: unknown, req: { path?: string }) => + req.path === "/" + ? { + enabled: true, + running: true, + pid: 1, + cdpPort: 18800, + chosenBrowser: "chrome", + userDataDir: "/tmp/openclaw", + color: "blue", + headless: true, + attachOnly: false, + } + : {}, + ), + runtime: { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + }, +})); + +vi.mock("./browser-cli-shared.js", () => ({ + callBrowserRequest: mocks.callBrowserRequest, +})); + +vi.mock("./cli-utils.js", () => ({ + runCommandWithRuntime: async ( + _runtime: unknown, + action: () => Promise, + onError: (err: unknown) => void, + ) => { + try { + await action(); + } catch (err) { + onError(err); + } + }, +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: mocks.runtime, +})); + +describe("browser manage start timeout option", () => { + function createProgram() { + const program = new Command(); + const browser = program + .command("browser") + .option("--browser-profile ", "Browser profile") + .option("--json", "Output JSON", false) + .option("--timeout ", "Timeout in ms", "30000"); + const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + registerBrowserManageCommands(browser, parentOpts); + return program; + } + + beforeEach(() => { + mocks.callBrowserRequest.mockClear(); + mocks.runtime.log.mockClear(); + mocks.runtime.error.mockClear(); + mocks.runtime.exit.mockClear(); + }); + + it("uses parent --timeout for browser start instead of hardcoded 15s", async () => { + const program = createProgram(); + await program.parseAsync(["browser", "--timeout", "60000", "start"], { from: "user" }); + + const startCall = mocks.callBrowserRequest.mock.calls.find( + (call) => ((call[1] ?? {}) as { path?: string }).path === "/start", + ) as [Record, { path?: string }, unknown] | undefined; + + expect(startCall).toBeDefined(); + expect(startCall?.[0]).toMatchObject({ timeout: "60000" }); + expect(startCall?.[2]).toBeUndefined(); + }); +}); diff --git a/src/cli/browser-cli-manage.ts b/src/cli/browser-cli-manage.ts index 600d7ac2b4d..cea1ea24cc3 100644 --- a/src/cli/browser-cli-manage.ts +++ b/src/cli/browser-cli-manage.ts @@ -34,15 +34,11 @@ async function runBrowserToggle( parent: BrowserParentOpts, params: { profile?: string; path: string }, ) { - await callBrowserRequest( - parent, - { - method: "POST", - path: params.path, - query: params.profile ? { profile: params.profile } : undefined, - }, - { timeoutMs: 15000 }, - ); + await callBrowserRequest(parent, { + method: "POST", + path: params.path, + query: params.profile ? { profile: params.profile } : undefined, + }); const status = await fetchBrowserStatus(parent, params.profile); if (parent?.json) { defaultRuntime.log(JSON.stringify(status, null, 2));