Print auth URL to console to support a remote auth flow#32
Conversation
Summary by CodeRabbit
WalkthroughThe ChangesCLI login paste URL alternative flow
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/lib/auth/login.ts (1)
65-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize the callback exchange instead of guarding it with
completed.
completedis only set afterawait state.handleCallback(...)returns. If the browser redirect and the pasted URL arrive close together, both branches can enterhandleCallback(...)with the same auth code. That makes the login outcome timing-dependent: one path can spend the code while the other rejects, andPromise.race(...)may fail even though the other path already completed successfully.Suggested fix
- let completed = false; + let completed = false; + let completionPromise: Promise<void> | undefined; + + const completeOnce = (callbackUrl: URL): Promise<void> => { + if (!completionPromise) { + completionPromise = (async () => { + await state.handleCallback(callbackUrl); + completed = true; + })(); + } + + return completionPromise; + }; const httpResult = new Promise<void>((resolve, reject) => { server.on("request", async (req, res) => { @@ try { - await state.handleCallback(url); - completed = true; + await completeOnce(url); } catch (error) { @@ const pasteResult = waitForPastedCallback({ input, output, signal: pasteAbort.signal, }).then(async (pastedUrl) => { if (pastedUrl === null || completed) return; - await state.handleCallback(pastedUrl); - completed = true; + await completeOnce(pastedUrl); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/lib/auth/login.ts` around lines 65 - 112, The race exists because both the HTTP handler and the pasteResult branch call state.handleCallback concurrently and only set completed after awaiting it; replace the boolean-guard pattern by serializing the exchange: introduce a single shared "inflight" promise/lock (e.g., handleCallbackPromise or similar) that both the server request handler and the waitForPastedCallback .then callback consult—if it's null they assign it to the Promise returned by state.handleCallback(url) and await it, otherwise they await the existing promise; set completed (or resolve the inflight promise) only after that promise settles and use that shared symbol in place of the current completed checks so handleCallback is invoked at most once (refer to completed, state.handleCallback, the server "request" handler, and the pasteResult callback).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/lib/auth/login.ts`:
- Around line 76-82: The server is being closed immediately when the pasted-flow
loses the Promise.race, which prevents late browser redirects from reaching the
request handler's completed branch (the branch that calls
state.resolveWorkspaceName() and renderSuccessPage). Modify the shutdown logic
so the HTTP server isn't closed until after any in-flight request can be
handled: either move the server.close() call into the request handler after
sending the success response (ensure you call server.close() only once), or, if
you must close from the race winner path, wait for a short grace period or check
a shared completed flag and defer closing until completed is true or the current
response finishes; update the code paths that call server.close() around the
Promise.race handling (the shutdown block currently invoked after the race) to
use this deferred-close approach so renderSuccessPage and
state.resolveWorkspaceName() can complete.
---
Outside diff comments:
In `@packages/cli/src/lib/auth/login.ts`:
- Around line 65-112: The race exists because both the HTTP handler and the
pasteResult branch call state.handleCallback concurrently and only set completed
after awaiting it; replace the boolean-guard pattern by serializing the
exchange: introduce a single shared "inflight" promise/lock (e.g.,
handleCallbackPromise or similar) that both the server request handler and the
waitForPastedCallback .then callback consult—if it's null they assign it to the
Promise returned by state.handleCallback(url) and await it, otherwise they await
the existing promise; set completed (or resolve the inflight promise) only after
that promise settles and use that shared symbol in place of the current
completed checks so handleCallback is invoked at most once (refer to completed,
state.handleCallback, the server "request" handler, and the pasteResult
callback).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48967437-4948-48bd-acb9-26b22d407a66
📒 Files selected for processing (1)
packages/cli/src/lib/auth/login.ts
| if (completed) { | ||
| // The paste path already completed the token exchange. Render the | ||
| // success page anyway so a late browser callback isn't left dangling. | ||
| const workspaceName = await state.resolveWorkspaceName(); | ||
| res.setHeader("Content-Type", "text/html; charset=utf-8"); | ||
| res.end(renderSuccessPage(workspaceName)); | ||
| return; |
There was a problem hiding this comment.
Don't tear down the listener before a late browser redirect can hit the success branch.
Once the pasted flow wins Promise.race(...), Line 117 starts closing the server immediately. A browser redirect that arrives after that point never reaches the completed branch on Lines 76-82, so the browser still gets a connection failure instead of the success page this branch is trying to preserve.
Also applies to: 115-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/lib/auth/login.ts` around lines 76 - 82, The server is being
closed immediately when the pasted-flow loses the Promise.race, which prevents
late browser redirects from reaching the request handler's completed branch (the
branch that calls state.resolveWorkspaceName() and renderSuccessPage). Modify
the shutdown logic so the HTTP server isn't closed until after any in-flight
request can be handled: either move the server.close() call into the request
handler after sending the success response (ensure you call server.close() only
once), or, if you must close from the race winner path, wait for a short grace
period or check a shared completed flag and defer closing until completed is
true or the current response finishes; update the code paths that call
server.close() around the Promise.race handling (the shutdown block currently
invoked after the race) to use this deferred-close approach so renderSuccessPage
and state.resolveWorkspaceName() can complete.
luanvdw
left a comment
luanvdw
left a comment
There was a problem hiding this comment.
Thanks @sneub, this adds a solid QoL improvement and gives users a practical escape hatch when the CLI is running on one machine and the browser is on another machine!
Could you/your agent review these small changes before we merge this in?
- Only enable the paste fallback when we can actually prompt
If stdin is not a TTY, there is nowhere for the user to paste the callback URL. In that case, let’s keep the existing browser-callback behaviour, i.e. don’t start/race the paste flow, and don’t treat paste as an available fallback if opening the browser fails.
Put differently, the paste path should only change behaviour in interactive terminals.
-
Make completion run once
Since the browser callback and pasted callback can both arrive, let’s centralize completion through a smallcompleteOnce(callbackUrl)helper. That helper should make surestate.handleCallback(...)runs only once, and both paths await the same completion if they arrive close together. -
Make the remote-machine instruction more concrete
I think “resulting callback URL” may be unclear to users. Could we make it more explicit
Suggested copy:
Open this URL to sign in: <auth-url>
If the browser opens on another machine, finish sign-in there. When it redirects to localhost, copy the full localhost URL from the address bar and paste it here.
- Keep login alive after a bad paste
If the user presses Enter too early or pastes the wrong thing, we should not end the whole login. Can we show a short message and ask again, while still leaving the browser callback path open? If this opens a big conditional loop, skip this for now - we can always return to this if needed.
2 participants
Summary
prisma-cli auth loginopens a browser and waits on alocalhost:<port>/auth/callbackHTTP server for the OAuth redirect. This breaks on remote machines (SSH, Docker,
cloud VMs): no browser to open, and even if the user copies the URL to a local
browser, the redirect lands on the local machine's
localhost, never reachingthe remote CLI.
This change adds a second completion path — paste-the-callback-URL — that runs in
parallel with the existing browser callback. No backend changes, no SDK changes,
no new flag. Laptop users see no behavior change; remote users get a path that
actually works.
How it works
readlineprompt (Paste URL here:) runs in parallel when stdin is a TTY.Promise.race([httpCallback, pastedUrl])— whichever delivers a callback URLfirst wins. Both paths call the same
state.handleCallback(url).completedguard ensures the OAuth code exchange runs exactly once even ifboth fire close together; the losing path is aborted via
AbortControllerandthe existing server-close
finally.open(url)failures are now swallowed instead of bubbling — the paste pathis a viable fallback when the browser launch silently fails on headless boxes.
UX notes
behavior is identical to today.
code) so we can keepstatevalidation intact with zero new parsing logic.
Scope
Single file:
packages/cli/src/lib/auth/login.ts(~70 net lines). No changes tothe controller, use-cases, types, SDK, or any backend/OAuth-provider config.