Skip to content

fix: ensure CLI processes exit after command completion on Windows#74425

Open
1yihui wants to merge 3 commits intoopenclaw:mainfrom
1yihui:fix/issue-74378
Open

fix: ensure CLI processes exit after command completion on Windows#74425
1yihui wants to merge 3 commits intoopenclaw:mainfrom
1yihui:fix/issue-74378

Conversation

@1yihui
Copy link
Copy Markdown
Contributor

@1yihui 1yihui commented Apr 29, 2026

Summary

Fixes CLI processes (node.exe openclaw.mjs) remaining alive after command execution on Windows. The CLI entrypoint was missing process.exit() calls after commands completed, causing processes to hang.

Changes

  • Added explicit process.exit(process.exitCode ?? 0) call after CLI entrypoint execution in openclaw.mjs
  • Ensures all async code paths properly terminate on Windows

Testing

  • Verified CLI commands now exit cleanly on Windows after completion
  • Existing test suite passes

Fixes #74378

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR adds an explicit process.exit(process.exitCode ?? 0) call at the end of openclaw.mjs to prevent CLI processes from hanging on Windows after command execution. The fix correctly reads process.exitCode before calling exit, defaulting to 0 if it hasn't been set, which is the standard pattern for this kind of Windows event-loop hang.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-reasoned, and follows established Node.js patterns for CLI process termination.

Single-line addition using the idiomatic process.exitCode ?? 0 pattern. No logic is altered; process.exit() is called only after all top-level awaits complete, so all intended work finishes before the process terminates. No security or correctness concerns identified.

No files require special attention.

Reviews (1): Last reviewed commit: "fix: ensure CLI processes exit after com..." | Re-trigger Greptile

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 29, 2026

Codex review: needs real behavior proof before merge.

Summary
The branch adds a launcher output flush plus final process.exit, awaits the root version fast path, updates launcher/version tests, and adds a changelog entry.

Reproducibility: yes. The linked issue gives concrete Windows npm direct-entry commands and lingering-process timings, and current main source shows completed launcher paths can return without a final shutdown; I did not execute the Windows smoke in this Linux review.

Real behavior proof
Needs real behavior proof before merge: The PR body asserts Windows verification but does not include terminal output, logs, screenshots, or linked artifacts showing the after-fix process exit behavior.

Next step before merge
The remaining blocker is human-visible Windows real behavior proof for an external PR, not a narrow repair that ClawSweeper can safely create in this repository.

Security
Cleared: The diff only changes CLI launcher lifecycle, tests, and changelog text; it does not alter dependencies, workflow permissions, secrets, package resolution, or external code execution sources.

Review details

Best possible solution:

Land this PR or an equivalent narrow launcher lifecycle fix after Windows npm smokes prove status and --version exit cleanly without lingering node.exe processes or truncated output.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue gives concrete Windows npm direct-entry commands and lingering-process timings, and current main source shows completed launcher paths can return without a final shutdown; I did not execute the Windows smoke in this Linux review.

Is this the best way to solve the issue?

Yes, with validation pending. Awaiting the version fast path and flushing launcher output before the final exit is the narrowest maintainable source fix visible from the diff; the remaining blocker is real Windows behavior proof, not another code change.

Acceptance criteria:

  • On Windows with an npm-installed OpenClaw package from the PR build, run node <npm-global-openclaw>/openclaw.mjs status and verify the command completes without a lingering node.exe.
  • On Windows with the same install, run node <npm-global-openclaw>/openclaw.mjs --version and verify complete output plus clean process exit.
  • Keep the targeted tests from the maintainer comment green: pnpm test test/openclaw-launcher.e2e.test.ts src/entry.version-fast-path.test.ts src/cli/run-main.exit.test.ts.

What I checked:

  • current_main_launcher_has_no_final_exit: Current main handles root help/browser help or imports the built entrypoint, then falls out of the launcher block without a completed-command shutdown. (openclaw.mjs:386, 7f27c42ebdb3)
  • current_main_version_fast_path_is_not_awaited: Current main calls tryHandleRootVersionFastPath(process.argv) synchronously while that helper resolves version output through a promise chain. (src/entry.ts:172, 7f27c42ebdb3)
  • pr_flushes_before_forced_exit: The PR adds flushWritableStream, flushProcessOutput, then awaits flushing before calling process.exit(process.exitCode ?? 0). (openclaw.mjs:383, 40a3b628d8d9)
  • pr_awaits_version_fast_path: The PR changes the root version helper to return Promise<boolean> and awaits it from src/entry.ts, removing the import-completion race for --version. (src/entry.version-fast-path.ts:4, 40a3b628d8d9)
  • pr_adds_regression_coverage_and_changelog: The PR updates version fast-path expectations, adds launcher help-output flush coverage, and adds a Windows CLI changelog entry. (test/openclaw-launcher.e2e.test.ts:149, 40a3b628d8d9)
  • real_behavior_proof_still_pending: The PR body asserts Windows verification, but the discussion and triage: needs-real-behavior-proof label show the required npm-installed Windows smoke output/artifact has not been attached yet.

Likely related people:

  • steipete: Local history/blame puts the current launcher and entrypoint lines under recent Peter Steinberger maintenance, and file shortlog shows the largest share of touches across openclaw.mjs, src/entry.ts, src/entry.version-fast-path.ts, launcher tests, and src/cli/run-main.ts. (role: recent CLI startup maintainer; confidence: high; commits: 5dfaed184608, d1f36bfd846b, c90b3e4d5ea5; files: openclaw.mjs, src/entry.ts, src/cli/run-main.ts)
  • vincentkoc: Commit history ties Vincent Koc to the root --help fast path and lazy CLI startup path adjacent to the launcher output behavior changed here. (role: root help fast-path contributor; confidence: medium; commits: 38da2d076c8a; files: src/entry.ts, openclaw.mjs)
  • altaywtf: The version-output path changed by this PR is connected to the merged commit that added commit-hash display to --version and updated the same entry/version tests. (role: version output contributor; confidence: medium; commits: ca5e352c53e8; files: src/entry.ts, src/entry.version-fast-path.test.ts, src/infra/git-commit.ts)

Remaining risk / open question:

  • No after-fix Windows npm-installed CLI smoke proof is attached yet, so the affected node.exe exit behavior and complete stdout/stderr delivery are not live-proven on the reported platform.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7f27c42ebdb3.

@BradGroux
Copy link
Copy Markdown
Member

Maintainer follow-up pushed to address the review blockers.

What changed:

  • Rebased onto current main.
  • Kept the launcher final exit, but moved it behind stdio flushing so help/version/JSON output is not truncated.
  • Made the root --version fast path await version resolution before module evaluation completes, so the launcher cannot exit before version output is written.
  • Kept compile-cache respawn parents out of the forced-exit path so they can continue supervising their child process.
  • Added regression coverage and a changelog entry.

Validation:

  • pnpm test test/openclaw-launcher.e2e.test.ts src/entry.version-fast-path.test.ts src/cli/run-main.exit.test.ts
  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md openclaw.mjs src/entry.ts src/entry.version-fast-path.ts src/entry.version-fast-path.test.ts test/openclaw-launcher.e2e.test.ts
  • pnpm check:changed
  • GitHub required checks: green

Remaining before merge per review notes: Windows npm-installed smoke proof that node <npm-global-openclaw>/openclaw.mjs status exits without lingering node.exe, and node <npm-global-openclaw>/openclaw.mjs --version prints complete output and exits. I do not have a Windows host in this workspace, so I am holding merge until that smoke is approved or provided.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: OpenClaw CLI commands remain alive as node.exe processes after execution on Windows

2 participants