Skip to content

[Bugfix #630] Fix terminal scroll-to-top from split escapes and WebGL context loss - #631

Merged
waleedkadous merged 5 commits into
mainfrom
builder/bugfix-630-bug-terminal-scroll-to-top-cau
Mar 19, 2026
Merged

[Bugfix #630] Fix terminal scroll-to-top from split escapes and WebGL context loss#631
waleedkadous merged 5 commits into
mainfrom
builder/bugfix-630-bug-terminal-scroll-to-top-cau

Conversation

@waleedkadous

@waleedkadous waleedkadous commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #630

Root Cause

Two distinct triggers cause the terminal to scroll to the top:

  1. Split escape sequences: PTY output containing escape sequences (e.g., \x1b[31m for red text) can be split across WebSocket frames. When xterm.js receives an incomplete escape sequence, it enters escape state and hits a parsing error on the next non-escape character, which can reset its internal buffer state and cause viewportY to jump to 0. This also causes filterDA to fail on split DA responses, leaving dangling ESC characters.

  2. WebGL context loss: When the browser's GPU context is lost (resource pressure, tab switching, GPU driver reset), xterm's WebGL renderer loses state. During the dispose/fallback transition to canvas renderer, the viewport resets to 0.

Fix

Two proactive fixes that prevent the root causes:

  1. EscapeBuffer (dashboard/src/lib/escapeBuffer.ts): Buffers incoming PTY data and holds back trailing incomplete escape sequences. When the next chunk arrives, the pending data is prepended, ensuring xterm always receives complete escape sequences. Also fixes filterDA failing to match DA responses split across frames. Buffer is flushed on reconnect to prevent stale bytes from contaminating new streams.

  2. WebGL context loss scroll preservation (dashboard/src/components/Terminal.tsx): The onContextLoss handler now saves the viewport position before disposing the WebGL addon and restores it after loading the canvas fallback.

Test Plan

  • Added 22 regression tests for EscapeBuffer (split CSI, OSC, DCS sequences, box-drawing chars, reconnect flush)
  • Added 1 regression test for ScrollController scroll-to-top warning (diagnostic)
  • All 67 dashboard unit tests pass
  • All 2247 main unit tests pass
  • Build succeeds
  • Porch checks pass

CMAP Review

Model Verdict Key Finding
Gemini REQUEST_CHANGES Auto-correction in ScrollController prevents manual scroll-to-top — fixed: removed correction, kept diagnostic warning
Codex REQUEST_CHANGES EscapeBuffer not flushed on reconnect — fixed: added escBuf.flush() in connect reset
Claude APPROVE (HIGH) "Well-crafted three-layer defense with thorough tests and clean code"

All REQUEST_CHANGES addressed. Final architecture: prevent root causes upstream (EscapeBuffer + WebGL handler), warn diagnostically on unexpected scroll-to-top without interfering with user scrolling.

…equences and WebGL context loss

Three fixes for scroll-to-top:
1. EscapeBuffer: buffers incomplete escape sequences split across WebSocket
   frames, preventing xterm parsing errors that reset viewport
2. ScrollController: auto-corrects unexpected scroll-to-top by restoring
   previous position instead of just warning
3. WebGL context loss: saves/restores scroll position during renderer
   transition from WebGL to canvas fallback
…anual scroll (CMAP feedback)

Auto-correction can't distinguish parsing-error resets from intentional
user scroll-to-top. Root causes are already prevented upstream by
EscapeBuffer and WebGL context loss handler. Keep diagnostic warning.
@waleedkadous
waleedkadous merged commit 292144b into main Mar 19, 2026
6 checks passed
@waleedkadous
waleedkadous deleted the builder/bugfix-630-bug-terminal-scroll-to-top-cau branch March 19, 2026 12:53
waleedkadous added a commit that referenced this pull request Mar 19, 2026
Restore auto-correction in ScrollController that was incorrectly removed
in #631. A >50 line jump to viewportY=0 is clearly an xterm reset, not
user scrolling. User scrolling generates incremental events.
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.

Bug: Terminal scroll-to-top caused by xterm parsing errors and WebGL context loss

1 participant