Smart scroll for the agent terminal

src/renderer/components/terminal/smartScroll.ts

@@ -0,0 +1,166 @@
+/**
+ * Smart scroll: keep the viewport stuck to the bottom while the agent streams
+ * output, but let the user scroll up to read past content without being yanked
+ * back. xterm's default is to follow the cursor on writes, which is what
+ * yanks them back — we counter that by snapshotting the user's distance from
+ * the bottom before each write and restoring it after the parser finishes.
+ */
+
+import type { Terminal } from '@xterm/xterm'
+
+export interface BufferState {
+  /** Buffer line index of the top of the viewport. */
+  viewportY: number
+  /** Buffer line index of the top of the bottom-most page (i.e. fully scrolled down). */
+  baseY: number
+}
+
+/** Lines of slack near the bottom that still count as "anchored". */
+export const ANCHOR_THRESHOLD = 3
+
+export function isAnchoredToBottom(buf: BufferState, threshold = ANCHOR_THRESHOLD): boolean {
+  return buf.viewportY >= buf.baseY - threshold
+}
+
+export function linesFromBottom(buf: BufferState): number {
+  return Math.max(0, buf.baseY - buf.viewportY)
+}
+
+export type ScrollAction =
+  | { type: 'none' }
+  | { type: 'scrollToBottom' }
+  | { type: 'scrollLines'; delta: number }
+
+export function computeScrollAction(args: {
+  wasAnchored: boolean
+  savedLinesFromBottom: number
+  buf: BufferState
+  visible: boolean
+}): ScrollAction {
+  const { wasAnchored, savedLinesFromBottom, buf, visible } = args
+
+  if (wasAnchored) {
+    return visible ? { type: 'scrollToBottom' } : { type: 'none' }
+  }
+
+  const targetY = Math.max(0, buf.baseY - savedLinesFromBottom)
+  const delta = targetY - buf.viewportY
+  return delta === 0 ? { type: 'none' } : { type: 'scrollLines', delta }
+}
+
+/** Pixel slack near the bottom that still counts as "anchored" in DOM space. */
+export const DOM_ANCHOR_TOLERANCE_PX = 8
+
+export interface DomScrollState {
+  scrollTop: number
+  scrollHeight: number
+  clientHeight: number
+}
+
+export function isDomAnchored(s: DomScrollState, tolerance = DOM_ANCHOR_TOLERANCE_PX): boolean {
+  return s.scrollTop + s.clientHeight >= s.scrollHeight - tolerance
+}
+
+export function distanceFromBottom(s: DomScrollState): number {
+  return Math.max(0, s.scrollHeight - s.clientHeight - s.scrollTop)
+}
+
+export type DomScrollAction =
+  | { type: 'none' }
+  | { type: 'scrollToBottom' }
+  | { type: 'setScrollTop'; scrollTop: number }
+
+/**
+ * Decide what to do with the DOM viewport after a write completes. Pure so we
+ * can unit-test exhaustively without a real Terminal.
+ */
+export function computeDomScrollAction(args: {
+  wasAnchored: boolean
+  savedDistanceFromBottom: number
+  state: DomScrollState
+  visible: boolean
+}): DomScrollAction {
+  const { wasAnchored, savedDistanceFromBottom, state, visible } = args
+
+  if (wasAnchored) {
+    return visible ? { type: 'scrollToBottom' } : { type: 'none' }
+  }
+
+  const target = Math.max(0, state.scrollHeight - state.clientHeight - savedDistanceFromBottom)
+  // No-op if we are already there (avoid spurious scroll events).
+  if (Math.abs(state.scrollTop - target) < 1) return { type: 'none' }
+  return { type: 'setScrollTop', scrollTop: target }
+}
+
+export interface SmartScrollController {
+  /** Write `data` to the terminal, preserving the user's scroll position. */
+  write: (data: string) => void
+  /** True when the viewport is anchored to the bottom. */
+  isAnchored: () => boolean
+  /** Tear down listeners. */
+  dispose: () => void
+}
+
+/**
+ * Attach smart-scroll behaviour to an xterm Terminal.
+ *
+ * Detection is done against the live DOM scroll position of `.xterm-viewport`
+ * — that is the only source of truth that updates synchronously with user
+ * input (wheel, scrollbar drag, touch, keyboard). xterm's `onScroll` event is
+ * driven off the *buffer*'s viewportY which is updated asynchronously from
+ * the DOM scroll, so we cannot rely on it during a stream of writes.
+ */
+export function attachSmartScroll(
+  term: Terminal,
+  getVisible: () => boolean,
+): SmartScrollController {
+  function getViewport(): HTMLElement | null {
+    return term.element?.querySelector<HTMLElement>('.xterm-viewport') ?? null
+  }
+
+  function readState(): DomScrollState | null {
+    const v = getViewport()
+    if (!v) return null
+    return { scrollTop: v.scrollTop, scrollHeight: v.scrollHeight, clientHeight: v.clientHeight }
+  }
+
+  function isAnchored(): boolean {
+    const s = readState()
+    return s ? isDomAnchored(s) : true
+  }
+
+  function write(data: string): void {
+    const stateBefore = readState()
+    const wasAnchored = stateBefore ? isDomAnchored(stateBefore) : true
+    const savedDistanceFromBottom = stateBefore ? distanceFromBottom(stateBefore) : 0
+
+    term.write(data, () => {
+      const v = getViewport()
+      const stateAfter = readState()
+      if (!v || !stateAfter) return
+
+      const action = computeDomScrollAction({
+        wasAnchored,
+        savedDistanceFromBottom,
+        state: stateAfter,
+        visible: getVisible(),
+      })
+      if (action.type === 'scrollToBottom') {
+        term.scrollToBottom()
+        // term.scrollToBottom updates the buffer; force the DOM scrollbar to
+        // match in case xterm hasn't repainted yet.
+        v.scrollTop = stateAfter.scrollHeight
+      } else if (action.type === 'setScrollTop') {
+        v.scrollTop = action.scrollTop
+      }
+    })
+  }
+
+  return {
+    write,
+    isAnchored,
+    dispose: () => {
+      // No persistent listeners; reserved for future use.
+    },
+  }
+}

src/renderer/components/terminal/useTerminal.ts

@@ -8,6 +8,7 @@ import { useTerminalStore } from '../../stores/terminalStore'
 import { useSettingsStore } from '../../stores/settingsStore'
 import { useSessionStore } from '../../stores/sessionStore'
 import { THEMES } from '../../../shared/themes'
+import { attachSmartScroll, type SmartScrollController } from './smartScroll'
 
 interface UseTerminalOptions {
   terminalId: string | null
@@ -19,7 +20,7 @@ interface UseTerminalOptions {
 // Global registry — keeps xterm instances alive for the lifetime of the app
 const terminalInstances = new Map<
   string,
-  { term: Terminal; fitAddon: FitAddon; attached: boolean; visible: boolean; anchoredToBottom: boolean; unsubData: (() => void) | null; unsubExit: (() => void) | null; cleanupWheel: (() => void) | null }
+  { term: Terminal; fitAddon: FitAddon; attached: boolean; visible: boolean; smartScroll: SmartScrollController; unsubData: (() => void) | null; unsubExit: (() => void) | null }
 >()
 
 /** Force the xterm viewport scrollbar to sync with the buffer position */
@@ -40,7 +41,7 @@ export function destroyTerminal(terminalId: string): void {
   if (!instance) return
   instance.unsubData?.()
   instance.unsubExit?.()
-  instance.cleanupWheel?.()
+  instance.smartScroll.dispose()
   instance.term.dispose()
   terminalInstances.delete(terminalId)
 }
@@ -112,44 +113,16 @@ export function useTerminal({ terminalId, sessionId, sessionName, visible = true
       }
     })
 
-    // Track user-initiated scrolls only (wheel/keyboard), not programmatic ones
-    // anchoredToBottom is stored on the instance object so the visibility effect can read it
-    let userScrolling = false
-
-    const el = containerRef.current!
-    const wheelHandler = () => {
-      userScrolling = true
-      requestAnimationFrame(() => {
-        const buf = term.buffer.active
-        const inst = terminalInstances.get(terminalId)
-        if (inst) inst.anchoredToBottom = buf.viewportY >= buf.baseY - 3
-        userScrolling = false
-      })
-    }
-    el.addEventListener('wheel', wheelHandler)
-
-    // Also catch keyboard scrolling (Shift+PageUp/Down etc)
-    term.onKey(({ domEvent }) => {
-      if (domEvent.key === 'PageUp' || domEvent.key === 'PageDown' ||
-          (domEvent.shiftKey && (domEvent.key === 'ArrowUp' || domEvent.key === 'ArrowDown'))) {
-        requestAnimationFrame(() => {
-          const buf = term.buffer.active
-          const inst = terminalInstances.get(terminalId)
-          if (inst) inst.anchoredToBottom = buf.viewportY >= buf.baseY - 3
-        })
-      }
+    // Smart scroll: see smartScroll.ts. The controller intercepts writes to
+    // preserve the user's scroll position when they have scrolled up.
+    const smartScroll = attachSmartScroll(term, () => {
+      return terminalInstances.get(terminalId)?.visible ?? false
     })
 
     // Receive data from pty — always active, even when hidden
     const unsubData = window.api.terminal.onData((id, data) => {
       if (id !== terminalId) return
-      term.write(data)
-
-      // Only scroll while visible — scrollToBottom on a hidden element desyncs the scrollbar
-      const inst = terminalInstances.get(terminalId)
-      if (inst?.visible && inst.anchoredToBottom && !userScrolling) {
-        term.scrollToBottom()
-      }
+      smartScroll.write(data)
 
       // Intervention detection
       lineBuffer.current += data
@@ -173,8 +146,7 @@ export function useTerminal({ terminalId, sessionId, sessionName, visible = true
       term.writeln(`\r\n[Process exited with code ${code}]`)
     })
 
-    const cleanupWheel = () => el.removeEventListener('wheel', wheelHandler)
-    terminalInstances.set(terminalId, { term, fitAddon, attached: true, visible: true, anchoredToBottom: true, unsubData, unsubExit, cleanupWheel })
+    terminalInstances.set(terminalId, { term, fitAddon, attached: true, visible: true, smartScroll, unsubData, unsubExit })
 
     // Initial fit + scroll to bottom
     requestAnimationFrame(() => {
@@ -213,7 +185,7 @@ export function useTerminal({ terminalId, sessionId, sessionName, visible = true
       const { cols, rows } = instance.term
       window.api.terminal.resize(terminalId, cols, rows)
 
-      if (instance.anchoredToBottom) {
+      if (instance.smartScroll.isAnchored()) {
         instance.term.scrollToBottom()
         // Force the viewport scrollbar to match — scrollToBottom alone can desync
         // when the terminal received writes while hidden

tests/e2e/smart-scroll.spec.ts

@@ -0,0 +1,96 @@
+import { test, expect, type Page } from '@playwright/test'
+
+/**
+ * E2E coverage for the smart-scroll behaviour in the agent terminal.
+ *
+ * Bug: while a session's pty streams output, the xterm viewport followed the
+ * cursor on every write — so a user who scrolled up to read past content was
+ * yanked back to the bottom on the next chunk.
+ *
+ * Fix: see src/renderer/components/terminal/smartScroll.ts. This test drives
+ * the actual app against the mock backend (which streams ~50 lines over a few
+ * seconds) and asserts the user-set scroll position survives subsequent
+ * writes.
+ */
+
+async function bootApp(page: Page) {
+  await page.goto('/', { waitUntil: 'domcontentloaded' })
+  await page.evaluate(() => localStorage.clear())
+  await page.goto('/', { waitUntil: 'domcontentloaded' })
+  await expect(page.getByText('CodeCrucible').first()).toBeVisible({ timeout: 10_000 })
+}
+
+async function openTerminalSession(page: Page) {
+  // Activating a session card auto-spawns a terminal (TerminalPanel useEffect).
+  const card = page.getByText('fix-terminal-resize').first()
+  await card.click()
+}
+
+async function viewport(page: Page) {
+  return page.locator('.xterm-viewport').first()
+}
+
+test.describe('Agent terminal — smart scroll', () => {
+  test.beforeEach(async ({ page }) => bootApp(page))
+
+  test('scrolling up while data streams keeps the user at their position', async ({ page }) => {
+    await openTerminalSession(page)
+    const vp = await viewport(page)
+    await expect(vp).toBeVisible({ timeout: 10_000 })
+
+    // Wait until the viewport has accumulated enough content to be scrollable.
+    await expect
+      .poll(async () => vp.evaluate((el: HTMLElement) => el.scrollHeight - el.clientHeight), {
+        timeout: 5_000,
+      })
+      .toBeGreaterThan(20)
+
+    // Scroll up to roughly the top while the mock is still streaming.
+    await vp.evaluate((el: HTMLElement) => {
+      el.scrollTop = 0
+    })
+    const scrollTopAfterUserScroll = await vp.evaluate((el: HTMLElement) => el.scrollTop)
+    expect(scrollTopAfterUserScroll).toBe(0)
+
+    // Let more data stream in. The mock pushes one chunk every 80ms for ~50
+    // chunks; 1.5s gives us plenty of writes after we scrolled.
+    await page.waitForTimeout(1500)
+
+    // Without smart-scroll, scrollTop would have been forced back near
+    // (scrollHeight - clientHeight). With it, we should still be near the top.
+    const finalScrollTop = await vp.evaluate((el: HTMLElement) => el.scrollTop)
+    const finalDistanceFromBottom = await vp.evaluate(
+      (el: HTMLElement) => el.scrollHeight - el.clientHeight - el.scrollTop,
+    )
+    expect(finalScrollTop).toBeLessThan(20)
+    expect(finalDistanceFromBottom).toBeGreaterThan(20)
+  })
+
+  test('returning to the bottom re-engages auto-follow', async ({ page }) => {
+    await openTerminalSession(page)
+    const vp = await viewport(page)
+    await expect(vp).toBeVisible({ timeout: 10_000 })
+
+    await expect
+      .poll(async () => vp.evaluate((el: HTMLElement) => el.scrollHeight - el.clientHeight), {
+        timeout: 5_000,
+      })
+      .toBeGreaterThan(20)
+
+    // Scroll up, then back to the bottom.
+    await vp.evaluate((el: HTMLElement) => {
+      el.scrollTop = 0
+    })
+    await page.waitForTimeout(200)
+    await vp.evaluate((el: HTMLElement) => {
+      el.scrollTop = el.scrollHeight
+    })
+
+    // After streaming continues, we should remain anchored at the bottom.
+    await page.waitForTimeout(1000)
+    const distanceFromBottom = await vp.evaluate(
+      (el: HTMLElement) => el.scrollHeight - el.clientHeight - el.scrollTop,
+    )
+    expect(distanceFromBottom).toBeLessThan(10)
+  })
+})