cli: silence commander tests and rethrow exits

altaywtf · altaywtf · commit 0de0c43111a8 · 2026-04-05T03:16:57.000+03:00
diff --git a/src/cli/program/build-program.test.ts b/src/cli/program/build-program.test.ts
@@ -1,5 +1,5 @@
 import process from "node:process";
-import { Command } from "commander";
+import { Command, CommanderError } from "commander";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { buildProgram } from "./build-program.js";
 import type { ProgramContext } from "./context.js";
@@ -31,14 +31,26 @@ vi.mock("./program-context.js", () => ({
 }));
 
 describe("buildProgram", () => {
-  function mockProcessExit() {
-    return vi.spyOn(process, "exit").mockImplementation(((code?: number) => {
-      throw new Error(`process.exit:${String(code)}`);
-    }) as typeof process.exit);
+  function mockProcessOutput() {
+    vi.spyOn(process.stdout, "write").mockImplementation(
+      ((() => true) as unknown) as typeof process.stdout.write,
+    );
+    vi.spyOn(process.stderr, "write").mockImplementation(
+      ((() => true) as unknown) as typeof process.stderr.write,
+    );
+  }
+
+  async function expectCommanderExit(promise: Promise<unknown>, exitCode: number) {
+    const error = await promise.catch((err) => err);
+
+    expect(error).toBeInstanceOf(CommanderError);
+    expect(error).toMatchObject({ exitCode });
+    return error as CommanderError;
   }
 
   beforeEach(() => {
     vi.clearAllMocks();
+    mockProcessOutput();
     createProgramContextMock.mockReturnValue({
       programVersion: "9.9.9-test",
       channelOptions: ["telegram"],
@@ -72,62 +84,57 @@ describe("buildProgram", () => {
 
   it("sets exitCode to 1 on argument errors (fixes #60905)", async () => {
     const program = buildProgram();
-    const exitSpy = mockProcessExit();
     program.command("test").description("Test command");
 
-    await expect(program.parseAsync(["test", "unexpected-arg"], { from: "user" })).rejects.toThrow(
-      "process.exit:1",
+    const error = await expectCommanderExit(
+      program.parseAsync(["test", "unexpected-arg"], { from: "user" }),
+      1,
     );
-    expect(exitSpy).toHaveBeenCalledWith(1);
+
+    expect(error.code).toBe("commander.excessArguments");
     expect(process.exitCode).toBe(1);
   });
 
   it("does not run the command action after an argument error", async () => {
     const program = buildProgram();
-    const exitSpy = mockProcessExit();
     const actionSpy = vi.fn();
     program.command("test").action(actionSpy);
 
-    await expect(program.parseAsync(["test", "unexpected-arg"], { from: "user" })).rejects.toThrow(
-      "process.exit:1",
-    );
-    expect(exitSpy).toHaveBeenCalledWith(1);
+    await expectCommanderExit(program.parseAsync(["test", "unexpected-arg"], { from: "user" }), 1);
+
     expect(actionSpy).not.toHaveBeenCalled();
   });
 
   it("preserves exitCode 0 for help display", async () => {
     const program = buildProgram();
-    const exitSpy = mockProcessExit();
     program.command("test").description("Test command");
 
-    await expect(program.parseAsync(["--help"], { from: "user" })).rejects.toThrow(
-      "process.exit:0",
-    );
-    expect(exitSpy).toHaveBeenCalledWith(0);
+    const error = await expectCommanderExit(program.parseAsync(["--help"], { from: "user" }), 0);
+
+    expect(error.code).toBe("commander.helpDisplayed");
     expect(process.exitCode).toBe(0);
   });
 
   it("preserves exitCode 0 for version display", async () => {
     const program = buildProgram();
-    const exitSpy = mockProcessExit();
     program.version("1.0.0");
 
-    await expect(program.parseAsync(["--version"], { from: "user" })).rejects.toThrow(
-      "process.exit:0",
-    );
-    expect(exitSpy).toHaveBeenCalledWith(0);
+    const error = await expectCommanderExit(program.parseAsync(["--version"], { from: "user" }), 0);
+
+    expect(error.code).toBe("commander.version");
     expect(process.exitCode).toBe(0);
   });
 
   it("preserves non-zero exitCode for help error flows", async () => {
     const program = buildProgram();
-    const exitSpy = mockProcessExit();
     program.helpCommand("help [command]");
 
-    await expect(program.parseAsync(["help", "missing"], { from: "user" })).rejects.toThrow(
-      "process.exit:1",
+    const error = await expectCommanderExit(
+      program.parseAsync(["help", "missing"], { from: "user" }),
+      1,
     );
-    expect(exitSpy).toHaveBeenCalledWith(1);
+
+    expect(error.code).toBe("commander.help");
     expect(process.exitCode).toBe(1);
   });
 });
diff --git a/src/cli/program/build-program.ts b/src/cli/program/build-program.ts
@@ -1,3 +1,4 @@
+import process from "node:process";
 import { Command } from "commander";
 import { registerProgramCommands } from "./command-registry.js";
 import { createProgramContext } from "./context.js";
@@ -8,11 +9,12 @@ import { setProgramContext } from "./program-context.js";
 export function buildProgram() {
   const program = new Command();
   program.enablePositionalOptions();
-  // Ensure Commander.js sets exit code on argument errors (fixes #60905)
-  // Without this, commands like `openclaw sessions list` would print an error
-  // but exit with code 0, breaking scripts and monitoring tools.
+  // Preserve Commander-computed exit codes while still aborting parse flow.
+  // Without this, commands like `openclaw sessions list` can print an error
+  // but still report success when exits are intercepted.
   program.exitOverride((err) => {
     process.exitCode = typeof err.exitCode === "number" ? err.exitCode : 1;
+    throw err;
   });
   const ctx = createProgramContext();
   const argv = process.argv;
diff --git a/src/cli/run-main.exit.test.ts b/src/cli/run-main.exit.test.ts
@@ -1,4 +1,5 @@
 import process from "node:process";
+import { CommanderError } from "commander";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { runCli } from "./run-main.js";
 
@@ -14,6 +15,9 @@ const startTaskRegistryMaintenanceMock = vi.hoisted(() => vi.fn());
 const outputRootHelpMock = vi.hoisted(() => vi.fn());
 const outputPrecomputedRootHelpTextMock = vi.hoisted(() => vi.fn(() => false));
 const buildProgramMock = vi.hoisted(() => vi.fn());
+const getProgramContextMock = vi.hoisted(() => vi.fn(() => null));
+const registerCoreCliByNameMock = vi.hoisted(() => vi.fn());
+const registerSubCliByNameMock = vi.hoisted(() => vi.fn());
 const maybeRunCliInContainerMock = vi.hoisted(() =>
   vi.fn<
     (argv: string[]) => { handled: true; exitCode: number } | { handled: false; argv: string[] }
@@ -73,11 +77,24 @@ vi.mock("./program.js", () => ({
   buildProgram: buildProgramMock,
 }));
 
+vi.mock("./program/program-context.js", () => ({
+  getProgramContext: getProgramContextMock,
+}));
+
+vi.mock("./program/command-registry.js", () => ({
+  registerCoreCliByName: registerCoreCliByNameMock,
+}));
+
+vi.mock("./program/register.subclis.js", () => ({
+  registerSubCliByName: registerSubCliByNameMock,
+}));
+
 describe("runCli exit behavior", () => {
   beforeEach(() => {
     vi.clearAllMocks();
     hasMemoryRuntimeMock.mockReturnValue(false);
     outputPrecomputedRootHelpTextMock.mockReturnValue(false);
+    getProgramContextMock.mockReturnValue(null);
   });
 
   it("does not force process.exit after successful routed command", async () => {
@@ -149,4 +166,22 @@ describe("runCli exit behavior", () => {
     expect(process.exitCode).toBe(7);
     process.exitCode = exitCode;
   });
+
+  it("swallows Commander parse exits after recording the exit code", async () => {
+    const exitCode = process.exitCode;
+    buildProgramMock.mockReturnValueOnce({
+      commands: [{ name: () => "status" }],
+      parseAsync: vi
+        .fn()
+        .mockRejectedValueOnce(
+          new CommanderError(1, "commander.excessArguments", "too many arguments for 'status'"),
+        ),
+    });
+
+    await expect(runCli(["node", "openclaw", "status"])).resolves.toBeUndefined();
+
+    expect(registerSubCliByNameMock).toHaveBeenCalledWith(expect.anything(), "status");
+    expect(process.exitCode).toBe(1);
+    process.exitCode = exitCode;
+  });
 });
diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts
@@ -2,6 +2,7 @@ import { existsSync } from "node:fs";
 import path from "node:path";
 import process from "node:process";
 import { fileURLToPath } from "node:url";
+import { CommanderError } from "commander";
 import type { OpenClawConfig } from "../config/config.js";
 import { resolveStateDir } from "../config/paths.js";
 import { normalizeEnv } from "../infra/env.js";
@@ -231,7 +232,14 @@ export async function runCli(argv: string[] = process.argv) {
       }
     }
 
-    await program.parseAsync(parseArgv);
+    try {
+      await program.parseAsync(parseArgv);
+    } catch (error) {
+      if (!(error instanceof CommanderError)) {
+        throw error;
+      }
+      process.exitCode = error.exitCode;
+    }
   } finally {
     await closeCliMemoryManagers();
   }
