fix(ui): route tray status repaints through relayoutMenu to fix item races

buildMenu reassigns every menu item pointer under menuMu, but the status
path (refreshMenuItemsForStatus, applyStatusIndicator), the click handlers
and the session ticker read them unguarded. Wails dispatches event
listeners and OnClick callbacks on fresh goroutines, so these reads race
the rebuilds and applyStatus even runs concurrently with itself.

- applyStatus drives a single aggregated relayoutMenu (status, daemon
  version and session-deadline changes); refreshMenuItemsForStatus is
  removed — relayoutMenu repaints the same items from the caches
- applyStatusIndicator is SetBitmap-only: its unguarded SetMenu could
  reinstall a stale tree and pushed a half-built one on every relayout
- Connect/Disconnect handlers receive the clicked item from the buildMenu
  closure instead of reading the menuMu-guarded fields
- applySessionExpiry is cache-only; the row is painted by relayoutMenu,
  the 30s ticker snapshots its item under menuMu
- drop loadProfiles' dead unguarded profileSubmenu nil-check

client/ui/CLAUDE.md

@@ -9,7 +9,8 @@ This is the Wails v3 desktop UI for NetBird. Go services live in `services/`; th
 ### Go (top-level package `main`)
 - `main.go` — app entry. Builds the shared gRPC `Conn`, constructs services, registers them with Wails, creates the main webview window, then starts (in order) the Linux SNI watcher → tray → `peers.Watch` → `app.Run`. CLI flags: `--daemon-addr`, `--log-file` (repeatable; default is **empty** so "no flag" is distinguishable from explicit `--log-file console` — when empty `parseFlagsAndInitLog` falls back to `console` for `InitLog` and returns `userSetLogFile=false`), `--log-level` (`trace|debug|info|warn|error`, default `info`). See "GUI debug logging" below for what `userSetLogFile` gates.
 - `tray.go` — `Tray` struct + menu. Subscribes to `EventStatus`, `EventSystem`, `EventUpdateAvailable`, `EventUpdateProgress`. Owns per-status icon/dot, Profiles submenu, Connect/Disconnect swap, About → Update, session-expired toast.
-- **Tray menu updates go through `relayoutMenu` (whole-tree rebuild), never in-place submenu mutation.** Any dynamic menu change — Profiles submenu (`tray_profiles.go loadProfiles` → caches rows under `profilesMu`, then `fillProfileSubmenu`), Exit Node submenu (`tray_exitnodes.go refreshExitNodes` → `fillExitNodeSubmenu`), daemon-version row (`tray_status.go`), and the About → Update row (`tray_update.go applyState` → `onMenuChange` callback) — rebuilds the entire menu via `Tray.relayoutMenu` (`buildMenu()` + repaint cached state + single `t.tray.SetMenu`). Serialised by `menuMu`. **Why:** on KDE/Plasma the StatusNotifierItem host caches a submenu's layout the first time it's opened (`GetLayout` for that submenu id) and never re-fetches it on a `LayoutUpdated(parent=0)` signal — so the old `submenu.Clear()`+`Add()` left both the visible rows AND the click→id mapping frozen on the first snapshot. Because `Clear()`+`Add()` allocates fresh monotonic item ids each time (Wails `menuitem.go`), clicks then sent ids the rebuilt `itemMap` no longer knew, and silently no-op'd ("Manage Profiles" stopped responding after the first switch). `buildMenu()` allocates a brand-new submenu container id each relayout, which Plasma treats as unseen and re-queries on next open — fixing both the stale paint and the dead clicks. Confirmed via `dbus-monitor`: a re-opened submenu issued no `GetLayout` until its container id changed. The whole-tree `SetMenu` also subsumes the older darwin detached-NSMenu workaround. `fill*Submenu` helpers are pure UI (read caches, no daemon fetch, no `SetMenu`) so `relayoutMenu` never recurses back into the fetchers.
+- **Tray menu updates go through `relayoutMenu` (whole-tree rebuild), never in-place submenu mutation.** Any dynamic menu change — status-text transitions (`tray_status.go applyStatus`, which also folds in daemon-version and session-deadline changes into one aggregated relayout per push), Profiles submenu (`tray_profiles.go loadProfiles` → caches rows under `profilesMu`, then `fillProfileSubmenu`), Exit Node submenu (`tray_exitnodes.go refreshExitNodes` → `fillExitNodeSubmenu`), and the About → Update row (`tray_update.go applyState` → `onMenuChange` callback) — rebuilds the entire menu via `Tray.relayoutMenu` (`buildMenu()` + repaint cached state + single `t.tray.SetMenu`). Serialised by `menuMu`. **Why:** on KDE/Plasma the StatusNotifierItem host caches a submenu's layout the first time it's opened (`GetLayout` for that submenu id) and never re-fetches it on a `LayoutUpdated(parent=0)` signal — so the old `submenu.Clear()`+`Add()` left both the visible rows AND the click→id mapping frozen on the first snapshot. Because `Clear()`+`Add()` allocates fresh monotonic item ids each time (Wails `menuitem.go`), clicks then sent ids the rebuilt `itemMap` no longer knew, and silently no-op'd ("Manage Profiles" stopped responding after the first switch). `buildMenu()` allocates a brand-new submenu container id each relayout, which Plasma treats as unseen and re-queries on next open — fixing both the stale paint and the dead clicks. Confirmed via `dbus-monitor`: a re-opened submenu issued no `GetLayout` until its container id changed. The whole-tree `SetMenu` also subsumes the older darwin detached-NSMenu workaround. `fill*Submenu` helpers are pure UI (read caches, no daemon fetch, no `SetMenu`) so `relayoutMenu` never recurses back into the fetchers.
+- **Tray concurrency model (menuMu domain).** Wails dispatches `Event.On` listeners and menu `OnClick` callbacks on **fresh goroutines** (so `applyStatus` runs are concurrent with each other and with relayouts), and Wails `MenuItem` setters are not goroutine-safe. Hence `t.menu` and every `*Item`/`*Submenu` field on `Tray` are `menuMu`-owned: `buildMenu` reassigns them all on each relayout, and repaints happen from the caches inside `relayoutMenu` (caches are committed *before* the menu is touched, which makes a write on an orphaned pre-relayout item self-healing). Two sanctioned out-of-lock accesses: the Connect/Disconnect `OnClick` closures capture their own item, and `refreshSessionExpiresLabel` (30s ticker) snapshots `sessionExpiresItem` under `menuMu`. `relayoutMenu` is the **only post-startup `tray.SetMenu` call site** — never push a menu pointer snapshotted outside `menuMu` (it can reinstall a stale tree); `applyStatusIndicator` is `SetBitmap`-only, the relayout's trailing `SetMenu` is what repaints the macOS dot. Tray code never runs on the OS main thread, so taking `menuMu` from any goroutine can't deadlock the setters' `dispatch_sync`/`InvokeSync`.
 - `tray_linux.go` — `init()` sets `WEBKIT_DISABLE_DMABUF_RENDERER=1` (blank-white window on VMs / minimal WMs) and `WEBKIT_DISABLE_COMPOSITING_MODE=1` (Intel/Mesa SIGSEGV in `g_application_run` via unimplemented DRM-format-modifier paths — DMABUF-disable alone doesn't cover the GL compositor). Both are skipped if the user already set the var. Also `WEBKIT_DISABLE_SANDBOX_THIS_IS_DANGEROUS=1` when unprivileged userns are blocked.
 - `tray_watcher_linux.go`, `xembed_host_linux.go`, `xembed_tray_linux.{c,h}` — in-process SNI watcher + XEmbed bridge for minimal WMs. See `LINUX-TRAY.md`.
 - `signal_unix.go` / `signal_windows.go` — `listenForShowSignal`. Unix uses SIGUSR1; Windows uses a named event `Global\NetBirdQuickActionsTriggerEvent`. Mirrors the legacy Fyne UI's external-trigger contract so the installer / CLI keep working.

client/ui/tray.go

@@ -89,13 +89,15 @@ type Tray struct {
 	// language switch.
 	loc *Localizer
 
+	// menu and the *Item/*Submenu fields below are reassigned by buildMenu
+	// on every relayout — touch them only with menuMu held. Exceptions:
+	// the Connect/Disconnect OnClick closures capture their own item, and
+	// refreshSessionExpiresLabel snapshots its item under menuMu.
 	menu       *application.Menu
 	statusItem *application.MenuItem
 	// sessionExpiresItem displays the SSO session deadline as a humanised
-	// remaining-time label ("Session: 47m"). Hidden when no deadline is
-	// tracked (non-SSO peer or login-expiration disabled on the account).
-	// Refreshed by applyStatus on every Status push and by a 1-minute
-	// ticker between pushes so the countdown moves naturally.
+	// remaining-time label ("Session: 47m"). Painted by relayoutMenu from
+	// the sessionMu cache; a 30s ticker keeps the countdown moving.
 	sessionExpiresItem *application.MenuItem
 	upItem             *application.MenuItem
 	downItem           *application.MenuItem
@@ -180,11 +182,10 @@ type Tray struct {
 	profiles     []services.Profile
 	profilesUser string
 
-	// menuMu serialises relayoutMenu — the full buildMenu + SetMenu cycle.
-	// loadProfiles (under profileLoadMu) and refreshExitNodes (under
-	// exitNodesRebuildMu) both drive a relayout from independent mutexes, and
-	// applyLanguage drives one from the Localizer goroutine; without this guard
-	// two relayouts could interleave their t.menu swap and SetMenu push.
+	// menuMu serialises relayoutMenu (buildMenu + SetMenu) and guards the
+	// menu/item-pointer fields above. relayoutMenu is the only post-startup
+	// SetMenu call site — a menu snapshot pushed outside the lock could
+	// reinstall a stale tree.
 	menuMu sync.Mutex
 
 	// exitNodesMu guards the t.exitNodes row cache so reading the cached
@@ -411,10 +412,13 @@ func (t *Tray) relayoutMenu() {
 		}
 	}
 	if t.upItem != nil {
+		// Connect stays visible in the NeedsLogin states too — Up drives
+		// the SSO re-auth flow; hidden only when it would be a no-op.
 		t.upItem.SetHidden(connected || connecting || daemonUnavailable)
 		t.upItem.SetEnabled(!connected && !connecting && !daemonUnavailable)
 	}
 	if t.downItem != nil {
+		// Disconnect doubles as the abort path while still Connecting.
 		t.downItem.SetHidden(!connected && !connecting)
 		t.downItem.SetEnabled(connected || connecting)
 	}
@@ -468,11 +472,16 @@ func (t *Tray) buildMenu() *application.Menu {
 	menu.AddSeparator()
 
 	// Only the action that applies to the current state is visible: Connect
-	// when disconnected, Disconnect when connected. applyStatus swaps them on
-	// each daemon status change.
-	t.upItem = menu.Add(t.loc.T("tray.menu.connect")).OnClick(func(*application.Context) { t.handleConnect() })
-	t.downItem = menu.Add(t.loc.T("tray.menu.disconnect")).OnClick(func(*application.Context) { t.handleDisconnect() })
-	t.downItem.SetHidden(true)
+	// when disconnected, Disconnect when connected. The OnClick closures
+	// capture the local item — t.upItem/t.downItem are menuMu-guarded and
+	// must not be read from the click goroutine.
+	upItem := menu.Add(t.loc.T("tray.menu.connect"))
+	upItem.OnClick(func(*application.Context) { t.handleConnect(upItem) })
+	t.upItem = upItem
+	downItem := menu.Add(t.loc.T("tray.menu.disconnect"))
+	downItem.OnClick(func(*application.Context) { t.handleDisconnect(downItem) })
+	downItem.SetHidden(true)
+	t.downItem = downItem
 
 	menu.AddSeparator()
 
@@ -584,7 +593,9 @@ func (t *Tray) buildMenu() *application.Menu {
 	return menu
 }
 
-func (t *Tray) handleConnect() {
+// handleConnect receives the clicked item from the buildMenu closure —
+// t.upItem is menuMu-guarded and must not be read here.
+func (t *Tray) handleConnect(upItem *application.MenuItem) {
 	// NeedsLogin/SessionExpired/LoginFailed mean the daemon won't honor a
 	// plain Up RPC ("up already in progress: current status NeedsLogin") —
 	// it needs the Login → WaitSSOLogin → Up sequence instead. Emit
@@ -601,7 +612,7 @@ func (t *Tray) handleConnect() {
 		t.app.Event.Emit(services.EventTriggerLogin)
 		return
 	}
-	t.upItem.SetEnabled(false)
+	upItem.SetEnabled(false)
 	// Arm the SSO auto-handoff: Up() is async and the daemon may flip to
 	// NeedsLogin once it detects an SSO peer with no cached token. The
 	// flag is consumed by applyStatus on that transition, which then
@@ -619,7 +630,7 @@ func (t *Tray) handleConnect() {
 			t.statusMu.Lock()
 			t.pendingConnectLogin = false
 			t.statusMu.Unlock()
-			t.upItem.SetEnabled(true)
+			upItem.SetEnabled(true)
 		}
 	}()
 }
@@ -630,8 +641,9 @@ func (t *Tray) handleConnect() {
 // no-op. Also clears Peers' optimistic-Connecting guard so the daemon's
 // Idle push (and any subsequent updates) paint through immediately
 // instead of being swallowed by the profile-switch suppression filter.
-func (t *Tray) handleDisconnect() {
-	t.downItem.SetEnabled(false)
+// Receives the clicked item from the buildMenu closure (see handleConnect).
+func (t *Tray) handleDisconnect(downItem *application.MenuItem) {
+	downItem.SetEnabled(false)
 	t.profileMu.Lock()
 	if t.switchCancel != nil {
 		t.switchCancel()
@@ -643,7 +655,7 @@ func (t *Tray) handleDisconnect() {
 		if err := t.svc.Connection.Down(context.Background()); err != nil {
 			log.Errorf("disconnect: %v", err)
 			t.notifyError(t.loc.T("notify.error.disconnect"))
-			t.downItem.SetEnabled(true)
+			downItem.SetEnabled(true)
 		}
 	}()
 }

client/ui/tray_profiles.go

@@ -54,9 +54,6 @@ func (t *Tray) loadConfig() {
 // into the live submenu) is what makes KDE/Plasma actually repaint and keep
 // the click→id mapping live — see relayoutMenu's doc comment.
 func (t *Tray) loadProfiles() {
-	if t.profileSubmenu == nil {
-		return
-	}
 	t.profileLoadMu.Lock()
 	defer t.profileLoadMu.Unlock()
 	ctx := context.Background()

client/ui/tray_session.go

@@ -50,11 +50,10 @@ func (t *Tray) handleSessionExpired() {
 	}
 }
 
-// applySessionExpiry refreshes the "Session: 47m" tray row from the latest
-// SSO deadline carried on the Status snapshot. Hidden when no deadline is
-// tracked or the tunnel is down; otherwise renders the remaining time via
-// formatSessionRemaining.
-func (t *Tray) applySessionExpiry(deadline *time.Time, connected bool) {
+// applySessionExpiry refreshes the cached SSO deadline and reports whether
+// it changed. Cache-only — the tray row is painted by relayoutMenu;
+// applyStatus drives a relayout when this returns true.
+func (t *Tray) applySessionExpiry(deadline *time.Time, connected bool) bool {
 	var d time.Time
 	if connected && deadline != nil {
 		d = *deadline
@@ -76,17 +75,7 @@ func (t *Tray) applySessionExpiry(deadline *time.Time, connected bool) {
 				deadline.Format(time.RFC3339), time.Until(*deadline), connected)
 		}
 	}
-
-	if t.sessionExpiresItem == nil {
-		return
-	}
-	if d.IsZero() {
-		t.sessionExpiresItem.SetHidden(true)
-		return
-	}
-	remaining := t.formatSessionRemaining(time.Until(d))
-	t.sessionExpiresItem.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining))
-	t.sessionExpiresItem.SetHidden(false)
+	return changed
 }
 
 // runSessionExpiryTicker keeps the "Expires in …" countdown row fresh by
@@ -99,10 +88,15 @@ func (t *Tray) runSessionExpiryTicker() {
 	}
 }
 
-// refreshSessionExpiresLabel recomputes the "Session expires in …" tray
-// row label from the cached SSO deadline.
+// refreshSessionExpiresLabel recomputes the countdown row label from the
+// cached SSO deadline. The item is snapshotted under menuMu (buildMenu
+// reassigns it on every relayout); no full relayout here — a 30s-cadence
+// rebuild could disturb an open menu.
 func (t *Tray) refreshSessionExpiresLabel() {
-	if t.sessionExpiresItem == nil {
+	t.menuMu.Lock()
+	item := t.sessionExpiresItem
+	t.menuMu.Unlock()
+	if item == nil {
 		return
 	}
 	t.sessionMu.Lock()
@@ -112,7 +106,7 @@ func (t *Tray) refreshSessionExpiresLabel() {
 		return
 	}
 	remaining := t.formatSessionRemaining(time.Until(deadline))
-	t.sessionExpiresItem.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining))
+	item.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining))
 }
 
 // formatSessionRemaining renders the time-to-deadline as a localised

client/ui/tray_status.go

@@ -58,9 +58,19 @@ func (t *Tray) applyStatus(st services.Status) {
 		t.app.Event.Emit(services.EventTriggerLogin)
 	}
 
+	// Cache-only; the row is painted by the relayout below.
+	sessionChanged := t.applySessionExpiry(st.SessionExpiresAt, connected)
+
 	if iconChanged {
 		t.applyIcon()
-		t.refreshMenuItemsForStatus(st, connected)
+	}
+	// All repainting goes through relayoutMenu (menuMu-serialised, paints
+	// from the caches committed above): applyStatus runs concurrently with
+	// itself and with relayouts (Wails dispatches listeners on fresh
+	// goroutines), so in-place item mutation here would race the buildMenu
+	// pointer swap.
+	if iconChanged || daemonVersionChanged || sessionChanged {
+		t.relayoutMenu()
 	}
 	// Re-fetch the selectable exit-node list whenever the daemon's routed-
 	// networks revision bumps (a route candidate added/removed, or a selection
@@ -71,18 +81,14 @@ func (t *Tray) applyStatus(st services.Status) {
 	if iconChanged || revisionChanged {
 		go t.refreshExitNodes()
 	}
-	if daemonVersionChanged && t.daemonVersionItem != nil {
-		// The version row lives in the About submenu, which KDE/Plasma caches on
-		// first open and never re-fetches on a plain SetLabel (see relayoutMenu's
-		// doc comment). Drive a full relayout so the new version actually paints.
-		// relayoutMenu repaints the label from the cached lastDaemonVersion.
-		t.relayoutMenu()
+	// The daemon emits no active-profile event, so profile flips driven
+	// elsewhere (CLI, autoconnect) surface via status transitions.
+	if iconChanged {
+		go t.loadProfiles()
 	}
 	if sessionExpiredEnter {
 		t.handleSessionExpired()
 	}
-
-	t.applySessionExpiry(st.SessionExpiresAt, connected)
 }
 
 // consumePendingConnectLogin acts on the SSO auto-handoff flag armed by
@@ -111,84 +117,14 @@ func (t *Tray) consumePendingConnectLogin(status string) bool {
 	return false
 }
 
-// refreshMenuItemsForStatus updates the status row, Connect/Disconnect
-// enablement, Settings/Profiles gating, and Profiles submenu on a status-text
-// transition (called from applyStatus only when iconChanged).
-func (t *Tray) refreshMenuItemsForStatus(st services.Status, connected bool) {
-	daemonUnavailable := strings.EqualFold(st.Status, services.StatusDaemonUnavailable)
-	connecting := strings.EqualFold(st.Status, services.StatusConnecting)
-	if t.statusItem != nil {
-		// Label-only: row is informational (no OnClick). Enablement
-		// is platform-dependent via statusRowEnabled — Windows
-		// keeps it enabled so the Win32 disabled-state mask does
-		// not desaturate the coloured dot; macOS/Linux disable it.
-		// Swap the displayed text so the user sees a familiar
-		// phrase instead of the raw daemon enum.
-		t.statusItem.SetLabel(t.loc.StatusLabel(st.Status))
-		t.statusItem.SetEnabled(statusRowEnabled())
-		t.applyStatusIndicator(st.Status)
-	}
-	if t.upItem != nil {
-		// Connect stays visible/clickable in NeedsLogin/SessionExpired/
-		// LoginFailed too — the daemon's Up RPC kicks off the SSO flow
-		// when re-auth is required, mirroring the legacy Fyne client
-		// where the same button drove the initial and the re-login
-		// paths. Hidden only when the action would be a no-op (tunnel
-		// up, daemon mid-connect — Disconnect takes the slot) or
-		// would fail with no useful side effect (daemon unreachable).
-		t.upItem.SetHidden(connected || connecting || daemonUnavailable)
-		t.upItem.SetEnabled(!connected && !connecting && !daemonUnavailable)
-	}
-	if t.downItem != nil {
-		// Disconnect is the abort path while the daemon is still
-		// retrying the management dial — without it the user has no
-		// way to stop the loop short of killing the daemon.
-		t.downItem.SetHidden(!connected && !connecting)
-		t.downItem.SetEnabled(connected || connecting)
-	}
-	// Exit Node parent-item enablement (greyed unless the tunnel is up
-	// AND at least one candidate exists) is owned by refreshExitNodes,
-	// triggered by applyStatus on this same transition. Settings just needs
-	// the daemon socket reachable.
-	if t.settingsItem != nil {
-		t.settingsItem.SetEnabled(!daemonUnavailable)
-	}
-	disableProfiles, _ := t.featuresDisabled()
-	if t.profileSubmenuItem != nil {
-		t.profileSubmenuItem.SetEnabled(!daemonUnavailable && !disableProfiles)
-	}
-	// Refresh the Profiles submenu on every status-text transition: the
-	// daemon does not emit an active-profile event, so the startup race
-	// (UI loads profiles before autoconnect picks the persisted profile)
-	// and a CLI "profile select && up" both surface here. loadProfiles
-	// fetches the rows and drives a full relayoutMenu (serialised by menuMu),
-	// so it cannot race the SetHidden/SetEnabled writes on the static items
-	// above — the Wails 3 alpha menu API is not goroutine-safe and reads
-	// item.disabled/item.hidden at NSMenuItem construction time.
-	go t.loadProfiles()
-}
-
-// applyStatusIndicator sets the small coloured dot shown on the status
-// menu entry. The dot mirrors the tray icon's state through a fixed
-// palette: green for Connected, yellow for Connecting, blue for the
-// login states, red for hard errors, grey for the idle/disconnected
-// pair and a darker grey when the daemon socket is unreachable.
-//
-// Wails v3 alpha's setMenuItemBitmap calls NSMenuItem.setImage from
-// whichever thread invoked SetBitmap — unlike setMenuItemLabel/Disabled/
-// Hidden/Checked which dispatch_sync onto the main queue. The off-thread
-// AppKit call leaves the visible dot stale until the next time the menu
-// is reopened (close+reopen workaround). Rebuilding via tray.SetMenu
-// reruns processMenu inside InvokeSync, so the bitmap is applied to a
-// fresh NSMenuItem on the main thread and macOS picks it up.
+// applyStatusIndicator sets the coloured status dot. Called only from
+// relayoutMenu (menuMu held): on macOS the bitmap repaints via the
+// relayout's trailing SetMenu — no SetMenu here, the tree is half-built.
 func (t *Tray) applyStatusIndicator(status string) {
 	if t.statusItem == nil {
 		return
 	}
 	t.statusItem.SetBitmap(statusIndicatorBitmap(status))
-	if t.menu != nil {
-		t.tray.SetMenu(t.menu)
-	}
 }
 
 func statusIndicatorBitmap(status string) []byte {