login: simplify WSL browser fallback to explorer.exe (alternative to part of #1708)#1748
login: simplify WSL browser fallback to explorer.exe (alternative to part of #1708)#1748gtrrz-victor wants to merge 4 commits into
Conversation
Detect WSL via /proc/version and open the Windows default browser (wslview, else cmd.exe) instead of xdg-open, which resolves to a Linux WSLg browser when one is installed. Entire-Checkpoint: 01KX6MB6768R7F7CH2947GD9E7
Under WSL the Windows browser may not reach the WSL loopback listener (depends on localhostForwarding). Race the OAuth callback against an Enter keypress; on Enter, fall back to the device-code flow. WSL-only. Entire-Checkpoint: 01KX6MC1W5QG8T224V579933Y8
- browserWaitError: ctx first param (revive context-as-argument, was a CI lint failure) - WSL cmd.exe fallback: set working dir best-effort so a customized automount root (no /mnt/c) doesn't hard-fail the launch - fallback race: prefer an already-arrived successful callback over the Enter signal so a completed login is never discarded Entire-Checkpoint: 01KX69WTD49ZTZD2WQDFR2RXPG
cmd.exe `/c start` reparses its command line, so the `&` separators in an OAuth authorization URL split it into multiple commands and open a truncated URL (WSL interop only quotes argv entries containing whitespace). explorer.exe takes the URL as a plain argument and hands it to the shell URL handler: no reparsing, no escaping needed, and no UNC working-directory warning — which also removes the /mnt/c cmd.Dir workaround and the dir return value from resolveBrowserLauncher. explorer.exe's exit code is meaningless (1 even on success); openBrowser only checks process start, and a comment now guards that invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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 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.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2c5b529. Configure here.
| // user switch to the device-code flow when the redirect never arrives. | ||
| var waitFallback func(context.Context) error | ||
| if facts.wsl { | ||
| waitFallback = waitForEnter |
There was a problem hiding this comment.
WSL fallback fires under test
Medium Severity
On WSL, the browser wait arms waitForEnter as the Enter fallback, but waitForEnter returns success immediately whenever interactive.UnderTest() is true (including spawned entire login with ENTIRE_TEST_TTY=1). That treats the fallback as pressed without user input, so the loopback flow aborts and switches to device-code sign-in while the listener may still be needed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2c5b529. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR refines the entire login browser-launch behavior on WSL by simplifying the “no wslview available” fallback to use explorer.exe <url> directly, avoiding cmd.exe command-line reparsing pitfalls for OAuth URLs.
Changes:
- Switch WSL fallback launcher from
cmd.exe /c start(and its UNC-cwd workarounds) toexplorer.exe <url>. - Centralize browser-launch selection via
resolveBrowserLauncher(...)and add targeted test coverage for the WSL launcher matrix. - Document the “don’t trust exit code” invariant for
explorer.exeand keepopenBrowsersemantics aligned (start + release only).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/entire/cli/login.go | Uses resolveBrowserLauncher so WSL without wslview opens URLs via explorer.exe, avoiding cmd.exe URL reparsing. |
| cmd/entire/cli/login_test.go | Updates/extends tests to cover the launcher-selection behavior (including WSL explorer.exe fallback). |
Draft proposal on top of #1708 — includes its commits (fork branch, so it can't be stacked as a base) plus one simplification commit:
2c5b529. Review only that last commit; the rest is #1708 unchanged.What
Replaces #1708's
cmd.exe /c start+/mnt/cworking-directory fallback (used on WSL whenwslviewis missing) with a bareexplorer.exe <url>.Why
&bug at the root.cmd.exereparses its command line, so the&separators in an OAuth authorization URL split it into multiple commands and open a truncated URL — WSL interop only quotes argv entries containing whitespace.explorer.exetakes the URL as a plain argument and hands it to the shell URL handler: no reparsing, no^&escaping./mnt/cworkaround. The UNC-cwd warning is cmd.exe-specific, so thedirreturn value, theos.Statbest-effort chdir, and the test-table column all go away (net −9 lines on top of login: open the Windows browser under WSL, with device-flow fallback #1708).explorer.exeships with every Windows install and is reached through the same PATH interop ascmd.exe— no new failure mode.Caveat
explorer.exeexits 1 even on success. Irrelevant today —openBrowseronly checkscmd.Start()+Process.Release(), never the exit code — and the doc comment now records that invariant so a futureWaitdoesn't "fix" a phantom failure.The native
windowsbranch (cmd /c start) has the same latent&issue for multi-param URLs; left untouched here since it predates #1708 — separate PR if we want the same cure there.Testing
go test ./cmd/entire/cli/ -race(login tests incl. launcher matrix, WSL fallback race) — passgolangci-lint run ./cmd/entire/cli/...— 0 issues🤖 Generated with Claude Code
Note
Medium Risk
Touches interactive auth flows and process spawning on WSL; mistakes could break login or open truncated OAuth URLs, though behavior is heavily unit-tested and non-WSL paths are largely unchanged.
Overview
Improves
entire loginon WSL by detecting the environment via/proc/version, opening auth URLs in the Windows browser (wslvieworexplorer.exeinstead ofxdg-open), and adding an Enter escape hatch when the loopback redirect never reaches WSL.When WSL is detected, browser sign-in races the OAuth callback against a second Enter press; abandoning the redirect switches to device-code sign-in and auto-opens the verification URL (no second prompt). Browser wait/exchange logic is refactored into
browserWaitErrorandexchangeAndPersist.resolveBrowserLaunchercentralizes the OS matrix; withoutwslview,explorer.exe <url>replacescmd.exe /c startso OAuth query strings with&are not reparsed and UNC-cwd workarounds are unnecessary. Tests cover WSL detection, launcher choice, and the callback-vs-Enter race.Reviewed by Cursor Bugbot for commit 2c5b529. Configure here.