From 661d7876ab757f293bfc7f0727acbbd5e0560eba Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Tue, 4 Aug 2026 14:43:52 -0500 Subject: [PATCH] Keep keystrokes working in the picker after login On Windows, arrow keys did nothing in the context picker shown after login: the background watcher that listens for ^D during the auth wait could not be stopped there, so it stayed parked on the terminal and swallowed the keys the picker was waiting for. It now steps aside on terminals it cannot watch safely. Fixes #263 Co-Authored-By: Claude Opus 5 (1M context) --- internal/authutil/login.go | 36 ++++++++----- internal/authutil/login_test.go | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/internal/authutil/login.go b/internal/authutil/login.go index ada3596..37485fd 100644 --- a/internal/authutil/login.go +++ b/internal/authutil/login.go @@ -483,6 +483,10 @@ func runDeviceFlow(ctx context.Context, providerURL string, clientID string, sco return pollDeviceToken(waitCtx, tokenURL, clientID, deviceResp.DeviceCode, deviceResp.Interval, deviceResp.ExpiresIn) } +// eofPollInterval bounds each watcher read so the loop can check for stop(). +// The same value probes deadline support before the watcher starts. +const eofPollInterval = 250 * time.Millisecond + // deadlineReader is a stream that supports read deadlines, satisfied by // *os.File (including os.Stdin and os.Pipe ends). type deadlineReader interface { @@ -504,13 +508,24 @@ func watchStdinEOF(parent context.Context) (context.Context, context.CancelFunc) // input intended for a later interactive prompt. // // Input received during the wait is read and discarded — nothing else reads -// the stream while login is blocking on the auth callback or device poll. On -// streams where read deadlines are unsupported the watcher falls back to a -// single blocking read; EOF still cancels, and any leftover reader is -// abandoned rather than stealing later input in the common (deadline-capable) -// case. +// the stream while login is blocking on the auth callback or device poll. +// +// Detecting EOF requires read deadlines, so that support is probed up front. +// Without them the only way to watch is a blocking read that stop() cannot +// interrupt, which parks a reader on the stream and swallows keystrokes meant +// for the interactive picker that runs right after login. In that case the +// watcher declines to run at all and the wait is cancellable by ^C only. +// Windows console handles are never pollable, so *os.File deadlines are +// always unsupported there — and ^D is not a console EOF convention on +// Windows regardless. func watchReaderEOF(parent context.Context, in deadlineReader) (context.Context, context.CancelFunc) { ctx, cancel := context.WithCancel(parent) + + if err := in.SetReadDeadline(time.Now().Add(eofPollInterval)); err != nil { + _ = in.SetReadDeadline(time.Time{}) + return ctx, cancel + } + done := make(chan struct{}) go func() { @@ -527,13 +542,10 @@ func watchReaderEOF(parent context.Context, in deadlineReader) (context.Context, default: } - if err := in.SetReadDeadline(time.Now().Add(250 * time.Millisecond)); err != nil { - // Deadlines unsupported for this stream; fall back to a - // single blocking read. EOF cancels; anything else ends the - // watcher to avoid a busy loop. - if _, rerr := in.Read(buf); errors.Is(rerr, io.EOF) { - cancel() - } + if err := in.SetReadDeadline(time.Now().Add(eofPollInterval)); err != nil { + // Unreachable: support was probed before this goroutine + // started. Never fall back to a blocking read here — it + // cannot be interrupted by stop(). return } diff --git a/internal/authutil/login_test.go b/internal/authutil/login_test.go index a180791..80377a8 100644 --- a/internal/authutil/login_test.go +++ b/internal/authutil/login_test.go @@ -3,6 +3,7 @@ package authutil import ( "context" "errors" + "io" "net/http" "net/http/httptest" "os" @@ -125,6 +126,95 @@ func TestWatchReaderEOF_NoCancelBeforeEOFOrStop(t *testing.T) { } } +// noDeadlineReader is a stream that does not support read deadlines, as every +// Windows console handle behaves (os.Stdin is never pollable there). Read +// blocks until the test releases it, standing in for a console read that no +// stop() can interrupt. +type noDeadlineReader struct { + reads atomic.Int32 + release chan struct{} +} + +func (r *noDeadlineReader) SetReadDeadline(time.Time) error { return os.ErrNoDeadline } + +func (r *noDeadlineReader) Read(p []byte) (int, error) { + r.reads.Add(1) + <-r.release + return 0, io.EOF +} + +// TestWatchReaderEOF_NoBlockingReadWhenDeadlinesUnsupported is the #263 +// regression test. When deadlines are unsupported the watcher must not read +// the stream at all: a blocking read there cannot be interrupted by stop(), +// so it stays parked on stdin and eats the arrow keys the context picker +// needs after login. +func TestWatchReaderEOF_NoBlockingReadWhenDeadlinesUnsupported(t *testing.T) { + in := &noDeadlineReader{release: make(chan struct{})} + defer close(in.release) + + ctx, stop := watchReaderEOF(context.Background(), in) + + // Span several would-be poll cycles. + select { + case <-ctx.Done(): + t.Fatal("context canceled while stream open and idle") + case <-time.After(500 * time.Millisecond): + } + + if got := in.reads.Load(); got != 0 { + t.Fatalf("watcher read the stream %d time(s); it must not read a stream it cannot poll", got) + } + + // The caller's contract still holds: stop() cancels. + stop() + select { + case <-ctx.Done(): + case <-time.After(3 * time.Second): + t.Fatal("stop() did not cancel the context") + } +} + +// TestWatchReaderEOF_UnsupportedDeadlineLeavesInputIntact asserts the +// user-visible property behind #263: every byte typed during the login wait is +// still there for the next reader, so the picker sees the user's keystrokes. +// A regular file is used because it reports the same ErrNoDeadline a Windows +// console handle does. +func TestWatchReaderEOF_UnsupportedDeadlineLeavesInputIntact(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "stdin") + if err != nil { + t.Fatalf("CreateTemp: %v", err) + } + defer f.Close() + + const typed = "hello\n" + if _, err := f.WriteString(typed); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := f.Seek(0, io.SeekStart); err != nil { + t.Fatalf("seek: %v", err) + } + + // Guard the premise: this test is meaningless if the file supports + // deadlines, since the watcher would then take the polling path. + if derr := f.SetReadDeadline(time.Now().Add(time.Second)); derr == nil { + t.Skip("regular files support read deadlines on this platform") + } + _ = f.SetReadDeadline(time.Time{}) + + _, stop := watchReaderEOF(context.Background(), f) + time.Sleep(300 * time.Millisecond) + stop() + time.Sleep(100 * time.Millisecond) + + rest, err := io.ReadAll(f) + if err != nil { + t.Fatalf("read: %v", err) + } + if string(rest) != typed { + t.Fatalf("watcher consumed input meant for the picker: got %q, want %q", string(rest), typed) + } +} + func mustPipe(t *testing.T) (*os.File, *os.File) { t.Helper() r, w, err := os.Pipe()