Merge pull request #228 from SoftwareSavants/overloaded-lockfile-587

fix(terminal): repaint running PTY terminals on dark/light mode flip

Author: iabdousd

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Running PTY terminals now repaint when you flip dark ↔ light mode. xterm v6's WebGL renderer caches its texture atlas, glyph cache, and rectangle-batch buffers per renderer instance, so setting `term.options.theme` updated the option object but the canvas kept painting the palette baked in at construction — old terminals stayed dark in light mode and new terminals stayed light when switching back. The appearance subscriber now disposes the WebGL addon and reloads a fresh instance after writing the new theme, so xterm runs `setRenderer → _fullRefresh` and every visible cell repaints in the new colors. DOM-renderer callers (BtsLogPane, the inert `Terminal.tsx` log view) were unaffected and stay on the simpler subscribe path.
+- Terminal panel chrome now flips with the theme: the outer container was baking in `bg-[#0a0a0a]` so the empty state and any gaps around the xterm canvas stayed black even after switching to light mode. Replaced with `bg-surface-0` so it tracks the active theme like the rest of the panel.
 - Side question (`/btw`) for Claude sessions: ask an ephemeral question without polluting the transcript. Type `/btw <question>` in the composer (auto-completes from the slash-command palette), or - while the agent is streaming and you've scrolled to the bottom of the chat - click the small "Ask sideways /btw" pill that takes the place of the scroll-to-bottom button (same screen real estate, swapped by context). Q/A are not persisted to `output_lines` and never re-enter conversation history on resume; `synthetic` fallbacks are badged distinctly. Wires through a new `ask_side_question` Tauri command on top of the existing control-request plumbing.
 - Mid-loop API errors (e.g. ConnectionRefused after tools already ran) now surface a `Continue` button instead of `Retry`. It sends `continue` to the resumed session so Claude picks up from the persisted tool results instead of redoing the whole turn from the original prompt. `Retry in new session` is hidden in this case since it would discard the in-flight progress. Fresh-failure turns (no assistant content yet) keep the original Retry / Retry-in-new-session pair
 - Cmd+Alt+Left/Right now cycles between sessions in the current task when viewing a session (wraps at edges). The shortcut still cycles editor tabs when a file is open, mirroring Ctrl+Tab's view-aware behavior

src/components/ShellTerminal.tsx

@@ -108,6 +108,22 @@ function initialFit(entry: XtermEntry, terminalId: string) {
     .catch(() => {})
 }
 
+/**
+ * Replace a terminal's WebGL renderer with a fresh instance carrying the
+ * current theme. Setting `term.options.theme = newTheme` fires xterm's
+ * `onChangeColors`, but xterm v6's WebglRenderer keeps its texture atlas,
+ * cell model, and rectangle-batch buffers across that event — visible cells
+ * keep painting with the colors baked in at construction time. The canonical
+ * fix used by VS Code is to swap the renderer entirely; doing that via the
+ * WebglAddon's lifecycle gives us a clean `setRenderer(...)` → `_fullRefresh()`
+ * sequence that paints the whole grid in the new palette.
+ */
+function reloadWebglAddon(entry: import('../store/terminals').XtermEntry): void {
+  if (!entry.webglFactory) return
+  entry.webglAddon?.dispose()
+  entry.webglAddon = entry.webglFactory()
+}
+
 /**
  * Force-redraw triggers that fix WebGL texture-atlas drift without relying on
  * the user resizing the window. VS Code (`xtermTerminal.ts: forceRedraw()`)
@@ -245,10 +261,14 @@ export const ShellTerminal: Component<Props> = (props) => {
         existing.term.focus()
       })
       resizeObserver = attachResizeObserver(terminalRef, existing, props.terminalId)
-      unsubAppearance = subscribeXtermToAppearance(existing.term, () => {
-        existing.fitAddon.fit()
-        existing.term.clearTextureAtlas()
-      })
+      unsubAppearance = subscribeXtermToAppearance(
+        existing.term,
+        () => {
+          existing.fitAddon.fit()
+          existing.term.clearTextureAtlas()
+        },
+        () => reloadWebglAddon(existing),
+      )
       unsubLifecycle = attachAtlasLifecycle(existing.term)
       return
     }
@@ -301,27 +321,43 @@ export const ShellTerminal: Component<Props> = (props) => {
       term.write(replay.data)
       markSeqWritten(props.terminalId, replay.seq)
     }
-    registerXterm(props.terminalId, term, fitAddon, searchAddon)
-
-    try {
-      const webgl = new WebglAddon()
-      // VS Code pattern: dispose on context loss so xterm falls back to the
-      // DOM renderer instead of leaving a dead GL context behind. Critical
-      // on macOS WebKit (Tauri) where the OS occasionally drops contexts on
-      // sleep / Mission Control / display switches.
-      webgl.onContextLoss(() => webgl.dispose())
-      term.loadAddon(webgl)
-    } catch {
-      // WebGL not available — xterm auto-falls back to the DOM renderer.
+    // Factored so the appearance subscriber can re-create a fresh WebGL
+    // renderer instance when the theme flips. xterm v6's WebglRenderer caches
+    // its texture atlas + glyph state across `term.options.theme = ...`
+    // assignments, so without a swap, running terminals keep painting the
+    // colors they were started with.
+    const makeWebgl = (): WebglAddon | undefined => {
+      try {
+        const webgl = new WebglAddon()
+        // VS Code pattern: dispose on context loss so xterm falls back to the
+        // DOM renderer instead of leaving a dead GL context behind. Critical
+        // on macOS WebKit (Tauri) where the OS occasionally drops contexts on
+        // sleep / Mission Control / display switches.
+        webgl.onContextLoss(() => webgl.dispose())
+        term.loadAddon(webgl)
+        return webgl
+      } catch {
+        // WebGL not available — xterm auto-falls back to the DOM renderer.
+        return undefined
+      }
     }
+    const initialWebgl = makeWebgl()
 
-    const entry = { term, fitAddon, searchAddon }
+    const entry: import('../store/terminals').XtermEntry = {
+      term, fitAddon, searchAddon, webglAddon: initialWebgl, webglFactory: makeWebgl,
+    }
+    // Re-register so the registry sees the WebGL handle alongside the term.
+    registerXterm(props.terminalId, term, fitAddon, searchAddon, initialWebgl, makeWebgl)
     initialFit(entry, props.terminalId)
     resizeObserver = attachResizeObserver(terminalRef, entry, props.terminalId)
-    unsubAppearance = subscribeXtermToAppearance(term, () => {
-      fitAddon.fit()
-      term.clearTextureAtlas()
-    })
+    unsubAppearance = subscribeXtermToAppearance(
+      term,
+      () => {
+        fitAddon.fit()
+        term.clearTextureAtlas()
+      },
+      () => reloadWebglAddon(entry),
+    )
     unsubLifecycle = attachAtlasLifecycle(term)
   })
 

src/components/TerminalPanel.test.tsx

@@ -0,0 +1,59 @@
+import { describe, test, expect, vi, beforeEach } from 'vitest'
+import { render, cleanup } from '@solidjs/testing-library'
+
+// Stub the terminals store: TerminalPanel reads from it but for this test we
+// only care about the chrome around the empty state, so an empty list suffices.
+const terminalsMocks = vi.hoisted(() => ({
+  terminalsForTask: vi.fn(() => []),
+  activeTerminalId: vi.fn(() => undefined),
+  setActiveTerminalForTask: vi.fn(),
+  spawnTerminal: vi.fn(() => Promise.resolve()),
+  closeTerminal: vi.fn(() => Promise.resolve()),
+  focusActiveTerminal: vi.fn(),
+  terminalExitCodes: vi.fn(() => ({})),
+  isTerminalStopped: vi.fn(() => false),
+  spawnStartCommand: vi.fn(() => Promise.resolve()),
+  // Treat the task as hydrated but with no terminals so the createEffect that
+  // would auto-spawn one is gated by the empty terminals + spawning guard.
+  isTaskHydrated: vi.fn(() => false),
+}))
+vi.mock('../store/terminals', () => terminalsMocks)
+
+vi.mock('../store/setup', () => ({ isSetupRunning: vi.fn(() => false) }))
+
+vi.mock('../lib/ipc', () => ({
+  stopHook: vi.fn(() => Promise.resolve()),
+  runHook: vi.fn(() => Promise.resolve()),
+  ptyClose: vi.fn(() => Promise.resolve()),
+}))
+
+// xterm needs a real DOM and WebGL — neither of which jsdom supplies. Stub the
+// terminal so the panel can mount and we can inspect its outer chrome.
+vi.mock('./ShellTerminal', () => ({
+  ShellTerminal: () => {
+    const el = document.createElement('div')
+    el.setAttribute('data-testid', 'shell-terminal')
+    return el
+  },
+}))
+
+import { TerminalPanel } from './TerminalPanel'
+
+beforeEach(() => {
+  cleanup()
+})
+
+describe('TerminalPanel chrome adapts to the theme', () => {
+  // Regression: the panel container baked in `bg-[#0a0a0a]`, leaving the empty
+  // state and any gaps around the xterm canvas stuck on a black background
+  // even after the user switched to light mode. Surface tokens flip with the
+  // theme; raw hex does not.
+  test('outer container uses a surface token, not a hardcoded dark hex', () => {
+    const { container } = render(() => <TerminalPanel taskId="t-1" />)
+    const outer = container.firstElementChild as HTMLElement
+    expect(outer).toBeTruthy()
+    const cls = outer.className
+    expect(cls).not.toMatch(/bg-\[#0a0a0a\]/)
+    expect(cls).toMatch(/bg-surface-0\b/)
+  })
+})

src/components/TerminalPanel.tsx

@@ -78,7 +78,7 @@ export const TerminalPanel: Component<Props> = (props) => {
   })
 
   return (
-    <div class="flex flex-col h-full bg-[#0a0a0a]">
+    <div class="flex flex-col h-full bg-surface-0">
       {/* Tab bar */}
       <div class="flex items-center px-2 py-1.5 gap-1 bg-surface-1 border-b border-border-subtle overflow-x-auto shrink-0">
         <For each={sortedTerminals()}>

src/lib/terminalTheme.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
-import { subscribeXtermToAppearance } from './terminalTheme'
+import { getXtermTheme, subscribeXtermToAppearance } from './terminalTheme'
 import { applyAppearance, DEFAULT_PREFS } from './theme'
 
 // Minimal xterm stub - records option writes and refresh/clearTextureAtlas calls.
@@ -55,6 +55,70 @@ describe('subscribeXtermToAppearance', () => {
     unsub()
   })
 
+  test('theme background flips when mode switches between dark and light', () => {
+    const { term } = makeStubTerm()
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const unsub = subscribeXtermToAppearance(term as any)
+
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'dark' })
+    const darkTheme = (term.options as Record<string, unknown>).theme as { background: string }
+
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })
+    const lightTheme = (term.options as Record<string, unknown>).theme as { background: string }
+
+    expect(darkTheme.background).toBeDefined()
+    expect(lightTheme.background).toBeDefined()
+    // The whole point of switching modes: the visible terminal canvas color must change.
+    expect(lightTheme.background.toLowerCase()).not.toBe(darkTheme.background.toLowerCase())
+
+    unsub()
+  })
+
+  test('getXtermTheme reads the resolved CSS surface for the current mode', () => {
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'dark' })
+    const darkBg = getXtermTheme().background
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })
+    const lightBg = getXtermTheme().background
+    expect(darkBg).toBeDefined()
+    expect(lightBg).toBeDefined()
+    expect(lightBg!.toLowerCase()).not.toBe(darkBg!.toLowerCase())
+  })
+
+  // Repro for the user-reported bug: terminals started in dark mode kept
+  // painting their original palette after switching to light. xterm v6's
+  // WebGL renderer caches its atlas + cell model across `term.options.theme`
+  // assignments, so the canonical fix is to dispose-and-reload the renderer.
+  // The subscriber must call `reloadRenderer` between option writes and the
+  // post-fit refresh — and only after the new theme is on the term, so the
+  // fresh renderer activates against it.
+  test('invokes reloadRenderer after theme is set, before refresh', () => {
+    const { term, calls } = makeStubTerm()
+    const reloadRenderer = vi.fn(() => calls.push('reloadRenderer'))
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const unsub = subscribeXtermToAppearance(term as any, undefined, reloadRenderer)
+
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })
+
+    expect(reloadRenderer).toHaveBeenCalledTimes(1)
+    const themeIdx = calls.indexOf('set theme')
+    const reloadIdx = calls.indexOf('reloadRenderer')
+    const refreshIdx = calls.findIndex(c => c.startsWith('refresh('))
+    expect(themeIdx).toBeGreaterThanOrEqual(0)
+    expect(reloadIdx).toBeGreaterThan(themeIdx)
+    expect(refreshIdx).toBeGreaterThan(reloadIdx)
+
+    unsub()
+  })
+
+  test('reloadRenderer is optional — DOM-only callers still work', () => {
+    const { term } = makeStubTerm()
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const unsub = subscribeXtermToAppearance(term as any)
+    expect(() => applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })).not.toThrow()
+    expect(term.refresh).toHaveBeenCalled()
+    unsub()
+  })
+
   test('unsub stops further updates', () => {
     const { term } = makeStubTerm()
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

src/lib/terminalTheme.ts

@@ -69,14 +69,31 @@ export function getXtermFontConfig(): { fontFamily: string; fontSize: number; cu
   }
 }
 
-/** Subscribe an xterm instance to live appearance updates. Returns cleanup. */
-export function subscribeXtermToAppearance(term: XTerm, onResize?: () => void): () => void {
+/** Subscribe an xterm instance to live appearance updates. Returns cleanup.
+ *
+ *  `reloadRenderer`, when provided, is invoked after the new theme has been
+ *  written to xterm's options. WebGL-rendered terminals must use this to swap
+ *  in a fresh renderer instance — xterm v6's WebGL renderer keeps its texture
+ *  atlas, glyph cache, and rectangle-batch buffers alive across theme changes,
+ *  so setting `term.options.theme` updates the option object but the visible
+ *  canvas keeps painting with the old palette. The DOM renderer doesn't have
+ *  this issue (it re-injects CSS on every theme change), so DOM-only callers
+ *  can leave it undefined.
+ */
+export function subscribeXtermToAppearance(
+  term: XTerm,
+  onResize?: () => void,
+  reloadRenderer?: () => void,
+): () => void {
   return onAppearanceChanged(() => {
     const cfg = getXtermFontConfig()
     term.options.theme = getXtermTheme()
     term.options.fontFamily = cfg.fontFamily
     term.options.fontSize = cfg.fontSize
     term.options.cursorBlink = cfg.cursorBlink
+    // Swap the renderer first so it activates against the just-written theme
+    // (its constructor caches `themeService.colors` once at activate time).
+    reloadRenderer?.()
     // The WebGL renderer caches a texture atlas keyed on font + theme. Without
     // invalidating it, font/color changes don't visibly repaint until the next
     // resize. The canvas renderer ignores this method.

src/lib/terminalThemeRepro.test.ts

@@ -0,0 +1,94 @@
+import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'
+import { Terminal as XTerm } from '@xterm/xterm'
+import { subscribeXtermToAppearance } from './terminalTheme'
+import { applyAppearance, DEFAULT_PREFS } from './theme'
+
+// xterm reaches into matchMedia / IntersectionObserver during open() — jsdom
+// doesn't ship them. Bare-minimum stubs are enough for the DOM renderer path,
+// which is what we hit here since jsdom has no canvas.
+beforeAll(() => {
+  if (!window.matchMedia) {
+    Object.defineProperty(window, 'matchMedia', {
+      configurable: true,
+      value: vi.fn().mockImplementation((query: string) => ({
+        matches: false,
+        media: query,
+        onchange: null,
+        addEventListener: vi.fn(),
+        removeEventListener: vi.fn(),
+        addListener: vi.fn(),
+        removeListener: vi.fn(),
+        dispatchEvent: vi.fn(),
+      })),
+    })
+  }
+  if (!('IntersectionObserver' in window)) {
+    class IO {
+      observe() {}
+      unobserve() {}
+      disconnect() {}
+      takeRecords() { return [] }
+    }
+    Object.defineProperty(window, 'IntersectionObserver', { configurable: true, value: IO })
+  }
+})
+
+describe('real XTerm responds to appearance changes', () => {
+  let container: HTMLDivElement
+  let term: XTerm
+
+  beforeEach(() => {
+    localStorage.clear()
+    container = document.createElement('div')
+    Object.defineProperty(container, 'clientWidth', { value: 800, configurable: true })
+    Object.defineProperty(container, 'clientHeight', { value: 400, configurable: true })
+    document.body.appendChild(container)
+  })
+
+  afterEach(() => {
+    term?.dispose()
+    container.remove()
+  })
+
+  test('rawOptions.theme is replaced when appearance flips dark→light', () => {
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'dark' })
+    term = new XTerm({ allowProposedApi: true, theme: { background: '#000000' } })
+    term.open(container)
+    const initial = (term.options as unknown as { theme: { background?: string } }).theme
+
+    const unsub = subscribeXtermToAppearance(term)
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })
+
+    const after = (term.options as unknown as { theme: { background?: string } }).theme
+    expect(after).toBeDefined()
+    expect(after).not.toBe(initial)
+    expect(after.background?.toLowerCase()).not.toBe(initial.background?.toLowerCase())
+    unsub()
+  })
+
+  // Reproduces the user-reported bug: terminals started in one mode keep their
+  // original colors after the user toggles to the other mode. xterm's DOM
+  // renderer paints the viewport background via the scrollable element's inline
+  // backgroundColor — so verifying that style flips proves the rendered output
+  // (not just the option object) tracks the theme.
+  test('rendered viewport background flips when appearance flips dark→light', () => {
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'dark' })
+    term = new XTerm({ allowProposedApi: true })
+    term.open(container)
+
+    // The scrollable element's backgroundColor is set by xterm's Viewport on
+    // every theme change.
+    const findScrollable = () => container.querySelector('.xterm-scrollable-element') as HTMLElement | null
+    const before = findScrollable()?.style.backgroundColor ?? ''
+    expect(before).toBeTruthy()
+
+    const unsub = subscribeXtermToAppearance(term)
+    applyAppearance({ ...DEFAULT_PREFS, mode: 'light' })
+
+    const after = findScrollable()?.style.backgroundColor ?? ''
+    expect(after).toBeTruthy()
+    expect(after).not.toBe(before)
+
+    unsub()
+  })
+})