diff --git a/cmd/moat/cli/exec.go b/cmd/moat/cli/exec.go index 2f79b5ef..0d9c7eed 100644 --- a/cmd/moat/cli/exec.go +++ b/cmd/moat/cli/exec.go @@ -481,10 +481,19 @@ func RunInteractiveAttached(ctx context.Context, manager *run.Manager, r *run.Ru attachCtx, attachCancel := context.WithCancel(ctx) defer attachCancel() + // Reserve one terminal row for the status bar so the child process is + // told the PTY is one row shorter than the host terminal. Without this, + // the child draws its own bottom-pinned UI on the same row as moat's + // footer and the two interleave on every redraw. + var reservedRows uint + if statusWriter != nil { + reservedRows = 1 + } + // Start with attachment - this ensures TTY is connected before process starts attachDone := make(chan error, 1) go func() { - attachDone <- manager.StartAttached(attachCtx, r.ID, stdin, stdout, os.Stderr) + attachDone <- manager.StartAttached(attachCtx, r.ID, stdin, stdout, os.Stderr, reservedRows) }() // Give container a moment to start, then resize TTY to match terminal. @@ -497,8 +506,12 @@ func RunInteractiveAttached(ctx context.Context, manager *run.Manager, r *run.Ru if term.IsTerminal(os.Stdout) { width, height := term.GetSize(os.Stdout) if width > 0 && height > 0 { - // #nosec G115 -- width/height are validated positive above - if err := manager.ResizeTTY(ctx, r.ID, uint(height), uint(width)); err != nil { + childHeight := uint(height) // #nosec G115 -- validated positive above + if childHeight > reservedRows { + childHeight -= reservedRows + } + // #nosec G115 -- width is validated positive above + if err := manager.ResizeTTY(ctx, r.ID, childHeight, uint(width)); err != nil { log.Debug("failed to resize TTY", "error", err) } } @@ -519,9 +532,14 @@ func RunInteractiveAttached(ctx context.Context, manager *run.Manager, r *run.Ru } ringRecorder.AddResize(width, height) _ = statusWriter.Resize(width, height) - // Also resize container TTY - // #nosec G115 -- width/height are validated positive above - _ = manager.ResizeTTY(ctx, r.ID, uint(height), uint(width)) + // Resize the container TTY to the host height minus + // the rows reserved for moat's status bar. + childHeight := uint(height) // #nosec G115 -- validated positive above + if childHeight > reservedRows { + childHeight -= reservedRows + } + // #nosec G115 -- width is validated positive above + _ = manager.ResizeTTY(ctx, r.ID, childHeight, uint(width)) } } continue // Don't break out of loop diff --git a/internal/deps/scripts/moat-init.sh b/internal/deps/scripts/moat-init.sh index 90342b98..bf3772fa 100644 --- a/internal/deps/scripts/moat-init.sh +++ b/internal/deps/scripts/moat-init.sh @@ -468,6 +468,20 @@ run_pre_run_hook() { # If we're root and moatuser exists, drop privileges with gosu. # If moatuser doesn't exist, fail - running as root defeats the security model. run_pre_run_hook + +# Clear the terminal so the user's command starts on a fresh screen. +# Without this, output from pre_run hooks (e.g. "pnpm install") and from +# moat-init's own setup steps remains on screen. TUIs that paint with +# relative cursor advances (\x1b[NC) instead of overwriting cells — Claude +# Code's startup banner among them — leave those characters bleeding +# through their layout. Clearing once at the boundary between init and the +# user's command avoids that. moat's CLI redraws its footer on the next +# debounce tick, so the brief disappearance is invisible in practice. +# `set -e` aborts above on hook failure, so a clear here only runs on success. +if [ -t 1 ]; then + printf '\033[2J\033[H' +fi + if [ "$(id -u)" != "0" ]; then # Already non-root (e.g., --user was passed to docker run) exec "$@" diff --git a/internal/e2e/e2e_test.go b/internal/e2e/e2e_test.go index 7932d2ce..aee0c457 100644 --- a/internal/e2e/e2e_test.go +++ b/internal/e2e/e2e_test.go @@ -1807,7 +1807,7 @@ func TestInteractiveContainer(t *testing.T) { // Run StartAttached - this should send input to cat and get it echoed back // Note: cat exits when stdin reaches EOF, so this will complete - err = mgr.StartAttached(ctx, r.ID, stdinReader, &stdoutBuf, &stdoutBuf) + err = mgr.StartAttached(ctx, r.ID, stdinReader, &stdoutBuf, &stdoutBuf, 0) if err != nil { t.Fatalf("StartAttached: %v", err) } diff --git a/internal/e2e/logs_capture_test.go b/internal/e2e/logs_capture_test.go index 39de7217..21ce4d6b 100644 --- a/internal/e2e/logs_capture_test.go +++ b/internal/e2e/logs_capture_test.go @@ -188,7 +188,7 @@ func TestLogsCapturedInInteractiveMode(t *testing.T) { // verify that StartAttached captures logs when the container exits doneCh := make(chan error, 1) go func() { - doneCh <- mgr.StartAttached(ctx, r.ID, os.Stdin, os.Stdout, os.Stderr) + doneCh <- mgr.StartAttached(ctx, r.ID, os.Stdin, os.Stdout, os.Stderr, 0) }() // Wait for completion diff --git a/internal/e2e/tui_test.go b/internal/e2e/tui_test.go index 7ead802d..719d90bc 100644 --- a/internal/e2e/tui_test.go +++ b/internal/e2e/tui_test.go @@ -69,7 +69,7 @@ func TestAppleTUIWriterPassthrough(t *testing.T) { defer writer.Cleanup() // Route container output through the tui.Writer - err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}) + err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}, 0) if err != nil { t.Fatalf("StartAttached: %v", err) } @@ -127,7 +127,7 @@ func TestAppleTUIWriterAltScreenDuringInit(t *testing.T) { _ = writer.Setup() defer writer.Cleanup() - err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}) + err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}, 0) if err != nil { t.Fatalf("StartAttached: %v", err) } @@ -189,7 +189,7 @@ func TestAppleTUIWriterMultipleWrites(t *testing.T) { _ = writer.Setup() defer writer.Cleanup() - err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}) + err = mgr.StartAttached(ctx, r.ID, strings.NewReader(""), writer, &bytes.Buffer{}, 0) if err != nil { t.Fatalf("StartAttached: %v", err) } diff --git a/internal/run/manager.go b/internal/run/manager.go index 8c7cc05d..cca0ff77 100644 --- a/internal/run/manager.go +++ b/internal/run/manager.go @@ -3073,7 +3073,14 @@ func (m *Manager) Start(ctx context.Context, runID string, opts StartOptions) er // This is required for TUI applications (like Codex CLI) that need the terminal // connected before the process starts to properly detect terminal capabilities. // Unlike Start + Attach, this ensures the TTY is ready when the container command begins. -func (m *Manager) StartAttached(ctx context.Context, runID string, stdin io.Reader, stdout, stderr io.Writer) error { +// StartAttached attaches stdio to the run's container and blocks until the +// process exits. +// +// reservedRows tells the manager how many rows of the host terminal are +// reserved by the CLI (e.g., for a status bar) and must NOT be advertised +// to the child process. The auto-detected initial PTY height is reduced +// by this amount; pass 0 when no rows are reserved. +func (m *Manager) StartAttached(ctx context.Context, runID string, stdin io.Reader, stdout, stderr io.Writer, reservedRows uint) error { m.mu.Lock() r, ok := m.runs[runID] if !ok { @@ -3120,12 +3127,21 @@ func (m *Manager) StartAttached(ctx context.Context, runID string, stdin io.Read // Pass initial terminal size so the container can be resized immediately // after starting, before the process queries terminal dimensions. + // + // reservedRows is subtracted so the child sees only the rows it can + // actually paint into. Otherwise the child draws its own bottom-pinned + // UI (e.g., Claude Code's input prompt and status lines) at the same + // row as moat's status bar, producing character-interleaved artifacts. if useTTY && term.IsTerminal(os.Stdout) { width, height := term.GetSize(os.Stdout) if width > 0 && height > 0 { // #nosec G115 -- width/height are validated positive above attachOpts.InitialWidth = uint(width) - attachOpts.InitialHeight = uint(height) + usableHeight := uint(height) + if usableHeight > reservedRows { + usableHeight -= reservedRows + } + attachOpts.InitialHeight = usableHeight } } diff --git a/internal/tui/writer.go b/internal/tui/writer.go index 1dab9b96..bf959742 100644 --- a/internal/tui/writer.go +++ b/internal/tui/writer.go @@ -37,6 +37,25 @@ var altScreenExit = [][]byte{ []byte("\x1b[?1047l"), } +// decstbmReset is the DECSTBM "reset scroll region to full screen" escape +// sequence. Some node-based TUIs (Claude Code among them) emit this once +// during startup as part of TTY normalization. When that happens, moat's +// scroll region — which reserves the bottom row for the footer — is wiped +// out, and any subsequent footer redraw at row H becomes regular text that +// scrolls up with content. We detect this sequence in the data stream, let +// it pass through, then immediately reassert moat's scroll region. The +// trace evidence is one reset at startup and one at exit per session, so +// this is a one-shot restore — not an ongoing fight with the child. +var decstbmReset = []byte("\x1b[r") + +// eraseScreen is the "erase entire display" sequence. moat-init.sh emits +// this between pre_run hooks and the user's command so the agent starts +// on a clean screen. After it lands, moat's footer at row H is gone too, +// and the 50ms debounce isn't reliable during a busy agent startup. We +// detect the sequence, pass it through, then immediately redraw the +// footer so it doesn't disappear for the duration of the splash sequence. +var eraseScreen = []byte("\x1b[2J") + // renderInterval is the compositor render tick rate (~60fps). const renderInterval = 16 * time.Millisecond @@ -241,8 +260,39 @@ func (w *Writer) processDataLocked(data []byte) error { continue } + // Detect DECSTBM "reset scroll region" — pass it through, then + // immediately reassert moat's scroll region so the footer keeps a + // home. Only meaningful in scroll mode; in compositor mode the + // emulator owns its own scroll state. + if bytes.HasPrefix(data, decstbmReset) { + if err := w.outputLocked(data[:len(decstbmReset)]); err != nil { + return err + } + data = data[len(decstbmReset):] + if !w.altScreen && w.height > 1 { + if err := w.reassertScrollRegionLocked(); err != nil { + return err + } + } + continue + } + + // Detect "erase entire screen". Pass it through, then redraw the + // footer immediately — the 50ms debounce isn't reliable during the + // agent's startup splash. Compositor mode owns its own surface. + if bytes.HasPrefix(data, eraseScreen) { + if err := w.outputLocked(data[:len(eraseScreen)]); err != nil { + return err + } + data = data[len(eraseScreen):] + if !w.altScreen && w.height > 1 { + w.redrawFooterLocked() + } + continue + } + // Check if this could be a partial match at the end of the buffer - if w.isPrefixOfAltScreen(data) && len(data) < maxAltScreenSeqLen() { + if w.isPrefixOfKnownSequence(data) && len(data) < maxKnownSeqLen() { // Buffer it for the next Write call w.escBuf = append(w.escBuf[:0], data...) return nil @@ -291,8 +341,10 @@ func (w *Writer) matchAltScreen(data []byte) (matched bool, enter bool, length i return false, false, 0 } -// isPrefixOfAltScreen returns true if data is a prefix of any alt screen sequence. -func (w *Writer) isPrefixOfAltScreen(data []byte) bool { +// isPrefixOfKnownSequence returns true if data is a prefix of any sequence +// the writer recognizes (alt screen enter/exit or DECSTBM reset). Used to +// defer processing of sequences that may be split across Write calls. +func (w *Writer) isPrefixOfKnownSequence(data []byte) bool { for _, seq := range altScreenEnter { if len(data) < len(seq) && bytes.HasPrefix(seq, data) { return true @@ -303,11 +355,17 @@ func (w *Writer) isPrefixOfAltScreen(data []byte) bool { return true } } + if len(data) < len(decstbmReset) && bytes.HasPrefix(decstbmReset, data) { + return true + } + if len(data) < len(eraseScreen) && bytes.HasPrefix(eraseScreen, data) { + return true + } return false } -// maxAltScreenSeqLen returns the length of the longest alt screen sequence. -func maxAltScreenSeqLen() int { +// maxKnownSeqLen returns the length of the longest sequence the writer recognizes. +func maxKnownSeqLen() int { max := 0 for _, seq := range altScreenEnter { if len(seq) > max { @@ -319,9 +377,30 @@ func maxAltScreenSeqLen() int { max = len(seq) } } + if len(decstbmReset) > max { + max = len(decstbmReset) + } + if len(eraseScreen) > max { + max = len(eraseScreen) + } return max } +// reassertScrollRegionLocked re-establishes moat's DECSTBM scroll region +// without disturbing the cursor. Used after the child process resets the +// scroll region (e.g., via `\x1b[r` during TTY normalization). The save +// and restore are needed because setting DECSTBM moves the cursor to the +// home position by default. +// Caller must hold the mutex. +func (w *Writer) reassertScrollRegionLocked() error { + var buf bytes.Buffer + buf.WriteString("\x1b7") // DECSC: save cursor + attrs + fmt.Fprintf(&buf, "\x1b[1;%dr", w.height-1) + buf.WriteString("\x1b8") // DECRC: restore cursor + attrs + _, err := w.out.Write(buf.Bytes()) + return err +} + // enterCompositorLocked switches from scroll mode to compositor mode. func (w *Writer) enterCompositorLocked() error { if w.altScreen { diff --git a/internal/tui/writer_test.go b/internal/tui/writer_test.go index e8df0a12..5e29649b 100644 --- a/internal/tui/writer_test.go +++ b/internal/tui/writer_test.go @@ -797,6 +797,167 @@ func TestWriter_Apple_BufferFullFallback(t *testing.T) { w.Cleanup() } +func TestWriter_DECSTBMReset_ReassertsScrollRegion(t *testing.T) { + var buf bytes.Buffer + bar := NewStatusBar("run_abc123", "my-agent", "docker") + bar.SetDimensions(60, 24) + + w := NewWriter(&buf, bar, "docker") + _ = w.Setup() + buf.Reset() + + // Child sends DECSTBM reset (\x1b[r). The writer must pass it through + // to the terminal AND immediately reassert moat's scroll region so the + // footer keeps a home below the content area. + _, err := w.Write([]byte("hello\x1b[rworld")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + + // Original ESC[r must reach the terminal — Claude (or its libs) may + // have side effects on it that we don't want to suppress. + if !strings.Contains(output, "\x1b[r") { + t.Errorf("expected DECSTBM reset to pass through, got %q", output) + } + + // Following the reset, the writer should have reasserted the scroll + // region for height=24 (so [1;23]). + if !strings.Contains(output, "\x1b[1;23r") { + t.Errorf("expected scroll region reassert (ESC[1;23r) after reset, got %q", output) + } + + // Reassertion must be wrapped in DECSC/DECRC so the cursor isn't moved. + if !strings.Contains(output, "\x1b7") || !strings.Contains(output, "\x1b8") { + t.Errorf("expected DECSC/DECRC around reassert, got %q", output) + } + + // Surrounding content still passes through. + if !strings.Contains(output, "hello") || !strings.Contains(output, "world") { + t.Errorf("expected surrounding content to pass through, got %q", output) + } + + w.Cleanup() +} + +func TestWriter_DECSTBMReset_NoReassertInCompositorMode(t *testing.T) { + var buf bytes.Buffer + bar := NewStatusBar("run_abc123", "my-agent", "docker") + bar.SetDimensions(60, 24) + + w := NewWriter(&buf, bar, "docker") + _ = w.Setup() + + // Enter alt screen — switch to compositor mode. + _, _ = w.Write([]byte("\x1b[?1049h")) + buf.Reset() + + // While in alt-screen, DECSTBM reset is meaningful only inside the + // emulator's screen; the real terminal already has its own scroll + // region untouched. We must NOT emit our reassert to the real terminal + // here, since the compositor's render loop owns that surface. + _, err := w.Write([]byte("\x1b[r")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + + // No reassert should hit the real terminal. + if strings.Contains(output, "\x1b[1;23r") { + t.Errorf("expected NO scroll region reassert in compositor mode, got %q", output) + } + + w.Cleanup() +} + +func TestWriter_DECSTBMReset_SplitAcrossWrites(t *testing.T) { + var buf bytes.Buffer + bar := NewStatusBar("run_abc123", "my-agent", "docker") + bar.SetDimensions(60, 24) + + w := NewWriter(&buf, bar, "docker") + _ = w.Setup() + buf.Reset() + + // Send "\x1b[r" in two pieces — the writer should buffer the partial + // prefix and detect the sequence on the next Write. + _, _ = w.Write([]byte("\x1b[")) + _, _ = w.Write([]byte("r")) + + output := buf.String() + if !strings.Contains(output, "\x1b[1;23r") { + t.Errorf("expected scroll region reassert after split DECSTBM reset, got %q", output) + } + + w.Cleanup() +} + +func TestWriter_EraseScreen_RedrawsFooterImmediately(t *testing.T) { + var buf bytes.Buffer + bar := NewStatusBar("run_abc123", "my-agent", "docker") + bar.SetDimensions(60, 24) + + w := NewWriter(&buf, bar, "docker") + _ = w.Setup() + buf.Reset() + + // moat-init.sh sends \x1b[2J\x1b[H between pre_run hooks and the user's + // command so the agent paints on a clean screen. The writer must redraw + // the footer immediately afterward — the 50ms debounce isn't reliable + // during a busy startup. + _, err := w.Write([]byte("\x1b[2J\x1b[H")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + + // The erase sequence itself must reach the terminal. + if !strings.Contains(output, "\x1b[2J") { + t.Errorf("expected erase screen to pass through, got %q", output) + } + + // The footer must have been redrawn — look for the cursor-to-row-H move + // that redrawFooterLocked emits and the run name in the bar text. + if !strings.Contains(output, "\x1b[24;1H") { + t.Errorf("expected footer redraw at row 24, got %q", output) + } + if !strings.Contains(output, "run_abc123") { + t.Errorf("expected status bar text after erase, got %q", output) + } + + w.Cleanup() +} + +func TestWriter_EraseScreen_NoRedrawInCompositorMode(t *testing.T) { + var buf bytes.Buffer + bar := NewStatusBar("run_abc123", "my-agent", "docker") + bar.SetDimensions(60, 24) + + w := NewWriter(&buf, bar, "docker") + _ = w.Setup() + + // Switch to compositor mode. + _, _ = w.Write([]byte("\x1b[?1049h")) + buf.Reset() + + // In compositor mode, the emulator owns the surface. We must NOT issue + // a footer redraw to the real terminal here. + _, _ = w.Write([]byte("\x1b[2J")) + + output := buf.String() + + // Footer redraw goes through redrawFooterLocked which writes ESC[24;1H. + // In compositor mode that path is skipped. + if strings.Contains(output, "\x1b[24;1H") { + t.Errorf("expected NO footer redraw in compositor mode, got %q", output) + } + + w.Cleanup() +} + func TestWriter_PassthroughANSI(t *testing.T) { var buf bytes.Buffer bar := NewStatusBar("run_abc123", "my-agent", "docker")