fix: NSWindow main-thread crash in native macOS GUI - #126
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (7 files)
Previous Review Summaries (4 snapshots, latest commit 4f91d72)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 4f91d72)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (7 files)
Previous review (commit 88225a1)Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The systray antipattern was excised with surgical precision. Moving to a direct webview call is the right architecture — no more system tray drama. 💀 Worst part: The CLI returning success on catalog failure. A tool that claims victory when it can't even list models is more optimistic than a VC during a funding round. 📊 Overall: Like a car with a new engine but the check engine light permanently disabled. It runs, but you'll never know when it's coasting on fumes. Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit ae13de7)Verdict: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Removing the systray dependency entirely — that was cruft. Moving to a direct webview call on macOS is the right architecture. 💀 Worst part: The non-CGO 📊 Overall: The Darwin CGO path is slick. The rest of the planet's platforms were apparently an afterthought — the proxy now works exactly as well as my motivation on a Friday afternoon. Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 0006426)Verdict: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Removing the systray dependency entirely — that was cruft. Moving to a direct webview call on macOS is the right architecture. 💀 Worst part: The non-CGO 📊 Overall: The Darwin CGO path is slick. The rest of the planet's platforms were apparently an afterthought — the proxy now works exactly as well as my motivation on a Friday afternoon. Files Reviewed (4 files)
Reviewed by step-3.7-flash · Input: 65.6K · Output: 14.2K · Cached: 212.1K Review guidance: REVIEW.md from base branch |
|
All issues addressed:
Build passes on all platforms. |
…tdown - Spawn webview in a goroutine so NSWindow runs on the main thread, fixing a crash on macOS when opening the native GUI window. - Return a done channel from openGUI instead of passing a cancel function; the channel closes when the native window is dismissed, enabling clean shutdown on window close. - Fix 'Test model' dropdown to show configured model_overrides and model_family_overrides from /api/proxy/config. - Fix 'Refresh catalog' by passing CatalogDir and CatalogSourceURL to the GUI server. - Fix 'Send' to proxy through POST /api/test/send on the GUI server, avoiding CORS issues. Includes body size limit (1 MiB), io.Copy error handling, and table-driven test coverage.
a430924 to
c1c9eb5
Compare
|
@samueltuyizere the Kilo review was rate limited... but we should be good to go now :) |
taking a quick look into this @stefanoverna |
The native GUI (webview) crashed on macOS with: NSInternalInconsistencyException: NSWindow should only be instantiated on the main thread! Root cause: systray.Run() calls its onReady callback from a goroutine (not the main thread), but webview.New() creates an NSWindow which macOS 14+ strictly requires on the main thread. This bug was present since the Unified start command PR but never surfaced because the Homebrew release binary is built with CGO_ENABLED=0 (make dist), which excludes the darwin&&cgo build tag and uses the nocgo fallback that just prints the URL. The crash only happens with make build-ui (CGO_ENABLED=1). Fix: remove systray integration, call webview directly from the main goroutine. Window close triggers clean shutdown via context cancel.
The webview approach had main-thread issues and added a heavy dependency. Switch to getlantern/systray which was already in the codebase but unwired. Dashboard now opens in the default browser; tray icon provides Open/Quit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
What
The GUI dashboard crashed on macOS 14+ with an NSWindow main-thread violation because
systray.Run()spawned the webview from a goroutine, which AppKit doesn't allow.Fix
Removed the systray integration and call the webview directly from the main goroutine. The systray added complexity without providing essential functionality — the webview dashboard already serves the same purpose.
Testing