fix(desktop): register app:quit IPC handler to fix E2E quit hang#737
Merged
fix(desktop): register app:quit IPC handler to fix E2E quit hang#737
Conversation
The E2E test calls app:quit IPC to cleanly exit the packaged app, but the channel was never registered. This caused the test to fall back to SIGTERM + osascript, which hung indefinitely on CI (14 min timeout). - Add app:quit to hostInvokeChannels, payload/result type maps - Add IPC case handler that delegates to quitWithDecision() - Add setQuitHandlerOpts() setter so IPC handler can access quit opts - Harden E2E quit with 15s SIGKILL timeout on both IPC and SIGTERM paths
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 716dc07680
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address review feedback: - Use runTeardownAndExit() (finally → app.exit(0)) instead of quitWithDecision() so the process always exits even if teardown throws - Handle run-in-background separately (only hide window, no teardown) - quitHandlerOpts null fallback still calls app.exit(0) directly
When quitHandlerOpts is null (CI / orchestrator mode), app:quit now calls gracefulShutdown() via an injected fallback instead of bare app.exit(0). This ensures orchestrator-managed child processes (controller, web, openclaw) are properly cleaned up before exit.
Siri-Ray
approved these changes
Apr 1, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Register the missing
app:quitIPC channel so E2E tests can cleanly exit the packaged app without hanging.Why
Desktop E2E CI runs hang for ~14 minutes during app quit, then get cancelled by the CI timeout. Root cause: the E2E test calls
window.nexuHost.invoke("app:quit")but that channel was never registered, causing a silent fallback to SIGTERM + osascript which doesn't reliably exit on headless CI.Ref: https://github.com/nexu-io/nexu/actions/runs/23830575922
How
app:quittohostInvokeChannels,HostInvokePayloadMap, andHostInvokeResultMapinshared/host.tsipc.tsthat delegates to the existingquitWithDecision()functionsetQuitHandlerOpts()setter (same pattern assetUpdateManager) so the IPC handler can access quit options after launchd bootstrapsetQuitHandlerOpts()call inindex.tsright afterinstallLaunchdQuitHandler()quitPackagedApp()with 15-second SIGKILL hard timeout on both IPC and SIGTERM fallback pathsAffected areas
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (1 unrelated flaky test: skill-dir-watcher-workspace)pnpm generate-typesrun (if API routes/schemas changed)anytypes introduced (useunknownwith narrowing)Notes for reviewers
quitWithDecision()already existed inquit-handler.tsbut was never callable from IPC — this PR just wires it upapp:quitchannel accepts{ decision: "quit-completely" | "run-in-background" }, matching the existingQuitDecisiontypequitHandlerOptsis not yet set (cold start still in progress), falls back toapp.exit(0)directly