[client] Fix Exit Node submenu separator accumulation on Windows#5691
Conversation
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 (netbirdio#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 netbirdio#4702
📝 WalkthroughWalkthroughAdds persistent tracking for the exit-node separator menu item and refactors menu recreation to remove and re-add that separator via a new helper, preventing duplicate separators when rebuilding the exit-node submenu. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
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.
|
|
Thanks for the contribution @tobsec |



Problem
Fixes #4702
On Windows, the tray app uses a 10-second background poller to refresh the Exit Node menu (because
TrayOpenedChis not supported on Windows). Every poll cycle that finds a selected exit node callss.mExitNode.AddSeparator()before adding the "Deselect All" item.AddSeparator()returns no handle, so the separator is never removed in the cleanup pass ofrecreateExitNodeMenu(). All other items — exit node checkboxes and "Deselect All" — are properly tracked and removed each cycle. The separator is not.After the client has been running for a while with an exit node selected, hundreds of separator lines stack up in the submenu, filling the full screen height with blank entries.
On Linux/FreeBSD this doesn't manifest because the parent
mExitNodeitem itself is removed and recreated each cycle (to work around a different systray limitation), which wipes all children including the orphaned separators.Fix
Replace the untracked
AddSeparator()call with a regular disabled sub-menu item. This gives us a handle that can be stored inmExitNodeSeparatorand removed at the start of eachrecreateExitNodeMenu()call, alongside the existingmExitNodeDeselectAllcleanup.The visual result is equivalent on Windows (a greyed-out, unclickable divider row), and the item is now properly cleaned up on every menu refresh.
Test
Run the Windows client with an exit node selected and leave it running for several minutes. Open the Exit Node submenu — previously it would grow with each poll; with this fix the menu stays clean regardless of uptime.
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit