[client] Fix UI stuck in "Connecting" state when daemon reports "Connected"

[pappz]

Fix UI stuck in "Connecting" state when daemon reports "Connected"

The UI can get stuck showing "Connecting" status even after the daemon successfully connects and reports "Connected" status. This occurs because the condition to update the UI to "Connected" state checks the wrong flag.

Describe your changes

Issue ticket number and link

Stack

Checklist

• Is it a bug fix
• Is a typo/documentation fix
• Is a feature enhancement
• It is a refactor
• Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

• I added/updated documentation for this change
• Documentation is not needed for this change (explain why)

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

• Bug Fixes
  • Improved connection status detection to ensure UI elements reliably reflect accurate connection state and respond appropriately to connection changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified the condition in the updateStatus method to gate the connected state update on the internal s.connected flag instead of the mUp menu item's disabled state, transferring control-flow logic from the UI menu item state to an explicit internal flag.

Changes

Cohort / File(s)	Summary
Connected state condition gate client/ui/client_ui.go	Changed condition from !s.mUp.Disabled() to !s.connected in updateStatus method, altering when connected state, icons, tooltips, and related UI elements are updated

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• [client] Fix Linux UI flickering on state updates #4886: Also modifies updateStatus in the same file and changes conditions for the connected branch behavior

Poem

A flag now guards the UI gate,
No more menu items seal the fate,
Internal state leads the dance,
With cleaner logic's second chance! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The pull request description covers the main issue and correctly identifies it as a bug fix. However, it is incomplete: the 'Describe your changes' section is empty, and no issue ticket number/link is provided despite the template requiring these sections.	Complete the 'Describe your changes' section with details about the condition change and provide an issue ticket number/link if one exists for tracking this bug.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title accurately and specifically describes the bug being fixed: UI stuck in 'Connecting' state when daemon reports 'Connected'. It is clear, concise, and directly relates to the main change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96cdd56 and a6bea48.

📒 Files selected for processing (1)

• client/ui/client_ui.go

🧰 Additional context used

🧬 Code graph analysis (1)

client/ui/client_ui.go (2)

client/internal/state.go (1)

• StatusConnected (14-14)

client/internal/peer/conn_status.go (1)

• StatusConnected (13-13)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)

• GitHub Check: Relay / Unit (386)
• GitHub Check: Management / Unit (amd64, sqlite)
• GitHub Check: Management / Unit (amd64, mysql)
• GitHub Check: Management / Unit (amd64, postgres)
• GitHub Check: Relay / Unit (amd64, -race)
• GitHub Check: Client / Unit (386)
• GitHub Check: Client (Docker) / Unit
• GitHub Check: Client / Unit (amd64)
• GitHub Check: JS / Lint
• GitHub Check: release_ui_darwin
• GitHub Check: release_ui
• GitHub Check: release
• GitHub Check: FreeBSD Port / Build & Test
• GitHub Check: Linux
• GitHub Check: Client / Unit
• GitHub Check: Client / Unit
• GitHub Check: Darwin
• GitHub Check: Windows
• GitHub Check: Client / Unit
• GitHub Check: Android / Build
• GitHub Check: iOS / Build

🔇 Additional comments (1)

client/ui/client_ui.go (1)

912-912: Excellent fix—correctly addresses the root cause.

The original condition created a circular dependency: it checked !s.mUp.Disabled() (whether the Up menu item was enabled) to decide when to update the UI to the connected state. However, during the "Connecting" state, mUp is explicitly disabled (line 996 in setConnectingStatus), which prevented the condition from ever triggering when transitioning from Connecting → Connected.

The fix correctly uses the internal s.connected flag instead. Since s.connected is set to false during the connecting state (line 992), this condition now properly triggers when the daemon reports "Connected" while the UI is in the "Connecting" state.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud