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

packages/codev/dashboard/__tests__/escapeBuffer.test.ts

@@ -0,0 +1,205 @@
+/**
+ * Unit tests for EscapeBuffer — ensures escape sequences split across
+ * WebSocket frames are reassembled before writing to xterm (Issue #630).
+ */
+import { describe, it, expect } from 'vitest';
+import { EscapeBuffer } from '../src/lib/escapeBuffer.js';
+
+describe('EscapeBuffer', () => {
+  describe('plain text (no escape sequences)', () => {
+    it('passes through plain text unchanged', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('Hello, world!')).toBe('Hello, world!');
+    });
+
+    it('passes through empty string', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('')).toBe('');
+    });
+  });
+
+  describe('complete escape sequences', () => {
+    it('passes through complete CSI sequence', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('\x1b[31mHello')).toBe('\x1b[31mHello');
+    });
+
+    it('passes through multiple complete CSI sequences', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('\x1b[31mHello\x1b[0m')).toBe('\x1b[31mHello\x1b[0m');
+    });
+
+    it('passes through complete OSC sequence with BEL', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('\x1b]0;title\x07')).toBe('\x1b]0;title\x07');
+    });
+
+    it('passes through complete OSC sequence with ST', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('\x1b]0;title\x1b\\')).toBe('\x1b]0;title\x1b\\');
+    });
+
+    it('passes through two-byte escape sequences (ESC =, ESC >)', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.write('\x1b=Hello')).toBe('\x1b=Hello');
+      expect(buf.write('\x1b>World')).toBe('\x1b>World');
+    });
+  });
+
+  describe('split CSI sequences', () => {
+    it('buffers trailing ESC at end of chunk', () => {
+      const buf = new EscapeBuffer();
+      // Frame 1: text ending with bare ESC
+      const out1 = buf.write('Hello\x1b');
+      expect(out1).toBe('Hello');
+      expect(buf.hasPending).toBe(true);
+
+      // Frame 2: continuation completes the sequence
+      const out2 = buf.write('[31mWorld');
+      expect(out2).toBe('\x1b[31mWorld');
+      expect(buf.hasPending).toBe(false);
+    });
+
+    it('buffers incomplete CSI (ESC [) at end of chunk', () => {
+      const buf = new EscapeBuffer();
+      const out1 = buf.write('Hello\x1b[');
+      expect(out1).toBe('Hello');
+
+      const out2 = buf.write('31m');
+      expect(out2).toBe('\x1b[31m');
+    });
+
+    it('buffers incomplete CSI with partial parameters', () => {
+      const buf = new EscapeBuffer();
+      // Split mid-sequence: ESC [ 3 (missing final byte)
+      const out1 = buf.write('Hello\x1b[3');
+      expect(out1).toBe('Hello');
+
+      const out2 = buf.write('1mWorld');
+      expect(out2).toBe('\x1b[31mWorld');
+    });
+
+    it('handles DA response split across frames', () => {
+      const buf = new EscapeBuffer();
+      // DA response: ESC [ ? 6 c — split after ESC [ ? 6
+      const out1 = buf.write('Hello\x1b[?6');
+      expect(out1).toBe('Hello');
+
+      const out2 = buf.write('cWorld');
+      expect(out2).toBe('\x1b[?6cWorld');
+    });
+
+    it('handles multiple splits in sequence', () => {
+      const buf = new EscapeBuffer();
+      // First split
+      const out1 = buf.write('A\x1b');
+      expect(out1).toBe('A');
+
+      // Second chunk also has split
+      const out2 = buf.write('[31mB\x1b[3');
+      expect(out2).toBe('\x1b[31mB');
+
+      // Third chunk completes
+      const out3 = buf.write('2mC');
+      expect(out3).toBe('\x1b[32mC');
+    });
+  });
+
+  describe('split OSC sequences', () => {
+    it('buffers incomplete OSC', () => {
+      const buf = new EscapeBuffer();
+      const out1 = buf.write('Hello\x1b]0;tit');
+      expect(out1).toBe('Hello');
+
+      const out2 = buf.write('le\x07World');
+      expect(out2).toBe('\x1b]0;title\x07World');
+    });
+  });
+
+  describe('split DCS sequences', () => {
+    it('buffers incomplete DCS', () => {
+      const buf = new EscapeBuffer();
+      const out1 = buf.write('Hello\x1bPdata');
+      expect(out1).toBe('Hello');
+
+      const out2 = buf.write('more\x1b\\World');
+      expect(out2).toBe('\x1bPdatamore\x1b\\World');
+    });
+  });
+
+  describe('flush', () => {
+    it('returns empty string when no pending data', () => {
+      const buf = new EscapeBuffer();
+      expect(buf.flush()).toBe('');
+    });
+
+    it('returns and clears pending data', () => {
+      const buf = new EscapeBuffer();
+      buf.write('Hello\x1b');
+      expect(buf.flush()).toBe('\x1b');
+      expect(buf.hasPending).toBe(false);
+    });
+
+    it('prevents stale pending bytes from leaking into next stream (reconnect scenario)', () => {
+      const buf = new EscapeBuffer();
+      // Connection 1: data ends with incomplete escape
+      buf.write('Hello\x1b[3');
+      expect(buf.hasPending).toBe(true);
+
+      // Disconnect — flush discards stale bytes
+      buf.flush();
+      expect(buf.hasPending).toBe(false);
+
+      // Connection 2: fresh data should not be contaminated
+      const out = buf.write('World\x1b[31m!');
+      expect(out).toBe('World\x1b[31m!');
+    });
+  });
+
+  describe('edge cases', () => {
+    it('handles chunk that is just ESC', () => {
+      const buf = new EscapeBuffer();
+      const out = buf.write('\x1b');
+      expect(out).toBe('');
+      expect(buf.hasPending).toBe(true);
+    });
+
+    it('handles chunk that is just ESC [', () => {
+      const buf = new EscapeBuffer();
+      const out = buf.write('\x1b[');
+      expect(out).toBe('');
+      expect(buf.hasPending).toBe(true);
+    });
+
+    it('does not buffer complete sequence followed by text', () => {
+      const buf = new EscapeBuffer();
+      const out = buf.write('\x1b[31mHello');
+      expect(out).toBe('\x1b[31mHello');
+      expect(buf.hasPending).toBe(false);
+    });
+
+    it('only buffers the LAST incomplete sequence', () => {
+      const buf = new EscapeBuffer();
+      // Complete CSI + incomplete CSI at end
+      const out = buf.write('\x1b[31mHello\x1b[');
+      expect(out).toBe('\x1b[31mHello');
+      expect(buf.hasPending).toBe(true);
+    });
+
+    it('handles box-drawing characters (code 0x2500) that trigger issue #630', () => {
+      // Box-drawing ─ (U+2500) is what the bug report mentioned:
+      // "Code 9472 = 0x2500 (box-drawing ─), currentState: 4 = escape state"
+      // This happens when ─ appears after a split ESC sequence
+      const buf = new EscapeBuffer();
+      const boxChar = String.fromCharCode(0x2500); // ─
+
+      // ESC split before box-drawing char — without buffering, xterm would
+      // receive ESC then ─ in escape state, causing parsing error
+      const out1 = buf.write('test\x1b');
+      expect(out1).toBe('test');
+
+      const out2 = buf.write(`[1m${boxChar}${boxChar}${boxChar}`);
+      expect(out2).toBe(`\x1b[1m${boxChar}${boxChar}${boxChar}`);
+    });
+  });
+});

packages/codev/dashboard/__tests__/scrollController.test.ts

@@ -336,7 +336,7 @@ describe('ScrollController', () => {
       expect(ctrl.state.wasAtBottom).toBe(true);
     });
 
-    it('warns on unexpected scroll-to-top in interactive phase', () => {
+    it('warns on unexpected scroll-to-top but does not auto-correct (Issue #630)', () => {
       const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
       const { ctrl, term } = createController();
       ctrl.enterInteractive();
@@ -350,10 +350,17 @@ describe('ScrollController', () => {
       term.buffer.active.viewportY = 0;
       term._triggerScroll();
 
+      // Should warn but NOT auto-correct — viewportY=0 is also the normal
+      // state when a user intentionally scrolls to the top of history.
+      // Root causes are prevented upstream by EscapeBuffer and WebGL handler.
       expect(warnSpy).toHaveBeenCalledWith(
         expect.stringContaining('unexpected scroll-to-top'),
         expect.any(String),
       );
+      // State IS updated (no correction, user may be at top intentionally)
+      expect(ctrl.state.viewportY).toBe(0);
+      expect(term.scrollToLine).not.toHaveBeenCalled();
+      expect(term.scrollToBottom).not.toHaveBeenCalled();
       warnSpy.mockRestore();
     });
   });

packages/codev/dashboard/src/components/Terminal.tsx

@@ -11,6 +11,7 @@ import { useMediaQuery } from '../hooks/useMediaQuery.js';
 import { MOBILE_BREAKPOINT } from '../lib/constants.js';
 import { uploadPasteImage } from '../lib/api.js';
 import { ScrollController } from '../lib/scrollController.js';
+import { EscapeBuffer } from '../lib/escapeBuffer.js';
 
 /**
  * Floating controls overlay for terminal windows — refresh (re-fit + resize)
@@ -246,8 +247,21 @@ export function Terminal({ wsPath, onFileOpen, persistent, toolbarExtra }: Termi
     try {
       const webglAddon = new WebglAddon();
       webglAddon.onContextLoss(() => {
+        // Save scroll position before renderer transition (Issue #630).
+        // WebGL context loss resets xterm's viewport, causing scroll-to-top.
+        const savedViewportY = term.buffer?.active?.viewportY ?? 0;
+        const savedBaseY = term.buffer?.active?.baseY ?? 0;
+        const wasAtBottom = !savedBaseY || savedViewportY >= savedBaseY;
+
         webglAddon.dispose();
         loadCanvasFallback();
+
+        // Restore scroll position after switching to canvas renderer
+        if (wasAtBottom) {
+          term.scrollToBottom();
+        } else if (savedViewportY > 0) {
+          term.scrollToLine(savedViewportY);
+        }
       });
       term.loadAddon(webglAddon);
     } catch {
@@ -375,19 +389,26 @@ export function Terminal({ wsPath, onFileOpen, persistent, toolbarExtra }: Termi
       return text;
     };
 
+    // Buffer incomplete escape sequences to prevent xterm parsing errors
+    // from split WebSocket frames causing scroll-to-top (Issue #630).
+    const escBuf = new EscapeBuffer();
+
     /** Create a WebSocket connection, optionally resuming from a sequence number. */
     const connect = (resumeSeq?: number) => {
       const wsUrl = resumeSeq !== undefined ? `${wsBase}?resume=${resumeSeq}` : wsBase;
       const ws = new WebSocket(wsUrl);
       ws.binaryType = 'arraybuffer';
       wsRef.current = ws;
 
-      // Reset DA filter state and scroll controller for this connection.
-      // On reconnection, the controller needs to return to initial-load so
-      // beginReplay()/endReplay() can properly suppress fit during replay.
+      // Reset DA filter state, escape buffer, and scroll controller for this
+      // connection. On reconnection, the controller needs to return to
+      // initial-load so beginReplay()/endReplay() can properly suppress fit
+      // during replay. Flush escape buffer to discard stale pending bytes
+      // from the previous connection (CMAP feedback, Issue #630).
       rc.initialPhase = true;
       rc.initialBuffer = '';
       if (rc.flushTimer) { clearTimeout(rc.flushTimer); rc.flushTimer = null; }
+      escBuf.flush();
       scrollCtrl.reset();
 
       const flushInitialBuffer = () => {
@@ -457,20 +478,12 @@ export function Terminal({ wsPath, onFileOpen, persistent, toolbarExtra }: Termi
               rc.flushTimer = setTimeout(flushInitialBuffer, 500);
             }
           } else {
-            const filtered = filterDA(text);
-            if (filtered) {
-              // Capture scroll position before write — term.write() can cause
-              // buffer reflow that resets viewportY to 0 without firing onScroll.
-              const preWriteViewportY = term.buffer?.active?.viewportY ?? 0;
-              const preWriteBaseY = term.buffer?.active?.baseY ?? 0;
-              term.write(filtered, () => {
-                const postViewportY = term.buffer?.active?.viewportY ?? 0;
-                if (postViewportY === 0 && preWriteViewportY > 0 && preWriteBaseY > 0) {
-                  console.warn('[Terminal] write caused scroll-to-top, correcting:',
-                    `was viewportY=${preWriteViewportY}, baseY=${preWriteBaseY}`);
-                  scrollCtrl.scrollToBottom();
-                }
-              });
+            // Buffer through EscapeBuffer first (ensures complete escape
+            // sequences), then strip DA responses before writing to xterm.
+            const complete = escBuf.write(text);
+            if (complete) {
+              const filtered = filterDA(complete);
+              if (filtered) term.write(filtered);
             }
           }
         } else if (prefix === FRAME_CONTROL) {