diff --git a/src/main/daemon/daemon-server.test.ts b/src/main/daemon/daemon-server.test.ts index b8167948d54..5537782cc36 100644 --- a/src/main/daemon/daemon-server.test.ts +++ b/src/main/daemon/daemon-server.test.ts @@ -183,6 +183,24 @@ describe('DaemonServer', () => { 'Session not found' ) }) + + it('emits exit when a fire-and-forget write targets a missing session', async () => { + await startServer() + const c = await connectClient() + + const exitEvent = new Promise((resolve) => { + c.onEvent((event) => resolve(event)) + }) + + c.notify('write', { sessionId: 'missing-session', data: 'hi' }) + + await expect(exitEvent).resolves.toMatchObject({ + type: 'event', + event: 'exit', + sessionId: 'missing-session', + payload: { code: -1 } + }) + }) }) describe('authentication', () => { diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index 781b5386af7..108f78df2b9 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -4,7 +4,13 @@ import { writeFileSync, chmodSync, unlinkSync } from 'fs' import { encodeNdjson, createNdjsonParser } from './ndjson' import { TerminalHost } from './terminal-host' import type { SubprocessHandle } from './session' -import { PROTOCOL_VERSION, NOTIFY_PREFIX, type HelloMessage, type DaemonRequest } from './types' +import { + PROTOCOL_VERSION, + NOTIFY_PREFIX, + SessionNotFoundError, + type HelloMessage, + type DaemonRequest +} from './types' export type DaemonServerOptions = { socketPath: string @@ -230,11 +236,25 @@ export class DaemonServer { } case 'write': - this.host.write(request.payload.sessionId, request.payload.data) + try { + this.host.write(request.payload.sessionId, request.payload.data) + } catch (err) { + if (err instanceof SessionNotFoundError) { + this.sendExitEvent(client, request.payload.sessionId, -1) + } + throw err + } return {} case 'resize': - this.host.resize(request.payload.sessionId, request.payload.cols, request.payload.rows) + try { + this.host.resize(request.payload.sessionId, request.payload.cols, request.payload.rows) + } catch (err) { + if (err instanceof SessionNotFoundError) { + this.sendExitEvent(client, request.payload.sessionId, -1) + } + throw err + } return {} case 'kill': @@ -274,4 +294,25 @@ export class DaemonServer { throw new Error(`Unknown request type: ${(request as { type: string }).type}`) } } + + private sendExitEvent( + client: ConnectedClient | undefined, + sessionId: string, + code: number + ): void { + if (!client?.streamSocket) { + return + } + // Why: write/resize are notification-heavy and intentionally do not wait + // for replies. If their target session is gone, this synthetic exit is the + // only signal the renderer gets to clear stale terminal pane bindings. + client.streamSocket.write( + encodeNdjson({ + type: 'event', + event: 'exit', + sessionId, + payload: { code } + }) + ) + } } diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 71ff8c39908..654ff1ebc6e 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -66,8 +66,13 @@ vi.mock('../providers/ssh-git-dispatch', () => ({ })) const { deployAndLaunchRelay } = await import('./ssh-relay-deploy') -const { registerSshPtyProvider, unregisterSshPtyProvider, getPtyIdsForConnection } = - await import('../ipc/pty') +const { + registerSshPtyProvider, + unregisterSshPtyProvider, + getPtyIdsForConnection, + clearProviderPtyState, + deletePtyOwnership +} = await import('../ipc/pty') const { registerSshFilesystemProvider, unregisterSshFilesystemProvider } = await import('../providers/ssh-filesystem-dispatch') const { registerSshGitProvider, unregisterSshGitProvider } = @@ -81,11 +86,12 @@ function createMockDeps() { const mockPortForward = { removeAllForwards: vi.fn() } as unknown as SshPortForwardManager - const getMainWindow = vi.fn().mockReturnValue({ + const mockWindow = { isDestroyed: () => false, webContents: { send: vi.fn() } - }) - return { mockConn, mockStore, mockPortForward, getMainWindow } + } + const getMainWindow = vi.fn().mockReturnValue(mockWindow) + return { mockConn, mockStore, mockPortForward, getMainWindow, mockWindow } } function mockDeploySuccess() { @@ -187,6 +193,36 @@ describe('SshRelaySession', () => { expect(mockAttach).toHaveBeenCalledWith('pty-2') }) + it('invalidates and broadcasts remote PTYs that cannot reattach after relay reconnect', async () => { + const { mockConn, mockStore, mockPortForward, getMainWindow, mockWindow } = createMockDeps() + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + await session.establish(mockConn) + vi.clearAllMocks() + mockDeploySuccess() + + const { getSshPtyProvider } = await import('../ipc/pty') + const mockAttach = vi + .fn() + .mockRejectedValueOnce(new Error('missing pty')) + .mockResolvedValueOnce(undefined) + vi.mocked(getSshPtyProvider).mockReturnValue({ + attach: mockAttach, + dispose: vi.fn() + } as unknown as ReturnType) + vi.mocked(getPtyIdsForConnection).mockReturnValue(['pty-stale', 'pty-live']) + + await session.reconnect(mockConn) + + expect(mockAttach).toHaveBeenCalledWith('pty-stale') + expect(mockAttach).toHaveBeenCalledWith('pty-live') + expect(clearProviderPtyState).toHaveBeenCalledWith('pty-stale') + expect(deletePtyOwnership).toHaveBeenCalledWith('pty-stale') + expect(mockWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { + id: 'pty-stale', + code: -1 + }) + }) + it('dispose transitions to disposed and unregisters providers', async () => { const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index b30e7ece1ad..e018d610a88 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -234,8 +234,21 @@ export class SshRelaySession { } try { await ptyProvider.attach(ptyId) - } catch { - // PTY may have exited during the disconnect — ignore + } catch (err) { + console.warn( + `[ssh-relay-session] Dropping stale PTY ${ptyId} for ${this.targetId} after relay reattach failed: ${ + err instanceof Error ? err.message : String(err) + }` + ) + clearProviderPtyState(ptyId) + deletePtyOwnership(ptyId) + // Why: if the new relay cannot reattach this id, the remote + // backing process is gone. Tell the renderer so it clears stale + // pane bindings instead of keeping a cursor-only terminal. + const win = this.getMainWindow() + if (win && !win.isDestroyed()) { + win.webContents.send('pty:exit', { id: ptyId, code: -1 }) + } } } } diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index 0d897fdd949..ed2f6b41054 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -1,8 +1,17 @@ -import React, { lazy, useEffect, useMemo, useRef, useState, type MutableRefObject } from 'react' +import { + lazy, + useCallback, + useEffect, + useMemo, + useRef, + useState, + type MutableRefObject +} from 'react' import { LazySection } from './LazySection' import { ChevronDown, ChevronRight, ExternalLink } from 'lucide-react' import { DiffEditor, type DiffOnMount } from '@monaco-editor/react' import type { editor as monacoEditor } from 'monaco-editor' +import { monaco } from '@/lib/monaco-setup' import { joinPath } from '@/lib/path' import { detectLanguage } from '@/lib/language-detect' import { useAppStore } from '@/store' @@ -11,56 +20,11 @@ import { findWorktreeById } from '@/store/slices/worktree-helpers' import { useDiffCommentDecorator } from '../diff-comments/useDiffCommentDecorator' import { DiffCommentPopover } from '../diff-comments/DiffCommentPopover' import { applyDiffEditorLineNumberOptions } from './diff-editor-line-number-options' +import { computeLineStats } from './diff-line-stats' import type { DiffComment, GitDiffResult } from '../../../../shared/types' const ImageDiffViewer = lazy(() => import('./ImageDiffViewer')) -/** - * Compute approximate added/removed line counts by matching lines - * between original and modified content using a multiset approach. - * Not a true Myers diff, but fast and accurate enough for stat display. - */ -function computeLineStats( - original: string, - modified: string, - status: string -): { added: number; removed: number } | null { - // Why: for very large files (e.g. package-lock.json), splitting and - // iterating synchronously in the React render cycle would block the - // main thread and freeze the UI. Return null to skip stats display. - if (original.length + modified.length > 500_000) { - return null - } - if (status === 'added') { - return { added: modified ? modified.split('\n').length : 0, removed: 0 } - } - if (status === 'deleted') { - return { added: 0, removed: original ? original.split('\n').length : 0 } - } - - const origLines = original.split('\n') - const modLines = modified.split('\n') - - const origMap = new Map() - for (const line of origLines) { - origMap.set(line, (origMap.get(line) ?? 0) + 1) - } - - let matched = 0 - for (const line of modLines) { - const count = origMap.get(line) ?? 0 - if (count > 0) { - origMap.set(line, count - 1) - matched++ - } - } - - return { - added: modLines.length - matched, - removed: origLines.length - matched - } -} - type DiffSection = { key: string path: string @@ -126,6 +90,10 @@ export function DiffSectionItem({ ) const language = detectLanguage(section.path) const isEditable = section.area === 'unstaged' + const modelPathBase = useMemo( + () => `diff-section:${encodeURIComponent(worktreeId)}:${encodeURIComponent(section.key)}`, + [section.key, worktreeId] + ) const editorFontSize = computeEditorFontSize( settings?.terminalFontSize ?? 13, editorFontZoomLevel @@ -136,6 +104,27 @@ export function DiffSectionItem({ const lineNumberOptionsSubRef = useRef<{ dispose: () => void } | null>(null) const [popover, setPopover] = useState<{ lineNumber: number; top: number } | null>(null) + const disposeDiffModels = useCallback(() => { + window.setTimeout(() => { + const originalModel = monaco.editor.getModel(monaco.Uri.parse(`${modelPathBase}:original`)) + const modifiedModel = monaco.editor.getModel(monaco.Uri.parse(`${modelPathBase}:modified`)) + if (!originalModel?.isAttachedToEditor()) { + originalModel?.dispose() + } + if (!modifiedModel?.isAttachedToEditor()) { + modifiedModel?.dispose() + } + }, 0) + }, [modelPathBase]) + + useEffect(() => { + if (section.collapsed) { + disposeDiffModels() + } + }, [disposeDiffModels, section.collapsed]) + + useEffect(() => () => disposeDiffModels(), [disposeDiffModels]) + useDiffCommentDecorator({ editor: modifiedEditor, filePath: section.path, @@ -397,6 +386,12 @@ export function DiffSectionItem({ modified={section.modifiedContent} theme={isDark ? 'vs-dark' : 'vs'} onMount={handleMount} + // Why: @monaco-editor/react can dispose models before widget teardown. + // Keep them through unmount and dispose unattached models next tick. + originalModelPath={`${modelPathBase}:original`} + modifiedModelPath={`${modelPathBase}:modified`} + keepCurrentOriginalModel + keepCurrentModifiedModel options={{ readOnly: !isEditable, originalEditable: false, diff --git a/src/renderer/src/components/editor/diff-line-stats.ts b/src/renderer/src/components/editor/diff-line-stats.ts new file mode 100644 index 00000000000..3d9c348ba05 --- /dev/null +++ b/src/renderer/src/components/editor/diff-line-stats.ts @@ -0,0 +1,41 @@ +/** + * Compute approximate added/removed line counts by matching lines between + * original and modified content using a multiset approach. + */ +export function computeLineStats( + original: string, + modified: string, + status: string +): { added: number; removed: number } | null { + // Why: for very large files, splitting in React render would block the UI. + if (original.length + modified.length > 500_000) { + return null + } + if (status === 'added') { + return { added: modified ? modified.split('\n').length : 0, removed: 0 } + } + if (status === 'deleted') { + return { added: 0, removed: original ? original.split('\n').length : 0 } + } + + const origLines = original.split('\n') + const modLines = modified.split('\n') + const origMap = new Map() + for (const line of origLines) { + origMap.set(line, (origMap.get(line) ?? 0) + 1) + } + + let matched = 0 + for (const line of modLines) { + const count = origMap.get(line) ?? 0 + if (count > 0) { + origMap.set(line, count - 1) + matched++ + } + } + + return { + added: modLines.length - matched, + removed: origLines.length - matched + } +} 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 0a54cbfb4ef..76659d82fd7 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -472,6 +472,52 @@ describe('connectPanePty', () => { expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(2, 'leaf-pty-2') }) + it('spawns a fresh PTY when a restored daemon split session cannot reattach', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async (opts: { sessionId?: string }) => { + if (opts.sessionId) { + return undefined + } + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as + | ((ptyId: string) => void) + | undefined + onPtySpawn?.('fresh-pty') + return 'fresh-pty' + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + settings: { + ...mockStoreState.settings, + experimentalTerminalDaemon: true + } + } as StoreState + const pane = createPane(2) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: 'pane:2', + restoredPtyIdByLeafId: { 'pane:2': 'stale-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await Promise.resolve() + await Promise.resolve() + + expect(transport.connect).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ sessionId: 'stale-pty' }) + ) + expect(transport.connect).toHaveBeenNthCalledWith( + 2, + expect.not.objectContaining({ sessionId: expect.any(String) }) + ) + expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(2, null) + expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'stale-pty') + expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(2, 'fresh-pty') + expect(deps.updateTabPtyId).toHaveBeenCalledWith('tab-1', 'fresh-pty') + }) + it('resets focus reporting after daemon snapshot replay without applying the full mode reset', 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 91c84fbc5f5..6a0a0e6ef30 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -11,7 +11,12 @@ import { createIpcPtyTransport } from './pty-transport' import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding' import type { PtyConnectionDeps } from './pty-connection-types' import { isPaneReplaying, replayIntoTerminal } from './replay-guard' -import { POST_REPLAY_MODE_RESET, POST_REPLAY_FOCUS_REPORTING_RESET } from './layout-serialization' +import { + paneLeafId, + POST_REPLAY_MODE_RESET, + POST_REPLAY_FOCUS_REPORTING_RESET +} from './layout-serialization' +import { warnTerminalLifecycleAnomaly } from './terminal-lifecycle-diagnostics' const pendingSpawnByTabId = new Map>() @@ -453,7 +458,10 @@ export function connectPanePty( } } - const handleReattachResult = (result: PtyConnectResult | string | void): void => { + const handleReattachResult = ( + result: PtyConnectResult | string | void, + staleSessionId?: string | null + ): void => { if (disposed) { return } @@ -462,10 +470,25 @@ export function connectPanePty( const ptyId = connectResult?.id ?? (typeof result === 'string' ? result : transport.getPtyId()) - if (ptyId) { - deps.syncPanePtyLayoutBinding(pane.id, ptyId) - deps.updateTabPtyId(deps.tabId, ptyId) + if (!ptyId) { + warnTerminalLifecycleAnomaly('restored PTY reattach returned no PTY id', { + tabId: deps.tabId, + worktreeId: deps.worktreeId, + leafId: deps.restoredLeafId ?? paneLeafId(pane.id), + paneId: pane.id, + ptyId: staleSessionId ?? null + }) + // Why: a stale restored daemon/SSH session can fail reattach after the + // pane is mounted. Do not leave xterm alive without a backing PTY. + deps.syncPanePtyLayoutBinding(pane.id, null) + if (staleSessionId) { + deps.clearTabPtyId(deps.tabId, staleSessionId) + } + startFreshSpawn() + return } + deps.syncPanePtyLayoutBinding(pane.id, ptyId) + deps.updateTabPtyId(deps.tabId, ptyId) if (connectResult?.coldRestore) { // Why: restoreScrollbackBuffers() already wrote the saved xterm @@ -492,13 +515,7 @@ export function connectPanePty( // the snapshot branch below: that branch reattaches to a live daemon // session where a running TUI may still depend on these modes. replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_MODE_RESET) - // Why: ptyId can be null if the transport was torn down during the - // reattach flight. Only IPC the ack when we have a real ptyId; - // the replay-into-xterm calls above remain unconditional because - // they write into the terminal buffer regardless. - if (ptyId) { - window.api.pty.ackColdRestore(ptyId) - } + 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 @@ -530,13 +547,11 @@ export function connectPanePty( }) } - if (ptyId) { - transport.resize(cols, rows) - // Why: POSIX only delivers SIGWINCH when terminal dimensions actually - // change. Sending it explicitly guarantees restored TUIs repaint at - // the correct cursor position after snapshot replay. - window.api.pty.signal(ptyId, 'SIGWINCH') - } + transport.resize(cols, rows) + // Why: POSIX only delivers SIGWINCH when terminal dimensions actually + // change. Sending it explicitly guarantees restored TUIs repaint at + // the correct cursor position after snapshot replay. + window.api.pty.signal(ptyId, 'SIGWINCH') scheduleRuntimeGraphSync() } @@ -605,7 +620,7 @@ export function connectPanePty( } : 'undefined' ) - handleReattachResult(result) + handleReattachResult(result, pendingSessionId) }) .catch((err) => { console.warn(`[pty-connection] Reattach FAILED for tab=${deps.tabId}:`, err) @@ -687,10 +702,22 @@ export function connectPanePty( void Promise.resolve(reattachPromise) .then((result) => { - handleReattachResult(result) + handleReattachResult(result, deferredReattachSessionId) }) .catch((err) => { - reportError(err instanceof Error ? err.message : String(err)) + const message = err instanceof Error ? err.message : String(err) + warnTerminalLifecycleAnomaly('restored PTY reattach threw', { + tabId: deps.tabId, + worktreeId: deps.worktreeId, + leafId: deps.restoredLeafId ?? paneLeafId(pane.id), + paneId: pane.id, + ptyId: deferredReattachSessionId, + reason: message + }) + reportError(message) + deps.syncPanePtyLayoutBinding(pane.id, null) + deps.clearTabPtyId(deps.tabId, deferredReattachSessionId) + startFreshSpawn() }) } else if (detachedLivePtyId) { allowInitialIdleCacheSeed = false diff --git a/src/renderer/src/components/terminal-pane/terminal-lifecycle-diagnostics.ts b/src/renderer/src/components/terminal-pane/terminal-lifecycle-diagnostics.ts new file mode 100644 index 00000000000..38fb6a2c33d --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-lifecycle-diagnostics.ts @@ -0,0 +1,34 @@ +type TerminalLifecycleDiagnosticDetails = { + tabId?: string + worktreeId?: string + leafId?: string | null + paneId?: number + ptyId?: string | null + reason?: string +} + +const emittedDiagnostics = new Set() +const MAX_EMITTED_DIAGNOSTICS = 500 + +export function warnTerminalLifecycleAnomaly( + event: string, + details: TerminalLifecycleDiagnosticDetails +): void { + const key = [ + event, + details.tabId ?? '', + details.worktreeId ?? '', + details.leafId ?? '', + details.paneId ?? '', + details.ptyId ?? '', + details.reason ?? '' + ].join('|') + if (emittedDiagnostics.has(key)) { + return + } + if (emittedDiagnostics.size >= MAX_EMITTED_DIAGNOSTICS) { + emittedDiagnostics.clear() + } + emittedDiagnostics.add(key) + console.warn(`[terminal-lifecycle] ${event}`, details) +} 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 d6f6b19aa6b..e99851d8318 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 @@ -591,7 +591,11 @@ export function useTerminalPaneLifecycle({ // selection also detaches those listeners (see // SelectionService._removeMouseDownListeners). managerRef.current?.getActivePane()?.terminal.clearSelection() - } + }, + // Why: TerminalPane instances stay mounted for hidden visited worktrees + // so PTYs survive navigation. Creating WebGL for those offscreen panes + // still consumes Chromium's context budget and can blank visible panes. + initialRenderingSuspended: !isVisibleRef.current }) managerRef.current = manager diff --git a/src/renderer/src/hooks/useEditorExternalWatch.ts b/src/renderer/src/hooks/useEditorExternalWatch.ts index 5a2bc9fa416..416ef1913b4 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.ts @@ -28,6 +28,15 @@ import type { OpenFile } from '@/store/slices/editor' const EXTERNAL_RELOAD_DEBOUNCE_MS = 75 const pendingExternalReloadTimers = new Map() +function warnExternalWatchFailure(target: WatchedTarget, err: unknown): void { + console.warn('[filesystem-watch] failed to watch worktree', { + worktreeId: target.worktreeId, + worktreePath: target.worktreePath, + connectionId: target.connectionId, + error: err instanceof Error ? err.message : String(err) + }) +} + function scheduleDebouncedExternalReload(notification: { worktreeId: string worktreePath: string @@ -147,10 +156,17 @@ export function useEditorExternalWatch(): void { }) } for (const target of added) { - void window.api.fs.watchWorktree({ - worktreePath: target.worktreePath, - connectionId: target.connectionId - }) + void window.api.fs + .watchWorktree({ + worktreePath: target.worktreePath, + connectionId: target.connectionId + }) + .catch((err) => { + // Why: remote SSH providers can disappear while tabs still reference + // the worktree. Watching should degrade to a diagnostic, not an + // uncaught renderer promise that looks like the terminal froze. + warnExternalWatchFailure(target, err) + }) } targetsRef.current = nextTargets // Why: this effect is intentionally differential — it does not unwatch on diff --git a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts index fcdc2682a36..fa84e01f7c7 100644 --- a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts @@ -43,6 +43,8 @@ function createPane(): ManagedPaneInternal { xtermContainer: {} as never, linkTooltip: {} as never, gpuRenderingEnabled: true, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, fitAddon: { fit: vi.fn(), proposeDimensions: vi.fn(() => ({ cols: 80, rows: 24 })) 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 aa418b2ec46..9d1f3c93fc3 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -1,8 +1,99 @@ -import { describe, expect, it } from 'vitest' -import { buildDefaultTerminalOptions } from './pane-lifecycle' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ManagedPaneInternal } from './pane-manager-types' +import { attachWebgl } from './pane-lifecycle' +import { buildDefaultTerminalOptions } from './pane-terminal-options' + +const webglMock = vi.hoisted(() => ({ + contextLossHandler: null as (() => void) | null, + dispose: vi.fn() +})) + +vi.mock('@xterm/addon-webgl', () => ({ + WebglAddon: vi.fn().mockImplementation(function WebglAddon() { + return { + onContextLoss: vi.fn((handler: () => void) => { + webglMock.contextLossHandler = handler + }), + dispose: webglMock.dispose + } + }) +})) + +function createPane(): ManagedPaneInternal { + return { + id: 1, + terminal: { + loadAddon: vi.fn(), + refresh: vi.fn(), + rows: 24 + } as never, + container: {} as never, + xtermContainer: {} as never, + linkTooltip: {} as never, + gpuRenderingEnabled: true, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, + fitAddon: { + fit: vi.fn() + } as never, + fitResizeObserver: null, + pendingObservedFitRafId: null, + searchAddon: {} as never, + serializeAddon: {} as never, + unicode11Addon: {} as never, + ligaturesAddon: null, + webLinksAddon: {} as never, + webglAddon: null, + compositionHandler: null, + pendingSplitScrollState: null, + pendingDragScrollState: null + } +} describe('buildDefaultTerminalOptions', () => { it('leaves macOS Option available for keyboard layout characters', () => { expect(buildDefaultTerminalOptions().macOptionIsMeta).toBe(false) }) }) + +describe('attachWebgl', () => { + beforeEach(() => { + webglMock.contextLossHandler = null + webglMock.dispose.mockClear() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(16) + return 1 + }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('keeps a pane on the DOM renderer after WebGL context loss', () => { + const pane = createPane() + + attachWebgl(pane) + expect(pane.terminal.loadAddon).toHaveBeenCalledTimes(1) + expect(webglMock.contextLossHandler).not.toBeNull() + + webglMock.contextLossHandler?.() + + expect(pane.webglAddon).toBeNull() + expect(pane.webglDisabledAfterContextLoss).toBe(true) + + attachWebgl(pane) + + expect(pane.terminal.loadAddon).toHaveBeenCalledTimes(1) + }) + + it('does not attach WebGL while initial rendering is deferred', () => { + const pane = createPane() + pane.webglAttachmentDeferred = true + + attachWebgl(pane) + + expect(pane.webglAddon).toBeNull() + expect(pane.terminal.loadAddon).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 0391618ae07..e645552fae2 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -22,6 +22,7 @@ import { attachPaneFitResizeObserver, detachPaneFitResizeObserver } from './pane-fit-resize-observer' +import { buildDefaultTerminalOptions } from './pane-terminal-options' // --------------------------------------------------------------------------- // Pane creation, terminal open/close, addon management @@ -29,35 +30,6 @@ import { const ENABLE_WEBGL_RENDERER = true -export function buildDefaultTerminalOptions(): ITerminalOptions { - return { - allowProposedApi: true, - cursorBlink: true, - cursorStyle: 'bar', - fontSize: 14, - // Cross-platform fallback chain — ensures the terminal can always find a - // usable monospace font regardless of OS, even if user settings haven't - // loaded yet. macOS-only fonts are harmlessly skipped on other platforms. - // Must stay in sync with FALLBACK_FONTS in layout-serialization.ts; the - // trailing Nerd Fonts let Powerline/PUA glyphs render even at first paint - // before the user's configured terminalFontFamily is applied. - fontFamily: - '"SF Mono", "Menlo", "Monaco", "Cascadia Mono", "Consolas", "DejaVu Sans Mono", "Liberation Mono", "Symbols Nerd Font Mono", "MesloLGS Nerd Font", "JetBrainsMono Nerd Font", "Hack Nerd Font", monospace', - fontWeight: '300', - fontWeightBold: '500', - scrollback: 10000, - allowTransparency: false, - // Why: on macOS, non-US layouts rely on Option to compose real characters - // like @ (German Option+L) and € (German Option+E). Enabling xterm's - // Meta mode here makes Option behave like Esc+key instead, which steals - // those composed characters before they reach the shell. - // Readline shortcuts (Option+B/F/D) are compensated in terminal-shortcut-policy.ts. - macOptionIsMeta: false, - macOptionClickForcesSelection: true, - drawBoldTextInBrightColors: true - } -} - function getTerminalUrlOpenHint(): string { return navigator.userAgent.includes('Mac') ? '⌘+click to open or ⇧⌘+click for system browser' @@ -137,6 +109,8 @@ export function createPaneDOM( xtermContainer, linkTooltip, gpuRenderingEnabled: ENABLE_WEBGL_RENDERER, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, fitAddon, fitResizeObserver: null, pendingObservedFitRafId: null, @@ -307,7 +281,12 @@ export function disposeWebgl(pane: ManagedPaneInternal): void { } export function attachWebgl(pane: ManagedPaneInternal): void { - if (!ENABLE_WEBGL_RENDERER || !pane.gpuRenderingEnabled) { + if ( + !ENABLE_WEBGL_RENDERER || + !pane.gpuRenderingEnabled || + pane.webglAttachmentDeferred || + pane.webglDisabledAfterContextLoss + ) { pane.webglAddon = null return } @@ -319,6 +298,10 @@ export function attachWebgl(pane: ManagedPaneInternal): void { pane.id, '— falling back to DOM renderer' ) + // Why: Chromium starts reclaiming terminal contexts under pressure. + // Recreating WebGL for this pane can loop context loss and leave xterm + // visually blank, so keep the pane on the DOM renderer until remount. + pane.webglDisabledAfterContextLoss = true webglAddon.dispose() pane.webglAddon = null // Why: when the WebGL context is lost (GPU memory pressure, Chromium diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index ef0ccd44beb..d119cceacc0 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -19,6 +19,7 @@ export type PaneManagerOptions = { onLayoutChanged?: () => void terminalOptions?: (paneId: number) => Partial onLinkClick?: (event: MouseEvent | undefined, url: string) => void + initialRenderingSuspended?: boolean } export type PaneStyleOptions = { @@ -60,6 +61,8 @@ export type ManagedPaneInternal = { xtermContainer: HTMLElement linkTooltip: HTMLElement gpuRenderingEnabled: boolean + webglAttachmentDeferred: boolean + webglDisabledAfterContextLoss: boolean webglAddon: WebglAddon | null // Why nullable: ligatures are opt-in per font and toggleable at runtime, // so the addon instance only exists while the feature is active. A null diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index b720571d9c7..d9c95702042 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -38,6 +38,7 @@ import { } from './pane-tree-ops' import { lockDragScroll, unlockDragScroll } from './pane-drag-scroll' import { scheduleSplitScrollRestore } from './pane-split-scroll' +import { toPublicPane } from './pane-public-view' export type { PaneManagerOptions, PaneStyleOptions, ManagedPane, DropZone } @@ -49,6 +50,7 @@ export class PaneManager { private options: PaneManagerOptions private styleOptions: PaneStyleOptions = {} private destroyed = false + private renderingSuspended: boolean // Drag-to-reorder state private dragState = createDragReorderState() @@ -56,6 +58,7 @@ export class PaneManager { constructor(root: HTMLElement, options: PaneManagerOptions) { this.root = root this.options = options + this.renderingSuspended = options.initialRenderingSuspended === true } // ----------------------------------------------------------------------- @@ -84,8 +87,8 @@ export class PaneManager { pane.terminal.focus() } - void this.options.onPaneCreated?.(this.toPublic(pane)) - return this.toPublic(pane) + void this.options.onPaneCreated?.(toPublicPane(pane)) + return toPublicPane(pane) } splitPane( @@ -128,7 +131,7 @@ export class PaneManager { this.applyDividerStylesWrapped() newPane.terminal?.focus() updateMultiPaneState(this.getDragCallbacks()) - void this.options.onPaneCreated?.(this.toPublic(newPane)) + void this.options.onPaneCreated?.(toPublicPane(newPane)) this.options.onLayoutChanged?.() scheduleSplitScrollRestore( @@ -138,7 +141,7 @@ export class PaneManager { () => this.destroyed ) - return this.toPublic(newPane) + return toPublicPane(newPane) } closePane(paneId: number): void { @@ -180,7 +183,7 @@ export class PaneManager { } getPanes(): ManagedPane[] { - return Array.from(this.panes.values()).map((p) => this.toPublic(p)) + return Array.from(this.panes.values()).map(toPublicPane) } fitAllPanes(): void { @@ -192,7 +195,7 @@ export class PaneManager { return null } const pane = this.panes.get(this.activePaneId) - return pane ? this.toPublic(pane) : null + return pane ? toPublicPane(pane) : null } setActivePane(paneId: number, opts?: { focus?: boolean }): void { @@ -209,7 +212,7 @@ export class PaneManager { } if (changed) { - this.options.onActivePaneChange?.(this.toPublic(pane)) + this.options.onActivePaneChange?.(toPublicPane(pane)) } } @@ -242,6 +245,9 @@ export class PaneManager { disposeWebgl(pane) return } + if (pane.webglAttachmentDeferred || pane.webglDisabledAfterContextLoss) { + return + } if (!pane.webglAddon) { attachWebgl(pane) safeFit(pane) @@ -249,14 +255,18 @@ export class PaneManager { } suspendRendering(): void { + this.renderingSuspended = true for (const pane of this.panes.values()) { + pane.webglAttachmentDeferred = true disposeWebgl(pane) } } resumeRendering(): void { + this.renderingSuspended = false for (const pane of this.panes.values()) { - if (pane.gpuRenderingEnabled && !pane.webglAddon) { + pane.webglAttachmentDeferred = false + if (pane.gpuRenderingEnabled && !pane.webglDisabledAfterContextLoss && !pane.webglAddon) { attachWebgl(pane) // Why: the fitPanes() optimization skips panes whose dimensions are // unchanged (common when a worktree goes hidden→visible at the same @@ -315,6 +325,7 @@ export class PaneManager { this.handlePaneMouseEnter(paneId, event) } ) + pane.webglAttachmentDeferred = this.renderingSuspended this.panes.set(id, pane) return pane } @@ -357,18 +368,6 @@ export class PaneManager { applyDividerStyles(this.root, this.styleOptions) } - private toPublic(pane: ManagedPaneInternal): ManagedPane { - return { - id: pane.id, - terminal: pane.terminal, - container: pane.container, - linkTooltip: pane.linkTooltip, - fitAddon: pane.fitAddon, - searchAddon: pane.searchAddon, - serializeAddon: pane.serializeAddon - } - } - /** Build the callbacks object for drag-reorder functions. */ private getDragCallbacks() { return { diff --git a/src/renderer/src/lib/pane-manager/pane-public-view.ts b/src/renderer/src/lib/pane-manager/pane-public-view.ts new file mode 100644 index 00000000000..c0ae326b386 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-public-view.ts @@ -0,0 +1,13 @@ +import type { ManagedPane, ManagedPaneInternal } from './pane-manager-types' + +export function toPublicPane(pane: ManagedPaneInternal): ManagedPane { + return { + id: pane.id, + terminal: pane.terminal, + container: pane.container, + linkTooltip: pane.linkTooltip, + fitAddon: pane.fitAddon, + searchAddon: pane.searchAddon, + serializeAddon: pane.serializeAddon + } +} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-options.ts b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts new file mode 100644 index 00000000000..64d485e69f9 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts @@ -0,0 +1,21 @@ +import type { ITerminalOptions } from '@xterm/xterm' + +export function buildDefaultTerminalOptions(): ITerminalOptions { + return { + allowProposedApi: true, + cursorBlink: true, + cursorStyle: 'bar', + fontSize: 14, + // Cross-platform fallback chain; keep in sync with FALLBACK_FONTS in layout-serialization.ts. + fontFamily: + '"SF Mono", "Menlo", "Monaco", "Cascadia Mono", "Consolas", "DejaVu Sans Mono", "Liberation Mono", "Symbols Nerd Font Mono", "MesloLGS Nerd Font", "JetBrainsMono Nerd Font", "Hack Nerd Font", monospace', + fontWeight: '300', + fontWeightBold: '500', + scrollback: 10000, + allowTransparency: false, + // Why: on macOS, non-US layouts rely on Option to compose characters like @ and €. + macOptionIsMeta: false, + macOptionClickForcesSelection: true, + drawBoldTextInBrightColors: true + } +} diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts index 964e4554e28..18466e7c2df 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts @@ -37,6 +37,8 @@ function createPane({ xtermContainer: {} as never, linkTooltip: {} as never, gpuRenderingEnabled: true, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, fitAddon: { fit, proposeDimensions diff --git a/src/renderer/src/runtime/sync-runtime-graph.ts b/src/renderer/src/runtime/sync-runtime-graph.ts index 46dc9722fc1..3ebe8fb08a5 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.ts @@ -1,4 +1,5 @@ import { paneLeafId, serializePaneTree } from '@/components/terminal-pane/layout-serialization' +import { warnTerminalLifecycleAnomaly } from '@/components/terminal-pane/terminal-lifecycle-diagnostics' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { AppState } from '@/store/types' import type { RuntimeSyncWindowGraph } from '../../../shared/runtime-types' @@ -83,13 +84,26 @@ async function syncRuntimeGraph(): Promise { layout: serializePaneTree(root) }) + const savedPtyIdsByLeafId = state.terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {} for (const pane of manager?.getPanes() ?? []) { + const leafId = paneLeafId(pane.id) + const ptyId = registeredTab.getPtyIdForPane(pane.id) + const savedPtyId = savedPtyIdsByLeafId[leafId] ?? null + if (!ptyId && savedPtyId) { + warnTerminalLifecycleAnomaly('mounted terminal leaf has saved PTY but no live transport', { + tabId, + worktreeId: registeredTab.worktreeId, + leafId, + paneId: pane.id, + ptyId: savedPtyId + }) + } graph.leaves.push({ tabId, worktreeId: registeredTab.worktreeId, - leafId: paneLeafId(pane.id), + leafId, paneRuntimeId: pane.id, - ptyId: registeredTab.getPtyIdForPane(pane.id) + ptyId }) } } diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index 619b7d0fb93..5dc17fd5490 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -628,6 +628,37 @@ describe('setActiveWorktree', () => { expect(groups[0].tabOrder).toEqual([terminal.id]) }) + it('publishes the first terminal and root tab group atomically', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + groupsByWorktree: {}, + activeGroupIdByWorktree: {}, + unifiedTabsByWorktree: {} + }) + + const snapshots: { terminalCount: number; unifiedCount: number; groupCount: number }[] = [] + const unsubscribe = store.subscribe((state) => { + snapshots.push({ + terminalCount: state.tabsByWorktree[wt]?.length ?? 0, + unifiedCount: state.unifiedTabsByWorktree[wt]?.length ?? 0, + groupCount: state.groupsByWorktree[wt]?.length ?? 0 + }) + }) + + store.getState().createTab(wt) + unsubscribe() + + // Why: task-page launches queue startup/setup commands before React mounts. + // A terminal-only intermediate state can mount the legacy host and race + // the split-group host, duplicating setup panes and PTYs. + expect(snapshots).toEqual([{ terminalCount: 1, unifiedCount: 1, groupCount: 1 }]) + }) + it('syncs the global active surface when focusing a different split group', () => { const store = createTestStore() const wt = 'repo1::/path/wt1' diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index b907c67dd99..cf9597453ce 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -12,6 +12,14 @@ import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers' import { isClaudeAgent, detectAgentStatusFromTitle } from '@/lib/agent-status' import { buildOrphanTerminalCleanupPatch, getOrphanTerminalIds } from './terminal-orphan-helpers' +import { + dedupeTabOrder, + ensureGroup, + findTabByEntityInGroup, + pushRecentTabId, + sanitizeRecentTabIds, + updateGroup +} from './tab-group-state' import { ensurePtyDispatcher, unregisterPtyDataHandlers @@ -293,6 +301,7 @@ export const createTerminalSlice: StateCreator let tab!: TerminalTab set((s) => { const orphanTerminalIds = getOrphanTerminalIds(s, worktreeId) + const orphanCleanupPatch = buildOrphanTerminalCleanupPatch(s, worktreeId, orphanTerminalIds) const existing = (s.tabsByWorktree[worktreeId] ?? []).filter( (entry) => !orphanTerminalIds.has(entry.id) ) @@ -312,17 +321,73 @@ export const createTerminalSlice: StateCreator sortOrder: existing.length, createdAt: Date.now() } + const validTargetGroupId = + targetGroupId && s.groupsByWorktree[worktreeId]?.some((group) => group.id === targetGroupId) + ? targetGroupId + : undefined + const { group, groupsByWorktree, activeGroupIdByWorktree } = ensureGroup( + s.groupsByWorktree, + s.activeGroupIdByWorktree, + worktreeId, + validTargetGroupId ?? s.activeGroupIdByWorktree[worktreeId] + ) + const nextActiveGroupIdByWorktree = validTargetGroupId + ? { ...activeGroupIdByWorktree, [worktreeId]: validTargetGroupId } + : activeGroupIdByWorktree + const existingUnifiedTabs = s.unifiedTabsByWorktree[worktreeId] ?? [] + const existingTerminalTab = findTabByEntityInGroup( + s.unifiedTabsByWorktree, + worktreeId, + group.id, + id, + 'terminal' + ) + const unifiedTab = existingTerminalTab ?? { + id, + entityId: id, + groupId: group.id, + worktreeId, + contentType: 'terminal' as const, + label: tab.title, + customLabel: tab.customTitle, + color: tab.color, + sortOrder: dedupeTabOrder(group.tabOrder).length, + createdAt: tab.createdAt + } + const nextGroupOrder = dedupeTabOrder([...group.tabOrder, unifiedTab.id]) + const nextRecent = pushRecentTabId( + sanitizeRecentTabIds(group.recentTabIds, nextGroupOrder), + unifiedTab.id + ) return { - ...buildOrphanTerminalCleanupPatch(s, worktreeId, orphanTerminalIds), + ...orphanCleanupPatch, tabsByWorktree: { - ...s.tabsByWorktree, + ...orphanCleanupPatch.tabsByWorktree, [worktreeId]: [...existing, tab] }, - activeGroupIdByWorktree: - targetGroupId && - s.groupsByWorktree[worktreeId]?.some((group) => group.id === targetGroupId) - ? { ...s.activeGroupIdByWorktree, [worktreeId]: targetGroupId } - : s.activeGroupIdByWorktree, + // Why: task-page launch queues startup/setup work before React mounts + // the terminal. Publishing the unified tab atomically with the runtime + // tab prevents a transient legacy mount from racing the split host. + unifiedTabsByWorktree: { + ...s.unifiedTabsByWorktree, + [worktreeId]: existingTerminalTab + ? existingUnifiedTabs + : [...existingUnifiedTabs, unifiedTab] + }, + groupsByWorktree: { + ...groupsByWorktree, + [worktreeId]: updateGroup(groupsByWorktree[worktreeId] ?? [], { + ...group, + activeTabId: unifiedTab.id, + tabOrder: nextGroupOrder, + recentTabIds: nextRecent + }) + }, + activeGroupIdByWorktree: nextActiveGroupIdByWorktree, + layoutByWorktree: { + ...s.layoutByWorktree, + [worktreeId]: s.layoutByWorktree[worktreeId] ?? { type: 'leaf', groupId: group.id } + }, activeTabId: tab.id, activeTabIdByWorktree: { ...s.activeTabIdByWorktree, [worktreeId]: tab.id }, ptyIdsByTabId: { ...s.ptyIdsByTabId, [tab.id]: [] }, @@ -332,29 +397,6 @@ export const createTerminalSlice: StateCreator } } }) - const state = get() - const resolvedTargetGroupId = - targetGroupId ?? - state.activeGroupIdByWorktree[worktreeId] ?? - state.groupsByWorktree[worktreeId]?.[0]?.id ?? - state.ensureWorktreeRootGroup?.(worktreeId) - if ( - resolvedTargetGroupId && - !state.findTabForEntityInGroup(worktreeId, resolvedTargetGroupId, id, 'terminal') - ) { - // Why: a brand-new worktree can auto-create its first terminal before - // Terminal.tsx has mounted and seeded a root tab group. Force a root - // group here so the first terminal always gets a visible unified tab - // instead of existing only in the legacy terminal slice. - state.createUnifiedTab(worktreeId, 'terminal', { - id, - entityId: id, - label: tab.title, - customLabel: tab.customTitle, - color: tab.color, - targetGroupId: resolvedTargetGroupId - }) - } return tab },