Skip to content

refactor(cli): normalize process exit through dispatchCommand - #1455

Merged
Wibias merged 3 commits into
devfrom
codex/cli-exit
Aug 11, 2026
Merged

Wibias merged 3 commits into
devfrom
codex/cli-exit

Conversation

@Wibias

@Wibias Wibias commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Normalize process exit handling so dispatch owns every exit code.

  • src/cli/dispatch.ts: CommandRunner becomes (deps) => Promise<number>. Every runner returns an exit code instead of calling process.exit or setting process.exitCode directly. dispatchCommand aggregates and returns the final code (unknown command → 1, help forms → 0).
  • src/cli/index.ts: the single process.exit(await dispatchCommand(...)) is now the only exit site in the CLI entry.
  • restore, route, integration, claude, ready, and health runners preserve their per-branch exit codes as return values.

Behavior is unchanged: exit code values and message text are identical; only the mechanism (return vs process.exit) changed.

Verification

  • bun run typecheck — exit 0.
  • ocx --version — exit 0.
  • ocx help nosuch — exit 1, Unknown command: nosuch.
  • ocx ready --timeout 5 — exit 64.
  • ocx sync --help — exit 0.
  • CLI-focused suite — 157 pass; the only failures are the known pre-existing cli-restore-back (2) and POST /api/sync (2) cases.

No GUI changes; no screenshot required.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review notes (stacked PR): stacks on #1451 (codex/cli-dispatch). Does not target dev. Diff is only the exit normalization (0bb19e1e..300c9cda): 3 files, +90/−66. Merge only after #1444, #1446, #1451 land.

Summary by CodeRabbit

  • Bug Fixes

    • Improved CLI exit-code handling so command results are reported consistently.
    • Preserved specific failure codes for readiness checks and other command errors.
    • Unknown commands now return a failure status instead of exiting unpredictably.
  • Tests

    • Updated CLI and readiness checks to validate returned exit codes and process termination behavior.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d2ee747d-0f34-4ab7-a85d-e3ee1796c399

📥 Commits

Reviewing files that changed from the base of the PR and between 2e134fa and a43ec84.

📒 Files selected for processing (3)
  • src/cli/dispatch.ts
  • src/cli/index.ts
  • tests/cli-ready.test.ts

📝 Walkthrough

Walkthrough

The CLI dispatch system now returns numeric exit codes from command runners through dispatchCommand to the top-level process.exit call. Readiness, health, integration, restore, update, tray, and other command paths preserve explicit status results.

Changes

CLI exit-code propagation

Layer / File(s) Summary
Command runner result propagation
src/cli/dispatch.ts
Command runners return explicit success and failure codes. Existing diagnostics, cleanup, side effects, and delegated results remain intact.
Dispatch and process exit contract
src/cli/dispatch.ts, src/cli/index.ts
dispatchCommand returns Promise<number>, including 0 for help and 1 for unknown commands. The CLI passes the returned code to process.exit.
Exit-code behavior validation
tests/cli-ready.test.ts
Tests verify the Promise<number> contract, dispatch result propagation, and readiness returning 64 when required arguments are missing.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: centralizing CLI process exit handling in dispatchCommand.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cli-exit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Aug 11, 2026
@Wibias
Wibias force-pushed the codex/cli-dispatch branch from 0bb19e1 to ee7fdbe Compare August 11, 2026 06:04
@Wibias
Wibias force-pushed the codex/cli-exit branch 2 times, most recently from dc4688b to b9889ef Compare August 11, 2026 06:32
@Wibias
Wibias force-pushed the codex/cli-dispatch branch from cb952c0 to 46b18f0 Compare August 11, 2026 08:48
@Wibias
Wibias changed the base branch from codex/cli-dispatch to dev August 11, 2026 09:00
Wibias added 2 commits August 11, 2026 11:00
Phase 4 of the CLI deepening: dispatch runners now RETURN exit codes
instead of calling process.exit/setting process.exitCode directly.
dispatchCommand aggregates and returns the final code; index.ts performs
the single process.exit(await dispatchCommand(...)).

- CommandRunner becomes (deps) => Promise<number>
- unknown command -> 1, help/--help/-h/undefined -> 0
- restore/route/integration/claude/ready/health runners return their codes
- behavior preserved: smoke tests (version 0, help nosuch 1, ready
  invalid 64, sync --help 0) pass; cli-ready runner assertion updated
Phase 4 exit normalization made runners return 0, but handlers passed via
deps (handleStart/handleStop/handleStatus/handleEnsure/handleUninstall/
handleRecoverHistory/handleProxyRestart/handleTrayProxyRestart) and
runInit set process.exitCode internally. The hardcoded return 0 overrode
those codes (e.g. ocx init exited 0 instead of 1 on stdin EOF). Return
process.exitCode ?? 0 so the handler-set codes are preserved.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/dispatch.ts`:
- Around line 374-375: Update the ready dispatch flow so normal results
propagate through the single top-level exit: change CliDispatchDeps.handleReady
to return Promise<number>, have index.ts handleReady return runReady(args)
instead of exiting, and preserve the missing-arguments 64 result. Add a focused
regression test alongside the existing ready dispatch tests covering a valid
ready command and asserting its result is returned.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e7552f0b-afe6-42ea-8f30-dee98ae10f71

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8c4fe and 2e134fa.

📒 Files selected for processing (3)
  • src/cli/dispatch.ts
  • src/cli/index.ts
  • tests/cli-ready.test.ts

Comment thread src/cli/dispatch.ts
Addresses CodeRabbit finding on #1455: handleReady was typed
Promise<never> and called process.exit(await runReady(args)), so a valid
'ocx ready' terminated below the dispatcher instead of returning its code
to the single top-level process.exit(await dispatchCommand(...)).

- handleReady now returns Promise<number> and returns runReady(args)
- CliDispatchDeps.handleReady typed Promise<number>
- cli-ready source test updated to assert the return-through-dispatch
  behavior (body returns runReady, no process.exit); invalid-args 64
  branch still covered
@Wibias
Wibias merged commit 316f675 into dev Aug 11, 2026
65 of 69 checks passed
@Wibias
Wibias deleted the codex/cli-exit branch August 11, 2026 09:38
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…jun#1455)

* refactor(cli): normalize process exit through dispatchCommand

Phase 4 of the CLI deepening: dispatch runners now RETURN exit codes
instead of calling process.exit/setting process.exitCode directly.
dispatchCommand aggregates and returns the final code; index.ts performs
the single process.exit(await dispatchCommand(...)).

- CommandRunner becomes (deps) => Promise<number>
- unknown command -> 1, help/--help/-h/undefined -> 0
- restore/route/integration/claude/ready/health runners return their codes
- behavior preserved: smoke tests (version 0, help nosuch 1, ready
  invalid 64, sync --help 0) pass; cli-ready runner assertion updated

* fix(cli): preserve handler-set exit codes in dispatch runners

Phase 4 exit normalization made runners return 0, but handlers passed via
deps (handleStart/handleStop/handleStatus/handleEnsure/handleUninstall/
handleRecoverHistory/handleProxyRestart/handleTrayProxyRestart) and
runInit set process.exitCode internally. The hardcoded return 0 overrode
those codes (e.g. ocx init exited 0 instead of 1 on stdin EOF). Return
process.exitCode ?? 0 so the handler-set codes are preserved.

* fix(cli): propagate ready result through dispatchCommand

Addresses CodeRabbit finding on lidge-jun#1455: handleReady was typed
Promise<never> and called process.exit(await runReady(args)), so a valid
'ocx ready' terminated below the dispatcher instead of returning its code
to the single top-level process.exit(await dispatchCommand(...)).

- handleReady now returns Promise<number> and returns runReady(args)
- CliDispatchDeps.handleReady typed Promise<number>
- cli-ready source test updated to assert the return-through-dispatch
  behavior (body returns runReady, no process.exit); invalid-args 64
  branch still covered
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant