From 801ec63f925fa3f896c0802fdd9d7aa16f39f5fa Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 13 Jul 2026 05:05:31 -0700 Subject: [PATCH] refactor(cli): rely on Commander contracts (#106366) --- .../browser/src/cli/browser-cli-inspect.ts | 6 ++-- src/cli/command-options.ts | 16 ++------- src/cli/cron-cli/register.cron-add.ts | 13 +++---- src/cli/program/action-reparse.test.ts | 16 ++++----- src/cli/program/action-reparse.ts | 4 +-- src/cli/program/helpers.test.ts | 14 -------- src/cli/program/helpers.ts | 23 ++----------- src/cli/program/json-mode.ts | 15 +------- src/cli/program/preaction.ts | 5 +-- src/cli/program/register-command-groups.ts | 7 ++-- src/cli/program/register-lazy-command.ts | 18 ++++------ src/cli/program/register.onboard.ts | 26 ++++---------- src/cli/program/register.setup.ts | 34 ++++--------------- 13 files changed, 42 insertions(+), 155 deletions(-) diff --git a/extensions/browser/src/cli/browser-cli-inspect.ts b/extensions/browser/src/cli/browser-cli-inspect.ts index 020154957ee3..c940439cd097 100644 --- a/extensions/browser/src/cli/browser-cli-inspect.ts +++ b/extensions/browser/src/cli/browser-cli-inspect.ts @@ -105,13 +105,11 @@ export function registerBrowserInspectCommands( .option("--labels", "Include label overlay screenshot with annotations", false) .option("--urls", "Append discovered link URLs to AI snapshots", false) .option("--out ", "Write snapshot to a file") - .action(async (opts, cmd) => { + .action(async (opts, cmd: Command) => { const parent = parentOpts(cmd); const profile = parent?.browserProfile; const format = opts.format === "aria" ? "aria" : "ai"; - const formatWasExplicit = - typeof cmd.getOptionValueSource === "function" && - cmd.getOptionValueSource("format") === "cli"; + const formatWasExplicit = cmd.getOptionValueSource("format") === "cli"; const configMode = !formatWasExplicit && format === "ai" && diff --git a/src/cli/command-options.ts b/src/cli/command-options.ts index db1a180d0667..7c57e68ed276 100644 --- a/src/cli/command-options.ts +++ b/src/cli/command-options.ts @@ -2,19 +2,9 @@ import type { Command } from "commander"; export function hasExplicitOptions(command: Command, names: readonly string[]): boolean { - if (typeof command.getOptionValueSource !== "function") { - return false; - } 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; @@ -27,7 +17,7 @@ export function inheritOptionFromParent( return undefined; } - const childSource = getOptionSource(command, name); + const childSource = command.getOptionValueSource(name); if (childSource && childSource !== "default") { return undefined; } @@ -35,9 +25,9 @@ export function inheritOptionFromParent( let depth = 0; let ancestor = command.parent; while (ancestor && depth < MAX_INHERIT_DEPTH) { - const source = getOptionSource(ancestor, name); + const source = ancestor.getOptionValueSource(name); if (source && source !== "default") { - return ancestor.opts>()[name] as T | undefined; + return ancestor.getOptionValue(name) as T | undefined; } depth += 1; ancestor = ancestor.parent; diff --git a/src/cli/cron-cli/register.cron-add.ts b/src/cli/cron-cli/register.cron-add.ts index 6c37e917b9fd..4f54697a9fa9 100644 --- a/src/cli/cron-cli/register.cron-add.ts +++ b/src/cli/cron-cli/register.cron-add.ts @@ -154,7 +154,7 @@ export function registerCronAddCommand(cron: Command) { nameArg: string | undefined, messageArg: string | undefined, opts: GatewayRpcOpts & Record, - cmd?: Command, + cmd: Command, ) => { try { const hasScheduleFlag = @@ -183,11 +183,6 @@ export function registerCronAddCommand(cron: Command) { const rawAgentId = normalizeOptionalString(opts.agent); const agentId = rawAgentId ? sanitizeAgentId(rawAgentId) : undefined; - const optionSource = - typeof cmd?.getOptionValueSource === "function" - ? (name: string) => cmd.getOptionValueSource(name) - : () => undefined; - const hasAnnounce = Boolean(opts.announce) || opts.deliver === true; const hasNoDeliver = opts.deliver === false; const webhookUrl = normalizeOptionalString(opts.webhook); @@ -284,7 +279,7 @@ export function registerCronAddCommand(cron: Command) { }; })(); - const sessionSource = optionSource("session"); + const sessionSource = cmd.getOptionValueSource("session"); const sessionTargetRaw = normalizeOptionalString(opts.session) ?? ""; const inferredSessionTarget = payload.kind === "agentTurn" || payload.kind === "command" ? "isolated" : "main"; @@ -331,7 +326,7 @@ export function registerCronAddCommand(cron: Command) { const threadId = parseCronThreadIdOption(opts.threadId); const hasThreadId = typeof threadId === "number"; const hasChatDeliveryTarget = - optionSource("channel") === "cli" || + cmd.getOptionValueSource("channel") === "cli" || typeof opts.to === "string" || Boolean(accountId) || hasThreadId; @@ -408,7 +403,7 @@ export function registerCronAddCommand(cron: Command) { declarationKey, displayName, description, - ...(declarationKey && optionSource("disabled") !== "cli" + ...(declarationKey && cmd.getOptionValueSource("disabled") !== "cli" ? {} : { enabled: !opts.disabled }), deleteAfterRun: opts.deleteAfterRun ? true : opts.keepAfterRun ? false : undefined, diff --git a/src/cli/program/action-reparse.test.ts b/src/cli/program/action-reparse.test.ts index 7f4ea01d3f90..d15ccd67ec7b 100644 --- a/src/cli/program/action-reparse.test.ts +++ b/src/cli/program/action-reparse.test.ts @@ -4,7 +4,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { reparseProgramFromActionArgs } from "./action-reparse.js"; const buildParseArgvMock = vi.hoisted(() => vi.fn()); -const resolveActionArgsMock = vi.hoisted(() => vi.fn()); const resolveCommandOptionArgsMock = vi.hoisted(() => vi.fn()); vi.mock("../argv.js", () => ({ @@ -12,7 +11,6 @@ vi.mock("../argv.js", () => ({ })); vi.mock("./helpers.js", () => ({ - resolveActionArgs: resolveActionArgsMock, resolveCommandOptionArgs: resolveCommandOptionArgsMock, })); @@ -47,7 +45,6 @@ describe("reparseProgramFromActionArgs", () => { beforeEach(() => { vi.clearAllMocks(); buildParseArgvMock.mockReturnValue(["node", "openclaw", "status"]); - resolveActionArgsMock.mockReturnValue([]); resolveCommandOptionArgsMock.mockReturnValue([]); }); @@ -58,8 +55,8 @@ describe("reparseProgramFromActionArgs", () => { const actionCommand = { name: () => "status", parent: program, + args: ["--json"], } as unknown as Command; - resolveActionArgsMock.mockReturnValue(["--json"]); await reparseProgramFromActionArgs(program, [actionCommand]); @@ -78,8 +75,8 @@ describe("reparseProgramFromActionArgs", () => { const actionCommand = { name: () => "", parent: program, + args: ["--json"], } as unknown as Command; - resolveActionArgsMock.mockReturnValue(["--json"]); await reparseProgramFromActionArgs(program, [actionCommand]); @@ -97,8 +94,8 @@ describe("reparseProgramFromActionArgs", () => { const actionCommand = { name: () => "open", parent: program, + args: ["about:blank"], } as unknown as Command; - resolveActionArgsMock.mockReturnValue(["about:blank"]); resolveCommandOptionArgsMock.mockReturnValue(["--json"]); await reparseProgramFromActionArgs(program, [actionCommand]); @@ -118,9 +115,9 @@ describe("reparseProgramFromActionArgs", () => { const workspaces = root.command("workspaces"); const audit = workspaces.command("audit"); const exportCommand = audit.command("export"); + exportCommand.args = ["--since", "1"]; const parseAsync = vi.spyOn(root, "parseAsync").mockResolvedValue(root); const auditParseAsync = vi.spyOn(audit, "parseAsync"); - resolveActionArgsMock.mockReturnValue(["--since", "1"]); await reparseProgramFromActionArgs(audit, [exportCommand]); @@ -140,9 +137,9 @@ describe("reparseProgramFromActionArgs", () => { const workspaces = root.command("workspaces"); const audit = workspaces.command("audit"); const exportCommand = audit.command("export"); + exportCommand.args = ["--since", "1"]; deleteRawArgs(root); const parseAsync = vi.spyOn(root, "parseAsync").mockResolvedValue(root); - resolveActionArgsMock.mockReturnValue(["--since", "1"]); await reparseProgramFromActionArgs(audit, [exportCommand]); @@ -160,7 +157,6 @@ describe("reparseProgramFromActionArgs", () => { await reparseProgramFromActionArgs(program, []); - expect(resolveActionArgsMock).toHaveBeenCalledWith(undefined); expect(buildParseArgvMock).toHaveBeenCalledWith({ programName: "openclaw", rawArgs: [], @@ -174,9 +170,9 @@ describe("reparseProgramFromActionArgs", () => { // still reparse from reconstructed argv if Commander stops exposing it. const root = new Command().name("openclaw"); const configCommand = root.command("config"); + configCommand.args = ["set", "key", "value"]; deleteRawArgs(root); const parseAsync = vi.spyOn(root, "parseAsync").mockResolvedValue(root); - resolveActionArgsMock.mockReturnValue(["set", "key", "value"]); await reparseProgramFromActionArgs(root, [configCommand]); diff --git a/src/cli/program/action-reparse.ts b/src/cli/program/action-reparse.ts index 8614c58e2346..17d38381603e 100644 --- a/src/cli/program/action-reparse.ts +++ b/src/cli/program/action-reparse.ts @@ -2,7 +2,7 @@ import { expectDefined } from "@openclaw/normalization-core"; // Reparse support for lazy commands after their placeholder has been replaced. import type { Command, Option } from "commander"; import { buildParseArgv } from "../argv.js"; -import { resolveActionArgs, resolveCommandOptionArgs } from "./helpers.js"; +import { resolveCommandOptionArgs } from "./helpers.js"; function getCommandPathFromRoot(command: Command | undefined): Command[] { const path: Command[] = []; @@ -17,7 +17,7 @@ function getCommandPathFromRoot(command: Command | undefined): Command[] { } function buildFallbackArgv(program: Command, actionCommand: Command | undefined): string[] { - const actionArgsList = resolveActionArgs(actionCommand); + const actionArgsList = actionCommand?.args ?? []; const parentOptionArgs = actionCommand?.parent === program ? resolveCommandOptionArgs(program) : []; const commandPath = getCommandPathFromRoot(actionCommand).map((command) => command.name()); diff --git a/src/cli/program/helpers.test.ts b/src/cli/program/helpers.test.ts index 5b2876d956c2..cc86700733fd 100644 --- a/src/cli/program/helpers.test.ts +++ b/src/cli/program/helpers.test.ts @@ -6,7 +6,6 @@ import { parsePositiveIntOrUndefined, parseStrictPositiveIntOption, parseStrictPositiveIntOrUndefined, - resolveActionArgs, resolveCommandOptionArgs, } from "./helpers.js"; @@ -63,19 +62,6 @@ describe("program helpers", () => { ); }); - it("resolveActionArgs returns args when command has arg array", () => { - const command = new Command(); - (command as Command & { args?: string[] }).args = ["one", "two"]; - expect(resolveActionArgs(command)).toEqual(["one", "two"]); - }); - - it("resolveActionArgs returns empty array for missing/invalid args", () => { - const command = new Command(); - (command as unknown as { args?: unknown }).args = "not-an-array"; - expect(resolveActionArgs(command)).toStrictEqual([]); - expect(resolveActionArgs(undefined)).toStrictEqual([]); - }); - it("resolveCommandOptionArgs serializes explicit options", () => { const command = new Command() .option("--json", "JSON output", false) diff --git a/src/cli/program/helpers.ts b/src/cli/program/helpers.ts index f206bb0f7847..5562ba2079e7 100644 --- a/src/cli/program/helpers.ts +++ b/src/cli/program/helpers.ts @@ -29,22 +29,6 @@ export function parseStrictPositiveIntOption(value: string, flag: string): numbe return parsed; } -/** Return positional args captured by a Commander action command. */ -export function resolveActionArgs(actionCommand?: Command): string[] { - if (!actionCommand) { - return []; - } - const args = (actionCommand as Command & { args?: string[] }).args; - return Array.isArray(args) ? args : []; -} - -function isDefaultOptionValue(command: Command, name: string): boolean { - if (typeof command.getOptionValueSource !== "function") { - return false; - } - return command.getOptionValueSource(name) === "default"; -} - function appendOptionValue(out: string[], flag: string, value: unknown): void { if (value === undefined) { return; @@ -79,14 +63,11 @@ function stringifyOptionValue(value: unknown): string | undefined { } /** Reconstruct explicit option tokens from a Commander command for lazy reparsing. */ -export function resolveCommandOptionArgs(command?: Command): string[] { - if (!command) { - return []; - } +export function resolveCommandOptionArgs(command: Command): string[] { const out: string[] = []; for (const option of command.options) { const name = option.attributeName(); - if (isDefaultOptionValue(command, name)) { + if (command.getOptionValueSource(name) === "default") { continue; } const flag = option.long ?? option.short; diff --git a/src/cli/program/json-mode.ts b/src/cli/program/json-mode.ts index 53cc1da5d349..902ff5d8c01b 100644 --- a/src/cli/program/json-mode.ts +++ b/src/cli/program/json-mode.ts @@ -26,19 +26,6 @@ function getDeclaredCommandJsonMode(command: Command): JsonMode | null { return null; } -function commandSelectedJsonFlag(command: Command, argv: string[]): boolean { - const commandWithGlobals = command as Command & { - optsWithGlobals?: () => Record; - }; - if (typeof commandWithGlobals.optsWithGlobals === "function") { - const resolved = commandWithGlobals.optsWithGlobals().json; - if (resolved === true) { - return true; - } - } - return hasFlag(argv, "--json"); -} - /** Mark a command as having a special JSON mode beyond ordinary JSON output. */ export function setCommandJsonMode(command: Command, mode: JsonMode): Command { (command as JsonModeCommand)[jsonModeSymbol] = mode; @@ -46,7 +33,7 @@ export function setCommandJsonMode(command: Command, mode: JsonMode): Command { } function getCommandJsonMode(command: Command, argv: string[] = process.argv): JsonMode | null { - if (!commandSelectedJsonFlag(command, argv)) { + if (command.optsWithGlobals<{ json?: unknown }>().json !== true && !hasFlag(argv, "--json")) { return null; } return getDeclaredCommandJsonMode(command); diff --git a/src/cli/program/preaction.ts b/src/cli/program/preaction.ts index 379fdeb74f8b..0b6dbb266c93 100644 --- a/src/cli/program/preaction.ts +++ b/src/cli/program/preaction.ts @@ -65,13 +65,10 @@ function getActionCommandPath(actionCommand: Command): string[] { function getCliLogLevel(actionCommand: Command): LogLevel | undefined { const root = getRootCommand(actionCommand); - if (typeof root.getOptionValueSource !== "function") { - return undefined; - } if (root.getOptionValueSource("logLevel") !== "cli") { return undefined; } - const logLevel = root.opts>().logLevel; + const logLevel = root.getOptionValue("logLevel"); return typeof logLevel === "string" ? (logLevel as LogLevel) : undefined; } diff --git a/src/cli/program/register-command-groups.ts b/src/cli/program/register-command-groups.ts index 29aea366389c..060da91e3dc6 100644 --- a/src/cli/program/register-command-groups.ts +++ b/src/cli/program/register-command-groups.ts @@ -1,5 +1,4 @@ // Lazy command-group registration: placeholder commands are replaced by real subcommand groups. -import { uniqueStrings } from "@openclaw/normalization-core/string-normalization"; import type { Command } from "commander"; import { removeCommandByName } from "./command-tree.js"; import { registerLazyCommand } from "./register-lazy-command.js"; @@ -70,10 +69,8 @@ export function registerLazyCommandGroup( name: placeholder.name, description: placeholder.description, options: placeholder.options, - removeNames: uniqueStrings(getCommandGroupNames(entry)), - register: async () => { - await entry.register(program); - }, + removeNames: getCommandGroupNames(entry), + register: () => entry.register(program), }); } diff --git a/src/cli/program/register-lazy-command.ts b/src/cli/program/register-lazy-command.ts index c539410eba65..8a4654b93b95 100644 --- a/src/cli/program/register-lazy-command.ts +++ b/src/cli/program/register-lazy-command.ts @@ -12,7 +12,7 @@ type RegisterLazyCommandParams = { flags: string; description: string; }[]; - removeNames?: string[]; + removeNames?: readonly string[]; register: () => Promise | void; }; @@ -29,18 +29,12 @@ export function registerLazyCommand({ for (const option of options ?? []) { placeholder.option(option.flags, option.description); } - placeholder.allowUnknownOption(true); - placeholder.allowExcessArguments(true); + placeholder.allowUnknownOption(true).allowExcessArguments(true); placeholder.action(async (...actionArgs) => { - const actionCommand = actionArgs.at(-1) as (Command & { args?: string[] }) | undefined; - if (actionCommand) { - // Commander separates option values from positional args on placeholders; restore them - // before reparsing so the real command sees the original token order. - actionCommand.args = [ - ...resolveCommandOptionArgs(actionCommand), - ...(actionCommand.args ?? []), - ]; - } + const actionCommand = actionArgs.at(-1) as Command; + // Commander separates option values from positional args on placeholders; restore them + // before reparsing so the real command sees the original token order. + actionCommand.args = [...resolveCommandOptionArgs(actionCommand), ...actionCommand.args]; for (const commandName of new Set(removeNames ?? [name])) { removeCommandByName(program, commandName); } diff --git a/src/cli/program/register.onboard.ts b/src/cli/program/register.onboard.ts index 74a64284cd28..4dcd98086159 100644 --- a/src/cli/program/register.onboard.ts +++ b/src/cli/program/register.onboard.ts @@ -20,26 +20,14 @@ import { runCommandWithRuntime } from "../cli-utils.js"; import { formatCliCommand } from "../command-format.js"; import { parsePort } from "../shared/parse-port.js"; -function resolveInstallDaemonFlag( - command: unknown, - opts: { installDaemon?: boolean }, -): boolean | undefined { - if (!command || typeof command !== "object") { - return undefined; - } - const getOptionValueSource = - "getOptionValueSource" in command ? command.getOptionValueSource : undefined; - if (typeof getOptionValueSource !== "function") { - return undefined; - } - +export function resolveInstallDaemonFlag(command: Command): boolean | undefined { // Commander doesn't support option conflicts natively; keep original behavior. // If --skip-daemon is explicitly passed, it wins. - if (getOptionValueSource.call(command, "skipDaemon") === "cli") { + if (command.getOptionValueSource("skipDaemon") === "cli") { return false; } - if (getOptionValueSource.call(command, "installDaemon") === "cli") { - return Boolean(opts.installDaemon); + if (command.getOptionValueSource("installDaemon") === "cli") { + return Boolean(command.getOptionValue("installDaemon")); } return undefined; } @@ -247,7 +235,7 @@ export function registerOnboardCommand(program: Command): void { .option("--import-secrets", "Import supported secrets during onboarding migration", false) .option("--json", "Output JSON summary", false); - command.action(async (opts, commandRuntime) => { + command.action(async (opts, commandRuntime: Command) => { const { defaultRuntime } = await import("../../runtime.js"); await runCommandWithRuntime(defaultRuntime, async () => { if (opts.modern) { @@ -291,9 +279,7 @@ export function registerOnboardCommand(program: Command): void { ); return; } - const installDaemon = resolveInstallDaemonFlag(commandRuntime, { - installDaemon: Boolean(opts.installDaemon), - }); + const installDaemon = resolveInstallDaemonFlag(commandRuntime); const gatewayPort = parsePort(opts.gatewayPort); const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand( diff --git a/src/cli/program/register.setup.ts b/src/cli/program/register.setup.ts index fa43d49133cc..7eb5f3479500 100644 --- a/src/cli/program/register.setup.ts +++ b/src/cli/program/register.setup.ts @@ -12,29 +12,11 @@ import type { } from "../../commands/onboard-types.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { parsePort } from "../shared/parse-port.js"; -import { pickOnboardAuthOptionValues, registerOnboardAuthOptions } from "./register.onboard.js"; - -function resolveInstallDaemonFlag( - command: unknown, - opts: { installDaemon?: boolean }, -): boolean | undefined { - if (!command || typeof command !== "object") { - return undefined; - } - const getOptionValueSource = - "getOptionValueSource" in command ? command.getOptionValueSource : undefined; - if (typeof getOptionValueSource !== "function") { - return undefined; - } - - if (getOptionValueSource.call(command, "skipDaemon") === "cli") { - return false; - } - if (getOptionValueSource.call(command, "installDaemon") === "cli") { - return Boolean(opts.installDaemon); - } - return undefined; -} +import { + pickOnboardAuthOptionValues, + registerOnboardAuthOptions, + resolveInstallDaemonFlag, +} from "./register.onboard.js"; /** Register the `setup` command as an onboarding alias. */ export function registerSetupCommand(program: Command): void { @@ -107,7 +89,7 @@ export function registerSetupCommand(program: Command): void { .option("--remote-url ", "Remote Gateway WebSocket URL") .option("--remote-token ", "Remote Gateway token (optional)") .option("--json", "Output JSON summary", false) - .action(async (opts, commandRuntime) => { + .action(async (opts, commandRuntime: Command) => { const { defaultRuntime } = await import("../../runtime.js"); await runCommandWithRuntime(defaultRuntime, async () => { if (opts.baseline) { @@ -115,9 +97,7 @@ export function registerSetupCommand(program: Command): void { await setupCommand({ workspace: opts.workspace as string | undefined }, defaultRuntime); return; } - const installDaemon = resolveInstallDaemonFlag(commandRuntime, { - installDaemon: Boolean(opts.installDaemon), - }); + const installDaemon = resolveInstallDaemonFlag(commandRuntime); const gatewayPort = parsePort(opts.gatewayPort); const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand(