Repository navigation
Conversation
📝 WalkthroughWalkthroughAdds WSL browser-login support for Claude accounts. The change creates temporary login configuration directories, captures authorization URLs through WSL handoff files, validates and opens approved URLs through Electron, and stops handoff processing during cleanup. WSL command execution exports the login path when required. Tests cover script generation, URL validation, polling behavior, login integration, account creation, reauthentication, and command wiring. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/claude-accounts/wsl-login-browser-opener.ts (2)
135-140: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAttach a rejection handler to the
onUrlresult.
onUrlis typed asvoid | Promise<void>.void args.onUrl(url)discards a rejected promise and produces an unhandled rejection. The service callback catches its own errors today, butwsl-login-opener-flow.test.tspassesopenWslLoginAuthorizationUrldirectly, which can reject.♻️ Proposed change
const url = parseWslLoginOpenerHandoff(raw) if (url) { - void args.onUrl(url) + void Promise.resolve(args.onUrl(url)).catch(() => args.onInvalid()) } else { args.onInvalid() }
152-154: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRevalidate the URL inside
openWslLoginAuthorizationUrl.The function forwards any string to
shell.openExternal. Validation lives only at the call site increateWslLoginOpenerHandoff. If a later caller skipsparseWslLoginOpenerHandoff, the allowlist no longer applies. Move the check into this function to keep one enforcement point.🛡️ Proposed change
export function openWslLoginAuthorizationUrl(url: string): Promise<void> { - return shell.openExternal(url) + const safeUrl = parseWslLoginOpenerHandoff(url) + if (!safeUrl) { + return Promise.reject(new Error('Refusing to open an unsupported authorization URL.')) + } + return shell.openExternal(safeUrl) }src/main/claude-accounts/service.ts (1)
1092-1092: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGate the opener PATH export on an explicit option.
The export is tied to
keepStdinOpen, which describes stdio behavior, not the login flow. A future caller that keeps stdin open for a different WSL command would also receive the opener path, and that config directory may contain noorca-openerdirectory. Add a separate option, for exampleexportWslLoginOpenerPath, and set it next tokeepStdinOpenat theauth logincall site.src/main/claude-accounts/service.test.ts (1)
1646-1649: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso unmock
../wslin thefinallyblock.The test calls
vi.doMock('../wsl', ...)but only unmocksnode:child_process. The../wslmock stays registered for the rest of the file. A later test that imports./serviceafter anothervi.resetModules()then receives the stubbedtoWindowsWslPath. Add the matchingvi.doUnmockcall.♻️ Proposed change
} finally { vi.doUnmock('node:child_process') + vi.doUnmock('../wsl') rmSync(windowsRoot, { recursive: true, force: true }) }src/main/claude-accounts/wsl-login-opener-flow.test.ts (1)
14-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove timer and directory cleanup into
afterEach.
vi.useRealTimers()andrmSyncrun at the end of the test body. If an assertion fails, both are skipped. Fake timers then stay active for later tests in the worker, and the temporary directory leaks. The sibling filewsl-login-browser-opener.test.tsalready restores timers inafterEach. Use the same pattern here.♻️ Proposed change
-import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' @@ describe('WSL Claude login opener flow', () => { + afterEach(() => { + vi.useRealTimers() + }) + it('delivers one URL to the Windows-side browser owner and consumes the request', () => { @@ watcher.stop() rmSync(root, { recursive: true, force: true }) - vi.useRealTimers() })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 22771039-4174-4ac7-aab8-08f90c051bfd
📒 Files selected for processing (5)
src/main/claude-accounts/service.test.tssrc/main/claude-accounts/service.tssrc/main/claude-accounts/wsl-login-browser-opener.test.tssrc/main/claude-accounts/wsl-login-browser-opener.tssrc/main/claude-accounts/wsl-login-opener-flow.test.ts
Greptile SummaryThis PR adds a fallback browser opener for WSL Claude sign-in on Windows: when the selected WSL distro has no
Confidence Score: 5/5Safe to merge. The new WSL browser opener shim is narrowly scoped to interactive WSL Claude login, leaves all existing code paths unchanged, and the URL handoff is guarded by an atomic rename, a byte-bounded read, and an exact-host HTTPS allowlist before anything reaches shell.openExternal. The handoff mechanism is carefully constructed: atomic claim-before-read prevents double-dispatch; three distinct error callbacks give the caller precise failure context; openerError is correctly prioritised over the raw login error in the catch block; PATH injection is append-only so existing distro openers retain precedence; and the provisioning script self-tests the shim before printing the directory marker. No functional defects were identified in the changed code. Files Needing Attention: No files require special attention. The service.ts integration and all new files reviewed cleanly.
|
| Filename | Overview |
|---|---|
| src/main/claude-accounts/wsl-login-browser-opener.ts | New module implementing WSL browser opener shim: atomic claim-before-read handoff, strict HTTPS URL allowlist validation, controlled cleanup ordering, and Electron shell opener. Well-structured with no issues found. |
| src/main/claude-accounts/service.ts | Wires the new opener handoff into the WSL login flow: creates the handoff watcher only when linuxPath is set, forwards distinct error messages for URL-open vs invalid-URL vs read-error cases, and always stops the watcher in finally. PATH export is correctly scoped to the keepStdinOpen (login) command only. |
| src/main/claude-accounts/wsl-login-browser-opener.test.ts | Comprehensive unit tests covering: self-test and script content, output parsing, URL validation allowlist (11 rejection cases and 4 acceptance cases), byte-length bounding, atomic claim-and-delete, and cleanup failure handling. |
| src/main/claude-accounts/wsl-login-opener-flow.test.ts | Integration-style test verifying end-to-end delivery of a URL to the Windows browser via openExternal, one-shot consumption, and no second delivery on a duplicate write. |
| src/main/claude-accounts/service.test.ts | Adds a comprehensive end-to-end test exercising the WSL bridge through both addAccount and reauthenticateAccount paths, including mktemp failure, PATH export injection, and openExternal call counts. |
Sequence Diagram
sequenceDiagram
participant S as ClaudeAccountService
participant H as HandoffWatcher
participant W as wsl.exe bash
participant F as open-url.request
participant E as shell.openExternal
S->>W: execFileSync buildWslLoginConfigDirScript
W-->>S: ORCA_CLAUDE_LOGIN_DIR path
S->>H: createWslLoginOpenerHandoff poll every 25ms
S->>W: "spawn claude auth login with PATH+=orca-opener"
W->>F: xdg-open shim writes URL atomically via mv
H->>F: renameSync to .consumed
H->>F: readSync bounded read
H->>F: rmSync .consumed
H->>H: parseWslLoginOpenerHandoff HTTPS allowlist check
H->>E: onUrl shell.openExternal
E-->>H: Windows browser opens OAuth URL
W-->>S: claude auth login exits 0
S->>S: auth status and captureAuthFromConfigDir
S->>H: handoff.stop in finally
S->>W: rm -rf temp dir
Reviews (3): Last reviewed commit: "Prevent WSL handoff cleanup failures fro..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/claude-accounts/wsl-login-browser-opener.test.ts (1)
81-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the new
onReadErrorbranch.These tests cover oversized readable input only. They do not execute the
catchatsrc/main/claude-accounts/wsl-login-browser-opener.tsLine 132. Add a deterministic read failure and assert exactly oneonReadErrorcall, no URL or invalid callback, and cleanup of the claimed file.
src/main/claude-accounts/service.tsLines 591-616 mapsonReadErrorto a distinct sign-in failure.Also applies to: 107-113
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8eb24e20-fe42-4cd5-a5fc-ec8564c12f4d
📒 Files selected for processing (5)
src/main/claude-accounts/service.test.tssrc/main/claude-accounts/service.tssrc/main/claude-accounts/wsl-login-browser-opener.test.tssrc/main/claude-accounts/wsl-login-browser-opener.tssrc/main/claude-accounts/wsl-login-opener-flow.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/claude-accounts/wsl-login-opener-flow.test.ts
- src/main/claude-accounts/service.test.ts
- src/main/claude-accounts/service.ts
|
Field report from Orca 1.4.190 on Windows + WSL2: I have not built or tested this PR, so this is field evidence only. Since this patch appends its shim to PATH to preserve existing openers, could you also validate the installed-but-nonfunctional opener case before merge? |
|
Thanks @rodboev for the work here. Closing as superseded: WSL Claude sign-in now creates its own BROWSER helper and opens the callback URL in the desktop browser (#29566, which also closed #11823). Credit for the original approach; appreciate the contribution, and feel free to open a fresh PR against current main if you see anything still missing. |
Summary
Give WSL Claude sign-in a private, temporary browser opener when the selected distro has no opener on PATH. The shim writes one atomic URL handoff into the existing temporary config directory; the main process validates and opens it through Electron. Existing distro openers keep precedence, and no renderer, IPC, preload, or mobile surface changes.
Closes #11823.
Screenshots
No visual change. The only user-visible change is a more specific existing failure-toast message when opener preparation fails.
Testing
Focused Vitest validation passed: the three touched bridge/service test files passed 52 tests.
pnpm run typecheck:node,pnpm run typecheck:web, changed-fileoxlint, and changed-fileoxfmt --checkpassed.Post-fix WSL opener validation passed: the focused bridge suite passed 17 tests, including cleanup-failure handling.
Real Windows + WSL2 and browser-owner checks remain unrun in this environment.
AI Review Report
Native host login, existing WSL openers, credential capture, persistence, rollback, IPC, preload, renderer, web, RPC, mobile, SSH, remote, macOS, Linux, and performance surfaces remain unchanged. The fallback is scoped to interactive WSL Claude account login and reauthentication.
Security Audit
The handoff uses one bounded read, atomically claims and deletes the request before validation, accepts only exact HTTPS hosts
claude.com,platform.claude.com,console.anthropic.com, andanthropic.com, and rejects userinfo, fragments, ports, localhost, suffix lookalikes, and arbitrary hosts. The shim uses a quoted heredoc, an atomic same-filesystem rename, mode 0700, and no stdin or output. PATH injection is limited to the interactive WSL login command.Notes
The live base is
origin/mainat8fc892dd023850df379c07cc4e7a46cff9a3ea84. The committed head is6c91419531233f6d93a406fe7417c8b909c56323.