Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jun 6, 2025

Closes #64.

Screenshot 2025-06-06 at 4 03 59 pm

Screenshot 2025-06-06 at 4 03 51 pm

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ethanndickson ethanndickson marked this pull request as ready for review June 6, 2025 06:13
@ethanndickson ethanndickson requested a review from Copilot June 6, 2025 06:13
Copy link

@Copilot Copilot AI left a 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.proto and regenerate Swift Protobuf code to include a new LastPing message.
  • Wire up lastPing and handshake data in MenuState, update AgentStatus logic 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 Add LastPing message and import duration.proto.
Coder-Desktop/VPNLib/vpn.pb.swift Generated Swift support for LastPing and new fields.
Coder-Desktop/VPNLib/FileSync/FileSyncManagement.swift Remove stale TODO comments.
Coder-Desktop/VPN/Manager.swift Tweak logger.log interpolation for message formatting.
Coder-Desktop/Coder-DesktopTests/VPNMenuStateTests.swift Add tests for poorConnection and adjust expected statuses.
Coder-Desktop/Coder-DesktopTests/AgentsTests.swift Include lastPing: nil in test agent fixtures.
Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenuItem.swift Add statusString and .help tooltip on status dot.
Coder-Desktop/Coder-Desktop/VPN/MenuState.swift Integrate LastPing into Agent model, add status logic, helpers.
Coder-Desktop/Coder-Desktop/Theme.swift Define tooltipDelay constant (250 ms).
Coder-Desktop/Coder-Desktop/Coder_DesktopApp.swift Set NSInitialToolTipDelay via UserDefaults.
Comments suppressed due to low confidence (3)

Coder-Desktop/Coder-Desktop/VPN/MenuState.swift:107

  • Inserting connecting with 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 = 1

Coder-Desktop/Coder-Desktop/VPN/MenuState.swift:290

  • There are no existing tests for the connecting status path when lastHandshake is 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)")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show latency on workspace view
2 participants