From 13807f1b3d75d06b2b513da5b7a11cfe09363fd7 Mon Sep 17 00:00:00 2001
From: tobsec <8700196+tobsec@users.noreply.github.com>
Date: Mon, 30 Mar 2026 10:41:38 +0200
Subject: [PATCH] [client] Fix Exit Node submenu separator accumulation on
 Windows (#5691)

* client/ui: fix Exit Node submenu separator accumulation on Windows

On Windows the tray uses a background poller (every 10s) instead of
TrayOpenedCh to keep the Exit Node menu fresh. Each poll that has a
selected exit node called s.mExitNode.AddSeparator() before the
"Deselect All" item. Because AddSeparator() returns no handle the
separator was never removed in the cleanup pass of
recreateExitNodeMenu(), while every other item (exit node checkboxes
and the "Deselect All" entry) was properly tracked and removed.

After the client has been running for a while with an exit node
selected this leaves hundreds of separator lines stacked in the
submenu, filling the screen height with blank entries (#4702).

On Linux/FreeBSD this is masked because the parent mExitNode item
itself is removed and recreated each cycle, wiping all children
including orphaned separators.

Fix: replace the untracked AddSeparator() call with a regular disabled
sub-menu item that is stored in mExitNodeSeparator and removed at the
start of each recreateExitNodeMenu() call alongside mExitNodeDeselectAll.

Fixes #4702

* client/ui: extract addExitNodeDeselectAll to reduce cognitive complexity

Move the separator + deselect-all creation and its goroutine listener
out of recreateExitNodeMenu into a dedicated helper, bringing the
function's cognitive complexity back under the SonarCloud threshold.
---
 client/ui/client_ui.go |  1 +
 client/ui/network.go   | 52 +++++++++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/client/ui/client_ui.go b/client/ui/client_ui.go
index 0574e53d0..b1e0aec41 100644
--- a/client/ui/client_ui.go
+++ b/client/ui/client_ui.go
@@ -324,6 +324,7 @@ type serviceClient struct {
 	exitNodeMu           sync.Mutex
 	mExitNodeItems       []menuHandler
 	exitNodeRetryCancel  context.CancelFunc
+	mExitNodeSeparator   *systray.MenuItem
 	mExitNodeDeselectAll *systray.MenuItem
 	logFile              string
 	wLoginURL            fyne.Window
diff --git a/client/ui/network.go b/client/ui/network.go
index ed03f5ada..571e871bb 100644
--- a/client/ui/network.go
+++ b/client/ui/network.go
@@ -421,6 +421,10 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) {
 		node.Remove()
 	}
 	s.mExitNodeItems = nil
+	if s.mExitNodeSeparator != nil {
+		s.mExitNodeSeparator.Remove()
+		s.mExitNodeSeparator = nil
+	}
 	if s.mExitNodeDeselectAll != nil {
 		s.mExitNodeDeselectAll.Remove()
 		s.mExitNodeDeselectAll = nil
@@ -453,31 +457,37 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) {
 	}
 
 	if showDeselectAll {
-		s.mExitNode.AddSeparator()
-		deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All")
-		s.mExitNodeDeselectAll = deselectAllItem
-		go func() {
-			for {
-				_, ok := <-deselectAllItem.ClickedCh
-				if !ok {
-					// channel closed: exit the goroutine
-					return
-				}
-				exitNodes, err := s.handleExitNodeMenuDeselectAll()
-				if err != nil {
-					log.Warnf("failed to handle deselect all exit nodes: %v", err)
-				} else {
-					s.exitNodeMu.Lock()
-					s.recreateExitNodeMenu(exitNodes)
-					s.exitNodeMu.Unlock()
-				}
-			}
-
-		}()
+		s.addExitNodeDeselectAll()
 	}
 
 }
 
+func (s *serviceClient) addExitNodeDeselectAll() {
+	sep := s.mExitNode.AddSubMenuItem("───────────────", "")
+	sep.Disable()
+	s.mExitNodeSeparator = sep
+
+	deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All")
+	s.mExitNodeDeselectAll = deselectAllItem
+
+	go func() {
+		for {
+			_, ok := <-deselectAllItem.ClickedCh
+			if !ok {
+				return
+			}
+			exitNodes, err := s.handleExitNodeMenuDeselectAll()
+			if err != nil {
+				log.Warnf("failed to handle deselect all exit nodes: %v", err)
+			} else {
+				s.exitNodeMu.Lock()
+				s.recreateExitNodeMenu(exitNodes)
+				s.exitNodeMu.Unlock()
+			}
+		}
+	}()
+}
+
 func (s *serviceClient) getExitNodes(conn proto.DaemonServiceClient) ([]*proto.Network, error) {
 	ctx, cancel := context.WithTimeout(s.ctx, defaultFailTimeout)
 	defer cancel()