diff --git a/pkg/leantui/leantui.go b/pkg/leantui/leantui.go index 6997c08aa6..ffada5e08e 100644 --- a/pkg/leantui/leantui.go +++ b/pkg/leantui/leantui.go @@ -11,6 +11,7 @@ import ( tea "charm.land/bubbletea/v2" "github.com/mattn/go-isatty" + "github.com/muesli/cancelreader" "github.com/docker/docker-agent/pkg/app" "github.com/docker/docker-agent/pkg/gitbranch" @@ -88,9 +89,7 @@ func Run(ctx context.Context, cfg Config) error { keys := make(chan ui.Key, 64) events := make(chan any, 256) - resizes := make(chan [2]int, 4) done := make(chan struct{}) - defer close(done) fileCompletion := completions.NewFileCompletionAt(loopCtx, cfg.WorkingDir) if loader, ok := fileCompletion.(completions.AsyncLoader); ok { @@ -103,7 +102,15 @@ func Run(ctx context.Context, cfg Config) error { }() } - go readKeys(term.Reader(), keys, done) + keysDone := make(chan struct{}) + go func() { + defer close(keysDone) + readKeys(term.Reader(), keys, done) + }() + defer func() { + close(done) + <-keysDone + }() subscriptionReady := make(chan struct{}) go func() { m.app.SubscribeReliable(loopCtx, func(msg tea.Msg) { @@ -117,19 +124,6 @@ func Run(ctx context.Context, cfg Config) error { }() <-subscriptionReady m.app.Start(loopCtx) - go func() { - for { - w, h, ok := term.Resized() - if !ok { - return - } - select { - case resizes <- [2]int{w, h}: - case <-done: - return - } - } - }() if cfg.FirstMessage != nil || cfg.FirstMessageAttachment != "" { first := "" @@ -148,6 +142,8 @@ func Run(ctx context.Context, cfg Config) error { animationTicker := time.NewTicker(100 * time.Millisecond) defer animationTicker.Stop() + resizeTicker := time.NewTicker(250 * time.Millisecond) + defer resizeTicker.Stop() branchChanges := branchWatcher.Changes() probeTimer := time.NewTimer(200 * time.Millisecond) @@ -178,10 +174,8 @@ func Run(ctx context.Context, cfg Config) error { m.handleEvent(loopCtx, ev) m.publishProgramStatus() m.render() - case sz := <-resizes: - m.width, m.height = sz[0], sz[1] - m.r.SetSize(sz[0], sz[1]) - m.render() + case <-resizeTicker.C: + m.resize(term.Size()) case <-animationTicker.C: if m.busy || m.screen.Transcript.ToolCount() > 0 { m.spinnerFrame++ @@ -212,7 +206,9 @@ func readKeys(r io.Reader, keys chan<- ui.Key, done <-chan struct{}) { err error } reads := make(chan readResult) + readerDone := make(chan struct{}) go func() { + defer close(readerDone) for { buf := make([]byte, 8192) n, err := r.Read(buf) @@ -226,6 +222,17 @@ func readKeys(r io.Reader, keys chan<- ui.Key, done <-chan struct{}) { } } }() + defer func() { + select { + case <-readerDone: + return + default: + } + // Some terminals cannot interrupt Read; do not hang shutdown in that case. + if reader, ok := r.(cancelreader.CancelReader); ok && reader.Cancel() { + <-readerDone + } + }() p := &ui.InputParser{} timer := time.NewTimer(time.Hour) timer.Stop() @@ -363,6 +370,15 @@ func newModel(term *ui.Terminal, cfg Config) *model { } } +func (m *model) resize(width, height int) { + if width == m.width && height == m.height { + return + } + m.width, m.height = width, height + m.r.SetSize(width, height) + m.render() +} + // render assembles the full frame and reconciles it with the terminal. func (m *model) render() { lines, cursorLine, cursorCol := m.buildLines() diff --git a/pkg/leantui/readkeys_test.go b/pkg/leantui/readkeys_test.go new file mode 100644 index 0000000000..1fbfa4f0d4 --- /dev/null +++ b/pkg/leantui/readkeys_test.go @@ -0,0 +1,91 @@ +package leantui + +import ( + "io" + "sync" + "testing" + "time" + + "github.com/muesli/cancelreader" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/leantui/ui" +) + +type blockedKeyReader struct { + entered chan struct{} + release chan struct{} + finished chan struct{} + once sync.Once +} + +func (r *blockedKeyReader) Read([]byte) (int, error) { + close(r.entered) + <-r.release + close(r.finished) + return 0, cancelreader.ErrCanceled +} + +func (r *blockedKeyReader) Cancel() bool { + r.once.Do(func() { close(r.release) }) + return true +} + +func (*blockedKeyReader) Close() error { return nil } + +func TestReadKeysJoinsCanceledReader(t *testing.T) { + t.Parallel() + + reader := &blockedKeyReader{ + entered: make(chan struct{}), release: make(chan struct{}), finished: make(chan struct{}), + } + done, returned := make(chan struct{}), make(chan struct{}) + go func() { + defer close(returned) + readKeys(reader, make(chan ui.Key), done) + }() + <-reader.entered + close(done) + select { + case <-returned: + select { + case <-reader.finished: + default: + t.Fatal("readKeys returned before the canceled read completed") + } + case <-time.After(5 * time.Second): + t.Fatal("readKeys did not cancel and join the reader") + } +} + +func TestReadKeysCancellationWithBlockedOutput(t *testing.T) { + t.Parallel() + + reader, writer := io.Pipe() + t.Cleanup(func() { _ = reader.Close(); _ = writer.Close() }) + done, returned := make(chan struct{}), make(chan struct{}) + go func() { + defer close(returned) + readKeys(reader, make(chan ui.Key), done) + }() + _, err := writer.Write([]byte("x")) + require.NoError(t, err) + close(done) + select { + case <-returned: + case <-time.After(5 * time.Second): + t.Fatal("readKeys remained blocked sending a key after cancellation") + } +} + +func TestReadKeysClosedReader(t *testing.T) { + t.Parallel() + + reader, writer := io.Pipe() + require.NoError(t, writer.Close()) + t.Cleanup(func() { _ = reader.Close() }) + keys := make(chan ui.Key) + readKeys(reader, keys, make(chan struct{})) + assert.Empty(t, keys) +} diff --git a/pkg/leantui/resize_test.go b/pkg/leantui/resize_test.go new file mode 100644 index 0000000000..0064ef2491 --- /dev/null +++ b/pkg/leantui/resize_test.go @@ -0,0 +1,64 @@ +package leantui + +import ( + "bufio" + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/leantui/ui" +) + +func TestResize(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + width, height int + }{ + {name: "width only", width: 40, height: 24}, + {name: "height only", width: 80, height: 8}, + {name: "grow", width: 120, height: 40}, + {name: "shrink", width: 30, height: 6}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + m := bareModel(24) + var output bytes.Buffer + m.r = ui.NewRenderer(bufio.NewWriter(&output), m.width, m.height) + text := strings.Repeat("resize this paragraph without losing content ", 12) + m.screen.Transcript.AppendAssistant(text) + m.screen.Editor.SetText("keep this draft") + m.render() + output.Reset() + + m.resize(m.width, m.height) + assert.Empty(t, output.String(), "unchanged polling must not render") + m.render() + assert.NotContains(t, output.String(), "\x1b[2J", "unchanged size must not invalidate the next frame") + output.Reset() + + m.resize(tc.width, tc.height) + assert.Equal(t, tc.width, m.width) + assert.Equal(t, tc.height, m.height) + assert.Contains(t, output.String(), "\x1b[2J", "changed dimensions must repaint") + lines, _, _ := m.buildLines() + assert.Equal(t, max(0, len(lines)-tc.height), m.r.ViewportTop()) + require.NotEmpty(t, lines) + for _, line := range lines { + assert.LessOrEqual(t, ui.DisplayWidth(line), tc.width) + } + + m.resize(80, 24) + assert.Equal(t, "keep this draft", m.screen.Editor.Text()) + assert.Contains(t, strings.Join(m.screen.Transcript.Lines(80, 0, false, m.sessionState, nil), "\n"), "resize this paragraph") + output.Reset() + m.resize(80, 24) + assert.Empty(t, output.String(), "repeated size must not render") + }) + } +} diff --git a/pkg/leantui/resize_unix_test.go b/pkg/leantui/resize_unix_test.go new file mode 100644 index 0000000000..2e6a8e21a6 --- /dev/null +++ b/pkg/leantui/resize_unix_test.go @@ -0,0 +1,134 @@ +//go:build !windows + +package leantui + +import ( + "bytes" + "context" + "io" + "os" + "os/exec" + "regexp" + "strings" + "sync" + "testing" + "time" + + "github.com/creack/pty" + "github.com/muesli/cancelreader" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/term" + + "github.com/docker/docker-agent/pkg/app" + "github.com/docker/docker-agent/pkg/history" + "github.com/docker/docker-agent/pkg/session" +) + +type resizeOutput struct { + mu sync.Mutex + buffer bytes.Buffer +} + +func (o *resizeOutput) Write(p []byte) (int, error) { + o.mu.Lock() + defer o.mu.Unlock() + return o.buffer.Write(p) +} + +func (o *resizeOutput) hasSeparator(width int) bool { + o.mu.Lock() + defer o.mu.Unlock() + return regexp.MustCompile(`(^|[^─])` + strings.Repeat("─", width) + `[^─]`).Match(o.buffer.Bytes()) +} + +func TestRunPollsResizeWhileIdle(t *testing.T) { + t.Parallel() + + master, tty, err := pty.Open() + require.NoError(t, err) + t.Cleanup(func() { _ = master.Close(); _ = tty.Close() }) + require.NoError(t, pty.Setsize(master, &pty.Winsize{Cols: 80, Rows: 24})) + initialState, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + + cancelRead, cancelWrite, err := os.Pipe() + require.NoError(t, err) + t.Cleanup(func() { _ = cancelRead.Close(); _ = cancelWrite.Close() }) + ctx, cancel := context.WithTimeout(t.Context(), 15*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestResizeHelperProcess$") + cmd.Env = append(os.Environ(), "DOCKER_AGENT_RESIZE_HELPER=1") + cmd.Dir = t.TempDir() + cmd.Stdin, cmd.Stdout = tty, tty + cmd.ExtraFiles = []*os.File{cancelRead} + var stderr bytes.Buffer + cmd.Stderr = &stderr + + reader, err := cancelreader.NewReader(master) + require.NoError(t, err) + var output resizeOutput + drained := make(chan struct{}) + go func() { + defer close(drained) + _, _ = io.Copy(&output, reader) + }() + t.Cleanup(func() { + reader.Cancel() + <-drained + require.NoError(t, reader.Close()) + }) + require.NoError(t, cmd.Start()) + waited := make(chan struct{}) + var waitErr error + go func() { waitErr = cmd.Wait(); close(waited) }() + t.Cleanup(func() { + cancel() + select { + case <-waited: + case <-time.After(5 * time.Second): + t.Error("resize helper did not exit") + } + }) + + require.Eventually(t, func() bool { return output.hasSeparator(80) }, 5*time.Second, 10*time.Millisecond) + for _, width := range []uint16{100, 40, 120} { + require.NoError(t, pty.Setsize(master, &pty.Winsize{Cols: width, Rows: 24})) + require.Eventually(t, func() bool { + return output.hasSeparator(int(width)) + }, 5*time.Second, 10*time.Millisecond, "idle UI did not redraw at width %d", width) + } + + require.NoError(t, cancelWrite.Close()) + select { + case <-waited: + require.NoError(t, waitErr, stderr.String()) + case <-ctx.Done(): + t.Fatal("resize helper did not exit after cancellation") + } + restoredState, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + assert.Equal(t, initialState, restoredState) +} + +func TestResizeHelperProcess(t *testing.T) { + if os.Getenv("DOCKER_AGENT_RESIZE_HELPER") != "1" { + return + } + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + cancelRead := os.NewFile(3, "cancel") + defer cancelRead.Close() + go func() { + _, _ = io.Copy(io.Discard, cancelRead) + cancel() + }() + cfg := Config{ + App: app.New(ctx, &cycleThinkingRuntime{}, session.New()), + WorkingDir: ".", + History: &history.History{}, + ShowBanner: new(false), + } + require.NoError(t, Run(ctx, cfg)) +} diff --git a/pkg/leantui/ui/terminal.go b/pkg/leantui/ui/terminal.go index 91eea741f6..cc3fb22f18 100644 --- a/pkg/leantui/ui/terminal.go +++ b/pkg/leantui/ui/terminal.go @@ -4,7 +4,6 @@ import ( "bufio" "fmt" "os" - "time" "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" @@ -24,12 +23,6 @@ type Terminal struct { reader cancelreader.CancelReader prevState *term.State - - width int - height int - - resize chan [2]int - stopResize chan struct{} } func NewTerminal(in, out *os.File) (*Terminal, error) { @@ -45,76 +38,21 @@ func NewTerminal(in, out *os.File) (*Terminal, error) { } t := &Terminal{ - in: in, - out: out, - writer: bufio.NewWriterSize(out, 16*1024), - reader: reader, - prevState: state, - resize: make(chan [2]int, 1), - stopResize: make(chan struct{}), + in: in, + out: out, + writer: bufio.NewWriterSize(out, 16*1024), + reader: reader, + prevState: state, } - t.width, t.height = normalizeTerminalSize(t.querySize()) - lipgloss.EnableLegacyWindowsANSI(out) t.writeString(seqEnableBracketedPaste) t.writeString(ansi.PushKittyKeyboard(ansi.KittyDisambiguateEscapeCodes)) t.flush() - t.startResizeWatcher() return t, nil } -// Resized blocks until the terminal is resized, then refreshes the cached -// dimensions and reports the new size. It returns ok=false once the resize -// channel is closed during shutdown. -func (t *Terminal) Resized() (w, h int, ok bool) { - size, open := <-t.resize - if !open { - return 0, 0, false - } - t.width, t.height = size[0], size[1] - return t.width, t.height, true -} - -func (t *Terminal) startResizeWatcher() { - lastW, lastH := t.width, t.height - go func() { - ticker := time.NewTicker(250 * time.Millisecond) - defer ticker.Stop() - defer close(t.resize) - - for { - select { - case <-ticker.C: - w, h := normalizeTerminalSize(t.querySize()) - if w == lastW && h == lastH { - continue - } - lastW, lastH = w, h - sendLatestResize(t.resize, [2]int{w, h}) - case <-t.stopResize: - return - } - } - }() -} - -func sendLatestResize(ch chan [2]int, size [2]int) { - select { - case ch <- size: - return - default: - } - - select { - case <-ch: - default: - } - - ch <- size -} - func (t *Terminal) querySize() (w, h int) { w, h, err := term.GetSize(int(t.out.Fd())) if err != nil { @@ -133,8 +71,9 @@ func normalizeTerminalSize(w, h int) (int, int) { return w, h } +// Size returns the current output terminal dimensions, falling back to 80×24. func (t *Terminal) Size() (w, h int) { - return t.width, t.height + return normalizeTerminalSize(t.querySize()) } func (t *Terminal) Reader() cancelreader.CancelReader { @@ -159,15 +98,13 @@ func (t *Terminal) flush() { } // Restore tears the terminal back down: it disables bracketed paste, cancels -// the reader, restores the saved terminal state and stops watching for resizes. +// the reader, and restores the saved terminal state. func (t *Terminal) Restore() { t.writeString(seqDisableBracketedPaste) t.writeString(ansi.PopKittyKeyboard(1)) t.writeString(seqShowCursor) t.flush() - close(t.stopResize) - t.reader.Cancel() _ = t.reader.Close() diff --git a/pkg/leantui/ui/terminal_test.go b/pkg/leantui/ui/terminal_test.go new file mode 100644 index 0000000000..91d52331e6 --- /dev/null +++ b/pkg/leantui/ui/terminal_test.go @@ -0,0 +1,46 @@ +package ui + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNormalizeTerminalSize(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + width, height int + wantW, wantH int + }{ + {name: "valid", width: 120, height: 40, wantW: 120, wantH: 40}, + {name: "zero", wantW: 80, wantH: 24}, + {name: "negative", width: -1, height: -1, wantW: 80, wantH: 24}, + {name: "width only", height: 40, wantW: 80, wantH: 40}, + {name: "height only", width: 120, wantW: 120, wantH: 24}, + {name: "tiny", width: 1, height: 1, wantW: 1, wantH: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + w, h := normalizeTerminalSize(tc.width, tc.height) + assert.Equal(t, tc.wantW, w) + assert.Equal(t, tc.wantH, h) + }) + } +} + +func TestTerminalSizeWithoutTTY(t *testing.T) { + t.Parallel() + + out, err := os.Open(os.DevNull) + require.NoError(t, err) + t.Cleanup(func() { _ = out.Close() }) + terminal := &Terminal{out: out} + + w, h := terminal.Size() + assert.Equal(t, 80, w) + assert.Equal(t, 24, h) +} diff --git a/pkg/leantui/ui/terminal_unix_test.go b/pkg/leantui/ui/terminal_unix_test.go new file mode 100644 index 0000000000..463cd46133 --- /dev/null +++ b/pkg/leantui/ui/terminal_unix_test.go @@ -0,0 +1,49 @@ +//go:build !windows + +package ui + +import ( + "testing" + + "github.com/creack/pty" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/term" +) + +func TestTerminalSizeReadsCurrentDimensions(t *testing.T) { + t.Parallel() + + master, tty, err := pty.Open() + require.NoError(t, err) + t.Cleanup(func() { _ = master.Close(); _ = tty.Close() }) + outputMaster, outputTTY, err := pty.Open() + require.NoError(t, err) + t.Cleanup(func() { _ = outputMaster.Close(); _ = outputTTY.Close() }) + require.NoError(t, pty.Setsize(master, &pty.Winsize{Cols: 200, Rows: 60})) + initialState, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + terminal, err := NewTerminal(tty, outputTTY) + require.NoError(t, err) + t.Cleanup(func() { + terminal.Restore() + restoredState, err := term.GetState(int(tty.Fd())) + require.NoError(t, err) + assert.Equal(t, initialState, restoredState) + }) + + for _, size := range []pty.Winsize{ + {Cols: 80, Rows: 24}, + {Cols: 120, Rows: 40}, + {Cols: 20, Rows: 5}, + {Cols: 1, Rows: 1}, + {Cols: 0, Rows: 40}, + {Cols: 120, Rows: 0}, + } { + require.NoError(t, pty.Setsize(outputMaster, &size)) + w, h := terminal.Size() + wantW, wantH := normalizeTerminalSize(int(size.Cols), int(size.Rows)) + assert.Equal(t, wantW, w) + assert.Equal(t, wantH, h) + } +}