fix(desktop): WSL backend connects with Docker bridges and slow cold starts - #6129
fix(desktop): WSL backend connects with Docker bridges and slow cold starts#6129rehanhaider wants to merge 4 commits into
Conversation
getDistroIp took the first IPv4 from `hostname -I`, which prints every bound address in interface order. A distro running Docker reports its br-*/docker0 bridges first, so the desktop pointed the renderer at an address Windows cannot reach and sat on "Connecting to WSL..." until the readiness budget expired. It also defeated the mirrored-networking check in DesktopBackendConfiguration, which only collapses to loopback when the reported address belongs to a Windows interface; a bridge address never matches. Print the default route's source address ahead of the `hostname -I` output so the existing first-match parser picks it up. `hostname -I` still trails it, so distros without iproute2, or with no default route, keep today's behaviour. The lookup targets 192.0.2.1 (RFC 5737) and is a routing-table query, so no packet is sent. Verified on Windows 11, WSL2 mirrored networking, three Docker bridges: hostname -I -> 172.18.0.1 172.17.0.1 172.19.0.1 192.168.1.5 route source -> 192.168.1.5 Before, readiness targeted the 172.18.0.1 bridge and never resolved. After, isLocalHostIpv4 matches 192.168.1.5 against the host adapter and readiness targets http://127.0.0.1:<port>, which answers 200. Closes pingdotgg#5211.
The desktop probes backend readiness once with a 60s budget. A WSL backend loads the server bundle and its dependencies across /mnt/c, so cold boot regularly exceeds that. When it does the probe gives up, the run is left alive but never ready, and the app sits on "Connecting to WSL..." forever even though the backend finishes booting seconds later and answers 200 on its readiness endpoint. Measured on Windows 11 with WSL2, two consecutive launches: run 1: gave up at 60000ms; backend healthy shortly after run 2: gave up at 60000ms; backend listening and answering 200 at 98s The probe polls every 100ms and resolves the instant the backend answers, so a larger budget costs nothing when boot is fast. Refs pingdotgg#4535, pingdotgg#5522.
|
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:
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. Comment |
github-actions
Bot
added
vouch:unvouched
ApprovabilityVerdict: Needs human review This PR modifies WSL connection logic, including how the distro IP is determined. While the intent is to fix connection issues, changes to IP detection on the critical connection path warrant human verification to ensure correctness across different WSL configurations. You can customize Macroscope's approvability policy. Learn more. |
rehanhaider
changed the title
rehanhaider
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cce7a8bf82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "--", | ||
| "sh", | ||
| "-c", | ||
| "ip -4 route get 192.0.2.1 2>/dev/null | sed -n 's/.* src \\([0-9.]*\\).*/\\1/p'; hostname -I", |
There was a problem hiding this comment.
This changes backend IP-selection behavior but adds no test exercising the new ordered output. In the reported Docker scenario, a spawner-backed test should verify that the route source wins when hostname -I lists a bridge first, and that the hostname result still wins when iproute2 is unavailable; otherwise shell quoting or output-order regressions can restore the inaccessible address while the existing suite remains green.
AGENTS.md reference: AGENTS.md:L108-L108
Useful? React with 👍 / 👎.
| "--", | ||
| "sh", | ||
| "-c", | ||
| "ip -4 route get 192.0.2.1 2>/dev/null | sed -n 's/.* src \\([0-9.]*\\).*/\\1/p'; hostname -I", |
There was a problem hiding this comment.
When a NAT-mode distro routes 192.0.2.1 through a VPN or exit-node policy table, ip route get returns the tunnel interface's source rather than the Windows-facing WSL address. The first-match parser then passes that address to resolveWslStartConfig, which uses it directly for the readiness and renderer URL; if Windows has no route to the distro's VPN address, the backend again remains stuck connecting. Select the Windows-facing interface/default gateway path rather than the source chosen for an arbitrary external destination.
Useful? React with 👍 / 👎.
| const MAX_PREFLIGHT_FAILURE_ATTEMPTS = 5; | ||
| const DEFAULT_BACKEND_READINESS_TIMEOUT = Duration.minutes(1); | ||
| // One-shot probe; a WSL backend over /mnt/c regularly needs 60-98s to answer. | ||
| const DEFAULT_BACKEND_READINESS_TIMEOUT = Duration.minutes(3); |
There was a problem hiding this comment.
The intended slow-cold-start behavior is not covered by the existing readiness test, which supplies its own 50 ms timeout and therefore cannot detect this default being reverted or wired to the wrong backend path. Add a TestClock manager test where readiness succeeds after more than one minute but before three minutes and assert that onReady fires; otherwise this backend behavior change can regress while the focused desktop suite remains green.
AGENTS.md reference: AGENTS.md:L108-L108
Useful? React with 👍 / 👎.
CDVolvik
left a comment
CDVolvik
left a comment
There was a problem hiding this comment.
Checked the IP half on a real WSL2 distro. The new command emits two lines, the route source then the hostname -I list, and the existing parser is
const candidate = raw.split(/\s+/).find((part) => IPV4_PATTERN.test(part));which splits on all whitespace including newlines and takes the first match, so the route source wins and the hostname -I tail stays a fallback exactly as intended. Worth stating explicitly in the comment, because the fix silently depends on that parser taking the first match across the whole output rather than parsing a single line. If anyone later "tidies" it to read one line or to prefer the last match, this regresses without a test noticing.
The fallback chain also holds up: no iproute2 means sed gets nothing and hostname -I carries it, and no default route means no src field and the same fallback. Both degrade to today's behaviour rather than to nothing.
The timeout half is the part I would push back on:
-const DEFAULT_BACKEND_READINESS_TIMEOUT = Duration.minutes(1);
+// One-shot probe; a WSL backend over /mnt/c regularly needs 60-98s to answer.
+const DEFAULT_BACKEND_READINESS_TIMEOUT = Duration.minutes(3);The justification is specific to a WSL backend running off /mnt/c, but the constant is the default for every backend. After this, a local backend that is simply dead keeps the app in "connecting" for three minutes instead of one, on machines that never had the problem. That is a real cost paid by the majority to fix a case the code can already distinguish, since the WSL path is a separate branch and could carry its own readiness timeout.
Worth flagging too that #5042 and #5769 both go after the cause rather than the symptom, by getting the backend off /mnt/c and onto the Linux filesystem. If either lands, the 60-98s figure this constant is sized against disappears, and a tripled global timeout is left behind as something nobody remembers to put back. A WSL-scoped override would not have that problem.
2 participants
What Changed
getDistroIpnow prefers the default route’s source address, withhostname -Ias the fallback.DEFAULT_BACKEND_READINESS_TIMEOUTincreases from 1 to 3 minutes.Scope: two files, 19 lines, two commits. No tests added—the changes are one shell string and one constant.
Why two commits and not one?
Both commits fix the same user-visible hang, so they ship together.
Both causes the exact same symptoms to the user.
Happy to split if you'd rather take them separately.
Why
On Windows, the desktop can remain on “Connecting to WSL…” when Docker is running inside the distro. The backend starts, listens, and answers readiness checks, but the app targets the wrong address and times out too early. Either bug can cause the hang.
Wrong address
getDistroIppreviously selected the first IPv4 address fromhostname -I, which lists all bound addresses in interface order. With Docker running, abr-*ordocker0bridge can appear first, directing the renderer to an address Windows cannot route to.This also breaks mirrored networking.
isLocalHostIpv4inDesktopBackendConfigurationmaps the backend to loopback only when the reported address belongs to a Windows interface. A Docker bridge does not match, so mirrored-mode users lose the intended loopback path.The new command asks the kernel for the default route’s source address using
ip -4 route get 192.0.2.1. This avoids interface-name matching and RFC1918 heuristics without adding another spawn. The route source is printed beforehostname -I, allowing the existing first-match parser and IPv4 validation to remain unchanged.hostname -Iremains the fallback, so distros without iproute2 or a default route behave as they do today.192.0.2.1is reserved by RFC 5737, and this is only a routing-table lookup—no packet is sent.Readiness timed out too early
Readiness currently receives a single 60-second budget. Loading the server bundle and dependencies across
/mnt/cregularly takes longer. When the probe expires, the backend process remains alive but is never marked ready, leaving the splash visible even if startup completes seconds later.The probe polls every 100 ms and resolves immediately when the backend responds, so increasing the budget does not slow fast launches.
Verification
Verified on Windows 11 with WSL2 mirrored networking and three active Docker bridges—the topology reported in #5211:
Before the timeout change, with the address fix already applied:
After both changes:
The address fix is evidenced by the route lookup above: hostname -I returns the bridge first, so the previous first-match selection resolves to 172.18.0.1, which Windows cannot route to. Measurements were taken against a dev build (dev:desktop), which also waits on the Vite server, so the 98s figure is not directly a packaged-build number.
Validation:
vp test run apps/desktop/src/wsl/DesktopWslEnvironment.test.ts apps/desktop/src/backend/DesktopBackendManager.test.tsAll 52 tests passed. Typecheck and lint are clean for both changed files.
Related Work
This area is active:
There could be more robustness in the solution but I have preferred simple, tested, and workable solution instead.
Closes #5211
Refs #4535 and #5522.
Checklist
Note
[!NOTE]
Fix WSL distro IP detection and extend backend readiness timeout
getDistroIpImplin DesktopWslEnvironment.ts to prefer the IPv4 source address of the default route (ip -4 route get 192.0.2.1) before falling back tohostname -I, which is more reliable in multi-interface WSL environments.DEFAULT_BACKEND_READINESS_TIMEOUTin DesktopBackendManager.ts from 1 minute to 3 minutes to accommodate slower WSL startup times.getDistroIpImplmay differ in environments with multiple network interfaces.Macroscope summarized cce7a8b.