- Notifications
You must be signed in to change notification settings - Fork3
feat: include ping and network stats on status tooltip#181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR adds ping and network statistics to the VPN status tooltip by extending the protocol, propagating new fields through the Swift models, and updating the UI and tests to surface ping-based status.
- Extend
vpn.protoand regenerate Swift Protobuf code to include a newLastPingmessage. - Wire up
lastPingand handshake data inMenuState, updateAgentStatuslogic to account for latency, and add UI tooltips. - Add and update unit tests to verify ping‐based status, and adjust tooltip timing in the app theme.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Coder-Desktop/VPNLib/vpn.proto | AddLastPing message and importduration.proto. |
| Coder-Desktop/VPNLib/vpn.pb.swift | Generated Swift support forLastPing and new fields. |
| Coder-Desktop/VPNLib/FileSync/FileSyncManagement.swift | Remove stale TODO comments. |
| Coder-Desktop/VPN/Manager.swift | Tweaklogger.log interpolation for message formatting. |
| Coder-Desktop/Coder-DesktopTests/VPNMenuStateTests.swift | Add tests forpoorConnection and adjust expected statuses. |
| Coder-Desktop/Coder-DesktopTests/AgentsTests.swift | IncludelastPing: nil in test agent fixtures. |
| Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenuItem.swift | AddstatusString and.help tooltip on status dot. |
| Coder-Desktop/Coder-Desktop/VPN/MenuState.swift | IntegrateLastPing intoAgent model, add status logic, helpers. |
| Coder-Desktop/Coder-Desktop/Theme.swift | DefinetooltipDelay constant (250 ms). |
| Coder-Desktop/Coder-Desktop/Coder_DesktopApp.swift | SetNSInitialToolTipDelay viaUserDefaults. |
Comments suppressed due to low confidence (3)
Coder-Desktop/Coder-Desktop/VPN/MenuState.swift:107
- Inserting
connectingwith raw value 1 shifts all subsequent enum raw values, which may break code or persisted data relying on those values. Consider maintaining backward compatibility by explicitly assigning the original values or migrating consumers.
case connecting = 1Coder-Desktop/Coder-Desktop/VPN/MenuState.swift:290
- There are no existing tests for the
connectingstatus path whenlastHandshakeis missing. Adding a unit test to cover this branch will ensure correct behavior for newly-started agents.
var status: AgentStatus {Coder-Desktop/VPN/Manager.swift:324
- [nitpick] The single privacy label applies to the entire interpolated string. If some fields should remain private, consider splitting the log call or marking each interpolation segment explicitly.
logger.log(level: level, "\(log.message, privacy: .public)\(fields.isEmpty ? "" : ": \(fields)", privacy: .public)")Uh oh!
There was an error while loading.Please reload this page.
745cf56 toa00c350CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closes#64.