From 985ec71c553837bd7084d3a64f6f14bfc80856b2 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Tue, 17 Feb 2026 20:57:09 -0500 Subject: [PATCH] CLI: resolve parent/subcommand option collisions (#18725) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: b7e51cf90950cdd3049ac3c7a3a949717b8ba261 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + docs/tools/browser.md | 4 +- src/cli/acp-cli.option-collisions.test.ts | 45 +++++ src/cli/acp-cli.ts | 6 +- src/cli/browser-cli-state.cookies-storage.ts | 32 +++- ...rowser-cli-state.option-collisions.test.ts | 87 ++++++++++ src/cli/browser-cli-state.ts | 15 +- src/cli/command-options.test.ts | 71 ++++++++ src/cli/command-options.ts | 36 ++++ .../register-service-commands.test.ts | 72 ++++++++ .../daemon-cli/register-service-commands.ts | 35 +++- .../register.option-collisions.test.ts | 160 ++++++++++++++++++ src/cli/gateway-cli/register.ts | 40 +++-- .../gateway-cli/run.option-collisions.test.ts | 143 ++++++++++++++++ src/cli/gateway-cli/run.ts | 53 +++++- src/cli/update-cli.option-collisions.test.ts | 68 ++++++++ src/cli/update-cli.ts | 26 ++- 17 files changed, 856 insertions(+), 38 deletions(-) create mode 100644 src/cli/acp-cli.option-collisions.test.ts create mode 100644 src/cli/browser-cli-state.option-collisions.test.ts create mode 100644 src/cli/command-options.test.ts create mode 100644 src/cli/daemon-cli/register-service-commands.test.ts create mode 100644 src/cli/gateway-cli/register.option-collisions.test.ts create mode 100644 src/cli/gateway-cli/run.option-collisions.test.ts create mode 100644 src/cli/update-cli.option-collisions.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index da57a44180..fcc3e2aff6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,7 @@ Docs: https://docs.openclaw.ai - Discord: optimize reaction notification handling to skip unnecessary message fetches in `off`/`all`/`allowlist` modes, streamline reaction routing, and improve reaction emoji formatting. (#18248) Thanks @thewilloftheshadow and @victorGPT. - CLI/Pairing: make `openclaw qr --remote` prefer `gateway.remote.url` over tailscale/public URL resolution and register the `openclaw clawbot qr` legacy alias path. (#18091) - CLI/QR: restore fail-fast validation for `openclaw qr --remote` when neither `gateway.remote.url` nor tailscale `serve`/`funnel` is configured, preventing unusable remote pairing QR flows. (#18166) Thanks @mbelinky. +- CLI: fix parent/subcommand option collisions across gateway, daemon, update, ACP, and browser command flows, while preserving legacy `browser set headers --json ` compatibility. - CLI/Doctor: ensure `openclaw doctor --fix --non-interactive --yes` exits promptly after completion so one-shot automation no longer hangs. (#18502) - CLI/Doctor: auto-repair `dmPolicy="open"` configs missing wildcard allowlists and write channel-correct repair paths (including `channels.googlechat.dm.allowFrom`) so `openclaw doctor --fix` no longer leaves Google Chat configs invalid after attempted repair. (#18544) - CLI/Doctor: detect gateway service token drift when the gateway token is only provided via environment variables, keeping service repairs aligned after token rotation. diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 74f4247243..4d8492f215 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -430,7 +430,7 @@ State: - `openclaw browser storage local set theme dark` - `openclaw browser storage session clear` - `openclaw browser set offline on` -- `openclaw browser set headers --json '{"X-Debug":"1"}'` +- `openclaw browser set headers --headers-json '{"X-Debug":"1"}'` - `openclaw browser set credentials user pass` - `openclaw browser set credentials --clear` - `openclaw browser set geo 37.7749 -122.4194 --origin "https://example.com"` @@ -542,7 +542,7 @@ These are useful for “make the site behave like X” workflows: - Cookies: `cookies`, `cookies set`, `cookies clear` - Storage: `storage local|session get|set|clear` - Offline: `set offline on|off` -- Headers: `set headers --json '{"X-Debug":"1"}'` (or `--clear`) +- Headers: `set headers --headers-json '{"X-Debug":"1"}'` (legacy `set headers --json '{"X-Debug":"1"}'` remains supported) - HTTP basic auth: `set credentials user pass` (or `--clear`) - Geolocation: `set geo --origin "https://example.com"` (or `--clear`) - Media: `set media dark|light|no-preference|none` diff --git a/src/cli/acp-cli.option-collisions.test.ts b/src/cli/acp-cli.option-collisions.test.ts new file mode 100644 index 0000000000..240ade624a --- /dev/null +++ b/src/cli/acp-cli.option-collisions.test.ts @@ -0,0 +1,45 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const runAcpClientInteractive = vi.fn(async () => {}); +const serveAcpGateway = vi.fn(async () => {}); + +const defaultRuntime = { + error: vi.fn(), + exit: vi.fn(), +}; + +vi.mock("../acp/client.js", () => ({ + runAcpClientInteractive: (opts: unknown) => runAcpClientInteractive(opts), +})); + +vi.mock("../acp/server.js", () => ({ + serveAcpGateway: (opts: unknown) => serveAcpGateway(opts), +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime, +})); + +describe("acp cli option collisions", () => { + beforeEach(() => { + runAcpClientInteractive.mockClear(); + serveAcpGateway.mockClear(); + defaultRuntime.error.mockClear(); + defaultRuntime.exit.mockClear(); + }); + + it("forwards --verbose to `acp client` when parent and child option names collide", async () => { + const { registerAcpCli } = await import("./acp-cli.js"); + const program = new Command(); + registerAcpCli(program); + + await program.parseAsync(["acp", "client", "--verbose"], { from: "user" }); + + expect(runAcpClientInteractive).toHaveBeenCalledWith( + expect.objectContaining({ + verbose: true, + }), + ); + }); +}); diff --git a/src/cli/acp-cli.ts b/src/cli/acp-cli.ts index c86deb48f2..978a9c6fda 100644 --- a/src/cli/acp-cli.ts +++ b/src/cli/acp-cli.ts @@ -4,6 +4,7 @@ import { serveAcpGateway } from "../acp/server.js"; import { defaultRuntime } from "../runtime.js"; import { formatDocsLink } from "../terminal/links.js"; import { theme } from "../terminal/theme.js"; +import { inheritOptionFromParent } from "./command-options.js"; export function registerAcpCli(program: Command) { const acp = program.command("acp").description("Run an ACP bridge backed by the Gateway"); @@ -49,14 +50,15 @@ export function registerAcpCli(program: Command) { .option("--server-args ", "Extra arguments for the ACP server") .option("--server-verbose", "Enable verbose logging on the ACP server", false) .option("--verbose, -v", "Verbose client logging", false) - .action(async (opts) => { + .action(async (opts, command) => { + const inheritedVerbose = inheritOptionFromParent(command, "verbose"); try { await runAcpClientInteractive({ cwd: opts.cwd as string | undefined, serverCommand: opts.server as string | undefined, serverArgs: opts.serverArgs as string[] | undefined, serverVerbose: Boolean(opts.serverVerbose), - verbose: Boolean(opts.verbose), + verbose: Boolean(opts.verbose || inheritedVerbose), }); } catch (err) { defaultRuntime.error(String(err)); diff --git a/src/cli/browser-cli-state.cookies-storage.ts b/src/cli/browser-cli-state.cookies-storage.ts index 47b4eec752..d71cb9a043 100644 --- a/src/cli/browser-cli-state.cookies-storage.ts +++ b/src/cli/browser-cli-state.cookies-storage.ts @@ -2,6 +2,20 @@ import type { Command } from "commander"; import { danger } from "../globals.js"; import { defaultRuntime } from "../runtime.js"; import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js"; +import { inheritOptionFromParent } from "./command-options.js"; + +function resolveTargetId(rawTargetId: unknown, command: Command): string | undefined { + const local = typeof rawTargetId === "string" ? rawTargetId.trim() : ""; + if (local) { + return local; + } + const inherited = inheritOptionFromParent(command, "targetId"); + if (typeof inherited !== "string") { + return undefined; + } + const trimmed = inherited.trim(); + return trimmed ? trimmed : undefined; +} export function registerBrowserCookiesAndStorageCommands( browser: Command, @@ -14,6 +28,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (opts, cmd) => { const parent = parentOpts(cmd); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd); try { const result = await callBrowserRequest<{ cookies?: unknown[] }>( parent, @@ -21,7 +36,7 @@ export function registerBrowserCookiesAndStorageCommands( method: "GET", path: "/cookies", query: { - targetId: opts.targetId?.trim() || undefined, + targetId, profile, }, }, @@ -48,6 +63,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (name: string, value: string, opts, cmd) => { const parent = parentOpts(cmd); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd); try { const result = await callBrowserRequest( parent, @@ -56,7 +72,7 @@ export function registerBrowserCookiesAndStorageCommands( path: "/cookies/set", query: profile ? { profile } : undefined, body: { - targetId: opts.targetId?.trim() || undefined, + targetId, cookie: { name, value, url: opts.url }, }, }, @@ -80,6 +96,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (opts, cmd) => { const parent = parentOpts(cmd); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd); try { const result = await callBrowserRequest( parent, @@ -88,7 +105,7 @@ export function registerBrowserCookiesAndStorageCommands( path: "/cookies/clear", query: profile ? { profile } : undefined, body: { - targetId: opts.targetId?.trim() || undefined, + targetId, }, }, { timeoutMs: 20000 }, @@ -117,6 +134,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (key: string | undefined, opts, cmd2) => { const parent = parentOpts(cmd2); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd2); try { const result = await callBrowserRequest<{ values?: Record }>( parent, @@ -125,7 +143,7 @@ export function registerBrowserCookiesAndStorageCommands( path: `/storage/${kind}`, query: { key: key?.trim() || undefined, - targetId: opts.targetId?.trim() || undefined, + targetId, profile, }, }, @@ -151,6 +169,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (key: string, value: string, opts, cmd2) => { const parent = parentOpts(cmd2); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd2); try { const result = await callBrowserRequest( parent, @@ -161,7 +180,7 @@ export function registerBrowserCookiesAndStorageCommands( body: { key, value, - targetId: opts.targetId?.trim() || undefined, + targetId, }, }, { timeoutMs: 20000 }, @@ -184,6 +203,7 @@ export function registerBrowserCookiesAndStorageCommands( .action(async (opts, cmd2) => { const parent = parentOpts(cmd2); const profile = parent?.browserProfile; + const targetId = resolveTargetId(opts.targetId, cmd2); try { const result = await callBrowserRequest( parent, @@ -192,7 +212,7 @@ export function registerBrowserCookiesAndStorageCommands( path: `/storage/${kind}/clear`, query: profile ? { profile } : undefined, body: { - targetId: opts.targetId?.trim() || undefined, + targetId, }, }, { timeoutMs: 20000 }, diff --git a/src/cli/browser-cli-state.option-collisions.test.ts b/src/cli/browser-cli-state.option-collisions.test.ts new file mode 100644 index 0000000000..b50b5faa0c --- /dev/null +++ b/src/cli/browser-cli-state.option-collisions.test.ts @@ -0,0 +1,87 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { BrowserParentOpts } from "./browser-cli-shared.js"; +import { registerBrowserStateCommands } from "./browser-cli-state.js"; + +const mocks = vi.hoisted(() => ({ + callBrowserRequest: vi.fn(async () => ({ ok: true })), + runBrowserResizeWithOutput: vi.fn(async () => {}), + runtime: { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + }, +})); + +vi.mock("./browser-cli-shared.js", () => ({ + callBrowserRequest: (...args: unknown[]) => mocks.callBrowserRequest(...args), +})); + +vi.mock("./browser-cli-resize.js", () => ({ + runBrowserResizeWithOutput: (params: unknown) => mocks.runBrowserResizeWithOutput(params), +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: mocks.runtime, +})); + +describe("browser state option collisions", () => { + beforeEach(() => { + mocks.callBrowserRequest.mockClear(); + mocks.runBrowserResizeWithOutput.mockClear(); + mocks.runtime.log.mockClear(); + mocks.runtime.error.mockClear(); + mocks.runtime.exit.mockClear(); + }); + + it("forwards parent-captured --target-id on `browser cookies set`", async () => { + const program = new Command(); + const browser = program + .command("browser") + .option("--browser-profile ", "Browser profile") + .option("--json", "Output JSON", false); + + const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + registerBrowserStateCommands(browser, parentOpts); + + await program.parseAsync( + [ + "browser", + "cookies", + "set", + "session", + "abc", + "--url", + "https://example.com", + "--target-id", + "tab-1", + ], + { from: "user" }, + ); + + const call = mocks.callBrowserRequest.mock.calls.at(-1); + expect(call).toBeDefined(); + const request = call?.[1] as { body?: { targetId?: string } }; + expect(request.body?.targetId).toBe("tab-1"); + }); + + it("accepts legacy parent `--json` by parsing payload via positional headers fallback", async () => { + const program = new Command(); + const browser = program + .command("browser") + .option("--browser-profile ", "Browser profile") + .option("--json", "Output JSON", false); + + const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + registerBrowserStateCommands(browser, parentOpts); + + await program.parseAsync(["browser", "set", "headers", "--json", '{"x-auth":"ok"}'], { + from: "user", + }); + + const call = mocks.callBrowserRequest.mock.calls.at(-1); + expect(call).toBeDefined(); + const request = call?.[1] as { body?: { headers?: Record } }; + expect(request.body?.headers).toEqual({ "x-auth": "ok" }); + }); +}); diff --git a/src/cli/browser-cli-state.ts b/src/cli/browser-cli-state.ts index db4a8a452d..338e0469fe 100644 --- a/src/cli/browser-cli-state.ts +++ b/src/cli/browser-cli-state.ts @@ -102,14 +102,21 @@ export function registerBrowserStateCommands( set .command("headers") .description("Set extra HTTP headers (JSON object)") - .requiredOption("--json ", "JSON object of headers") + .argument("[headersJson]", "JSON object of headers (alternative to --headers-json)") + .option("--headers-json ", "JSON object of headers") .option("--target-id ", "CDP target id (or unique prefix)") - .action(async (opts, cmd) => { + .action(async (headersJson: string | undefined, opts, cmd) => { const parent = parentOpts(cmd); await runBrowserCommand(async () => { - const parsed = JSON.parse(String(opts.json)) as unknown; + const headersJsonValue = + (typeof opts.headersJson === "string" && opts.headersJson.trim()) || + (headersJson?.trim() ? headersJson.trim() : undefined); + if (!headersJsonValue) { + throw new Error("Missing headers JSON (pass --headers-json or positional JSON argument)"); + } + const parsed = JSON.parse(String(headersJsonValue)) as unknown; if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { - throw new Error("headers json must be an object"); + throw new Error("Headers JSON must be a JSON object"); } const headers: Record = {}; for (const [k, v] of Object.entries(parsed as Record)) { diff --git a/src/cli/command-options.test.ts b/src/cli/command-options.test.ts new file mode 100644 index 0000000000..89d40ab716 --- /dev/null +++ b/src/cli/command-options.test.ts @@ -0,0 +1,71 @@ +import { Command } from "commander"; +import { describe, expect, it } from "vitest"; +import { inheritOptionFromParent } from "./command-options.js"; + +describe("inheritOptionFromParent", () => { + it("inherits from grandparent when parent does not define the option", async () => { + const program = new Command().option("--token ", "Root token"); + const gateway = program.command("gateway"); + let inherited: string | undefined; + + gateway + .command("run") + .option("--token ", "Run token") + .action((_opts, command) => { + inherited = inheritOptionFromParent(command, "token"); + }); + + await program.parseAsync(["--token", "root-token", "gateway", "run"], { from: "user" }); + expect(inherited).toBe("root-token"); + }); + + it("prefers nearest ancestor value when multiple ancestors set the same option", async () => { + const program = new Command().option("--token ", "Root token"); + const gateway = program.command("gateway").option("--token ", "Gateway token"); + let inherited: string | undefined; + + gateway + .command("run") + .option("--token ", "Run token") + .action((_opts, command) => { + inherited = inheritOptionFromParent(command, "token"); + }); + + await program.parseAsync( + ["--token", "root-token", "gateway", "--token", "gateway-token", "run"], + { from: "user" }, + ); + expect(inherited).toBe("gateway-token"); + }); + + it("does not inherit when the child option was set explicitly", async () => { + const program = new Command().option("--token ", "Root token"); + const gateway = program.command("gateway").option("--token ", "Gateway token"); + const run = gateway.command("run").option("--token ", "Run token"); + + program.setOptionValueWithSource("token", "root-token", "cli"); + gateway.setOptionValueWithSource("token", "gateway-token", "cli"); + run.setOptionValueWithSource("token", "run-token", "cli"); + + expect(inheritOptionFromParent(run, "token")).toBeUndefined(); + }); + + it("does not inherit from ancestors beyond the bounded traversal depth", async () => { + const program = new Command().option("--token ", "Root token"); + const level1 = program.command("level1"); + const level2 = level1.command("level2"); + let inherited: string | undefined; + + level2 + .command("run") + .option("--token ", "Run token") + .action((_opts, command) => { + inherited = inheritOptionFromParent(command, "token"); + }); + + await program.parseAsync(["--token", "root-token", "level1", "level2", "run"], { + from: "user", + }); + expect(inherited).toBeUndefined(); + }); +}); diff --git a/src/cli/command-options.ts b/src/cli/command-options.ts index 2c2bc27c98..0f1830c95c 100644 --- a/src/cli/command-options.ts +++ b/src/cli/command-options.ts @@ -6,3 +6,39 @@ export function hasExplicitOptions(command: Command, names: readonly string[]): } return names.some((name) => command.getOptionValueSource(name) === "cli"); } + +function getOptionSource(command: Command, name: string): string | undefined { + if (typeof command.getOptionValueSource !== "function") { + return undefined; + } + return command.getOptionValueSource(name); +} + +// Defensive guardrail: allow expected parent/grandparent inheritance without unbounded deep traversal. +const MAX_INHERIT_DEPTH = 2; + +export function inheritOptionFromParent( + command: Command | undefined, + name: string, +): T | undefined { + if (!command) { + return undefined; + } + + const childSource = getOptionSource(command, name); + if (childSource && childSource !== "default") { + return undefined; + } + + let depth = 0; + let ancestor = command.parent; + while (ancestor && depth < MAX_INHERIT_DEPTH) { + const source = getOptionSource(ancestor, name); + if (source && source !== "default") { + return ancestor.opts>()[name] as T | undefined; + } + depth += 1; + ancestor = ancestor.parent; + } + return undefined; +} diff --git a/src/cli/daemon-cli/register-service-commands.test.ts b/src/cli/daemon-cli/register-service-commands.test.ts new file mode 100644 index 0000000000..03fcf41387 --- /dev/null +++ b/src/cli/daemon-cli/register-service-commands.test.ts @@ -0,0 +1,72 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { addGatewayServiceCommands } from "./register-service-commands.js"; + +const runDaemonInstall = vi.fn(async () => {}); +const runDaemonRestart = vi.fn(async () => {}); +const runDaemonStart = vi.fn(async () => {}); +const runDaemonStatus = vi.fn(async () => {}); +const runDaemonStop = vi.fn(async () => {}); +const runDaemonUninstall = vi.fn(async () => {}); + +vi.mock("./runners.js", () => ({ + runDaemonInstall: (opts: unknown) => runDaemonInstall(opts), + runDaemonRestart: (opts: unknown) => runDaemonRestart(opts), + runDaemonStart: (opts: unknown) => runDaemonStart(opts), + runDaemonStatus: (opts: unknown) => runDaemonStatus(opts), + runDaemonStop: (opts: unknown) => runDaemonStop(opts), + runDaemonUninstall: (opts: unknown) => runDaemonUninstall(opts), +})); + +function createGatewayParentLikeCommand() { + const gateway = new Command().name("gateway"); + // Mirror overlapping root gateway options that conflict with service subcommand options. + gateway.option("--port ", "Port for the gateway WebSocket"); + gateway.option("--token ", "Gateway token"); + gateway.option("--password ", "Gateway password"); + gateway.option("--force", "Gateway run --force", false); + addGatewayServiceCommands(gateway); + return gateway; +} + +describe("addGatewayServiceCommands", () => { + beforeEach(() => { + runDaemonInstall.mockClear(); + runDaemonRestart.mockClear(); + runDaemonStart.mockClear(); + runDaemonStatus.mockClear(); + runDaemonStop.mockClear(); + runDaemonUninstall.mockClear(); + }); + + it("forwards install option collisions from parent gateway command", async () => { + const gateway = createGatewayParentLikeCommand(); + await gateway.parseAsync(["install", "--force", "--port", "19000", "--token", "tok_test"], { + from: "user", + }); + + expect(runDaemonInstall).toHaveBeenCalledWith( + expect.objectContaining({ + force: true, + port: "19000", + token: "tok_test", + }), + ); + }); + + it("forwards status auth collisions from parent gateway command", async () => { + const gateway = createGatewayParentLikeCommand(); + await gateway.parseAsync(["status", "--token", "tok_status", "--password", "pw_status"], { + from: "user", + }); + + expect(runDaemonStatus).toHaveBeenCalledWith( + expect.objectContaining({ + rpc: expect.objectContaining({ + token: "tok_status", + password: "pw_status", + }), + }), + ); + }); +}); diff --git a/src/cli/daemon-cli/register-service-commands.ts b/src/cli/daemon-cli/register-service-commands.ts index 1cfe3881ec..ff511e71e9 100644 --- a/src/cli/daemon-cli/register-service-commands.ts +++ b/src/cli/daemon-cli/register-service-commands.ts @@ -1,4 +1,6 @@ import type { Command } from "commander"; +import type { DaemonInstallOptions, GatewayRpcOpts } from "./types.js"; +import { inheritOptionFromParent } from "../command-options.js"; import { runDaemonInstall, runDaemonRestart, @@ -8,6 +10,31 @@ import { runDaemonUninstall, } from "./runners.js"; +function resolveInstallOptions( + cmdOpts: DaemonInstallOptions, + command?: Command, +): DaemonInstallOptions { + const parentForce = inheritOptionFromParent(command, "force"); + const parentPort = inheritOptionFromParent(command, "port"); + const parentToken = inheritOptionFromParent(command, "token"); + return { + ...cmdOpts, + force: Boolean(cmdOpts.force || parentForce), + port: cmdOpts.port ?? parentPort, + token: cmdOpts.token ?? parentToken, + }; +} + +function resolveRpcOptions(cmdOpts: GatewayRpcOpts, command?: Command): GatewayRpcOpts { + const parentToken = inheritOptionFromParent(command, "token"); + const parentPassword = inheritOptionFromParent(command, "password"); + return { + ...cmdOpts, + token: cmdOpts.token ?? parentToken, + password: cmdOpts.password ?? parentPassword, + }; +} + export function addGatewayServiceCommands(parent: Command, opts?: { statusDescription?: string }) { parent .command("status") @@ -19,9 +46,9 @@ export function addGatewayServiceCommands(parent: Command, opts?: { statusDescri .option("--no-probe", "Skip RPC probe") .option("--deep", "Scan system-level services", false) .option("--json", "Output JSON", false) - .action(async (cmdOpts) => { + .action(async (cmdOpts, command) => { await runDaemonStatus({ - rpc: cmdOpts, + rpc: resolveRpcOptions(cmdOpts, command), probe: Boolean(cmdOpts.probe), deep: Boolean(cmdOpts.deep), json: Boolean(cmdOpts.json), @@ -36,8 +63,8 @@ export function addGatewayServiceCommands(parent: Command, opts?: { statusDescri .option("--token ", "Gateway token (token auth)") .option("--force", "Reinstall/overwrite if already installed", false) .option("--json", "Output JSON", false) - .action(async (cmdOpts) => { - await runDaemonInstall(cmdOpts); + .action(async (cmdOpts, command) => { + await runDaemonInstall(resolveInstallOptions(cmdOpts, command)); }); parent diff --git a/src/cli/gateway-cli/register.option-collisions.test.ts b/src/cli/gateway-cli/register.option-collisions.test.ts new file mode 100644 index 0000000000..77093e206f --- /dev/null +++ b/src/cli/gateway-cli/register.option-collisions.test.ts @@ -0,0 +1,160 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const callGatewayCli = vi.fn(async () => ({ ok: true })); +const gatewayStatusCommand = vi.fn(async () => {}); + +const runtimeLogs: string[] = []; +const runtimeErrors: string[] = []; +const defaultRuntime = { + log: (msg: string) => runtimeLogs.push(msg), + error: (msg: string) => runtimeErrors.push(msg), + exit: (code: number) => { + throw new Error(`__exit__:${code}`); + }, +}; + +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, +})); + +vi.mock("../../commands/gateway-status.js", () => ({ + gatewayStatusCommand: (opts: unknown, runtime: unknown) => gatewayStatusCommand(opts, runtime), +})); + +vi.mock("./call.js", () => ({ + gatewayCallOpts: (cmd: Command) => + cmd + .option("--url ", "Gateway WebSocket URL") + .option("--token ", "Gateway token") + .option("--password ", "Gateway password") + .option("--timeout ", "Timeout in ms", "10000") + .option("--expect-final", "Wait for final response (agent)", false) + .option("--json", "Output JSON", false), + callGatewayCli: (method: string, opts: unknown, params?: unknown) => + callGatewayCli(method, opts, params), +})); + +vi.mock("./run.js", () => ({ + addGatewayRunCommand: (cmd: Command) => + cmd + .option("--token ", "Gateway token") + .option("--password ", "Gateway password"), +})); + +vi.mock("../daemon-cli.js", () => ({ + addGatewayServiceCommands: () => undefined, +})); + +vi.mock("../../commands/health.js", () => ({ + formatHealthChannelLines: () => [], +})); + +vi.mock("../../config/config.js", () => ({ + loadConfig: () => ({}), +})); + +vi.mock("../../infra/bonjour-discovery.js", () => ({ + discoverGatewayBeacons: async () => [], +})); + +vi.mock("../../infra/widearea-dns.js", () => ({ + resolveWideAreaDiscoveryDomain: () => undefined, +})); + +vi.mock("../../terminal/health-style.js", () => ({ + styleHealthChannelLine: (line: string) => line, +})); + +vi.mock("../../terminal/links.js", () => ({ + formatDocsLink: () => "docs.openclaw.ai/cli/gateway", +})); + +vi.mock("../../terminal/theme.js", () => ({ + colorize: (_rich: boolean, _fn: (value: string) => string, value: string) => value, + isRich: () => false, + theme: { + heading: (value: string) => value, + muted: (value: string) => value, + success: (value: string) => value, + }, +})); + +vi.mock("../../utils/usage-format.js", () => ({ + formatTokenCount: () => "0", + formatUsd: () => "$0.00", +})); + +vi.mock("../help-format.js", () => ({ + formatHelpExamples: () => "", +})); + +vi.mock("../progress.js", () => ({ + withProgress: async (_opts: unknown, fn: () => Promise) => await fn(), +})); + +vi.mock("./discover.js", () => ({ + dedupeBeacons: (beacons: unknown[]) => beacons, + parseDiscoverTimeoutMs: () => 2000, + pickBeaconHost: () => null, + pickGatewayPort: () => 18789, + renderBeaconLines: () => [], +})); + +describe("gateway register option collisions", () => { + beforeEach(() => { + runtimeLogs.length = 0; + runtimeErrors.length = 0; + callGatewayCli.mockClear(); + gatewayStatusCommand.mockClear(); + }); + + it("forwards --token to gateway call when parent and child option names collide", async () => { + const { registerGatewayCli } = await import("./register.js"); + const program = new Command(); + registerGatewayCli(program); + + await program.parseAsync(["gateway", "call", "health", "--token", "tok_call", "--json"], { + from: "user", + }); + + expect(callGatewayCli).toHaveBeenCalledWith( + "health", + expect.objectContaining({ + token: "tok_call", + }), + {}, + ); + }); + + it("forwards --token to gateway probe when parent and child option names collide", async () => { + const { registerGatewayCli } = await import("./register.js"); + const program = new Command(); + registerGatewayCli(program); + + await program.parseAsync(["gateway", "probe", "--token", "tok_probe", "--json"], { + from: "user", + }); + + expect(gatewayStatusCommand).toHaveBeenCalledWith( + expect.objectContaining({ + token: "tok_probe", + }), + defaultRuntime, + ); + }); +}); diff --git a/src/cli/gateway-cli/register.ts b/src/cli/gateway-cli/register.ts index b85783382f..29a06a845f 100644 --- a/src/cli/gateway-cli/register.ts +++ b/src/cli/gateway-cli/register.ts @@ -11,6 +11,7 @@ import { formatDocsLink } from "../../terminal/links.js"; import { colorize, isRich, theme } from "../../terminal/theme.js"; import { formatTokenCount, formatUsd } from "../../utils/usage-format.js"; import { runCommandWithRuntime } from "../cli-utils.js"; +import { inheritOptionFromParent } from "../command-options.js"; import { addGatewayServiceCommands } from "../daemon-cli.js"; import { formatHelpExamples } from "../help-format.js"; import { withProgress } from "../progress.js"; @@ -46,6 +47,19 @@ function parseDaysOption(raw: unknown, fallback = 30): number { return fallback; } +function resolveGatewayRpcOptions( + opts: T, + command?: Command, +): T { + const parentToken = inheritOptionFromParent(command, "token"); + const parentPassword = inheritOptionFromParent(command, "password"); + return { + ...opts, + token: opts.token ?? parentToken, + password: opts.password ?? parentPassword, + }; +} + function renderCostUsageSummary(summary: CostUsageSummary, days: number, rich: boolean): string[] { const totalCost = formatUsd(summary.totals.totalCost) ?? "$0.00"; const totalTokens = formatTokenCount(summary.totals.totalTokens) ?? "0"; @@ -103,11 +117,12 @@ export function registerGatewayCli(program: Command) { .description("Call a Gateway method") .argument("", "Method name (health/status/system-presence/cron.*)") .option("--params ", "JSON object string for params", "{}") - .action(async (method, opts) => { + .action(async (method, opts, command) => { await runGatewayCommand(async () => { + const rpcOpts = resolveGatewayRpcOptions(opts, command); const params = JSON.parse(String(opts.params ?? "{}")); - const result = await callGatewayCli(method, opts, params); - if (opts.json) { + const result = await callGatewayCli(method, rpcOpts, params); + if (rpcOpts.json) { defaultRuntime.log(JSON.stringify(result, null, 2)); return; } @@ -125,11 +140,12 @@ export function registerGatewayCli(program: Command) { .command("usage-cost") .description("Fetch usage cost summary from session logs") .option("--days ", "Number of days to include", "30") - .action(async (opts) => { + .action(async (opts, command) => { await runGatewayCommand(async () => { + const rpcOpts = resolveGatewayRpcOptions(opts, command); const days = parseDaysOption(opts.days); - const result = await callGatewayCli("usage.cost", opts, { days }); - if (opts.json) { + const result = await callGatewayCli("usage.cost", rpcOpts, { days }); + if (rpcOpts.json) { defaultRuntime.log(JSON.stringify(result, null, 2)); return; } @@ -146,10 +162,11 @@ export function registerGatewayCli(program: Command) { gateway .command("health") .description("Fetch Gateway health") - .action(async (opts) => { + .action(async (opts, command) => { await runGatewayCommand(async () => { - const result = await callGatewayCli("health", opts); - if (opts.json) { + const rpcOpts = resolveGatewayRpcOptions(opts, command); + const result = await callGatewayCli("health", rpcOpts); + if (rpcOpts.json) { defaultRuntime.log(JSON.stringify(result, null, 2)); return; } @@ -180,9 +197,10 @@ export function registerGatewayCli(program: Command) { .option("--password ", "Gateway password (applies to all probes)") .option("--timeout ", "Overall probe budget in ms", "3000") .option("--json", "Output JSON", false) - .action(async (opts) => { + .action(async (opts, command) => { await runGatewayCommand(async () => { - await gatewayStatusCommand(opts, defaultRuntime); + const rpcOpts = resolveGatewayRpcOptions(opts, command); + await gatewayStatusCommand(rpcOpts, defaultRuntime); }); }); diff --git a/src/cli/gateway-cli/run.option-collisions.test.ts b/src/cli/gateway-cli/run.option-collisions.test.ts new file mode 100644 index 0000000000..a97e9b2fa0 --- /dev/null +++ b/src/cli/gateway-cli/run.option-collisions.test.ts @@ -0,0 +1,143 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const startGatewayServer = vi.fn(async () => ({ + close: vi.fn(async () => {}), +})); +const setGatewayWsLogStyle = vi.fn(); +const setVerbose = vi.fn(); +const forceFreePortAndWait = vi.fn(async () => ({ + killed: [], + waitedMs: 0, + escalatedToSigkill: false, +})); +const ensureDevGatewayConfig = vi.fn(async () => {}); +const runGatewayLoop = vi.fn(async ({ start }: { start: () => Promise }) => { + await start(); +}); + +const runtimeLogs: string[] = []; +const runtimeErrors: string[] = []; +const defaultRuntime = { + log: (msg: string) => runtimeLogs.push(msg), + error: (msg: string) => runtimeErrors.push(msg), + exit: (code: number) => { + throw new Error(`__exit__:${code}`); + }, +}; + +vi.mock("../../config/config.js", () => ({ + getConfigPath: () => "/tmp/openclaw-test-missing-config.json", + loadConfig: () => ({}), + readConfigFileSnapshot: async () => ({ exists: false }), + resolveStateDir: () => "/tmp", + resolveGatewayPort: () => 18789, +})); + +vi.mock("../../gateway/auth.js", () => ({ + resolveGatewayAuth: (params: { authConfig?: { token?: string }; env?: NodeJS.ProcessEnv }) => ({ + mode: "token", + token: params.authConfig?.token ?? params.env?.OPENCLAW_GATEWAY_TOKEN, + password: undefined, + allowTailscale: false, + }), +})); + +vi.mock("../../gateway/server.js", () => ({ + startGatewayServer: (port: number, opts?: unknown) => startGatewayServer(port, opts), +})); + +vi.mock("../../gateway/ws-logging.js", () => ({ + setGatewayWsLogStyle: (style: string) => setGatewayWsLogStyle(style), +})); + +vi.mock("../../globals.js", () => ({ + setVerbose: (enabled: boolean) => setVerbose(enabled), +})); + +vi.mock("../../infra/gateway-lock.js", () => ({ + GatewayLockError: class GatewayLockError extends Error {}, +})); + +vi.mock("../../infra/ports.js", () => ({ + formatPortDiagnostics: () => [], + inspectPortUsage: async () => ({ status: "free" }), +})); + +vi.mock("../../logging/console.js", () => ({ + setConsoleSubsystemFilter: () => undefined, + setConsoleTimestampPrefix: () => undefined, +})); + +vi.mock("../../logging/subsystem.js", () => ({ + createSubsystemLogger: () => ({ + info: () => undefined, + warn: () => undefined, + error: () => undefined, + }), +})); + +vi.mock("../../runtime.js", () => ({ + defaultRuntime, +})); + +vi.mock("../command-format.js", () => ({ + formatCliCommand: (cmd: string) => cmd, +})); + +vi.mock("../ports.js", () => ({ + forceFreePortAndWait: (port: number, opts: unknown) => forceFreePortAndWait(port, opts), +})); + +vi.mock("./dev.js", () => ({ + ensureDevGatewayConfig: (opts?: unknown) => ensureDevGatewayConfig(opts), +})); + +vi.mock("./run-loop.js", () => ({ + runGatewayLoop: (params: { start: () => Promise }) => runGatewayLoop(params), +})); + +describe("gateway run option collisions", () => { + beforeEach(() => { + runtimeLogs.length = 0; + runtimeErrors.length = 0; + startGatewayServer.mockClear(); + setGatewayWsLogStyle.mockClear(); + setVerbose.mockClear(); + forceFreePortAndWait.mockClear(); + ensureDevGatewayConfig.mockClear(); + runGatewayLoop.mockClear(); + }); + + it("forwards parent-captured options to `gateway run` subcommand", async () => { + const { addGatewayRunCommand } = await import("./run.js"); + const program = new Command(); + const gateway = addGatewayRunCommand(program.command("gateway")); + addGatewayRunCommand(gateway.command("run")); + + await program.parseAsync( + [ + "gateway", + "run", + "--token", + "tok_run", + "--allow-unconfigured", + "--ws-log", + "full", + "--force", + ], + { from: "user" }, + ); + + expect(forceFreePortAndWait).toHaveBeenCalledWith(18789, expect.anything()); + expect(setGatewayWsLogStyle).toHaveBeenCalledWith("full"); + expect(startGatewayServer).toHaveBeenCalledWith( + 18789, + expect.objectContaining({ + auth: expect.objectContaining({ + token: "tok_run", + }), + }), + ); + }); +}); diff --git a/src/cli/gateway-cli/run.ts b/src/cli/gateway-cli/run.ts index 58b0ceae47..d85f7db200 100644 --- a/src/cli/gateway-cli/run.ts +++ b/src/cli/gateway-cli/run.ts @@ -1,7 +1,8 @@ +import type { Command } from "commander"; import fs from "node:fs"; import path from "node:path"; -import type { Command } from "commander"; import type { GatewayAuthMode } from "../../config/config.js"; +import type { GatewayWsLogStyle } from "../../gateway/ws-logging.js"; import { CONFIG_PATH, loadConfig, @@ -11,7 +12,6 @@ import { } from "../../config/config.js"; import { resolveGatewayAuth } from "../../gateway/auth.js"; import { startGatewayServer } from "../../gateway/server.js"; -import type { GatewayWsLogStyle } from "../../gateway/ws-logging.js"; import { setGatewayWsLogStyle } from "../../gateway/ws-logging.js"; import { setVerbose } from "../../globals.js"; import { GatewayLockError } from "../../infra/gateway-lock.js"; @@ -20,6 +20,7 @@ import { setConsoleSubsystemFilter, setConsoleTimestampPrefix } from "../../logg import { createSubsystemLogger } from "../../logging/subsystem.js"; import { defaultRuntime } from "../../runtime.js"; import { formatCliCommand } from "../command-format.js"; +import { inheritOptionFromParent } from "../command-options.js"; import { forceFreePortAndWait } from "../ports.js"; import { ensureDevGatewayConfig } from "./dev.js"; import { runGatewayLoop } from "./run-loop.js"; @@ -53,6 +54,50 @@ type GatewayRunOpts = { const gatewayLog = createSubsystemLogger("gateway"); +const GATEWAY_RUN_VALUE_KEYS = [ + "port", + "bind", + "token", + "auth", + "password", + "tailscale", + "wsLog", + "rawStreamPath", +] as const; + +const GATEWAY_RUN_BOOLEAN_KEYS = [ + "tailscaleResetOnExit", + "allowUnconfigured", + "dev", + "reset", + "force", + "verbose", + "claudeCliLogs", + "compact", + "rawStream", +] as const; + +function resolveGatewayRunOptions(opts: GatewayRunOpts, command?: Command): GatewayRunOpts { + const resolved: GatewayRunOpts = { ...opts }; + + for (const key of GATEWAY_RUN_VALUE_KEYS) { + const inherited = inheritOptionFromParent(command, key); + if (key === "wsLog") { + // wsLog has a child default ("auto"), so prefer inherited parent CLI value when present. + resolved[key] = inherited ?? resolved[key]; + continue; + } + resolved[key] = resolved[key] ?? inherited; + } + + for (const key of GATEWAY_RUN_BOOLEAN_KEYS) { + const inherited = inheritOptionFromParent(command, key); + resolved[key] = Boolean(resolved[key] || inherited); + } + + return resolved; +} + async function runGatewayCommand(opts: GatewayRunOpts) { const isDevProfile = process.env.OPENCLAW_PROFILE?.trim().toLowerCase() === "dev"; const devMode = Boolean(opts.dev) || isDevProfile; @@ -353,7 +398,7 @@ export function addGatewayRunCommand(cmd: Command): Command { .option("--compact", 'Alias for "--ws-log compact"', false) .option("--raw-stream", "Log raw model stream events to jsonl", false) .option("--raw-stream-path ", "Raw stream jsonl path") - .action(async (opts) => { - await runGatewayCommand(opts); + .action(async (opts, command) => { + await runGatewayCommand(resolveGatewayRunOptions(opts, command)); }); } diff --git a/src/cli/update-cli.option-collisions.test.ts b/src/cli/update-cli.option-collisions.test.ts new file mode 100644 index 0000000000..baa39d6403 --- /dev/null +++ b/src/cli/update-cli.option-collisions.test.ts @@ -0,0 +1,68 @@ +import { Command } from "commander"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const updateCommand = vi.fn(async () => {}); +const updateStatusCommand = vi.fn(async () => {}); +const updateWizardCommand = vi.fn(async () => {}); + +const defaultRuntime = { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), +}; + +vi.mock("./update-cli/update-command.js", () => ({ + updateCommand: (opts: unknown) => updateCommand(opts), +})); + +vi.mock("./update-cli/status.js", () => ({ + updateStatusCommand: (opts: unknown) => updateStatusCommand(opts), +})); + +vi.mock("./update-cli/wizard.js", () => ({ + updateWizardCommand: (opts: unknown) => updateWizardCommand(opts), +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime, +})); + +describe("update cli option collisions", () => { + beforeEach(() => { + updateCommand.mockClear(); + updateStatusCommand.mockClear(); + updateWizardCommand.mockClear(); + defaultRuntime.log.mockClear(); + defaultRuntime.error.mockClear(); + defaultRuntime.exit.mockClear(); + }); + + it("forwards parent-captured --json/--timeout to `update status`", async () => { + const { registerUpdateCli } = await import("./update-cli.js"); + const program = new Command(); + registerUpdateCli(program); + + await program.parseAsync(["update", "status", "--json", "--timeout", "9"], { from: "user" }); + + expect(updateStatusCommand).toHaveBeenCalledWith( + expect.objectContaining({ + json: true, + timeout: "9", + }), + ); + }); + + it("forwards parent-captured --timeout to `update wizard`", async () => { + const { registerUpdateCli } = await import("./update-cli.js"); + const program = new Command(); + registerUpdateCli(program); + + await program.parseAsync(["update", "wizard", "--timeout", "13"], { from: "user" }); + + expect(updateWizardCommand).toHaveBeenCalledWith( + expect.objectContaining({ + timeout: "13", + }), + ); + }); +}); diff --git a/src/cli/update-cli.ts b/src/cli/update-cli.ts index 6d7e965ea7..0a7dc5dcd5 100644 --- a/src/cli/update-cli.ts +++ b/src/cli/update-cli.ts @@ -2,6 +2,7 @@ import type { Command } from "commander"; import { defaultRuntime } from "../runtime.js"; import { formatDocsLink } from "../terminal/links.js"; import { theme } from "../terminal/theme.js"; +import { inheritOptionFromParent } from "./command-options.js"; import { formatHelpExamples } from "./help-format.js"; import { type UpdateCommandOptions, @@ -15,6 +16,21 @@ import { updateWizardCommand } from "./update-cli/wizard.js"; export { updateCommand, updateStatusCommand, updateWizardCommand }; export type { UpdateCommandOptions, UpdateStatusOptions, UpdateWizardOptions }; +function inheritedUpdateJson(command?: Command): boolean { + return Boolean(inheritOptionFromParent(command, "json")); +} + +function inheritedUpdateTimeout( + opts: { timeout?: unknown }, + command?: Command, +): string | undefined { + const timeout = opts.timeout as string | undefined; + if (timeout) { + return timeout; + } + return inheritOptionFromParent(command, "timeout"); +} + export function registerUpdateCli(program: Command) { const update = program .command("update") @@ -89,10 +105,10 @@ ${theme.muted("Docs:")} ${formatDocsLink("/cli/update", "docs.openclaw.ai/cli/up "after", `\n${theme.muted("Docs:")} ${formatDocsLink("/cli/update", "docs.openclaw.ai/cli/update")}\n`, ) - .action(async (opts) => { + .action(async (opts, command) => { try { await updateWizardCommand({ - timeout: opts.timeout as string | undefined, + timeout: inheritedUpdateTimeout(opts, command), }); } catch (err) { defaultRuntime.error(String(err)); @@ -118,11 +134,11 @@ ${theme.muted("Docs:")} ${formatDocsLink("/cli/update", "docs.openclaw.ai/cli/up "Docs:", )} ${formatDocsLink("/cli/update", "docs.openclaw.ai/cli/update")}`, ) - .action(async (opts) => { + .action(async (opts, command) => { try { await updateStatusCommand({ - json: Boolean(opts.json), - timeout: opts.timeout as string | undefined, + json: Boolean(opts.json) || inheritedUpdateJson(command), + timeout: inheritedUpdateTimeout(opts, command), }); } catch (err) { defaultRuntime.error(String(err));