Repository navigation
fix(companion): stop owned gateway during desktop exit - #212
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe desktop ChangesShutdown ordering
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to Desktop exit now stops the owned gateway before asynchronous WebSocket cleanup, preventing the local gateway from being left running after the application closes. No merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, aligns with the described shutdown behavior, and uses an existing bounded synchronous disposal path to reliably stop the owned gateway before the exit handler can yield.
Pull request overview
Fixes a desktop shutdown race in OpenClaw.Companion where the owned local gateway process could be orphaned because desktop.Exit uses an async void handler and does not await post-await cleanup work.
Changes:
- Dispose the owned
ManagedGatewayServicesynchronously at the start of the desktop exit handler to ensure the gateway process is stopped before the handler reaches its firstawait. - Keep WebSocket client cleanup asynchronous (
DisposeAsync) without blocking guaranteed owned-process cleanup.
File summaries
| File | Description |
|---|---|
| src/OpenClaw.Companion/App.axaml.cs | Moves owned gateway shutdown to a synchronous disposal before the first await in the desktop exit handler to avoid orphaned child processes. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The v0.2.0 desktop smoke reproduced an orphaned gateway: closing Companion exited the parent, but the owned gateway stayed alive on 127.0.0.1:18789 with PPID 1. The desktop Exit event does not await its async handler, so awaiting WebSocket disposal could prevent managed-process cleanup from running.
Stop the owned gateway synchronously before the handler reaches its first await. The existing bounded synchronous disposal also kills child processes. This keeps connection cleanup asynchronous while guaranteeing owned-process cleanup is attempted during shutdown.
Validation: the packaged pre-fix binary reproduced the failure during setup/start/restart/close testing. The locally published fixed candidate passed the same process-level check: it auto-started and connected, then closing the window exited both Companion PID 7938 and owned gateway PID 8138 and freed port 18789. The final release bundle will receive the same check before publication. The v0.2.0 release remains a draft until this passes.