diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 6437d3cd4ca..740f7f9b6a9 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -76,7 +76,6 @@ export default function TerminalPane({ const paneMode2031Ref = useRef>(new Map()) const paneLastThemeModeRef = useRef>(new Map()) const panePtyBindingsRef = useRef>(new Map()) - const pendingWritesRef = useRef>(new Map()) // Why: tracks panes currently replaying recorded PTY bytes into xterm // (cold-restore, daemon snapshot, scrollback restore, eager-buffer flush). // While non-zero, pty-connection.ts drops xterm onData so auto-replies to @@ -468,7 +467,6 @@ export default function TerminalPane({ paneMode2031Ref, paneLastThemeModeRef, panePtyBindingsRef, - pendingWritesRef, replayingPanesRef, isActiveRef, isVisibleRef, @@ -534,7 +532,6 @@ export default function TerminalPane({ cwd, startup: { command: 'codex' }, paneTransportsRef, - pendingWritesRef, replayingPanesRef, isActiveRef, isVisibleRef, @@ -634,7 +631,6 @@ export default function TerminalPane({ managerRef, containerRef, paneTransportsRef, - pendingWritesRef, isActiveRef, isVisibleRef, toggleExpandPane @@ -823,15 +819,9 @@ export default function TerminalPane({ if (panes.length === 0) { return } - // Flush pending background PTY output into terminals before serializing. - // terminal.write() is async so some trailing bytes may be lost — best effort. - for (const pane of panes) { - const pending = pendingWritesRef.current.get(pane.id) - if (pending) { - pane.terminal.write(pending) - pendingWritesRef.current.set(pane.id, '') - } - } + // No renderer-side pending writes to flush — PTY output writes live + // into xterm regardless of visibility, so the SerializeAddon already + // sees the freshest possible state at this point. const buffers: Record = {} for (const pane of panes) { try { diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.ts b/src/renderer/src/components/terminal-pane/layout-serialization.ts index bab1a94dd1d..6127da76526 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.ts @@ -264,10 +264,9 @@ export function restoreScrollbackBuffers( } if (buf.length > 0) { // Why replayIntoTerminal: the serialized buffer can contain query - // sequences that leaked in via the pendingWritesRef flush before - // serialization (see TerminalPane capture hook). Writing those - // through xterm would trigger auto-replies that land in the new - // shell's stdin. See replay-guard.ts. + // sequences from the prior session (DA1, DECRQM, OSC 10/11, focus, + // CPR). Writing those through xterm.write would trigger auto-replies + // that land in the new shell's stdin. See replay-guard.ts. replayIntoTerminal(pane, replayingPanesRef, buf) // Ensure cursor is on a new line so the new shell prompt // doesn't trigger zsh's PROMPT_EOL_MARK (%) indicator. diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index bb41e96990c..ce1b90a496c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -16,7 +16,6 @@ export type PtyConnectionDeps = { restoredLeafId?: string | null restoredPtyIdByLeafId?: Record paneTransportsRef: React.RefObject> - pendingWritesRef: React.RefObject> replayingPanesRef: ReplayingPanesRef isActiveRef: React.RefObject isVisibleRef: React.RefObject diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index c6e561bd31f..cfa771ce9f3 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -177,7 +177,6 @@ function createDeps(overrides: Record = {}) { restoredLeafId: null, restoredPtyIdByLeafId: {}, paneTransportsRef: { current: new Map() }, - pendingWritesRef: { current: new Map() }, replayingPanesRef: { current: new Map() }, isActiveRef: { current: true }, isVisibleRef: { current: true }, @@ -659,6 +658,115 @@ describe('connectPanePty', () => { ) }) + // Why: when a reattach result carries both snapshot and replay (the daemon + // host serves the snapshot, the relay replay buffer covers the same tail), + // painting both into xterm doubles the same lines. This is the duplicated- + // TUI-output symptom users saw on worktree switch. Snapshot is the freshest + // authoritative source and wins by precedence. + it('paints only the daemon snapshot when reattach result includes both snapshot and replay', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + return { + id: sessionId, + snapshot: 'snapshot-payload', + replay: 'replay-payload' + } + } + return null + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'tab-pty' }] + } + } as StoreState + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + restoredLeafId: 'pane:1', + restoredPtyIdByLeafId: { 'pane:1': 'tab-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(20) + + expect(pane.terminal.write).toHaveBeenCalledWith('snapshot-payload', expect.any(Function)) + expect(pane.terminal.write).not.toHaveBeenCalledWith('replay-payload', expect.any(Function)) + }) + + it('paints only relay replay when reattach result has replay and coldRestore but no snapshot', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + return { + id: sessionId, + replay: 'replay-payload', + coldRestore: { scrollback: 'cold-payload' } + } + } + return null + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'tab-pty' }] + } + } as StoreState + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + restoredLeafId: 'pane:1', + restoredPtyIdByLeafId: { 'pane:1': 'tab-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(20) + + expect(pane.terminal.write).toHaveBeenCalledWith('replay-payload', expect.any(Function)) + expect(pane.terminal.write).not.toHaveBeenCalledWith('cold-payload', expect.any(Function)) + // Why: the replay branch supersedes cold-restore but must still ack so + // the daemon does not redeliver the cold-restore payload on the next + // reattach. + expect(window.api.pty.ackColdRestore).toHaveBeenCalledWith('tab-pty') + }) + + // Regression for the dim-mismatch bug — guarantees that we never + // reintroduce visibility-gated buffering. Bytes go straight to xterm, + // which lets the WebGL/DOM-fallback renderer parse them at live cols. + // Hidden panes still get writes; the visibility prop only controls + // WebGL suspend/resume in use-terminal-pane-global-effects. + it('writes PTY bytes straight to xterm regardless of visibility', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + capturedDataCallback.current?.('hello\r\n') + + expect(pane.terminal.write).toHaveBeenCalledWith('hello\r\n') + }) + it('reattaches via daemon sessionId when an in-session PTY is live', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 45790fdf0f7..c37803c7fdc 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -12,7 +12,7 @@ import type { PtyConnectionDeps } from './pty-connection-types' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' import { isPtyLocked } from '@/lib/pane-manager/mobile-driver-state' -import { isPaneReplaying, replayIntoTerminal, replayIntoTerminalAsync } from './replay-guard' +import { isPaneReplaying, replayIntoTerminal } from './replay-guard' import { paneLeafId, POST_REPLAY_MODE_RESET, @@ -436,7 +436,6 @@ export function connectPanePty( // Defer PTY spawn/attach to next frame so FitAddon has time to calculate // the correct terminal dimensions from the laid-out container. - deps.pendingWritesRef.current.set(pane.id, '') connectFrame = requestAnimationFrame(() => { connectFrame = null if (disposed) { @@ -460,13 +459,6 @@ export function connectPanePty( deps.onPtyErrorRef?.current?.(pane.id, message) } - // Why: 512 KB cap keeps the pending buffer from growing without bound - // when an agent runs for minutes in a background worktree. When the - // cap is reached, the oldest output is trimmed so the most recent - // terminal state is preserved. This matches the MAX_BUFFER_BYTES - // constant used for serialized scrollback capture. - const MAX_PENDING_BYTES = 512 * 1024 - // Why: shared registration so both fresh-spawn and reattach paths install // the same SerializeAddon-backed serializer plus the onTitleChange wrapper // that drives lastTitle parity for mobile subscribers. Wires the resulting @@ -483,14 +475,6 @@ export function connectPanePty( } const unregisterSerializer = registerPtySerializer(ptyId, async (opts) => { try { - const pending = deps.pendingWritesRef.current.get(pane.id) - if (pending) { - deps.pendingWritesRef.current.set(pane.id, '') - // Why: hidden/background panes buffer PTY output instead of writing - // to xterm. Mobile snapshots must include that pending output, and - // replay guard prevents xterm query auto-replies from hitting stdin. - await replayIntoTerminalAsync(pane, deps.replayingPanesRef, pending) - } // Why: alt-screen TUIs (vim, claude-code) hold transient state in // the alternate screen. The hydration path requests // altScreenForcesZeroRows so normal-buffer scrollback isn't bled @@ -581,49 +565,27 @@ export function connectPanePty( pendingSpawnByPaneKey.set(pendingSpawnKey, trackedPromise) } - // Why: replay bytes (eager-buffer flush, attach-time screen clear) must - // always go through the replay guard so xterm's auto-replies to embedded - // query sequences don't leak to the shell. Unlike dataCallback we do not - // honor isVisibleRef — the visibility branch is a perf batching strategy - // for live output that defers parsing until the worktree is foregrounded, - // but deferring replay parsing drops the bytes into pendingWritesRef, - // which later flushes through plain pane.terminal.write (in - // use-terminal-pane-global-effects) with no guard engaged. xterm's - // write() buffers internally regardless of DOM visibility, and the guard - // stays engaged via the write-completion callback until xterm finishes - // parsing — so writing directly here is both correct and safe. + // The replay path uses the guard so xterm auto-replies to embedded query + // sequences don't leak into the shell. xterm.write() buffers internally + // regardless of DOM visibility and the guard stays engaged via the + // write-completion callback until xterm finishes parsing. const replayDataCallback = (data: string): void => { - // Why: the relay's replay buffer holds the full last 100 KB of output, - // including data already rendered in xterm before the disconnect. - // Clearing before writing prevents duplication on SSH reconnect. + // Relay replay buffer holds the last 100 KB of output, which may + // overlap with content already rendered in xterm before the + // disconnect. Clear first to prevent duplication on SSH reconnect. replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') replayIntoTerminal(pane, deps.replayingPanesRef, data) } const dataCallback = (data: string): void => { maybeShowDeveloperPermissionHint(deps.worktreeId, data) - - if (deps.isVisibleRef.current) { - pane.terminal.write(data) - } else { - const pending = deps.pendingWritesRef.current - let buf = (pending.get(pane.id) ?? '') + data - if (buf.length > MAX_PENDING_BYTES) { - // Why: slicing at an arbitrary offset can bisect a multi-byte - // character or an ANSI escape sequence (e.g. \x1b[38;2;255;0m), - // producing garbled output when the buffer is later flushed. - // Snapping forward to the next newline ensures the cut lands on - // a line boundary where escape state is far less likely to be - // mid-sequence. - let cutAt = buf.length - MAX_PENDING_BYTES - const nl = buf.indexOf('\n', cutAt) - if (nl !== -1 && nl < cutAt + 256) { - cutAt = nl + 1 - } - buf = buf.slice(cutAt) - } - pending.set(pane.id, buf) - } + // Always-live writes: PTY output goes straight into xterm regardless + // of visibility. xterm's internal write queue handles batching, and + // suspending WebGL while hidden (use-terminal-pane-global-effects) + // keeps GPU resources from leaking. Visibility-gated buffering used + // to feed bytes into xterm at stale dimensions on resume, which was + // the root of the cursor-on-strange-line and broken-wide-char bugs. + pane.terminal.write(data) if (pendingStartupCommand) { if (startupInjectTimer !== null) { @@ -680,13 +642,45 @@ export function connectPanePty( // main-process hydration path has full status parity. registerPaneSerializerFor(ptyId) - if (connectResult?.coldRestore) { - // Why: restoreScrollbackBuffers() already wrote the saved xterm - // buffer before this rAF ran. The cold-restore scrollback from - // disk history overlaps with that content. Without clearing first, - // the terminal shows duplicated output. - // Why replayIntoTerminal: the recorded scrollback is raw PTY output - // that may contain query sequences the previous agent CLI emitted; + // Strict precedence: snapshot > replay > coldRestore. Paint exactly + // one source per reattach. Painting snapshot AND replay produced the + // duplicated TUI output users saw on worktree switch (the relay replay + // buffer's tail typically overlaps with the daemon snapshot's tail, so + // both writing into xterm doubles the same lines). Snapshot wins + // because the daemon's authoritative buffer is freshest when present; + // replay wins over coldRestore because the relay's last 100 KB is + // newer than disk-recorded scrollback. If we ever return all three, + // the daemon and relay are by definition tracking the same session + // and only the freshest source belongs on screen. + if (connectResult?.snapshot) { + replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') + replayIntoTerminal(pane, deps.replayingPanesRef, connectResult.snapshot) + // Snapshot reattach keeps a live session, so avoid the broader mode + // reset. Focus reporting is the unsafe exception: preserving `?1004h` + // can make restored shells ring BEL on pane focus/blur. + replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_FOCUS_REPORTING_RESET) + if (connectResult.coldRestore) { + // Snapshot superseded the cold-restore payload — ack it so the + // daemon does not redeliver it on the next reattach. + window.api.pty.ackColdRestore(ptyId) + } + } else if (connectResult?.replay) { + // Relay replay holds the last 100 KB of raw output. The xterm may + // already hold pre-disconnect content; clear first to avoid + // duplication. Focus-reporting reset prevents BEL from stale mode + // bits in the replayed data. + replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') + replayIntoTerminal(pane, deps.replayingPanesRef, connectResult.replay) + replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_FOCUS_REPORTING_RESET) + if (connectResult.coldRestore) { + window.api.pty.ackColdRestore(ptyId) + } + } else if (connectResult?.coldRestore) { + // restoreScrollbackBuffers() already wrote the saved xterm buffer + // before this rAF ran. The cold-restore scrollback overlaps with + // that content; clear first. + // replayIntoTerminal: the recorded scrollback is raw PTY output that + // may contain query sequences the previous agent CLI emitted; // writing them through xterm.write would trigger auto-replies that // land in the new shell's stdin. See replay-guard.ts. replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') @@ -696,39 +690,12 @@ export function connectPanePty( deps.replayingPanesRef, '\r\n\x1b[2m--- session restored ---\x1b[0m\r\n\r\n' ) - // Why: the cold-restore scrollback is raw PTY output from the prior - // session, so mode-setting bytes emitted by a crashed TUI (e.g. - // Claude's \e[?1004h) come through verbatim and re-enable those modes - // in xterm. Cold-restore means the daemon lost the session and spawned - // a fresh shell — there is no TUI consuming these modes anymore, so - // reset them to match the fresh shell's expectations. Not applied to - // the snapshot branch below: that branch reattaches to a live daemon - // session where a running TUI may still depend on these modes. + // Cold-restore means the daemon lost the session and spawned a + // fresh shell — no TUI is consuming the mode-setting bytes that a + // crashed TUI (e.g. Claude's \e[?1004h) left in the scrollback, so + // reset them to match the fresh shell's expectations. replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_MODE_RESET) window.api.pty.ackColdRestore(ptyId) - } else if (connectResult?.snapshot) { - // Why: always clear before writing the daemon/SSH snapshot to prevent - // duplication with scrollback restored earlier. The replay guard also - // prevents terminal auto-replies from leaking into the live shell. - replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') - replayIntoTerminal(pane, deps.replayingPanesRef, connectResult.snapshot) - // Why: snapshot restore keeps a live session, so avoid the broader mode - // reset. Focus reporting is the unsafe exception: preserving `?1004h` - // can make restored shells ring BEL on pane focus/blur. - replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_FOCUS_REPORTING_RESET) - } - if (connectResult?.replay) { - // Why: the relay's replay buffer is the authoritative terminal state - // (last 100 KB of raw output). On SSH reattach the local xterm may - // already hold pre-disconnect content that overlaps with the buffer. - // Clearing before writing prevents duplication — same approach the - // snapshot path uses above. Focus-reporting reset prevents BEL on - // pane focus/blur from stale mode bits in the replayed data. - if (!connectResult.snapshot && !connectResult.coldRestore) { - replayIntoTerminal(pane, deps.replayingPanesRef, '\x1b[2J\x1b[3J\x1b[H') - } - replayIntoTerminal(pane, deps.replayingPanesRef, connectResult.replay) - replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_FOCUS_REPORTING_RESET) } if (connectResult?.sessionExpired) { toast.info('Previous SSH session expired.', { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 8fc59553b4a..1ebdfbd0936 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -6,7 +6,7 @@ import { type FocusTerminalPaneDetail } from '@/constants/terminal' import type { PaneManager } from '@/lib/pane-manager/pane-manager' -import { fitAndFocusPanes, fitPanes, hasDimensionsChanged } from './pane-helpers' +import { fitAndFocusPanes, fitPanes } from './pane-helpers' import type { PtyTransport } from './pty-transport' import { handleTerminalFileDrop } from './terminal-drop-handler' @@ -19,7 +19,6 @@ type UseTerminalPaneGlobalEffectsArgs = { managerRef: React.RefObject containerRef: React.RefObject paneTransportsRef: React.RefObject> - pendingWritesRef: React.RefObject> isActiveRef: React.RefObject isVisibleRef: React.RefObject toggleExpandPane: (paneId: number) => void @@ -34,7 +33,6 @@ export function useTerminalPaneGlobalEffects({ managerRef, containerRef, paneTransportsRef, - pendingWritesRef, isActiveRef, isVisibleRef, toggleExpandPane @@ -43,150 +41,37 @@ export function useTerminalPaneGlobalEffects({ worktreeIdRef.current = worktreeId const cwdRef = useRef(cwd) cwdRef.current = cwd - // Why: starts as `true` so the first render with isVisible=false triggers - // suspendRendering(). Without this, background worktrees that mount hidden - // (isVisible=false from the start) never suspend their WebGL contexts — - // openTerminal() unconditionally creates a WebGL addon, but this effect - // only suspends on true→false transitions. The leaked contexts exhaust - // Chromium's ~8-context budget, causing "webglcontextlost" on visible - // terminals and making them unresponsive. + // Starts true so the first render with isVisible=false triggers a + // suspendRendering(). Background worktrees that mount hidden would + // otherwise leak WebGL contexts — openTerminal() unconditionally creates + // one — and exhaust Chromium's ~8-context budget across worktrees. const wasVisibleRef = useRef(true) - // Why: tracks any in-progress chunked pending-write flush so the cleanup - // function can cancel it if the pane deactivates mid-flush. - const pendingFlushRef = useRef | null>(null) - - // Why: the deferred rAF (guardedFit) must be cancellable when the pane - // deactivates before the rAF fires — otherwise it would call - // fitAndFocusPanes() on a suspended manager. - const pendingRafRef = useRef(null) - - // Why: two independent code paths schedule fitPanes() after a worktree - // switch — the isActive effect (after pending-write drain) and the - // ResizeObserver (after its 150 ms debounce). On Windows, each redundant - // fit() call adds non-trivial overhead (clear + refresh of 10 000 - // scrollback lines). An epoch counter lets whichever path fires first - // serve the activation, while the second path skips. The staleness - // check also rejects callbacks from a prior activation during rapid - // worktree switches (A→B→C). - const fitEpochRef = useRef(0) - const fitRanForEpochRef = useRef(-1) - useEffect(() => { const manager = managerRef.current if (!manager) { return } if (isVisible) { - // Why: resume WebGL immediately so the terminal shows its last-known - // state on the first painted frame. Without this, the browser paints - // 1+ frames of stale xterm DOM-fallback content at wrong dimensions - // (the "stretched" flash). On macOS, WebGL context creation is ~5 ms - // — fast enough to feel instant. On Windows (ANGLE → D3D11) it can - // take 100–500 ms, but the alternative (deferring to a rAF) leaves - // the terminal visibly distorted, which is a worse UX tradeoff. + // Resume WebGL immediately so the terminal shows its last-known state + // on the first painted frame. macOS context creation is ~5 ms; on + // Windows (ANGLE → D3D11) it can be 100–500 ms but a deferred resume + // would paint a stretched DOM-fallback flash, which is worse UX. manager.resumeRendering() - - fitEpochRef.current++ - const epoch = fitEpochRef.current - - // Why: while a worktree is in the background, PTY output accumulates - // in pendingWritesRef with no size cap. A Claude agent running for - // minutes can produce hundreds of KB. Writing it all in one - // synchronous terminal.write() blocks the renderer for 2–5 s on - // Windows, freezing the UI on every worktree switch. - // - // Fix: drain each pane's pending buffer in 32 KB chunks with a - // setTimeout(0) yield between chunks. This lets the browser paint - // frames and process input events between chunks so the UI stays - // responsive while the scrollback catches up. The fit is deferred - // until after the final chunk so xterm only reflows once. - const CHUNK_SIZE = 32 * 1024 - const entries = Array.from(pendingWritesRef.current.entries()).filter( - ([, buf]) => buf.length > 0 - ) - // Clear all pending buffers immediately so new PTY output arriving - // during the flush goes into a fresh buffer instead of being lost. - for (const [paneId] of entries) { - pendingWritesRef.current.set(paneId, '') - } - - const guardedFit = (): void => { - pendingRafRef.current = null - const mgr = managerRef.current - if (!mgr) { - return - } - // Why: three-layer guard prevents redundant and stale fits. - // 1. Staleness — reject callbacks from a superseded activation - // (e.g. rapid A→B→C worktree switch). - if (epoch !== fitEpochRef.current) { - return - } - // 2. Dimension check — if a window resize changed the container - // size, the fit must run even if one already ran for this epoch. - const dimensionsChanged = hasDimensionsChanged(mgr) - // 3. Dedup — if dims are the same and a fit already ran, skip. - if (!dimensionsChanged && fitRanForEpochRef.current >= epoch) { - return - } - fitRanForEpochRef.current = epoch - if (isActive) { - fitAndFocusPanes(mgr) - return - } - fitPanes(mgr) - } - - if (entries.length === 0) { - pendingRafRef.current = requestAnimationFrame(guardedFit) + // Single fit on resume. xterm has been writing live the whole time + // (no visibility-gated buffering), so cols/rows are already correct + // for the new container; this fit is just to absorb any container + // dimension change that happened while we were hidden (e.g. sidebar + // toggle on another worktree). + if (isActive) { + fitAndFocusPanes(manager) } else { - let entryIdx = 0 - let offset = 0 - - const drainNextChunk = (): void => { - if (entryIdx >= entries.length) { - pendingFlushRef.current = null - pendingRafRef.current = requestAnimationFrame(guardedFit) - return - } - - const [paneId, buffer] = entries[entryIdx] - const pane = manager.getPanes().find((p) => p.id === paneId) - if (!pane) { - entryIdx++ - offset = 0 - pendingFlushRef.current = setTimeout(drainNextChunk, 0) - return - } - - const chunk = buffer.slice(offset, offset + CHUNK_SIZE) - pane.terminal.write(chunk) - offset += CHUNK_SIZE - - if (offset >= buffer.length) { - entryIdx++ - offset = 0 - } - - // Yield to the browser between chunks so the UI stays responsive. - pendingFlushRef.current = setTimeout(drainNextChunk, 0) - } - - drainNextChunk() + fitPanes(manager) } } else if (wasVisibleRef.current) { - // Cancel any in-progress chunked flush before suspending. - if (pendingFlushRef.current !== null) { - clearTimeout(pendingFlushRef.current) - pendingFlushRef.current = null - } - // Cancel any pending rAF so guardedFit doesn't run on a - // suspended manager. - if (pendingRafRef.current !== null) { - cancelAnimationFrame(pendingRafRef.current) - pendingRafRef.current = null - } + // Suspend WebGL when going hidden. xterm.write() continues to land in + // the (now DOM-renderer-fallback or paused-canvas) terminal; the + // suspend is purely a GPU resource decision. manager.suspendRendering() } wasVisibleRef.current = isVisible @@ -295,17 +180,9 @@ export function useTerminalPaneGlobalEffects({ if (!manager) { return } - // Why: apply the same epoch-based deduplication as the isActive - // effect's rAF path. Read the current epoch at fire time (not a - // captured value) because the ResizeObserver persists across the - // activation. Dimension changes (e.g. window resize) bypass the - // dedup so legitimate refits are never suppressed. - const currentEpoch = fitEpochRef.current - const dimensionsChanged = hasDimensionsChanged(manager) - if (!dimensionsChanged && fitRanForEpochRef.current >= currentEpoch) { - return - } - fitRanForEpochRef.current = currentEpoch + // safeFit early-returns when proposeDimensions matches current + // cols/rows, so a no-op resize is cheap. Always-live writes mean + // there is no "deferred drain" race; fit can run unconditionally. fitPanes(manager) }, RESIZE_DEBOUNCE_MS) }) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index c60facc8012..57ccca7b368 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -95,7 +95,6 @@ type UseTerminalPaneLifecycleDeps = { paneMode2031Ref: React.RefObject> paneLastThemeModeRef: React.RefObject> panePtyBindingsRef: React.RefObject> - pendingWritesRef: React.RefObject> replayingPanesRef: ReplayingPanesRef isActiveRef: React.RefObject isVisibleRef: React.RefObject @@ -183,7 +182,6 @@ export function useTerminalPaneLifecycle({ paneMode2031Ref, paneLastThemeModeRef, panePtyBindingsRef, - pendingWritesRef, replayingPanesRef, isActiveRef, isVisibleRef, @@ -267,7 +265,6 @@ export function useTerminalPaneLifecycle({ const expandedStyleSnapshots = expandedStyleSnapshotRef.current const paneTransports = paneTransportsRef.current const panePtyBindings = panePtyBindingsRef.current - const pendingWrites = pendingWritesRef.current const linkDisposables = linkProviderDisposablesRef.current const selectionDisposables = selectionDisposablesRef.current const mouseHideDisposables = mouseHideDisposablesRef.current @@ -331,7 +328,6 @@ export function useTerminalPaneLifecycle({ cwd, startup, paneTransportsRef, - pendingWritesRef, replayingPanesRef, isActiveRef, isVisibleRef, @@ -589,7 +585,6 @@ export function useTerminalPaneLifecycle({ } clearRuntimePaneTitle(tabId, paneId) paneFontSizesRef.current.delete(paneId) - pendingWritesRef.current.delete(paneId) replayingPanesRef.current.delete(paneId) // Clean up pane title state so closed panes don't leave stale entries. setPaneTitles((prev) => { @@ -909,7 +904,6 @@ export function useTerminalPaneLifecycle({ } panePtyBindings.clear() paneTransports.clear() - pendingWrites.clear() manager.destroy() managerRef.current = null if (e2eConfig.exposeStore) { diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index 3d6ba79be83..a5d2b370af9 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { WebglAddon } from '@xterm/addon-webgl' import type { ManagedPaneInternal } from './pane-manager-types' -import { attachWebgl, resetTerminalWebglSuggestion } from './pane-lifecycle' +import { attachWebgl, openTerminal, resetTerminalWebglSuggestion } from './pane-lifecycle' import { buildDefaultTerminalOptions } from './pane-terminal-options' const webglMock = vi.hoisted(() => ({ @@ -160,3 +160,112 @@ describe('attachWebgl', () => { expect(forcedPane.terminal.loadAddon).toHaveBeenCalledTimes(1) }) }) + +describe('openTerminal — Unicode 11 ordering', () => { + beforeEach(() => { + vi.stubGlobal('requestAnimationFrame', () => 1) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + // Why: CJK / emoji / ZWJ widths get baked into the buffer at the active + // unicode version on write. If anything writes bytes through xterm before + // unicode v11 is activated (still on default v6 width tables), wide chars + // lay out as single cells. The bug surfaces as the broken `?`-style glyphs + // users saw on worktree switch. + it('activates unicode 11 before any caller-driven write would be possible', () => { + const events: string[] = [] + + const fitAddon = { fit: vi.fn() } as unknown as ManagedPaneInternal['fitAddon'] + const searchAddon = {} as unknown as ManagedPaneInternal['searchAddon'] + const serializeAddon = {} as unknown as ManagedPaneInternal['serializeAddon'] + const unicode11Addon = {} as unknown as ManagedPaneInternal['unicode11Addon'] + const webLinksAddon = {} as unknown as ManagedPaneInternal['webLinksAddon'] + + const unicodeProxy = { + _version: '6' as '6' | '11', + get activeVersion(): '6' | '11' { + return this._version + }, + set activeVersion(v: '6' | '11') { + events.push(`activeVersion=${v}`) + this._version = v + } + } + + const fakeContainer = { + appendChild: vi.fn(), + addEventListener: vi.fn() + } as unknown as HTMLDivElement + const fakeTooltip = {} as unknown as HTMLDivElement + + const terminal = { + element: null as HTMLElement | null, + textarea: null, + cols: 80, + rows: 24, + open: vi.fn(() => { + events.push('open') + }), + loadAddon: vi.fn((addon: object) => { + if (addon === fitAddon) { + events.push('loadAddon:fit') + } else if (addon === searchAddon) { + events.push('loadAddon:search') + } else if (addon === serializeAddon) { + events.push('loadAddon:serialize') + } else if (addon === unicode11Addon) { + events.push('loadAddon:unicode11') + } else if (addon === webLinksAddon) { + events.push('loadAddon:webLinks') + } + }), + write: vi.fn(() => { + events.push('write') + }), + unicode: unicodeProxy, + buffer: { active: { cursorX: 0, cursorY: 0 } } + } as unknown as ManagedPaneInternal['terminal'] + + const pane: ManagedPaneInternal = { + id: 1, + terminal, + container: fakeContainer, + xtermContainer: fakeContainer, + linkTooltip: fakeTooltip, + terminalGpuAcceleration: 'off', + gpuRenderingEnabled: false, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, + fitAddon, + fitResizeObserver: null, + pendingObservedFitRafId: null, + searchAddon, + serializeAddon, + unicode11Addon, + ligaturesAddon: null, + webLinksAddon, + webglAddon: null, + compositionHandler: null, + pendingSplitScrollState: null, + debugLabel: null + } + + openTerminal(pane) + + expect(events).toContain('loadAddon:unicode11') + expect(events).toContain('activeVersion=11') + + const unicodeIdx = events.indexOf('activeVersion=11') + const writeIdx = events.indexOf('write') + if (writeIdx !== -1) { + expect(unicodeIdx).toBeLessThan(writeIdx) + } + + const loadUnicodeIdx = events.indexOf('loadAddon:unicode11') + expect(loadUnicodeIdx).toBeLessThan(unicodeIdx) + expect(events.indexOf('open')).toBeLessThan(loadUnicodeIdx) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 1ab960f6c5d..3fb6b304ad6 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -182,7 +182,15 @@ export function openTerminal(pane: ManagedPaneInternal): void { terminal.loadAddon(unicode11Addon) terminal.loadAddon(webLinksAddon) - // Activate unicode 11 + // Activate Unicode 11 widths *before* any caller-driven write. CJK / emoji / + // ZWJ codepoints get baked into the buffer at the active unicode version on + // write — if a restore (snapshot, scrollback, cold-restore) writes bytes + // through xterm while the default v6 width tables are still active, wide + // chars lay out as single cells and any subsequent re-measurement breaks + // pairing (visible as broken `?`-style glyphs). All restore paths + // (replayTerminalLayout → splitPane/createInitialPane → openTerminal, + // restoreScrollbackBuffers, handleReattachResult) run after openTerminal, + // so the activation must stay at this position. terminal.unicode.activeVersion = '11' // Why: the OS reads the focused textarea's screen rect at compositionstart to