From a78f767df059140c034dfc4d05f1c8b0d81d88ff Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 17 Jun 2026 10:40:15 +0200 Subject: [PATCH 1/4] feat(auth): launch browser login regardless of TTY 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) --- src/cli.ts | 54 ++++++++++++++++++++++++------------------- test/e2e/auth.test.ts | 18 +++++++++++++++ 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 9a74aadbc..4852489cd 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -447,42 +447,48 @@ export async function runCli(cliArgs: string[]): Promise { /** * Auto-authentication middleware. * - * Catches auth errors (not_authenticated, expired) in interactive TTYs - * and runs the login flow. On success, retries through the full middleware - * chain so inner middlewares (e.g., trial prompt) also apply to the retry. + * Catches auth errors (not_authenticated, expired) and runs the login flow. + * On success, retries through the full middleware chain so inner middlewares + * (e.g., trial prompt) also apply to the retry. + * + * Runs regardless of TTY: the device flow opens the browser when possible and + * otherwise prints the verification URL + QR code, so it works when stdin is + * piped/redirected too. */ const autoAuthMiddleware: ErrorMiddleware = async (next, argv) => { try { await next(argv); } catch (err) { - // Use isatty(0) for reliable stdin TTY detection (process.stdin.isTTY can be undefined in Bun) - // Errors can opt-out via skipAutoAuth (e.g., auth status command) + // Only recover auth errors that haven't opted out (e.g. auth status sets + // skipAutoAuth); rethrow everything else unchanged. if ( - err instanceof AuthError && - (err.reason === "not_authenticated" || err.reason === "expired") && - !err.skipAutoAuth && - isatty(0) + !(err instanceof AuthError) || + err.skipAutoAuth || + !["not_authenticated", "expired"].includes(err.reason) ) { - process.stderr.write( - err.reason === "expired" - ? "Authentication expired. Starting login flow...\n\n" - : "Authentication required. Starting login flow...\n\n" - ); - - const loginSuccess = await runInteractiveLogin(); + throw err; + } - if (loginSuccess) { - process.stderr.write("\nRetrying command...\n\n"); - await next(argv); - return; - } + process.stderr.write( + err.reason === "expired" + ? "Authentication expired. Starting login flow...\n\n" + : "Authentication required. Starting login flow...\n\n" + ); - // Login failed or was cancelled - process.exitCode = 1; + const loginSuccess = await runInteractiveLogin(); + if (loginSuccess) { + process.stderr.write("\nRetrying command...\n\n"); + await next(argv); return; } - throw err; + // Login failed or was cancelled. In a non-TTY, re-throw so the original + // auth error's standard message and exit code surface ("Not + // authenticated", exit 10); in a TTY the flow already reported it. + if (!isatty(0)) { + throw err; + } + process.exitCode = 1; } }; diff --git a/test/e2e/auth.test.ts b/test/e2e/auth.test.ts index 14ccefec0..3ad6b2242 100644 --- a/test/e2e/auth.test.ts +++ b/test/e2e/auth.test.ts @@ -76,6 +76,24 @@ describe("sentry auth status", () => { }); }); +describe("non-TTY auto-auth", () => { + // Auth-required commands attempt the OAuth device flow even in a non-TTY + // (the test subprocess has no TTY on stdin). The device-code request fails + // fast against the mock (no /oauth/device/code/ route → 404), so the command + // still exits 10 with the standard not-authenticated message — but only + // after attempting to start the login flow. + test("attempts login flow then exits not-authenticated", async () => { + const result = await ctx.run(["api", "organizations/"]); + + const output = result.stdout + result.stderr; + // The login flow was attempted (gate is no longer TTY-only)... + expect(output).toMatch(/starting login flow/i); + // ...but it failed, so the standard not-authenticated path still wins. + expect(output).toMatch(/not authenticated|login/i); + expect(result.exitCode).toBe(EXIT.AUTH_NOT_AUTHENTICATED); + }); +}); + describe("sentry auth login --token", () => { test("stores valid API token", { timeout: 10_000 }, async () => { const result = await ctx.run([ From ddf0e7a2b3adfb3273bd9e0687e3dbf83acf25e6 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 17 Jun 2026 11:44:53 +0200 Subject: [PATCH 2/4] fix(auth): exit 1 on cancelled login when only stderr is a TTY 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) --- src/cli.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 4852489cd..8ca2bacf6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -188,6 +188,11 @@ export async function runCli(cliArgs: string[]): Promise { // that rely on the original token positions. const hoistedArgs = hoistGlobalFlags(cliArgs); + // A human can complete the device-flow prompt when a terminal is attached to + // stdin or stderr (the URL/QR prints to stderr), so treat either as + // interactive when deciding how a failed login surfaces. + const hasInteractiveTerminal = (): boolean => isatty(0) || isatty(2); + // --------------------------------------------------------------------------- // Error-recovery middleware // --------------------------------------------------------------------------- @@ -482,10 +487,11 @@ export async function runCli(cliArgs: string[]): Promise { return; } - // Login failed or was cancelled. In a non-TTY, re-throw so the original - // auth error's standard message and exit code surface ("Not - // authenticated", exit 10); in a TTY the flow already reported it. - if (!isatty(0)) { + // Login failed or was cancelled. Re-throw the original auth error (exit + // 10) only when fully headless — no terminal on stdin or stderr. If a + // human saw the prompt (the URL/QR prints to stderr) and cancelled, the + // flow already reported it, so exit 1. + if (!hasInteractiveTerminal()) { throw err; } process.exitCode = 1; From 494a5a8e6d9fd9f2e5c5a8f7699138bde636c219 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 17 Jun 2026 15:02:39 +0200 Subject: [PATCH 3/4] test(auth): assert the not-authenticated message, not "login" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- test/e2e/auth.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/auth.test.ts b/test/e2e/auth.test.ts index 3ad6b2242..d00fc61e5 100644 --- a/test/e2e/auth.test.ts +++ b/test/e2e/auth.test.ts @@ -89,7 +89,7 @@ describe("non-TTY auto-auth", () => { // The login flow was attempted (gate is no longer TTY-only)... expect(output).toMatch(/starting login flow/i); // ...but it failed, so the standard not-authenticated path still wins. - expect(output).toMatch(/not authenticated|login/i); + expect(output).toMatch(/not authenticated/i); expect(result.exitCode).toBe(EXIT.AUTH_NOT_AUTHENTICATED); }); }); From 4325abd9ac9440e3116f5120b8fad7e51d2b7c0e Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 17 Jun 2026 15:47:02 +0200 Subject: [PATCH 4/4] fix(auth): skip auto-login when fully headless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/cli.ts | 19 +++++++++---------- test/e2e/auth.test.ts | 15 ++++++--------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 8ca2bacf6..49e131464 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -190,7 +190,7 @@ export async function runCli(cliArgs: string[]): Promise { // A human can complete the device-flow prompt when a terminal is attached to // stdin or stderr (the URL/QR prints to stderr), so treat either as - // interactive when deciding how a failed login surfaces. + // interactive when deciding whether to attempt auto-login. const hasInteractiveTerminal = (): boolean => isatty(0) || isatty(2); // --------------------------------------------------------------------------- @@ -465,11 +465,15 @@ export async function runCli(cliArgs: string[]): Promise { await next(argv); } catch (err) { // Only recover auth errors that haven't opted out (e.g. auth status sets - // skipAutoAuth); rethrow everything else unchanged. + // skipAutoAuth); rethrow everything else unchanged. Skip the login flow + // when fully headless — no terminal on stdin or stderr means nobody can + // complete the device flow, so fail fast with the original auth error + // instead of polling for minutes. if ( !(err instanceof AuthError) || err.skipAutoAuth || - !["not_authenticated", "expired"].includes(err.reason) + !["not_authenticated", "expired"].includes(err.reason) || + !hasInteractiveTerminal() ) { throw err; } @@ -487,13 +491,8 @@ export async function runCli(cliArgs: string[]): Promise { return; } - // Login failed or was cancelled. Re-throw the original auth error (exit - // 10) only when fully headless — no terminal on stdin or stderr. If a - // human saw the prompt (the URL/QR prints to stderr) and cancelled, the - // flow already reported it, so exit 1. - if (!hasInteractiveTerminal()) { - throw err; - } + // Login failed or was cancelled. A terminal was attached (the prompt + // printed to stdin/stderr), so the flow already reported it; exit 1. process.exitCode = 1; } }; diff --git a/test/e2e/auth.test.ts b/test/e2e/auth.test.ts index d00fc61e5..707efa668 100644 --- a/test/e2e/auth.test.ts +++ b/test/e2e/auth.test.ts @@ -77,18 +77,15 @@ describe("sentry auth status", () => { }); describe("non-TTY auto-auth", () => { - // Auth-required commands attempt the OAuth device flow even in a non-TTY - // (the test subprocess has no TTY on stdin). The device-code request fails - // fast against the mock (no /oauth/device/code/ route → 404), so the command - // still exits 10 with the standard not-authenticated message — but only - // after attempting to start the login flow. - test("attempts login flow then exits not-authenticated", async () => { + // The test subprocess is fully headless (no TTY on stdin or stderr), so + // nobody can complete the device flow. Auto-auth is skipped entirely and the + // command fails fast with the standard not-authenticated error (exit 10) + // instead of polling the device flow for minutes. + test("skips login flow and exits not-authenticated when fully headless", async () => { const result = await ctx.run(["api", "organizations/"]); const output = result.stdout + result.stderr; - // The login flow was attempted (gate is no longer TTY-only)... - expect(output).toMatch(/starting login flow/i); - // ...but it failed, so the standard not-authenticated path still wins. + expect(output).not.toMatch(/starting login flow/i); expect(output).toMatch(/not authenticated/i); expect(result.exitCode).toBe(EXIT.AUTH_NOT_AUTHENTICATED); });