refactor(cli): rely on Commander contracts (#106366)

This commit is contained in:
Peter Steinberger
2026-07-13 05:05:31 -07:00
committed by GitHub
parent a03df5fd58
commit 801ec63f92
13 changed files with 42 additions and 155 deletions

View File

@@ -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 <path>", "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" &&

View File

@@ -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<T = unknown>(
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<T = unknown>(
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<Record<string, unknown>>()[name] as T | undefined;
return ancestor.getOptionValue(name) as T | undefined;
}
depth += 1;
ancestor = ancestor.parent;

View File

@@ -154,7 +154,7 @@ export function registerCronAddCommand(cron: Command) {
nameArg: string | undefined,
messageArg: string | undefined,
opts: GatewayRpcOpts & Record<string, unknown>,
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,

View File

@@ -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]);

View File

@@ -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());

View File

@@ -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)

View File

@@ -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;

View File

@@ -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<string, unknown>;
};
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);

View File

@@ -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<Record<string, unknown>>().logLevel;
const logLevel = root.getOptionValue("logLevel");
return typeof logLevel === "string" ? (logLevel as LogLevel) : undefined;
}

View File

@@ -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),
});
}

View File

@@ -12,7 +12,7 @@ type RegisterLazyCommandParams = {
flags: string;
description: string;
}[];
removeNames?: string[];
removeNames?: readonly string[];
register: () => Promise<void> | 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);
}

View File

@@ -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(

View File

@@ -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 <url>", "Remote Gateway WebSocket URL")
.option("--remote-token <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(