feat: send WakeRequest to the tunnel on system resume#175
Draft
EhabY wants to merge 4 commits into
Draft
Conversation
After the machine sleeps and wakes, the tunnel can stay unusable for several minutes because a short, same-network wake often produces no link-change event that would trigger recovery. The tunnel binary (tunnel protocol 1.3) accepts a WakeRequest hint that forces network path re-discovery. Subscribe to SystemEvents.PowerModeChanged and, on resume, send a WakeRequest over the existing tunnel RPC channel when the negotiated protocol version supports it. Failures are logged and never affect the running tunnel. Fixes #172
- Sync WakeResponse with upstream (success field was removed) - Convert SystemResumeMonitor to a standard event-based class wired into Manager, instead of a hosted service calling into IManager - Trim comments and condense tests
Rename SupportsFeature to IsAtLeast with standard version ordering so a future major protocol bump keeps sending wake requests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #172
After the machine sleeps and wakes, the Coder Connect tunnel can stay unusable for several minutes until magicsock's periodic re-STUN recovers the path on its own. The daemon side (coder/coder#26739, tunnel protocol
1.3) added aWakeRequestRPC that forces network path re-discovery on demand. This PR makes the Windows manager emit it on system resume.Changes
Vpn.Proto/vpn.proto: synced with upstreamcoder/coder; addsWakeRequest/WakeResponse(bindings generated at build time).RpcVersion:Currentbumped1.1->1.3; newIsAtLeast()ordered comparison for version-gating.Speaker: exposesNegotiatedVersion, captured during the handshake (previously discarded).TunnelSupervisor: surfaces the tunnel'sNegotiatedVersionthroughITunnelSupervisor.SystemResumeMonitor(new): event-based class raisingResumedfromSystemEvents.PowerModeChanged; registered as a singleton.Manager: subscribes toResumed(mirroring the existingIManagerRpc.OnReceivewiring) and sendsWakeRequestwhen the negotiated version is>= 1.3, with a 5s reply timeout. The send is fire-and-forget off the SystemEvents broadcast thread;SendWakeRequestlogs and swallows all failures by construction, so nothing can fault the discarded task or affect the running tunnel.Tests
SpeakerTest: negotiated version exposed after handshake.RpcVersionTest:IsAtLeastordering.ManagerTest(new): wake request sent on the resume event; skipped when the tunnel isn't running or the version is too old; send failures are swallowed.Implementation decisions
1.2'slast_pingfields were already present in this repo's proto; only the version constant was never bumped, soCurrentgoes straight to1.3.SystemEvents.PowerModeChanged(as suggested in the issue): works in both service and console mode, and SystemEvents runs its own broadcast window thread so no message pump is needed. The service-control-handler alternative (CanHandlePowerEvent/OnPowerEvent) requires subclassingWindowsServiceLifetimeand doesn't fire in console/debug mode.IsAtLeast) rather than an exact-major match, so a future major protocol bump keeps sending wake requests.NegotiatedVersion == nullto detect a stopped tunnel, keepingSendWakeRequestside-effect free and unit-testable with a fakeITunnelSupervisor.This PR was generated with Coder Agents on behalf of @EhabY.