From c98c80fde052a61b5245069c24fcbe6c7eacd741 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 1 Jul 2026 04:37:09 +0000 Subject: [PATCH 1/3] fix(attach): replay at attached terminal size --- internal/session/host.go | 63 +++++++++---- internal/session/session_test.go | 150 +++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 17 deletions(-) diff --git a/internal/session/host.go b/internal/session/host.go index 5e92fe0..00384e1 100644 --- a/internal/session/host.go +++ b/internal/session/host.go @@ -366,16 +366,44 @@ func titleSequence(label string) string { return "\x1b]0;" + clean + "\x07" } -func (h *host) applyResize(cols, rows int) { - if cols <= 0 || rows <= 0 || cols > 1000 || rows > 1000 { +func validSize(cols, rows int) bool { + return cols > 0 && rows > 0 && cols <= 1000 && rows <= 1000 +} + +func resizeNudge(cols, rows int) (int, int, bool) { + if !validSize(cols, rows) { + return 0, 0, false + } + if rows > 1 { + return cols, rows - 1, true + } + if cols > 1 { + return cols - 1, rows, true + } + return 0, 0, false +} + +func (h *host) applyResizeLocked(cols, rows int) { + if !validSize(cols, rows) { return } - h.mu.Lock() - defer h.mu.Unlock() h.term.Resize(cols, rows) + h.applyPTYSizeLocked(cols, rows) +} + +func (h *host) applyPTYSizeLocked(cols, rows int) { + if !validSize(cols, rows) { + return + } _ = pty.Setsize(h.ptmx, &pty.Winsize{Cols: uint16(cols), Rows: uint16(rows)}) // #nosec G115 -- bounds checked above } +func (h *host) applyResize(cols, rows int) { + h.mu.Lock() + defer h.mu.Unlock() + h.applyResizeLocked(cols, rows) +} + func (h *host) handleAttach(conn net.Conn, br *bufio.Reader, req request) { cl := &attachClient{conn: conn, out: make(chan []byte, attachBufFrames)} h.mu.Lock() @@ -387,24 +415,25 @@ func (h *host) handleAttach(conn net.Conn, br *bufio.Reader, req request) { _ = conn.Close() return } - // Register and queue the screen replay atomically, so no live broadcast - // can interleave ahead of it. The replay paints the current screen - // immediately; the follow-up resize makes full-screen TUIs repaint - // themselves with real colors/attributes. + // Apply the attaching geometry, then register and queue the screen replay + // atomically so no live broadcast can interleave ahead of it. h.mu.Lock() - curCols, curRows := h.term.Size() - cl.out <- append([]byte(titleSequence(label)), h.term.Redraw()...) - h.clients[cl] = struct{}{} - h.mu.Unlock() - go h.attachWriter(cl) - if req.Cols > 0 && req.Rows > 0 { + if validSize(req.Cols, req.Rows) { + curCols, curRows := h.term.Size() if req.Cols == curCols && req.Rows == curRows { // Same geometry produces no SIGWINCH; nudge the size so the - // agent's TUI still repaints for the new viewer. - h.applyResize(req.Cols, req.Rows-1) + // agent's TUI still repaints for the new viewer without truncating + // the replay buffer before Redraw. + if cols, rows, ok := resizeNudge(req.Cols, req.Rows); ok { + h.applyPTYSizeLocked(cols, rows) + } } - h.applyResize(req.Cols, req.Rows) + h.applyResizeLocked(req.Cols, req.Rows) } + cl.out <- append([]byte(titleSequence(label)), h.term.Redraw()...) + h.clients[cl] = struct{}{} + h.mu.Unlock() + go h.attachWriter(cl) h.attachReader(cl, br) } diff --git a/internal/session/session_test.go b/internal/session/session_test.go index a459aa1..16a4ee6 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -447,6 +447,156 @@ func TestAttachOwnsTerminalStateOnTTY(t *testing.T) { } } +func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { + c := newTestClient(t) + ctx := context.Background() + name := "uam-fake-eeee9999" + cmd := []string{"/bin/sh", "-c", "printf '\033[999;1Hedge\033[999;999H'; sleep 60"} + if err := c.CreateSession(ctx, name, t.TempDir(), nil, cmd); err != nil { + t.Fatalf("CreateSession: %v", err) + } + waitFor(t, "edge", func() bool { + out, _ := c.Capture(ctx, name, 10) + return strings.Contains(out, "edge") + }) + + ptmx, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + defer func() { _ = ptmx.Close(); _ = tty.Close() }() + if err := pty.Setsize(ptmx, &pty.Winsize{Cols: 80, Rows: 24}); err != nil { + t.Fatal(err) + } + + done := make(chan error, 1) + go func() { done <- runAttachWithOptions(c.Dir, name, tty, tty, attachOptions{quiet: true}) }() + + var mu sync.Mutex + var got []byte + go func() { + buf := make([]byte, 4096) + for { + n, rerr := ptmx.Read(buf) + if n > 0 { + mu.Lock() + got = append(got, buf[:n]...) + mu.Unlock() + } + if rerr != nil { + return + } + } + }() + snapshot := func() string { + mu.Lock() + defer mu.Unlock() + return string(got) + } + + waitFor(t, "initial replay cursor", func() bool { + out := snapshot() + return strings.Contains(out, "\x1b[24;80H") || strings.Contains(out, "\x1b[50;200H") + }) + out := snapshot() + if !strings.Contains(out, "edge") { + t.Fatalf("attach replay missing edge marker: %q", out) + } + if !strings.Contains(out, "\x1b[24;80H") { + t.Fatalf("attach replay must park cursor using attached terminal size: %q", out) + } + if strings.Contains(out, "\x1b[50;200H") { + t.Fatalf("attach replay must not use detached terminal size: %q", out) + } + + if _, err := ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("runAttach: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("detach chord did not detach") + } +} + +func TestSameSizeAttachNudgeDoesNotTruncateReplay(t *testing.T) { + c := newTestClient(t) + ctx := context.Background() + name := "uam-fake-dddd9999" + cmd := []string{"/bin/sh", "-c", `while IFS= read -r line; do [ "$line" = paint ] && printf '\033[Htop-row-safe\033[24;1Hbottom-row-guard\033[H'; done`} + if err := c.CreateSession(ctx, name, t.TempDir(), nil, cmd); err != nil { + t.Fatalf("CreateSession: %v", err) + } + + attachOnce := func(what string, ready func(string) bool) string { + ptmx, tty, err := pty.Open() + if err != nil { + t.Fatal(err) + } + defer func() { _ = ptmx.Close(); _ = tty.Close() }() + if err := pty.Setsize(ptmx, &pty.Winsize{Cols: 80, Rows: 24}); err != nil { + t.Fatal(err) + } + + done := make(chan error, 1) + go func() { done <- runAttachWithOptions(c.Dir, name, tty, tty, attachOptions{quiet: true}) }() + + var mu sync.Mutex + var got []byte + go func() { + buf := make([]byte, 4096) + for { + n, rerr := ptmx.Read(buf) + if n > 0 { + mu.Lock() + got = append(got, buf[:n]...) + mu.Unlock() + } + if rerr != nil { + return + } + } + }() + snapshot := func() string { + mu.Lock() + defer mu.Unlock() + return string(got) + } + + waitFor(t, what, func() bool { return ready(snapshot()) }) + out := snapshot() + if _, err := ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("runAttach: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("detach chord did not detach") + } + return out + } + + attachOnce("initial empty replay", func(out string) bool { return strings.Contains(out, "\x1b[1;1H") }) + if err := c.SendLine(ctx, name, "paint"); err != nil { + t.Fatalf("SendLine: %v", err) + } + waitFor(t, "painted screen", func() bool { + out, _ := c.Capture(ctx, name, 30) + return strings.Contains(out, "top-row-safe") && strings.Contains(out, "bottom-row-guard") + }) + + out := attachOnce("same-size replay", func(out string) bool { return strings.Contains(out, "bottom-row-guard") }) + if !strings.Contains(out, "top-row-safe") { + t.Fatalf("same-size attach replay lost top-row content: %q", out) + } +} + // Detaching while the agent is mid-burst must not scribble the primary // screen: bytes still buffered in the host→terminal pump at detach time have // to be drained inside the alternate screen, so nothing follows the From 61520077439ba6c20fb74de1103fe108960c3afa Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 1 Jul 2026 04:44:38 +0000 Subject: [PATCH 2/3] test(attach): simplify fullscreen replay regressions --- internal/session/session_test.go | 138 +++++++++++++------------------ 1 file changed, 59 insertions(+), 79 deletions(-) diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 16a4ee6..54735a7 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -447,31 +447,29 @@ func TestAttachOwnsTerminalStateOnTTY(t *testing.T) { } } -func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { - c := newTestClient(t) - ctx := context.Background() - name := "uam-fake-eeee9999" - cmd := []string{"/bin/sh", "-c", "printf '\033[999;1Hedge\033[999;999H'; sleep 60"} - if err := c.CreateSession(ctx, name, t.TempDir(), nil, cmd); err != nil { - t.Fatalf("CreateSession: %v", err) - } - waitFor(t, "edge", func() bool { - out, _ := c.Capture(ctx, name, 10) - return strings.Contains(out, "edge") - }) +type attachedPTY struct { + ptmx *os.File + done chan error + snapshot func() string +} +func startQuietAttach(t *testing.T, dir, name string, cols, rows uint16) *attachedPTY { + t.Helper() ptmx, tty, err := pty.Open() if err != nil { t.Fatal(err) } - defer func() { _ = ptmx.Close(); _ = tty.Close() }() - if err := pty.Setsize(ptmx, &pty.Winsize{Cols: 80, Rows: 24}); err != nil { + t.Cleanup(func() { _ = ptmx.Close(); _ = tty.Close() }) + if err := pty.Setsize(ptmx, &pty.Winsize{Cols: cols, Rows: rows}); err != nil { t.Fatal(err) } done := make(chan error, 1) - go func() { done <- runAttachWithOptions(c.Dir, name, tty, tty, attachOptions{quiet: true}) }() + go func() { done <- runAttachWithOptions(dir, name, tty, tty, attachOptions{quiet: true}) }() + return &attachedPTY{ptmx: ptmx, done: done, snapshot: capturePTYOutput(ptmx)} +} +func capturePTYOutput(ptmx *os.File) func() string { var mu sync.Mutex var got []byte go func() { @@ -488,17 +486,49 @@ func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { } } }() - snapshot := func() string { + return func() string { mu.Lock() defer mu.Unlock() return string(got) } +} + +func (a *attachedPTY) Snapshot() string { return a.snapshot() } + +func (a *attachedPTY) Detach(t *testing.T) { + t.Helper() + if _, err := a.ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d + t.Fatal(err) + } + select { + case err := <-a.done: + if err != nil { + t.Fatalf("runAttach: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("detach chord did not detach") + } +} + +func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { + c := newTestClient(t) + ctx := context.Background() + name := "uam-fake-eeee9999" + cmd := []string{"/bin/sh", "-c", "printf '\033[999;1Hedge\033[999;999H'; sleep 60"} + if err := c.CreateSession(ctx, name, t.TempDir(), nil, cmd); err != nil { + t.Fatalf("CreateSession: %v", err) + } + waitFor(t, "edge", func() bool { + out, _ := c.Capture(ctx, name, 10) + return strings.Contains(out, "edge") + }) + attached := startQuietAttach(t, c.Dir, name, 80, 24) waitFor(t, "initial replay cursor", func() bool { - out := snapshot() + out := attached.Snapshot() return strings.Contains(out, "\x1b[24;80H") || strings.Contains(out, "\x1b[50;200H") }) - out := snapshot() + out := attached.Snapshot() if !strings.Contains(out, "edge") { t.Fatalf("attach replay missing edge marker: %q", out) } @@ -508,18 +538,7 @@ func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { if strings.Contains(out, "\x1b[50;200H") { t.Fatalf("attach replay must not use detached terminal size: %q", out) } - - if _, err := ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d - t.Fatal(err) - } - select { - case err := <-done: - if err != nil { - t.Fatalf("runAttach: %v", err) - } - case <-time.After(10 * time.Second): - t.Fatal("detach chord did not detach") - } + attached.Detach(t) } func TestSameSizeAttachNudgeDoesNotTruncateReplay(t *testing.T) { @@ -532,57 +551,16 @@ func TestSameSizeAttachNudgeDoesNotTruncateReplay(t *testing.T) { } attachOnce := func(what string, ready func(string) bool) string { - ptmx, tty, err := pty.Open() - if err != nil { - t.Fatal(err) - } - defer func() { _ = ptmx.Close(); _ = tty.Close() }() - if err := pty.Setsize(ptmx, &pty.Winsize{Cols: 80, Rows: 24}); err != nil { - t.Fatal(err) - } - - done := make(chan error, 1) - go func() { done <- runAttachWithOptions(c.Dir, name, tty, tty, attachOptions{quiet: true}) }() - - var mu sync.Mutex - var got []byte - go func() { - buf := make([]byte, 4096) - for { - n, rerr := ptmx.Read(buf) - if n > 0 { - mu.Lock() - got = append(got, buf[:n]...) - mu.Unlock() - } - if rerr != nil { - return - } - } - }() - snapshot := func() string { - mu.Lock() - defer mu.Unlock() - return string(got) - } - - waitFor(t, what, func() bool { return ready(snapshot()) }) - out := snapshot() - if _, err := ptmx.Write([]byte{0x02, 'd'}); err != nil { // Ctrl+B d - t.Fatal(err) - } - select { - case err := <-done: - if err != nil { - t.Fatalf("runAttach: %v", err) - } - case <-time.After(10 * time.Second): - t.Fatal("detach chord did not detach") - } + attached := startQuietAttach(t, c.Dir, name, 80, 24) + waitFor(t, what, func() bool { return ready(attached.Snapshot()) }) + out := attached.Snapshot() + attached.Detach(t) return out } - attachOnce("initial empty replay", func(out string) bool { return strings.Contains(out, "\x1b[1;1H") }) + attachOnce("initial empty replay", func(out string) bool { + return strings.Contains(out, "\x1b[1;1H") + }) if err := c.SendLine(ctx, name, "paint"); err != nil { t.Fatalf("SendLine: %v", err) } @@ -591,7 +569,9 @@ func TestSameSizeAttachNudgeDoesNotTruncateReplay(t *testing.T) { return strings.Contains(out, "top-row-safe") && strings.Contains(out, "bottom-row-guard") }) - out := attachOnce("same-size replay", func(out string) bool { return strings.Contains(out, "bottom-row-guard") }) + out := attachOnce("same-size replay", func(out string) bool { + return strings.Contains(out, "bottom-row-guard") + }) if !strings.Contains(out, "top-row-safe") { t.Fatalf("same-size attach replay lost top-row content: %q", out) } From f18f3a6e95e513c4117bc23f070fd40a86cadd42 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 1 Jul 2026 04:49:01 +0000 Subject: [PATCH 3/3] test(attach): cover resize nudge bounds --- internal/session/session_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 54735a7..cb8d6a4 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -510,6 +510,29 @@ func (a *attachedPTY) Detach(t *testing.T) { } } +func TestResizeNudgeBounds(t *testing.T) { + cols, rows, ok := resizeNudge(80, 24) + if !ok || cols != 80 || rows != 23 { + t.Fatalf("row nudge = %dx%d %v, want 80x23 true", cols, rows, ok) + } + + cols, rows, ok = resizeNudge(2, 1) + if !ok || cols != 1 || rows != 1 { + t.Fatalf("column nudge = %dx%d %v, want 1x1 true", cols, rows, ok) + } + + if _, _, ok := resizeNudge(1, 1); ok { + t.Fatal("1x1 terminal must not be nudged") + } + if _, _, ok := resizeNudge(0, 24); ok { + t.Fatal("invalid terminal size must not be nudged") + } + + h := &host{} + h.applyResizeLocked(0, 24) + h.applyPTYSizeLocked(80, 0) +} + func TestAttachReplayUsesCurrentTerminalSize(t *testing.T) { c := newTestClient(t) ctx := context.Background()