refactor(cli): use Commander argv for lazy reparses (#106382)

src/cli/program/action-reparse.test.ts

@@ -1,27 +1,12 @@
 // Action reparse tests cover Commander action reparsing for nested CLI commands.
 import { Command } from "commander";
-import { beforeEach, describe, expect, it, vi } from "vitest";
-import { reparseProgramFromActionArgs } from "./action-reparse.js";
-
-const buildParseArgvMock = vi.hoisted(() => vi.fn());
-const resolveCommandOptionArgsMock = vi.hoisted(() => vi.fn());
-
-vi.mock("../argv.js", () => ({
-  buildParseArgv: buildParseArgvMock,
-}));
-
-vi.mock("./helpers.js", () => ({
-  resolveCommandOptionArgs: resolveCommandOptionArgsMock,
-}));
+import { describe, expect, it, vi } from "vitest";
+import { reparseProgramFromActionCommand } from "./action-reparse.js";
 
 function setRawArgs(command: Command, rawArgs: string[]): void {
   (command as Command & { rawArgs: string[] }).rawArgs = rawArgs;
 }
 
-function deleteRawArgs(command: Command): void {
-  delete (command as Command & { rawArgs?: string[] }).rawArgs;
-}
-
 async function expectReparseArgv(params: {
   parent: Command;
   action: Command;
@@ -33,157 +18,37 @@ async function expectReparseArgv(params: {
     root = root.parent;
   }
   setRawArgs(root, params.argv);
-  buildParseArgvMock.mockReturnValue(params.argv);
   const parseAsync = vi.spyOn(root, "parseAsync").mockResolvedValue(root);
 
-  await reparseProgramFromActionArgs(params.parent, [params.action]);
+  await reparseProgramFromActionCommand(params.parent, params.action);
 
   expect(parseAsync).toHaveBeenCalledWith(params.expected);
 }
 
-describe("reparseProgramFromActionArgs", () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
-    buildParseArgvMock.mockReturnValue(["node", "openclaw", "status"]);
-    resolveCommandOptionArgsMock.mockReturnValue([]);
-  });
-
-  it("uses action command name + args as fallback argv", async () => {
-    const program = new Command().name("openclaw");
-    setRawArgs(program, ["node", "openclaw", "status", "--json"]);
-    const parseAsync = vi.spyOn(program, "parseAsync").mockResolvedValue(program);
-    const actionCommand = {
-      name: () => "status",
-      parent: program,
-      args: ["--json"],
-    } as unknown as Command;
-
-    await reparseProgramFromActionArgs(program, [actionCommand]);
-
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: ["node", "openclaw", "status", "--json"],
-      fallbackArgv: ["status", "--json"],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
-  });
-
-  it("falls back to action args without command name when action has no name", async () => {
-    const program = new Command().name("openclaw");
-    setRawArgs(program, ["node", "openclaw"]);
-    const parseAsync = vi.spyOn(program, "parseAsync").mockResolvedValue(program);
-    const actionCommand = {
-      name: () => "",
-      parent: program,
-      args: ["--json"],
-    } as unknown as Command;
-
-    await reparseProgramFromActionArgs(program, [actionCommand]);
-
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: ["node", "openclaw"],
-      fallbackArgv: ["--json"],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
-  });
-
-  it("preserves explicit parent command options in fallback argv", async () => {
-    const program = new Command().name("browser");
-    const parseAsync = vi.spyOn(program, "parseAsync").mockResolvedValue(program);
-    const actionCommand = {
-      name: () => "open",
-      parent: program,
-      args: ["about:blank"],
-    } as unknown as Command;
-    resolveCommandOptionArgsMock.mockReturnValue(["--json"]);
-
-    await reparseProgramFromActionArgs(program, [actionCommand]);
-
-    expect(resolveCommandOptionArgsMock).toHaveBeenCalledWith(program);
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "browser",
-      rawArgs: [],
-      fallbackArgv: ["--json", "open", "about:blank"],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
-  });
-
+describe("reparseProgramFromActionCommand", () => {
   it("uses root raw args and reparses the root for nested lazy commands", async () => {
     const root = new Command().name("openclaw");
     setRawArgs(root, ["node", "openclaw", "workspaces", "audit", "export", "--since", "1"]);
     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");
 
-    await reparseProgramFromActionArgs(audit, [exportCommand]);
+    await reparseProgramFromActionCommand(audit, exportCommand);
 
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: ["node", "openclaw", "workspaces", "audit", "export", "--since", "1"],
-      fallbackArgv: ["workspaces", "audit", "export", "--since", "1"],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
+    expect(parseAsync).toHaveBeenCalledWith([
+      "node",
+      "openclaw",
+      "workspaces",
+      "audit",
+      "export",
+      "--since",
+      "1",
+    ]);
     expect(auditParseAsync).not.toHaveBeenCalled();
   });
 
-  it("reconstructs the full nested command path when Commander rawArgs is missing", async () => {
-    // #83893: nested lazy commands still need their ancestor path if
-    // Commander stops exposing root rawArgs at runtime.
-    const root = new Command().name("openclaw");
-    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);
-
-    await reparseProgramFromActionArgs(audit, [exportCommand]);
-
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: undefined,
-      fallbackArgv: ["workspaces", "audit", "export", "--since", "1"],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
-  });
-
-  it("uses program root when action command is missing", async () => {
-    const program = new Command().name("openclaw");
-    const parseAsync = vi.spyOn(program, "parseAsync").mockResolvedValue(program);
-
-    await reparseProgramFromActionArgs(program, []);
-
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: [],
-      fallbackArgv: [],
-    });
-    expect(parseAsync).toHaveBeenCalledWith(["node", "openclaw", "status"]);
-  });
-
-  it("falls back to fallbackArgv when Commander rawArgs is missing from the root command", async () => {
-    // #83893: rawArgs is a Commander runtime field, so the root command must
-    // 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);
-
-    await reparseProgramFromActionArgs(root, [configCommand]);
-
-    expect(buildParseArgvMock).toHaveBeenCalledWith({
-      programName: "openclaw",
-      rawArgs: undefined,
-      fallbackArgv: ["config", "set", "key", "value"],
-    });
-    expect(parseAsync).toHaveBeenCalled();
-  });
-
   it("hoists a trailing lazy-parent option before the loaded command", async () => {
     const root = new Command().name("openclaw");
     const browser = root.command("browser").option("--browser-profile <name>");

src/cli/program/action-reparse.ts

@@ -1,8 +1,6 @@
-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 { resolveCommandOptionArgs } from "./helpers.js";
 
 function getCommandPathFromRoot(command: Command | undefined): Command[] {
   const path: Command[] = [];
@@ -16,25 +14,6 @@ function getCommandPathFromRoot(command: Command | undefined): Command[] {
   return path;
 }
 
-function buildFallbackArgv(program: Command, actionCommand: Command | undefined): string[] {
-  const actionArgsList = actionCommand?.args ?? [];
-  const parentOptionArgs =
-    actionCommand?.parent === program ? resolveCommandOptionArgs(program) : [];
-  const commandPath = getCommandPathFromRoot(actionCommand).map((command) => command.name());
-  if (commandPath.length === 0) {
-    return [...parentOptionArgs, ...actionArgsList];
-  }
-  return [
-    ...commandPath.slice(0, -1),
-    ...parentOptionArgs,
-    expectDefined(
-      commandPath[commandPath.length - 1],
-      "command path entry at command path.length 1",
-    ),
-    ...actionArgsList,
-  ];
-}
-
 function findRootCommand(cmd: Command): Command {
   let current: Command = cmd;
   while (current.parent) {
@@ -188,25 +167,14 @@ function hoistLazyParentOptions(
     : [...argv.slice(0, lazyCommandIndex), ...hoisted, lazyCommandName, ...remaining];
 }
 
-/** Rebuild argv from Commander action args and re-run parsing after lazy registration. */
-export async function reparseProgramFromActionArgs(
+/** Re-run parsing after replacing a lazy command placeholder. */
+export async function reparseProgramFromActionCommand(
   program: Command,
-  actionArgs: unknown[],
+  actionCommand: Command,
 ): Promise<void> {
-  const actionCommand = actionArgs.at(-1) as Command | undefined;
-  // Use the true root program for argv reconstruction and parsing.
-  // Commander keeps rawArgs as a JS runtime field, not a typed API; if a
-  // future version removes it, buildParseArgv falls back to reconstructed argv.
-  const rootProgram = findRootCommand(actionCommand ?? program);
-  const rawArgs = (rootProgram as Command & { rawArgs?: string[] }).rawArgs;
-  const fallbackArgv = buildFallbackArgv(program, actionCommand);
-  const parseArgv = buildParseArgv({
-    programName: rootProgram.name(),
-    rawArgs,
-    fallbackArgv,
-  });
-  const normalizedArgv = actionCommand
-    ? hoistLazyParentOptions(parseArgv, program, actionCommand.name())
-    : parseArgv;
+  const rootProgram = findRootCommand(actionCommand) as Command & { rawArgs: string[] };
+  // Commander 15 snapshots the full parse input on the root before actions run.
+  const parseArgv = buildParseArgv(rootProgram.rawArgs, rootProgram.name());
+  const normalizedArgv = hoistLazyParentOptions(parseArgv, program, actionCommand.name());
   await rootProgram.parseAsync(normalizedArgv);
 }

src/cli/program/helpers.test.ts

@@ -1,12 +1,10 @@
 // Program helper tests cover shared command registration and help helpers.
-import { Command } from "commander";
 import { describe, expect, it } from "vitest";
 import {
   collectOption,
   parsePositiveIntOrUndefined,
   parseStrictPositiveIntOption,
   parseStrictPositiveIntOrUndefined,
-  resolveCommandOptionArgs,
 } from "./helpers.js";
 
 describe("program helpers", () => {
@@ -61,46 +59,4 @@ describe("program helpers", () => {
       "--limit must be a positive integer.",
     );
   });
-
-  it("resolveCommandOptionArgs serializes explicit options", () => {
-    const command = new Command()
-      .option("--json", "JSON output", false)
-      .option("--timeout <ms>", "Timeout", "30000")
-      .option("--tag <name>", "Tag", collectOption)
-      .option("--no-progress", "Disable progress");
-
-    command.parse([
-      "node",
-      "test",
-      "--json",
-      "--timeout",
-      "10",
-      "--tag",
-      "a",
-      "--tag",
-      "b",
-      "--no-progress",
-    ]);
-
-    expect(resolveCommandOptionArgs(command)).toEqual([
-      "--json",
-      "--timeout",
-      "10",
-      "--tag",
-      "a",
-      "--tag",
-      "b",
-      "--no-progress",
-    ]);
-  });
-
-  it("resolveCommandOptionArgs skips defaults", () => {
-    const command = new Command()
-      .option("--json", "JSON output", false)
-      .option("--timeout <ms>", "Timeout", "30000");
-
-    command.parse(["node", "test"]);
-
-    expect(resolveCommandOptionArgs(command)).toStrictEqual([]);
-  });
 });

src/cli/program/helpers.ts

@@ -1,5 +1,5 @@
-// Shared Commander registration helpers for repeated options, positive ints, and lazy reparse args.
-import { InvalidArgumentError, type Command } from "commander";
+// Shared Commander registration helpers for repeated options and positive integers.
+import { InvalidArgumentError } from "commander";
 import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
 
 /** Commander option collector for repeatable string flags. */
@@ -28,60 +28,3 @@ export function parseStrictPositiveIntOption(value: string, flag: string): numbe
   }
   return parsed;
 }
-
-function appendOptionValue(out: string[], flag: string, value: unknown): void {
-  if (value === undefined) {
-    return;
-  }
-  if (value === false) {
-    if (flag.startsWith("--no-")) {
-      out.push(flag);
-    }
-    return;
-  }
-  if (value === true) {
-    out.push(flag);
-    return;
-  }
-  const arg = stringifyOptionValue(value);
-  if (arg !== undefined) {
-    out.push(flag, arg);
-  }
-}
-
-function stringifyOptionValue(value: unknown): string | undefined {
-  if (typeof value === "string") {
-    return value;
-  }
-  if (typeof value === "number" && Number.isFinite(value)) {
-    return String(value);
-  }
-  if (typeof value === "bigint") {
-    return value.toString();
-  }
-  return undefined;
-}
-
-/** Reconstruct explicit option tokens from a Commander command for lazy reparsing. */
-export function resolveCommandOptionArgs(command: Command): string[] {
-  const out: string[] = [];
-  for (const option of command.options) {
-    const name = option.attributeName();
-    if (command.getOptionValueSource(name) === "default") {
-      continue;
-    }
-    const flag = option.long ?? option.short;
-    if (!flag) {
-      continue;
-    }
-    const value = command.getOptionValue(name);
-    if (Array.isArray(value)) {
-      for (const item of value) {
-        appendOptionValue(out, flag, item);
-      }
-      continue;
-    }
-    appendOptionValue(out, flag, value);
-  }
-  return out;
-}

src/cli/program/register-lazy-command.ts

@@ -1,8 +1,7 @@
 // Lazy Commander placeholder registration used to keep CLI startup imports small.
 import type { Command } from "commander";
-import { reparseProgramFromActionArgs } from "./action-reparse.js";
+import { reparseProgramFromActionCommand } from "./action-reparse.js";
 import { removeCommandByName } from "./command-tree.js";
-import { resolveCommandOptionArgs } from "./helpers.js";
 
 type RegisterLazyCommandParams = {
   program: Command;
@@ -32,13 +31,10 @@ export function registerLazyCommand({
   placeholder.allowUnknownOption(true).allowExcessArguments(true);
   placeholder.action(async (...actionArgs) => {
     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);
     }
     await register();
-    await reparseProgramFromActionArgs(program, actionArgs);
+    await reparseProgramFromActionCommand(program, actionCommand);
   });
 }