Conversation
login previously refused immediately when stdin and stderr both lacked a terminal, on the assumption that nobody could see or complete the login. That assumption does not hold when the process runs on a machine with an active desktop session but no tty on its own streams (a code agent's shell tool, for example) — open::that still puts a real browser window in front of a real person there. Drop the early terminal check and let login attempt to open the browser in every case; the existing login_can_be_completed check after the open::that call still refuses when the browser genuinely could not be shown and stderr is redirected. Known gap, not yet addressed: this reintroduces the failure mode login_needs_a_terminal existed to prevent for genuinely headless processes (CI, containers without a display) that still have a real desktop-less environment — register_client now runs, and a wait for the OAuth callback can run out CALLBACK_TIMEOUT, before that's discovered. The non_interactive test suite documents and exercises the old contract and needs to be revisited together with this change.
…minal" This reverts commit 8568706.
1d260a7 to
4bdca03
Compare
|
Closing. CI confirmed the concern in the description in the worst way: on macOS and Windows runners
|
1 participant
What
mapbox auth loginrefused immediately whenever stdin and stderr both lacked a terminal, on the theory that nobody could see or complete the login. This drops that early check and letsloginalways tryopen::that; the existing post-open check (login_can_be_completed) still refuses when the browser genuinely could not be shown and stderr is redirected.Why
The terminal-on-stdin-or-stderr signal assumes "no tty" means "no human, no display." That's true for CI and for a container with no desktop, but not for a process launched by a code agent's shell tool on a real desktop machine: stdin/stderr are pipes, but the machine has an active session, so
open::thatgenuinely puts a browser in front of a real person, who can complete the login. Verified locally: with this change, running under an agent's non-tty shell on a desktop Mac opens the browser and completes login normally.Known gap — this is a draft on purpose
Removing the early check reintroduces the exact failure mode
login_needs_a_terminalwas written to prevent for genuinely headless processes (CI runners, containers without a display):register_clientnow runs unconditionally, so a headless run leaves a real, orphaned OAuth client registration on every attempt.open::thatfails (no display) the command still reacheslogin_can_be_completed, but only after that registration call — and if it doesn't fail cleanly, the process can sit out the fullCALLBACK_TIMEOUT(5 minutes) waiting for a callback nobody will send.non_interactivetest suite (oss/tests/non_interactive.rs) asserts and documents the old immediate-refusal contract;login_without_a_terminal_refuses_instead_of_waiting,the_refusal_carries_a_code_and_a_fix,the_refusal_creates_no_credential_directory, andyes_does_not_buy_a_login_a_terminalare not updated here and will now either hang (on a machine with a desktop session) or hit the network (in true headless CI).Opening this as a draft to discuss the right fix before landing — most likely a more precise signal than tty presence (e.g. detecting whether a display/browser is actually available) rather than dropping the check outright, plus updated tests. Not ready for review as-is.