From 2b4ae30e44bcd42dbcb6a598a6a57003f358e670 Mon Sep 17 00:00:00 2001 From: youdie006 Date: Sat, 20 Jun 2026 08:56:51 +0900 Subject: [PATCH] fix: return terminal control to parent (e.g. K9s) on quit In stdin mode (e.g. kubectl logs -f ... | gonzo), readStdinAsync's inner goroutine blocks on a read of os.Stdin that never sees EOF on a live pipe, so on 'q' the context is cancelled but the blocked read keeps holding the TTY/pipe fd and the parent (K9s) only regains control after a follow-up Ctrl-C. Close the stdin reader on quit (sync.Once) so the blocked read returns immediately and the goroutine exits, and return on EOF to fix a busy-spin. Adds a teardown test. (Incidental: gofmt import reorder in the touched file.) Closes #58 --- cmd/gonzo/app.go | 68 ++++++++++++++++++---- cmd/gonzo/app_quit_test.go | 113 +++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 cmd/gonzo/app_quit_test.go diff --git a/cmd/gonzo/app.go b/cmd/gonzo/app.go index eeaa0e1..c9c28ca 100644 --- a/cmd/gonzo/app.go +++ b/cmd/gonzo/app.go @@ -10,20 +10,21 @@ import ( "path/filepath" "runtime" "strings" + "sync" "time" "io/fs" "github.com/control-theory/gonzo/internal/analyzer" "github.com/control-theory/gonzo/internal/engine" - "github.com/control-theory/gonzo/internal/releases" - "github.com/control-theory/gonzo/internal/state" "github.com/control-theory/gonzo/internal/filereader" "github.com/control-theory/gonzo/internal/formats" "github.com/control-theory/gonzo/internal/k8s" "github.com/control-theory/gonzo/internal/memory" "github.com/control-theory/gonzo/internal/otlplog" "github.com/control-theory/gonzo/internal/otlpreceiver" + "github.com/control-theory/gonzo/internal/releases" + "github.com/control-theory/gonzo/internal/state" "github.com/control-theory/gonzo/internal/tui" versioncheck "github.com/control-theory/gonzo/internal/version" "github.com/control-theory/gonzo/internal/vmlogs" @@ -213,6 +214,13 @@ type simpleTuiModel struct { cancelFunc context.CancelFunc versionChecker *versioncheck.Checker + // stdin is the source for stdin-mode reading. It defaults to os.Stdin but is + // an interface so the quit-teardown path can be unit-tested. On quit we close + // it to unblock a goroutine parked in a blocking read(2) on a live pipe + // (e.g. `kubectl logs -f ... | gonzo`); see handleQuitKey and issue #58. + stdin io.ReadCloser + stopStdinOnce sync.Once + // Internal state finished bool logCount int @@ -354,6 +362,7 @@ func (m *simpleTuiModel) Init() tea.Cmd { // stdin is a pipe or file, we have data m.hasStdinData = true m.inputChan = make(chan string, 100) + m.stdin = os.Stdin // Start goroutine to read stdin without blocking go m.readStdinAsync() @@ -512,11 +521,45 @@ func (m *simpleTuiModel) readFilesAsync() { } } +// handleQuitKey cancels the app context and stops the input reader when the +// user presses a quit key ("q" or Ctrl-C). This is the lifecycle teardown seam +// for issue #58: on quit, bubbletea restores the terminal via tea.Quit, but the +// stdin-reading goroutine can stay parked in a blocking read(2) on a live pipe +// (e.g. `kubectl logs -f ... | gonzo`) whose writer never sends EOF. Without +// closing stdin, that read holds the terminal and control is not returned to the +// parent (K9s) until a follow-up Ctrl-C. It returns true if msg was a quit key. +func (m *simpleTuiModel) handleQuitKey(msg tea.KeyMsg) bool { + if s := msg.String(); s != "q" && s != "ctrl+c" { + return false + } + if m.cancelFunc != nil { + m.cancelFunc() + } + m.stopInput() + return true +} + +// stopInput unblocks the stdin reader by closing the stdin source exactly once. +// Closing the read end makes a blocked Scan/read(2) return immediately so the +// reader goroutine can exit and the terminal is released to the parent process. +func (m *simpleTuiModel) stopInput() { + m.stopStdinOnce.Do(func() { + if m.stdin != nil { + _ = m.stdin.Close() + } + }) +} + // readStdinAsync reads from stdin in a goroutine without blocking func (m *simpleTuiModel) readStdinAsync() { defer close(m.inputChan) - scanner := bufio.NewScanner(os.Stdin) + // Default to os.Stdin; tests inject a different reader via m.stdin. + if m.stdin == nil { + m.stdin = os.Stdin + } + + scanner := bufio.NewScanner(m.stdin) // Set larger buffer size (1MB) to handle long OTLP JSON lines const maxScanTokenSize = 1024 * 1024 // 1MB @@ -535,11 +578,15 @@ func (m *simpleTuiModel) readStdinAsync() { // Wait for either scan result or context cancellation select { case <-m.ctx.Done(): + // On quit, stopInput closes stdin, which unblocks the in-flight + // scanner.Scan above so its goroutine can exit instead of leaking. + m.stopInput() return case hasLine := <-scanChan: if !hasLine { - // EOF or error - exit gracefully - break + // EOF or error - exit gracefully (return, not break, so we do + // not re-spawn Scan and busy-spin on a closed input). + return } line := scanner.Text() @@ -547,6 +594,7 @@ func (m *simpleTuiModel) readStdinAsync() { select { case m.inputChan <- line: case <-m.ctx.Done(): + m.stopInput() return } } @@ -582,12 +630,10 @@ func (m *simpleTuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.KeyMsg: - // Check for quit keys and cancel context - if msg.String() == "q" || msg.String() == "ctrl+c" { - if m.cancelFunc != nil { - m.cancelFunc() - } - } + // On a quit key, cancel the app context and stop the stdin reader so + // control returns to the parent process (e.g. K9s) without requiring a + // follow-up Ctrl-C. See handleQuitKey and issue #58. + m.handleQuitKey(msg) // Always forward to dashboard first - let it decide whether to quit newDashboard, cmd := m.dashboard.Update(msg) m.dashboard = newDashboard.(*tui.DashboardModel) diff --git a/cmd/gonzo/app_quit_test.go b/cmd/gonzo/app_quit_test.go new file mode 100644 index 0000000..25bd28d --- /dev/null +++ b/cmd/gonzo/app_quit_test.go @@ -0,0 +1,113 @@ +package main + +import ( + "context" + "io" + "sync" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +// blockingReader simulates a live pipe (e.g. `kubectl logs -f ... | gonzo`) +// whose writer never closes, so Read blocks indefinitely until the reader is +// closed. This is the exact condition behind issue #58: the stdin-reading +// goroutine stays blocked on read(2), holding the terminal so control is not +// returned to the parent (K9s) until the user hits Ctrl-C. +type blockingReader struct { + closed chan struct{} + once sync.Once +} + +func newBlockingReader() *blockingReader { + return &blockingReader{closed: make(chan struct{})} +} + +func (b *blockingReader) Read(p []byte) (int, error) { + // Block until Close is called, then report EOF like a closed pipe. + <-b.closed + return 0, io.EOF +} + +func (b *blockingReader) Close() error { + b.once.Do(func() { close(b.closed) }) + return nil +} + +// TestQuitKeyTriggersInputTeardown is the regression test for issue #58. +// Pressing "q" must (1) cancel the app context and (2) stop the stdin reader +// so that a goroutine blocked on a live pipe unblocks immediately, returning +// control to the parent process without requiring Ctrl-C. +func TestQuitKeyTriggersInputTeardown(t *testing.T) { + for _, key := range []string{"q", "ctrl+c"} { + t.Run(key, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + reader := newBlockingReader() + m := &simpleTuiModel{ + dashboard: nil, // not exercised: quit handling happens before dashboard dispatch + ctx: ctx, + cancelFunc: cancel, + stdin: reader, + hasStdinData: true, + inputChan: make(chan string, 1), + } + + // Launch the stdin reader; it will block on reader.Read. + done := make(chan struct{}) + go func() { + m.readStdinAsync() + close(done) + }() + + // The reader must currently be blocked (no EOF yet). + select { + case <-done: + t.Fatal("readStdinAsync returned before quit was requested") + case <-time.After(50 * time.Millisecond): + } + + // Simulate the quit key. handleQuitKey is the cancellation seam: + // it must cancel the context and stop the input reader. + if !m.handleQuitKey(tea.KeyMsg{Type: keyType(key), Runes: keyRunes(key)}) { + t.Fatalf("handleQuitKey(%q) = false, want true (key should be recognized as quit)", key) + } + + // Context must be cancelled. + select { + case <-ctx.Done(): + default: + t.Errorf("quit key %q did not cancel app context", key) + } + + // The blocked stdin goroutine must unblock and return promptly. + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatalf("quit key %q did not stop the stdin reader; goroutine still blocked", key) + } + }) + } +} + +// keyType/keyRunes build a tea.KeyMsg matching what bubbletea produces for the +// given logical key, so the test exercises the same msg.String() path as the app. +func keyType(key string) tea.KeyType { + switch key { + case "ctrl+c": + return tea.KeyCtrlC + default: + return tea.KeyRunes + } +} + +func keyRunes(key string) []rune { + switch key { + case "ctrl+c": + return nil + default: + return []rune(key) + } +}