fix(desktop): resolve WSL distro IP from the default route, not hostname -I order - #8438
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…ame -I order The WSL backend hung on "Connecting to WSL..." whenever Docker bridge networks existed in the distro: `hostname -I` lists bridge addresses first, and the first IPv4 was assumed to be the reachable eth0 address, so the renderer polled an unreachable 172.x bridge IP forever. Resolve the address from the `src` field of `ip -4 route get 1.1.1.1` instead (a pure routing-table lookup; bridge interfaces never own the default route), keep `hostname -I` as the fallback for distros without a default route, and log the probe output and the chosen address. This also repairs mirrored-mode detection in the same scenario: the probe now reports the mirrored host IP, which isLocalHostIpv4 matches, so the renderer URL correctly falls back to loopback. Fixes pingdotgg#5211
600b9e0 to
d4511a2
Compare
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The PR fixes WSL connectivity by replacing a simple hostname-order heuristic with a multi-stage route, interface, and subnet-selection algorithm that changes a production connection path. Although well-scoped and tested, the networking logic and new layer dependency are substantial enough to warrant human review. You can add or adjust custom eligibility rules. Learn more. |
Trusting the Internet route's `src` regressed distros where a VPN, tunnel, or VRF owns the route to 1.1.1.1: its src is a tunnel address Windows cannot reach, even though eth0 is available. Collect candidates from both probes (route src first, then the `hostname -I` list) and pick the first one that is Windows-reachable: either equal to a Windows interface address (mirrored networking) or inside a Windows interface's subnet (the NAT-mode WSL vEthernet adapter). Docker bridges and in-distro tunnels match neither. When no candidate matches, fall back to the first one, preserving the previous behavior, and log the full candidate list with the chosen address.
github-actions
Bot
added
size:L
There was a problem hiding this comment.
One convention violation found: the new distro-IP probe reads the Windows network interfaces through a direct node:os call inside the Effect service implementation instead of acquiring the existing DesktopNetworkInterfaces service from the environment.
Posted via Macroscope — Effect Service Conventions
The distro-IP probe called node:os networkInterfaces() imperatively inside the service implementation, hiding the dependency from the layer's requirements and making it unsubstitutable in tests. Acquire the existing DesktopNetworkInterfaces service in the layer, pass its read effect into the probe, and keep the IPv4 flattening as a pure helper over the returned NetworkInterfaces map.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3024429. Configure here.
…t win The subnet test accepted a candidate against any Windows adapter, so a Windows-side VPN whose 10.x/172.x space overlaps an in-distro tunnel or Docker bridge could validate the wrong candidate — and the route src being first in candidate order made it win, recreating the hang this change prevents. Select in ranked passes instead: an exact interface-address match (mirrored mode) first, then a subnet match on a WSL-named vEthernet adapter (NAT mode), and only then a subnet match on any other adapter (renamed/custom switches). The flattening now carries the adapter name to make that possible.
1 participant
What Changed
getDistroIpImplinapps/desktop/src/wsl/DesktopWslEnvironment.tsno longer trustshostname -Iordering. It now gathers IPv4 candidates from two probes — thesrcfield ofip -4 route get 1.1.1.1(a pure routing-table lookup, no packet is sent), then thehostname -Ilist — and validates them against the Windows-side interfaces in ranked passes, strongest signal first: a candidate equal to a Windows interface address (mirrored networking, where the existingisLocalHostIpv4detection then swaps the renderer URL to loopback), then a candidate inside the subnet of a WSL-named vEthernet adapter (NAT mode), then a candidate inside any other Windows interface's subnet (renamed/custom Hyper-V switches). The ranking means a Windows-side VPN whose 10.x/172.x space overlaps an in-distro tunnel or Docker bridge cannot capture the probe while the real WSL adapter has a match. When no candidate matches, it falls back to the first candidate, preserving the previous behavior, and the probe logs the full candidate list plus the chosen address.The parsing/selection is extracted into exported
parseDistroIpCandidates/pickDistroIp/windowsIpv4Interfacespure helpers with unit tests, following the file's existing pattern for the other probe parsers. The Windows interfaces are read through the existingDesktopNetworkInterfacesservice (acquired in the layer, passed into the probe) rather than a directnode:oscall, so the dependency is visible in the layer's requirements and substitutable in tests.Why
Fixes #5211.
The desktop app hangs on "Connecting to WSL..." forever whenever Docker has created bridge networks inside the distro:
hostname -Ilists the bridge addresses (172.17.x.x, ...) before eth0, and the code took the first IPv4 as the renderer-visible host — an address Windows can never reach.docker network pruneonly rotates which unreachable bridge IP gets picked.Validating against the Windows-side interfaces (rather than preferring the Internet route's
srcoutright) also keeps distros with an in-distro VPN/tunnel/VRF working: there the route to 1.1.1.1 reports the tunnel's address, which Windows cannot reach, while eth0 remains the correct choice.This additionally repairs mirrored-mode detection in the Docker scenario — with a bridge IP the
isLocalHostIpv4host-interface comparison always failed, so the loopback fallback (which the issue reporter confirmed reachable) was never taken.Verified with
vp test run apps/desktop/src/wsl/DesktopWslEnvironment.test.ts(41 passed, including 13 new cases covering the Docker-bridge fixture from the issue, the in-distro VPN case, the overlapping Windows-VPN case, mirrored mode, custom switch names, the no-match fallback, and the interface flattening), plus desktop typecheck, lint, andvp fmt --checkon the touched files.Checklist
Note
Resolve WSL distro IP from default route and Windows interfaces, not
hostname -Iorderhostname -Iheuristic with a combined probe:DISTRO_IP_SCRIPTemits theip -4 route getsource IP followed byhostname -Iaddresses, parsed byparseDistroIpCandidatesin DesktopWslEnvironment.tspickDistroIpvalidates candidates against Windows IPv4 interfaces (viawindowsIpv4Interfaces) in three ranked passes: exact IP equality (mirrored mode), subnet match on WSL-named adapters (NAT mode eth0), then subnet match on any adapter (custom/renamed switch)DesktopWslEnvironment.layerprovider now requiresDesktopNetworkInterfacesfrom context and wires it intogetDistroIpImpl; theDesktopBackendConfigurationtest layer is updated to supply a stubparseDistroIpCandidates,pickDistroIp, andwindowsIpv4Interfacescovering ordering, deduplication, CRLF handling, and selection precedenceDesktopWslEnvironment.layernow depends onDesktopNetworkInterfacesbeing provided in the context; any layer composition that omits it will fail to buildMacroscope summarized 646b8cb.
Note
Medium Risk
Changes WSL backend URL selection and adds a new layer dependency; wrong IP choice would break connectivity, but behavior is heavily tested and falls back to the first candidate when nothing matches.
Overview
Fixes stuck "Connecting to WSL..." when Docker bridge IPs appear before eth0 in
hostname -I(#5211). WSL distro IP discovery no longer takes the first IPv4 fromhostname -I. It runs a combined probe (ip -4 route get 1.1.1.1plushostname -I), parses candidates withparseDistroIpCandidates, and picks an address withpickDistroIpby checking Windows reachability: exact match to a Windows interface (mirrored mode) or same subnet as a WSL-named vEthernet adapter (NAT), with weaker subnet matches ranked last so VPN/Docker overlaps do not win incorrectly.DesktopWslEnvironment.layernow depends onDesktopNetworkInterfacesfor that validation; tests stub an emptyreadwhere the real layer is built. Logging records the candidate list and chosen IP. Eleven new unit tests cover Docker bridges, in-distro VPN routesrc, mirrored mode, and interface flattening.Reviewed by Cursor Bugbot for commit 646b8cb. Bugbot is set up for automated code reviews on this repo. Configure here.