Skip to content

fix: clamp cursor-up to viewport height, preventing terminal scroll-to-top - #917

Open
MagnetonIO wants to merge 2 commits into
vadimdemedes:masterfrom
MagnetonIO:fix/clamp-cursor-up-to-viewport
Open

fix: clamp cursor-up to viewport height, preventing terminal scroll-to-top#917
MagnetonIO wants to merge 2 commits into
vadimdemedes:masterfrom
MagnetonIO:fix/clamp-cursor-up-to-viewport

Conversation

@MagnetonIO

Copy link
Copy Markdown

Summary

  • Clamps all eraseLines() and cursorUp() calls in log-update.ts to stream.rows (viewport height)
  • Prevents ANSI cursor-up sequences from moving the cursor above the visible viewport into terminal scrollback
  • Falls back to Infinity (no clamping) for non-TTY streams
  • Adds 6 new tests verifying viewport clamping in both standard and incremental rendering modes

Root Cause

When Ink re-renders content that exceeds the terminal viewport, eraseLines(previousLineCount) and cursorUp(previousLines.length - 1) emit cursor-up escape sequences (\e[NA) where N can far exceed the viewport height. Terminals follow the cursor above the visible area and snap to the top of scrollback history.

This affects all terminals — iTerm2, VS Code, tmux, Windows Terminal, kitty, GNOME Terminal, and xterm.js embeddings.

The Fix

Two small helpers read stream.rows from TTY streams and clamp line counts:

const getViewportRows = (stream: Writable): number =>
    (stream as NodeJS.WriteStream).rows || Infinity;

const clampToViewport = (lineCount: number, stream: Writable): number =>
    Math.min(lineCount, getViewportRows(stream));

Applied to all 6 eraseLines/cursorUp call sites in both createStandard and createIncremental. The clamp is semantically correct: lines above the viewport have already scrolled into scrollback and cannot be erased or navigated to.

Tests

  • All 944 existing tests pass with zero regressions
  • 6 new tests verify clamping behavior:
    • Standard/incremental: eraseLines clamped when content exceeds viewport
    • Standard/incremental: cursorUp clamped when content exceeds viewport
    • Standard/incremental: no clamping when content fits within viewport

Downstream Impact

This is the root cause behind a wave of scroll-to-top bugs in tools built on Ink, most notably Claude Code:

Also affects Gemini CLI and any Ink-based TUI that renders content taller than the viewport.

🤖 Generated with Claude Code

…o-top

When rendered content exceeds the terminal viewport, eraseLines() and
cursorUp() emit ANSI cursor-up sequences that move the cursor above
the visible area into scrollback. This causes terminals to snap the
viewport to the top of scrollback history — a widespread bug affecting
iTerm2, VS Code, tmux, Windows Terminal, kitty, and others.

The fix reads stream.rows (available on TTY streams) and clamps all
cursor-up operations to the viewport height. Lines beyond the viewport
have already scrolled into terminal scrollback and cannot be erased,
so the clamp is semantically correct. Non-TTY streams fall back to
Infinity (no clamping).

Fixes the root cause behind:
- anthropics/claude-code#34845
- anthropics/claude-code#33814
- anthropics/claude-code#826
- anthropics/claude-code#36582

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@MorningLightMountain713

Copy link
Copy Markdown

Send it

@sindresorhus

Copy link
Copy Markdown
Collaborator

I think this is still incomplete.

buildCursorSuffix() still emits an unclamped cursorUp(visibleLineCount - cursorPosition.y). I manually checked the branch with a 5-row fake TTY and 20 lines of output, and both standard and incremental rendering still write \x1b[20A as soon as a cursor position is set. So anything using useCursor can still trigger the same scroll-to-top behavior. The new tests only cover the erase/rewrite paths, so this sneaks through.

Also, CI is failing.

@OutThisLife

Copy link
Copy Markdown

Need this asap

Address review feedback: buildCursorSuffix() was still emitting unclamped
cursorUp sequences when useCursor positioned the cursor far from the
bottom of tall output. Add viewportHeight parameter to buildCursorSuffix
and buildCursorOnlySequence, and pass stream.rows from all call sites.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants