fix(auth): open browser on WSL via cmd.exe; always surface URL#29
Conversation
`runOAuthFlow` relied on the `open` npm package to launch the user's browser. On WSL `open` routes through `xdg-open` / `wslview`, both of which can silently no-op — the spawn resolves cleanly but no browser ever appears, leaving the user stuck until the 3-minute callback timeout. The same silent-failure mode hits SSH sessions, containers, CI runners, and headless servers. - Detect WSL via `/proc/version` and spawn `cmd.exe /c start "" <url>` directly so the real Windows browser opens. - Detect headless Linux (no DISPLAY / WAYLAND_DISPLAY / $BROWSER) and skip the doomed spawn entirely; the URL print is the only path. - Print the authorize URL unconditionally — even when the opener succeeds — so every user has a copy-pasteable fallback regardless of platform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
There was a problem hiding this comment.
This PR significantly improves the reliability of the OAuth flow by proactively handling silent browser launch failures on WSL and headless Linux, while ensuring the authorization URL is always surfaced as a fallback. Bypassing the unreliable open package in these environments provides a much smoother authentication experience for users. There are a few remaining details to address, including a URL parsing bug with cmd.exe on WSL where ampersands require explicit quoting, the addition of targeted tests for the new platform-specific opener logic, and a quick update to the README to reflect the new unconditional URL surfacing behavior.
Every test was re-listing the same seven boilerplate option fields (`scopes`, `readOnly`, `flags`, `preferredPort`, `renderSuccess`, `renderError`, `timeoutMs`) and re-implementing the same `state → fetch(redirect)` callback driver. Hoist both into helpers so each test reads as variants over the shared baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review feedback on #29: - **P1 (correctness)**: `cmd.exe /c` re-parses the reconstructed command line, and WSL interop only auto-quotes args containing spaces. OAuth URLs have no spaces but plenty of `&`s, so `cmd.exe` was treating the first `&` as a statement separator and only passing the prefix to `start`. Wrap the URL in literal double quotes so it stays one token, and correct the comment — `execFile`'s no-shell guarantee doesn't apply when the target is itself a shell. - **P2 (test coverage)**: Partial-mock `node:fs` + `node:child_process` so the default-opener branches can be exercised through the public `runOAuthFlow` surface. New tests assert: WSL routes through `cmd.exe` with the quoted URL, headless Linux skips the opener entirely, and `$BROWSER` overrides the headless short-circuit. - **P3 (docs)**: README still said the URL was surfaced only when `open` was missing or threw. Update both spots to reflect the new always-print behavior + the WSL / headless routing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The $BROWSER-routing test previously fell through to the real `open` package. `open` captures `process.platform` at module-load time and ignores our runtime `Object.defineProperty` stub, so on a macOS dev machine it ran its darwin branch and spawned the real \`/usr/bin/open\` — launching live browser tabs from the test runner. Mock `open` at module scope so its mock fn records calls without ever spawning, and assert the mock was invoked with the authorize URL to actually verify the routing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@doistbot /review |
doistbot
left a comment
There was a problem hiding this comment.
This PR improves OAuth login reliability on WSL and headless Linux environments by introducing targeted browser launch strategies and unconditionally surfacing the authorization URL. These adjustments will provide a much smoother and fail-safe authentication experience for users in locked-down or containerized setups. There are a few remaining details to refine, particularly around escaping URL percent-encoding for cmd.exe, isolating the new URL notification hook from the core login flow, and strengthening the test coverage and mock restoration.
…rizeUrl Second review pass on #29: - **P1**: `cmd.exe` expands `%VAR%` even inside quoted strings. An OAuth URL's percent-encoded bytes (`%3A`, `%2F`, …) could chance-match a defined env var (`%PATH%`, `%TEMP%`, …) and get mangled before `start` sees them. Double every `%` to `%%`; cmd.exe collapses them back to a single `%`. WSL test now uses a URL with both `&` and `%` to exercise both escapes. - **P2**: Isolate `onAuthorizeUrl` from flow control. The hook is purely an output channel; wrap its invocation in try/catch so a buggy logger can't abort an otherwise-working login. New test pins this. - **P2**: Widen `onAuthorizeUrl` to `(url) => void | Promise<void>` and await it. The public contract now matches the sync-or-async reality consumers (and the test helper) already use. - **P2**: Add a no-hook test that pins the `console.log` default print path, so a regression there can't hide behind the hook-driven tests. - **P2**: Headless test now asserts `openBrowserModule` was not called, locking down that no opener path was taken (previously a fall-through to `open` would have left the test green). - **P2**: `mockRestore()` instead of `mockReset()` in the opener-tests afterEach so the factory wraps (`actual.readFileSync`, `actual.execFile`) are restored between tests rather than stripped. - **P3**: `util.promisify(execFile)` replaces the manual Promise wrapper in `openViaCmdExe`. - **P3**: `flowOptions` doc / Pick now reflect that only `provider` and `store` are required; `openBrowser` was already optional via `RunOAuthFlowOptions`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## [0.16.1](v0.16.0...v0.16.1) (2026-05-16) ### Bug Fixes * **auth:** open browser on WSL via cmd.exe; always surface URL ([#29](#29)) ([4ac2824](4ac2824))
|
🎉 This PR is included in version 0.16.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
opennpm package (which routes throughxdg-open/wslviewand silently no-ops on most WSL installs) and spawncmd.exe /c start "" <url>directly so the user's real Windows browser launches.DISPLAY/WAYLAND_DISPLAY/$BROWSER): skip the doomedopenspawn entirely — SSH sessions, containers, CI runners, and headless servers all hit the same silent-failure mode as WSL but with no Windows side to bounce through.Why
runOAuthFlowpreviously relied onopen's exception handling to trigger the URL fallback, but on WSL the spawn resolves cleanly with no browser ever appearing — the barecatch {}never fires and the user sits untilwaitForCallbackhits its 3-minuteAUTH_CALLBACK_TIMEOUT. Reported by a real WSL user after the auth flow was extracted into cli-core.$BROWSERis honored so Codespaces / custom remote-bridge setups still go through the headless path's intended opener.Test plan
npm run check— oxlint + oxfmt cleannpm run type-check— tsc passesnpm test— 313 tests pass, including new success-path test assertingonAuthorizeUrlfires even when the opener resolves cleanlynpm linkinto a consumer CLI (todoist-cli / twist-cli / outline-cli), runlogin, confirm the Windows browser opens and the URL prints🤖 Generated with Claude Code