Skip to content

fix: NSWindow main-thread crash in native macOS GUI#126

Open
stefanoverna wants to merge 2 commits into
routatic:mainfrom
stefanoverna:fix/gui-nswindow-main-thread-crash
Open

fix: NSWindow main-thread crash in native macOS GUI#126
stefanoverna wants to merge 2 commits into
routatic:mainfrom
stefanoverna:fix/gui-nswindow-main-thread-crash

Conversation

@stefanoverna

Copy link
Copy Markdown
Contributor

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

  • Built and verified GUI launches without crash on macOS 15
  • No regression on Linux/Windows (systray was already no-op there)

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.
)

func openGUI(guiURL string) error {
func openGUI(guiURL string, _ context.CancelFunc) error {

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.

🔥 The Roast: On Linux/Windows this path prints a URL and returns. Because cancel() was passed in from main.go:391 and is called right here on function exit, the proxy's ctx is cancelled immediately after servers are started — killing both the proxy and HTTP servers before they serve a single request. The Ctrl+C signal listener below is dead on arrival.

🩹 The Fix:

Suggested change
func openGUI(guiURL string, _ context.CancelFunc) error {
func openGUI(guiURL string, _ context.CancelFunc) error {

📏 Severity: critical


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

)

func openGUI(guiURL string) error {
func openGUI(guiURL string, _ context.CancelFunc) error {

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.

🔥 The Roast: Same disease as the nocgo Darwin path — cancel() fires as soon as this function returns, murdering every server the proxy just started. On Linux and Windows, the start() function wastes CPU cycles spinning up proxy and GUI servers just to nuke them milliseconds later. Efficiency!

🩹 The Fix:

Suggested change
func openGUI(guiURL string, _ context.CancelFunc) error {
func openGUI(guiURL string, _ context.CancelFunc) error {

📏 Severity: critical


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

fmt.Printf("\nDashboard: %s\n", guiURL)
fmt.Println("\nPress Ctrl+C to stop.")
}
// Open GUI (macOS: native webview, Linux/Windows: print URL)

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.

🔥 The Roast: Your comment is at 2-tab indent while the conditional it describes is at 3 tabs. The comment is shallower than the code it's annotating — this is the programming equivalent of wearing a hat indoors and calling it "a personal choice."

🩹 The Fix:

Suggested change
// Open GUI (macOS: native webview, Linux/Windows: print URL)
// Open GUI (macOS: native webview, Linux/Windows: print URL)

📏 Severity: nitpick


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Code Review Roast 🔥

Verdict: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 2
💡 suggestion 0
🤏 nitpick 1
Issue Details (click to expand)
File Line Roast
cmd/routatic-proxy/start_gui_darwin_nocgo.go 10 cancel() fires immediately after function returns, killing all servers on Linux without CGO before they serve a request
cmd/routatic-proxy/start_gui_other.go 10 Same catastrophic behavior on every non-darwin platform — servers die before the startup log is printed
cmd/routatic-proxy/main.go 390 Comment indented one level shallower than the code it describes

🏆 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 openGUI paths are now antimatter for the proxy. Every server is started, then immediately murdered by an overzealous cancel() call. You've built a distributed system that self-destructs in under a millisecond.

📊 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)
  • cmd/routatic-proxy/main.go – 1 issue (signature change propagates cancel to non-blocking paths)
  • cmd/routatic-proxy/start_gui_darwin.go – no issues (blocking path, cancel behavior is correct)
  • cmd/routatic-proxy/start_gui_darwin_nocgo.go – 1 critical issue
  • cmd/routatic-proxy/start_gui_other.go – 1 critical issue

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash · Input: 49.7K · Output: 9.4K · Cached: 314.1K

Review guidance: REVIEW.md from base branch main (truncated)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants