feat(auth): launch browser login regardless of TTY#1100
Open
betegon wants to merge 4 commits into
Open
Conversation
Contributor
|
Contributor
Codecov Results 📊❌ Patch coverage is 0.00%. Project has 5040 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 81.23% 81.22% -0.01%
==========================================
Files 386 386 —
Lines 26831 26831 —
Branches 17427 17428 +1
==========================================
+ Hits 21795 21791 -4
- Misses 5036 5040 +4
- Partials 1810 1811 +1Generated by Codecov Action |
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
from
9bee57b to
cb2c8cc
Compare
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
from
cb2c8cc to
dd6678d
Compare
betegon
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
from
dd6678d to
334d11f
Compare
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
2 times, most recently
from
905fff2 to
98e02db
Compare
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
2 times, most recently
from
5e75d4a to
aef2027
Compare
autoAuthMiddleware only ran the OAuth device flow (which opens the browser) in an interactive terminal, so an unauthenticated command in a non-TTY (piped output, redirected stdin, CI) failed immediately. Remove the isatty(0) gate so the device flow runs regardless of TTY; on success the command retries as before. If login doesn't complete, behavior is unchanged from today: a non-TTY re-throws the original auth error (exit 10, "Not authenticated"), an interactive terminal exits 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
from
40e3b82 to
a78f767
Compare
betegon
changed the title
The auto-auth failure branch re-threw the original auth error (exit 10) whenever `isatty(0)` was false. But the device-flow prompt (URL/QR) prints to stderr, so with piped stdin and an interactive stderr (`cat x | sentry cmd`) a human sees the prompt; cancelling it should exit 1, not 10. Gate the re-throw on both fds so exit 10 is reserved for the fully-headless case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The non-TTY auto-auth test matched /not authenticated|login/i, but the `|login` alternative is trivially satisfied by the "Starting login flow…" line already asserted above, so it couldn't catch a regression in the not-authenticated message. Drop the alternative. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
betegon
force-pushed
the
feat/auth-non-tty-browser
branch
from
f723eb3 to
494a5a8
Compare
Contributor
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.
Reviewed by Cursor Bugbot for commit 494a5a8. Configure here.
Dropping the `isatty(0)` gate made `autoAuthMiddleware` run the device flow for every recoverable auth error, so a fully headless run (no TTY on stdin or stderr) — where nobody can complete OAuth — sat through the multi-minute poll before failing, instead of failing immediately as it did before this branch. Gate auto-login on `hasInteractiveTerminal()` (stdin or stderr a TTY): a human at a terminal (incl. piped stdin with interactive stderr) still gets the login flow, while fully headless runs fail fast with the original auth error. This also makes the post-login re-throw guard unreachable, so it's removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When you run a command without being logged in, the CLI already starts the OAuth device flow and opens your browser to sign in — but only in an interactive terminal. This drops that restriction so the same login flow runs regardless of TTY (piped output, redirected stdin, CI).
Changes
One change: remove the
isatty(0)gate fromautoAuthMiddlewareinsrc/cli.ts.No new timeout logic, no new helpers — just the gate removal plus preserving today's failure behavior.
Test Plan
test/e2e/auth.test.ts: an unauthenticated non-TTY run now prints "Starting login flow…" and still exits 10 (the device-code request 404s against the mock, so it fails fast).biome checkandtsc --noEmitclean.