Skip to content

Commit dca2156

Browse files
Linux2010altaywtf
andauthored
fix(cli): set non-zero exit code on argument errors (openclaw#60923)
Merged via squash. Prepared head SHA: 0de0c43 Co-authored-by: Linux2010 <35169750+Linux2010@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf
1 parent f299bb8 commit dca2156

5 files changed

Lines changed: 134 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ Docs: https://docs.openclaw.ai
114114
- Control UI/Overview: prevent gateway access token/password visibility toggle buttons from overlapping their inputs at narrow widths. (#56924) Thanks @bbddbb1.
115115
- 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.
116116
- 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.
117+
- 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.
117118

118119
## 2026.4.2
119120

src/cli/program/build-program.test.ts

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import process from "node:process";
2-
import { Command } from "commander";
3-
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import { Command, CommanderError } from "commander";
3+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44
import { buildProgram } from "./build-program.js";
55
import type { ProgramContext } from "./context.js";
66

@@ -31,8 +31,26 @@ vi.mock("./program-context.js", () => ({
3131
}));
3232

3333
describe("buildProgram", () => {
34+
function mockProcessOutput() {
35+
vi.spyOn(process.stdout, "write").mockImplementation(
36+
((() => true) as unknown) as typeof process.stdout.write,
37+
);
38+
vi.spyOn(process.stderr, "write").mockImplementation(
39+
((() => true) as unknown) as typeof process.stderr.write,
40+
);
41+
}
42+
43+
async function expectCommanderExit(promise: Promise<unknown>, exitCode: number) {
44+
const error = await promise.catch((err) => err);
45+
46+
expect(error).toBeInstanceOf(CommanderError);
47+
expect(error).toMatchObject({ exitCode });
48+
return error as CommanderError;
49+
}
50+
3451
beforeEach(() => {
3552
vi.clearAllMocks();
53+
mockProcessOutput();
3654
createProgramContextMock.mockReturnValue({
3755
programVersion: "9.9.9-test",
3856
channelOptions: ["telegram"],
@@ -41,6 +59,11 @@ describe("buildProgram", () => {
4159
} satisfies ProgramContext);
4260
});
4361

62+
afterEach(() => {
63+
process.exitCode = undefined;
64+
vi.restoreAllMocks();
65+
});
66+
4467
it("wires context/help/preaction/command registration with shared context", () => {
4568
const argv = ["node", "openclaw", "status"];
4669
const originalArgv = process.argv;
@@ -58,4 +81,60 @@ describe("buildProgram", () => {
5881
process.argv = originalArgv;
5982
}
6083
});
84+
85+
it("sets exitCode to 1 on argument errors (fixes #60905)", async () => {
86+
const program = buildProgram();
87+
program.command("test").description("Test command");
88+
89+
const error = await expectCommanderExit(
90+
program.parseAsync(["test", "unexpected-arg"], { from: "user" }),
91+
1,
92+
);
93+
94+
expect(error.code).toBe("commander.excessArguments");
95+
expect(process.exitCode).toBe(1);
96+
});
97+
98+
it("does not run the command action after an argument error", async () => {
99+
const program = buildProgram();
100+
const actionSpy = vi.fn();
101+
program.command("test").action(actionSpy);
102+
103+
await expectCommanderExit(program.parseAsync(["test", "unexpected-arg"], { from: "user" }), 1);
104+
105+
expect(actionSpy).not.toHaveBeenCalled();
106+
});
107+
108+
it("preserves exitCode 0 for help display", async () => {
109+
const program = buildProgram();
110+
program.command("test").description("Test command");
111+
112+
const error = await expectCommanderExit(program.parseAsync(["--help"], { from: "user" }), 0);
113+
114+
expect(error.code).toBe("commander.helpDisplayed");
115+
expect(process.exitCode).toBe(0);
116+
});
117+
118+
it("preserves exitCode 0 for version display", async () => {
119+
const program = buildProgram();
120+
program.version("1.0.0");
121+
122+
const error = await expectCommanderExit(program.parseAsync(["--version"], { from: "user" }), 0);
123+
124+
expect(error.code).toBe("commander.version");
125+
expect(process.exitCode).toBe(0);
126+
});
127+
128+
it("preserves non-zero exitCode for help error flows", async () => {
129+
const program = buildProgram();
130+
program.helpCommand("help [command]");
131+
132+
const error = await expectCommanderExit(
133+
program.parseAsync(["help", "missing"], { from: "user" }),
134+
1,
135+
);
136+
137+
expect(error.code).toBe("commander.help");
138+
expect(process.exitCode).toBe(1);
139+
});
61140
});

src/cli/program/build-program.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import process from "node:process";
12
import { Command } from "commander";
23
import { registerProgramCommands } from "./command-registry.js";
34
import { createProgramContext } from "./context.js";
@@ -8,6 +9,13 @@ import { setProgramContext } from "./program-context.js";
89
export function buildProgram() {
910
const program = new Command();
1011
program.enablePositionalOptions();
12+
// Preserve Commander-computed exit codes while still aborting parse flow.
13+
// Without this, commands like `openclaw sessions list` can print an error
14+
// but still report success when exits are intercepted.
15+
program.exitOverride((err) => {
16+
process.exitCode = typeof err.exitCode === "number" ? err.exitCode : 1;
17+
throw err;
18+
});
1119
const ctx = createProgramContext();
1220
const argv = process.argv;
1321

src/cli/run-main.exit.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import process from "node:process";
2+
import { CommanderError } from "commander";
23
import { beforeEach, describe, expect, it, vi } from "vitest";
34
import { runCli } from "./run-main.js";
45

@@ -14,6 +15,9 @@ const startTaskRegistryMaintenanceMock = vi.hoisted(() => vi.fn());
1415
const outputRootHelpMock = vi.hoisted(() => vi.fn());
1516
const outputPrecomputedRootHelpTextMock = vi.hoisted(() => vi.fn(() => false));
1617
const buildProgramMock = vi.hoisted(() => vi.fn());
18+
const getProgramContextMock = vi.hoisted(() => vi.fn(() => null));
19+
const registerCoreCliByNameMock = vi.hoisted(() => vi.fn());
20+
const registerSubCliByNameMock = vi.hoisted(() => vi.fn());
1721
const maybeRunCliInContainerMock = vi.hoisted(() =>
1822
vi.fn<
1923
(argv: string[]) => { handled: true; exitCode: number } | { handled: false; argv: string[] }
@@ -73,11 +77,24 @@ vi.mock("./program.js", () => ({
7377
buildProgram: buildProgramMock,
7478
}));
7579

80+
vi.mock("./program/program-context.js", () => ({
81+
getProgramContext: getProgramContextMock,
82+
}));
83+
84+
vi.mock("./program/command-registry.js", () => ({
85+
registerCoreCliByName: registerCoreCliByNameMock,
86+
}));
87+
88+
vi.mock("./program/register.subclis.js", () => ({
89+
registerSubCliByName: registerSubCliByNameMock,
90+
}));
91+
7692
describe("runCli exit behavior", () => {
7793
beforeEach(() => {
7894
vi.clearAllMocks();
7995
hasMemoryRuntimeMock.mockReturnValue(false);
8096
outputPrecomputedRootHelpTextMock.mockReturnValue(false);
97+
getProgramContextMock.mockReturnValue(null);
8198
});
8299

83100
it("does not force process.exit after successful routed command", async () => {
@@ -149,4 +166,22 @@ describe("runCli exit behavior", () => {
149166
expect(process.exitCode).toBe(7);
150167
process.exitCode = exitCode;
151168
});
169+
170+
it("swallows Commander parse exits after recording the exit code", async () => {
171+
const exitCode = process.exitCode;
172+
buildProgramMock.mockReturnValueOnce({
173+
commands: [{ name: () => "status" }],
174+
parseAsync: vi
175+
.fn()
176+
.mockRejectedValueOnce(
177+
new CommanderError(1, "commander.excessArguments", "too many arguments for 'status'"),
178+
),
179+
});
180+
181+
await expect(runCli(["node", "openclaw", "status"])).resolves.toBeUndefined();
182+
183+
expect(registerSubCliByNameMock).toHaveBeenCalledWith(expect.anything(), "status");
184+
expect(process.exitCode).toBe(1);
185+
process.exitCode = exitCode;
186+
});
152187
});

src/cli/run-main.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { existsSync } from "node:fs";
22
import path from "node:path";
33
import process from "node:process";
44
import { fileURLToPath } from "node:url";
5+
import { CommanderError } from "commander";
56
import type { OpenClawConfig } from "../config/config.js";
67
import { resolveStateDir } from "../config/paths.js";
78
import { normalizeEnv } from "../infra/env.js";
@@ -231,7 +232,14 @@ export async function runCli(argv: string[] = process.argv) {
231232
}
232233
}
233234

234-
await program.parseAsync(parseArgv);
235+
try {
236+
await program.parseAsync(parseArgv);
237+
} catch (error) {
238+
if (!(error instanceof CommanderError)) {
239+
throw error;
240+
}
241+
process.exitCode = error.exitCode;
242+
}
235243
} finally {
236244
await closeCliMemoryManagers();
237245
}

0 commit comments

Comments
 (0)