Greptile Summary

This PR fixes a long-standing CLI bug where openclaw sessions list (and similar invalid invocations) would print a Commander error but still exit with code 0, breaking scripts and monitoring tools. The fix registers an exitOverride callback on the Commander Program that sets process.exitCode from the Commander error's own exit code and rethrows the error, cleanly aborting the parse.

The implementation correctly addresses both concerns raised in the prior review round:

• The callback now rethrows so Commander's parse is aborted and action handlers do not run after an argument error.
• The callback no longer contains special-cased dead-code branches for commander.version; it simply mirrors err.exitCode for every error code.

Key observations:

• src/cli/program/build-program.ts: Clean and minimal. The exitOverride callback sets process.exitCode (with a fallback to 1 if exitCode is not a number) then rethrows.
• src/cli/run-main.ts: The existing try/catch around program.parseAsync (lines 235-242) now activates for Commander errors because exitOverride throws. It redundantly re-sets process.exitCode = error.exitCode, which is harmless since it is the same value that exitOverride already wrote.
• src/cli/program/build-program.test.ts: The new \"does not run the command action after an argument error\" test is the most important addition — it directly validates the abort-before-action behavior that was the crux of the original issue.
• src/cli/run-main.exit.test.ts: New integration-level test file; enableConsoleCapture and installUnhandledRejectionHandler are invoked with real implementations (both have global side effects), as noted in the inline comment."

Confidence Score: 5/5

Safe to merge — the logic fix is correct, the prior P1 rethrow concern is resolved, and no new correctness issues were found.

All remaining findings are P2 style/test-isolation concerns. The core behavior (setting exit code, aborting parse before running the action handler) is verified by the new unit tests. No P0 or P1 issues remain.

src/cli/run-main.exit.test.ts — missing mocks for enableConsoleCapture and installUnhandledRejectionHandler could cause test contamination across the suite.

This is a comment left during a code review.
Path: src/cli/run-main.exit.test.ts
Line: 170-186

Comment:
**Missing mocks for side-effectful imports in integration test**

The `"swallows Commander parse exits"` test exercises the full `runCli` code path through the Commander `parseAsync` call, which means two un-mocked modules are invoked with their real implementations:

1. **`enableConsoleCapture`** (static import from `../logging.js`, called at `run-main.ts:175`) — patches global `console.*` methods and registers EPIPE listeners on `process.stdout`/`process.stderr`. These patches persist for the lifetime of the test process.

2. **`installUnhandledRejectionHandler`** (dynamically imported from `../infra/unhandled-rejections.js`, called at `run-main.ts:183`) — attaches a `process.unhandledRejection` listener that can invoke `process.exit(1)` for fatal rejections. Because the listener is never removed, it remains active in all subsequent tests in the same process.

Neither module appears in the `vi.mock()` list for this file. Mocking both would prevent global console patching and stop the persistent `unhandledRejection` listener from triggering unexpected exits in other tests.

```ts
vi.mock("../logging.js", () => ({ enableConsoleCapture: vi.fn() }));
vi.mock("../infra/unhandled-rejections.js", () => ({
  installUnhandledRejectionHandler: vi.fn(),
}));
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "cli: silence commander tests and rethrow..." | Re-trigger Greptile}

@greptileai review

@greptileai review

Merged via squash.

• Prepared head SHA: 0de0c43111a82c14ae0235ba01a5aa89ed89d378
• Merge commit: dca21563c6386f32dc573a81d0506e0f6e66df86

Thanks @Linux2010!