diff --git a/CHANGELOG.md b/CHANGELOG.md index 489d0ef26d4..8faa6b7d777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ Docs: https://docs.openclaw.ai - Control UI/Overview: prevent gateway access token/password visibility toggle buttons from overlapping their inputs at narrow widths. (#56924) Thanks @bbddbb1. - Control UI/cron: highlight the Cron refresh button while refresh is in flight so the page's loading state stays visible even when prior data remains on screen. (#60394) Thanks @coder-zhuzm. - MS Teams: replace the deprecated Teams SDK HttpPlugin stub with `httpServerAdapter` so recurring gateway deprecation warnings stop firing and the Express 5 compatibility workaround stays on the supported SDK path. (#60939) Thanks @coolramukaka-sys. +- CLI/Commander: preserve Commander-computed exit codes for argument and help-error paths, and cover the user-argv parse mode in the regression tests so invalid CLI invocations no longer report success when exits are intercepted. (#60923) Thanks @Linux2010. ## 2026.4.2 diff --git a/src/cli/program/build-program.test.ts b/src/cli/program/build-program.test.ts index 08b5fd825cb..ededb772233 100644 --- a/src/cli/program/build-program.test.ts +++ b/src/cli/program/build-program.test.ts @@ -1,6 +1,6 @@ import process from "node:process"; -import { Command } from "commander"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +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,8 +31,26 @@ vi.mock("./program-context.js", () => ({ })); describe("buildProgram", () => { + 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, 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"], @@ -41,6 +59,11 @@ describe("buildProgram", () => { } satisfies ProgramContext); }); + afterEach(() => { + process.exitCode = undefined; + vi.restoreAllMocks(); + }); + it("wires context/help/preaction/command registration with shared context", () => { const argv = ["node", "openclaw", "status"]; const originalArgv = process.argv; @@ -58,4 +81,60 @@ describe("buildProgram", () => { process.argv = originalArgv; } }); + + it("sets exitCode to 1 on argument errors (fixes #60905)", async () => { + const program = buildProgram(); + program.command("test").description("Test command"); + + const error = await expectCommanderExit( + program.parseAsync(["test", "unexpected-arg"], { from: "user" }), + 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 actionSpy = vi.fn(); + program.command("test").action(actionSpy); + + 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(); + program.command("test").description("Test command"); + + 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(); + program.version("1.0.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(); + program.helpCommand("help [command]"); + + const error = await expectCommanderExit( + program.parseAsync(["help", "missing"], { from: "user" }), + 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 index 75d9de3dbd3..9ca0427a75d 100644 --- 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,6 +9,13 @@ import { setProgramContext } from "./program-context.js"; export function buildProgram() { const program = new Command(); program.enablePositionalOptions(); + // 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 index 4f3810ca4a8..21522dcfdef 100644 --- 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 index eba7718ab84..5ca1ea316ef 100644 --- 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(); }