From ad9c7fa43353ab6fabe07ba44f36445afe7d6b6e Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Wed, 25 Feb 2026 14:44:01 -0800 Subject: [PATCH] [Bugfix #563] fix: Revert safeFit() condition to use only buffer baseY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #561 changed safeFit()'s early-return condition from `if (!baseY)` to `if (!baseY && !scrollState.baseY)`, adding a dependency on the externally- tracked scrollState.baseY. This was unnecessary because baseY (from term.buffer.active.baseY) reflects actual buffer scrollback and is NOT affected by display:none toggling — unlike viewportY which the external tracking was designed to protect. The stale scrollState.baseY could cause safeFit() to take the scroll- preserving path when the buffer had no scrollback (e.g., after a clear), leading to incorrect scroll restoration with stale position values. Fix: Revert to `if (!baseY)` while keeping scrollState.wasAtBottom and scrollState.viewportY for position values (the core Issue #560 fix). Adds regression test for the stale scrollState.baseY scenario. Fixes #563 --- .../__tests__/Terminal.fit-scroll.test.tsx | 32 +++++++++++++++++++ .../dashboard/src/components/Terminal.tsx | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/codev/dashboard/__tests__/Terminal.fit-scroll.test.tsx b/packages/codev/dashboard/__tests__/Terminal.fit-scroll.test.tsx index 30f2e305..fc442c0e 100644 --- a/packages/codev/dashboard/__tests__/Terminal.fit-scroll.test.tsx +++ b/packages/codev/dashboard/__tests__/Terminal.fit-scroll.test.tsx @@ -310,4 +310,36 @@ describe('Terminal fit() scroll position preservation (Issue #423, #560)', () => expect(mockTermInstance.scrollToBottom).not.toHaveBeenCalled(); expect(mockTermInstance.scrollToLine).not.toHaveBeenCalled(); }); + + it('takes simple fit path when buffer is cleared even if scrollState.baseY is stale (Bugfix #563)', () => { + // Regression test: when the terminal buffer is cleared (baseY=0) but + // scrollState.baseY retains a stale positive value from before the clear, + // safeFit() should take the simple path (just fit) — not the scroll- + // preserving path with potentially stale position values. + render(); + mockContainerRect(); + + // Step 1: Simulate terminal with scrollback — scrollState gets updated + simulateScroll(500, 200); + + // Step 2: Simulate buffer clear — baseY goes to 0, but scrollState + // retains old values because onScroll may not fire during clear + mockTermInstance.buffer.active.baseY = 0; + mockTermInstance.buffer.active.viewportY = 0; + // NOTE: we do NOT call simulateScroll here — the clear may not + // trigger onScroll, so scrollState.baseY stays at 500 (stale) + + mockFitInstance.fit.mockClear(); + mockTermInstance.scrollToBottom.mockClear(); + mockTermInstance.scrollToLine.mockClear(); + + // Trigger ResizeObserver → debouncedFit → safeFit + mockResizeObserverCallback?.(); + vi.advanceTimersByTime(150); + + // safeFit should take the simple path: just fit(), no scroll restoration + expect(mockFitInstance.fit).toHaveBeenCalled(); + expect(mockTermInstance.scrollToBottom).not.toHaveBeenCalled(); + expect(mockTermInstance.scrollToLine).not.toHaveBeenCalled(); + }); }); diff --git a/packages/codev/dashboard/src/components/Terminal.tsx b/packages/codev/dashboard/src/components/Terminal.tsx index 608ea745..cf73a187 100644 --- a/packages/codev/dashboard/src/components/Terminal.tsx +++ b/packages/codev/dashboard/src/components/Terminal.tsx @@ -386,7 +386,7 @@ export function Terminal({ wsPath, onFileOpen, persistent, toolbarExtra }: Termi if (!rect || rect.width === 0 || rect.height === 0) return; const baseY = term.buffer?.active?.baseY; - if (!baseY && !scrollState.baseY) { + if (!baseY) { fitAddon.fit(); return; }