From f399cd8ce29b13e669d6fc456679aae6a954bfaf Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 19:07:37 +0100 Subject: [PATCH 1/6] feat(terminal): integrate WebGL context manager for GPU-accelerated rendering Wire up the existing webgl-context-manager into the terminal rendering pipeline with a user-configurable GPU Acceleration setting (auto/on/off). WebGL gives 3-5x rendering performance for terminals while falling back gracefully on unsupported browsers (Safari, some Linux+NVIDIA setups). - Add GpuAcceleration type and gpuAcceleration field to AppSettings - Add GPU Acceleration dropdown in Display Settings (i18n: en + fr) - Register/acquire WebGL context after xterm.open(), unregister on dispose - Respect user setting: 'off' skips WebGL, 'auto'/'on' acquires context - Add 13 tests covering WebGL lifecycle and DisplaySettings GPU dropdown Co-Authored-By: Claude Opus 4.6 --- .../components/settings/DisplaySettings.tsx | 38 +++- .../__tests__/DisplaySettings.test.tsx | 151 ++++++++++++++ .../terminal/__tests__/useXterm.test.ts | 184 ++++++++++++++++++ .../renderer/components/terminal/useXterm.ts | 13 ++ apps/frontend/src/shared/constants/config.ts | 4 +- .../src/shared/i18n/locales/en/settings.json | 7 + .../src/shared/i18n/locales/fr/settings.json | 7 + apps/frontend/src/shared/types/settings.ts | 5 + 8 files changed, 407 insertions(+), 2 deletions(-) create mode 100644 apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx diff --git a/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx b/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx index 13bd0695..fd922e05 100644 --- a/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx +++ b/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx @@ -6,7 +6,7 @@ import { Label } from '../ui/label'; import { SettingsSection } from './SettingsSection'; import { useSettingsStore } from '../../stores/settings-store'; import { UI_SCALE_MIN, UI_SCALE_MAX, UI_SCALE_DEFAULT, UI_SCALE_STEP } from '../../../shared/constants'; -import type { AppSettings } from '../../../shared/types'; +import type { AppSettings, GpuAcceleration } from '../../../shared/types'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select'; interface DisplaySettingsProps { @@ -274,6 +274,42 @@ export function DisplaySettings({ settings, onSettingsChange }: DisplaySettingsP + + {/* GPU Acceleration Setting */} +
+
+
+ +

+ {t('gpuAcceleration.description')} +

+
+ +
+
); diff --git a/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx b/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx new file mode 100644 index 00000000..37e71ff0 --- /dev/null +++ b/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx @@ -0,0 +1,151 @@ +/** + * @vitest-environment jsdom + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import '../../../../shared/i18n'; +import { DisplaySettings } from '../DisplaySettings'; +import type { AppSettings } from '../../../../shared/types'; + +// Mock the settings store +vi.mock('../../../stores/settings-store', () => ({ + useSettingsStore: vi.fn(() => ({ + updateSettings: vi.fn() + })) +})); + +// Track onValueChange callbacks per Select instance +let selectCallbacks: Array<(v: string) => void> = []; + +// Mock Radix Select to make it testable in jsdom (portals don't work in jsdom) +vi.mock('../../ui/select', () => { + return { + Select: ({ value, onValueChange, children }: { value: string; onValueChange: (v: string) => void; children: React.ReactNode }) => { + selectCallbacks.push(onValueChange); + const idx = selectCallbacks.length - 1; + return
{children}
; + }, + SelectTrigger: ({ id, children }: { id?: string; className?: string; children: React.ReactNode }) => ( + + ), + SelectValue: () => null, + SelectContent: ({ children }: { className?: string; children: React.ReactNode }) => ( +
{children}
+ ), + SelectItem: ({ value, children }: { value: string; children: React.ReactNode }) => ( +
+ {children} +
+ ) + }; +}); + +const defaultSettings: AppSettings = { + uiScale: 100, + logOrder: 'chronological', + gpuAcceleration: 'auto' +} as AppSettings; + +describe('DisplaySettings - GPU Acceleration Dropdown', () => { + let mockOnSettingsChange: (settings: AppSettings) => void; + + beforeEach(() => { + vi.clearAllMocks(); + selectCallbacks = []; + mockOnSettingsChange = vi.fn(); + }); + + it('should render the GPU acceleration dropdown with all 3 options', () => { + render( + + ); + + expect(screen.getByText('GPU Acceleration')).toBeInTheDocument(); + expect(screen.getByTestId('select-item-auto')).toBeInTheDocument(); + expect(screen.getByTestId('select-item-on')).toBeInTheDocument(); + expect(screen.getByTestId('select-item-off')).toBeInTheDocument(); + }); + + it('should display the correct translated labels for each option', () => { + render( + + ); + + expect(screen.getByText('Auto (recommended)')).toBeInTheDocument(); + expect(screen.getByText('Always on')).toBeInTheDocument(); + expect(screen.getByText('Off')).toBeInTheDocument(); + }); + + it('should display the current GPU acceleration value from settings', () => { + const settingsWithOn: AppSettings = { ...defaultSettings, gpuAcceleration: 'on' }; + + const { container } = render( + + ); + + // The GPU acceleration select is the second Select rendered (index 1, after logOrder) + const gpuSelect = container.querySelector('[data-testid="select-root-1"]'); + expect(gpuSelect).toHaveAttribute('data-value', 'on'); + }); + + it('should default to "auto" when gpuAcceleration is not set', () => { + const settingsWithoutGpu: AppSettings = { ...defaultSettings, gpuAcceleration: undefined }; + + const { container } = render( + + ); + + const gpuSelect = container.querySelector('[data-testid="select-root-1"]'); + expect(gpuSelect).toHaveAttribute('data-value', 'auto'); + }); + + it('should call onSettingsChange with gpuAcceleration "on" when selected', () => { + render( + + ); + + // selectCallbacks[1] is the GPU acceleration Select's onValueChange + selectCallbacks[1]('on'); + + expect(mockOnSettingsChange).toHaveBeenCalledWith( + expect.objectContaining({ gpuAcceleration: 'on' }) + ); + }); + + it('should call onSettingsChange with gpuAcceleration "off" when selected', () => { + render( + + ); + + selectCallbacks[1]('off'); + + expect(mockOnSettingsChange).toHaveBeenCalledWith( + expect.objectContaining({ gpuAcceleration: 'off' }) + ); + }); + + it('should call onSettingsChange with gpuAcceleration "auto" when selected', () => { + const settingsWithOff: AppSettings = { ...defaultSettings, gpuAcceleration: 'off' }; + + render( + + ); + + selectCallbacks[1]('auto'); + + expect(mockOnSettingsChange).toHaveBeenCalledWith( + expect.objectContaining({ gpuAcceleration: 'auto' }) + ); + }); + + it('should render the GPU acceleration description text', () => { + render( + + ); + + expect( + screen.getByText('Use WebGL for terminal rendering (faster with many terminals)') + ).toBeInTheDocument(); + }); +}); diff --git a/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts b/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts index 49494676..9ff33504 100644 --- a/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts +++ b/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts @@ -66,11 +66,35 @@ vi.mock('@xterm/addon-serialize', () => ({ vi.mock('../../../../lib/terminal-buffer-manager', () => ({ terminalBufferManager: { get: vi.fn(() => ''), + getAndClear: vi.fn(() => ''), set: vi.fn(), clear: vi.fn() } })); +// Mock WebGL context manager +const mockWebglRegister = vi.fn(); +const mockWebglAcquire = vi.fn(); +const mockWebglUnregister = vi.fn(); +vi.mock('../../../lib/webgl-context-manager', () => ({ + webglContextManager: { + register: (...args: unknown[]) => mockWebglRegister(...args), + acquire: (...args: unknown[]) => mockWebglAcquire(...args), + unregister: (...args: unknown[]) => mockWebglUnregister(...args), + } +})); + +// Mock settings store (for gpuAcceleration setting) +const mockSettingsStoreState = { + settings: { gpuAcceleration: 'auto' as string | undefined } +}; +vi.mock('../../../stores/settings-store', () => ({ + useSettingsStore: Object.assign(vi.fn(), { + getState: () => mockSettingsStoreState, + subscribe: vi.fn(() => vi.fn()), + }) +})); + // Mock navigator.platform for platform detection const originalNavigatorPlatform = navigator.platform; @@ -837,3 +861,163 @@ describe('useXterm keyboard handlers', () => { }); }); }); + +describe('useXterm WebGL context management', () => { + // Mock requestAnimationFrame for jsdom environment + const originalRequestAnimationFrame = global.requestAnimationFrame; + const originalCancelAnimationFrame = global.cancelAnimationFrame; + + beforeAll(() => { + global.requestAnimationFrame = vi.fn((cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number); + global.cancelAnimationFrame = vi.fn((id: number) => clearTimeout(id)); + }); + + afterAll(() => { + global.requestAnimationFrame = originalRequestAnimationFrame; + global.cancelAnimationFrame = originalCancelAnimationFrame; + }); + + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + + // Reset gpuAcceleration to default + mockSettingsStoreState.settings.gpuAcceleration = 'auto'; + + // Mock ResizeObserver + global.ResizeObserver = vi.fn().mockImplementation(function() { + return { observe: vi.fn(), unobserve: vi.fn(), disconnect: vi.fn() }; + }); + + // Mock window.electronAPI + (window as unknown as { electronAPI: unknown }).electronAPI = { + sendTerminalInput: vi.fn(), + openExternal: vi.fn(), + }; + }); + + afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + /** + * Helper to render useXterm and wait for initialization + */ + async function renderUseXterm(terminalId = 'test-webgl-terminal') { + // Set up XTerm mock with dispose tracking + const mockDispose = vi.fn(); + (XTerm as unknown as Mock).mockImplementation(function() { + return { + open: vi.fn(), + loadAddon: vi.fn(), + attachCustomKeyEventHandler: vi.fn(), + hasSelection: vi.fn(() => false), + getSelection: vi.fn(() => ''), + paste: vi.fn(), + input: vi.fn(), + onData: vi.fn(), + onResize: vi.fn(), + dispose: mockDispose, + write: vi.fn(), + cols: 80, + rows: 24, + options: { + cursorBlink: true, + cursorStyle: 'block', + fontSize: 14, + fontFamily: 'monospace', + fontWeight: 'normal', + lineHeight: 1, + letterSpacing: 0, + theme: { cursorAccent: '#000000' }, + scrollback: 1000 + }, + refresh: vi.fn() + }; + }); + + const { FitAddon } = await import('@xterm/addon-fit'); + (FitAddon as unknown as Mock).mockImplementation(function() { + return { fit: vi.fn(), dispose: vi.fn() }; + }); + + const { WebLinksAddon } = await import('@xterm/addon-web-links'); + (WebLinksAddon as unknown as Mock).mockImplementation(function() { + return {}; + }); + + const { SerializeAddon } = await import('@xterm/addon-serialize'); + (SerializeAddon as unknown as Mock).mockImplementation(function() { + return { serialize: vi.fn(() => ''), dispose: vi.fn() }; + }); + + let disposeHook: (() => void) | null = null; + + const TestWrapper = () => { + const result = useXterm({ terminalId }); + // Expose dispose via ref so tests can call it + disposeHook = result.dispose; + return React.createElement('div', { ref: result.terminalRef }); + }; + + await act(async () => { + render(React.createElement(TestWrapper)); + }); + + return { disposeHook: () => disposeHook?.() }; + } + + it('should register and acquire WebGL context when gpuAcceleration is "auto"', async () => { + mockSettingsStoreState.settings.gpuAcceleration = 'auto'; + + await renderUseXterm('terminal-auto'); + + expect(mockWebglRegister).toHaveBeenCalledWith('terminal-auto', expect.anything()); + expect(mockWebglAcquire).toHaveBeenCalledWith('terminal-auto'); + }); + + it('should register and acquire WebGL context when gpuAcceleration is "on"', async () => { + mockSettingsStoreState.settings.gpuAcceleration = 'on'; + + await renderUseXterm('terminal-on'); + + expect(mockWebglRegister).toHaveBeenCalledWith('terminal-on', expect.anything()); + expect(mockWebglAcquire).toHaveBeenCalledWith('terminal-on'); + }); + + it('should register but NOT acquire WebGL context when gpuAcceleration is "off"', async () => { + mockSettingsStoreState.settings.gpuAcceleration = 'off'; + + await renderUseXterm('terminal-off'); + + expect(mockWebglRegister).toHaveBeenCalledWith('terminal-off', expect.anything()); + expect(mockWebglAcquire).not.toHaveBeenCalled(); + }); + + it('should unregister WebGL context on terminal disposal', async () => { + mockSettingsStoreState.settings.gpuAcceleration = 'auto'; + + const { disposeHook } = await renderUseXterm('terminal-dispose'); + + expect(mockWebglRegister).toHaveBeenCalledWith('terminal-dispose', expect.anything()); + + // Dispose the terminal + act(() => { + disposeHook(); + }); + + expect(mockWebglUnregister).toHaveBeenCalledWith('terminal-dispose'); + }); + + it('should fallback to "auto" when gpuAcceleration is undefined (upgrading users)', async () => { + mockSettingsStoreState.settings.gpuAcceleration = undefined; + + await renderUseXterm('terminal-undefined'); + + // When undefined, the ?? 'auto' fallback means acquire should be called + expect(mockWebglRegister).toHaveBeenCalledWith('terminal-undefined', expect.anything()); + expect(mockWebglAcquire).toHaveBeenCalledWith('terminal-undefined'); + }); +}); diff --git a/apps/frontend/src/renderer/components/terminal/useXterm.ts b/apps/frontend/src/renderer/components/terminal/useXterm.ts index f2382c7d..618723bc 100644 --- a/apps/frontend/src/renderer/components/terminal/useXterm.ts +++ b/apps/frontend/src/renderer/components/terminal/useXterm.ts @@ -10,6 +10,8 @@ import { isWindows as checkIsWindows, isLinux as checkIsLinux } from '../../lib/ import { debounce } from '../../lib/debounce'; import { DEFAULT_TERMINAL_THEME } from '../../lib/terminal-theme'; import { debugLog, debugError } from '../../../shared/utils/debug-logger'; +import { webglContextManager } from '../../lib/webgl-context-manager'; +import { useSettingsStore } from '../../stores/settings-store'; interface UseXtermOptions { terminalId: string; @@ -117,6 +119,14 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea xterm.open(terminalRef.current); + // WebGL acceleration: register terminal and acquire context based on user setting + // Must happen AFTER xterm.open() — xterm.js requires the terminal to be mounted first + const gpuAcceleration = useSettingsStore.getState().settings.gpuAcceleration ?? 'auto'; + webglContextManager.register(terminalId, xterm); + if (gpuAcceleration !== 'off') { + webglContextManager.acquire(terminalId); + } + // Platform detection for copy/paste shortcuts // Use existing os-detection module instead of custom implementation const isWindows = checkIsWindows(); @@ -552,6 +562,9 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea // Serialize buffer before disposing to preserve ANSI formatting serializeBuffer(); + // Release WebGL context before disposing addons and xterm + webglContextManager.unregister(terminalId); + // Dispose addons explicitly before disposing xterm // While xterm.dispose() handles loaded addons, explicit disposal ensures // resources are freed in a predictable order and prevents potential leaks diff --git a/apps/frontend/src/shared/constants/config.ts b/apps/frontend/src/shared/constants/config.ts index a9ae47eb..110900b7 100644 --- a/apps/frontend/src/shared/constants/config.ts +++ b/apps/frontend/src/shared/constants/config.ts @@ -67,7 +67,9 @@ export const DEFAULT_APP_SETTINGS = { // Anonymous error reporting (Sentry) - enabled by default to help improve the app sentryEnabled: true, // Auto-name Claude terminals based on initial message (enabled by default) - autoNameClaudeTerminals: true + autoNameClaudeTerminals: true, + // GPU acceleration for terminal rendering (auto = WebGL when supported) + gpuAcceleration: 'auto' as const }; // ============================================ diff --git a/apps/frontend/src/shared/i18n/locales/en/settings.json b/apps/frontend/src/shared/i18n/locales/en/settings.json index ab3ee21f..0eba8f26 100644 --- a/apps/frontend/src/shared/i18n/locales/en/settings.json +++ b/apps/frontend/src/shared/i18n/locales/en/settings.json @@ -192,6 +192,13 @@ "chronological": "Chronological (oldest first)", "reverseChronological": "Reverse-chronological (newest first)" }, + "gpuAcceleration": { + "label": "GPU Acceleration", + "description": "Use WebGL for terminal rendering (faster with many terminals)", + "auto": "Auto (recommended)", + "on": "Always on", + "off": "Off" + }, "general": { "otherAgentSettings": "Other Agent Settings", "otherAgentSettingsDescription": "Additional agent configuration options", diff --git a/apps/frontend/src/shared/i18n/locales/fr/settings.json b/apps/frontend/src/shared/i18n/locales/fr/settings.json index edcc812b..baf91f41 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/settings.json +++ b/apps/frontend/src/shared/i18n/locales/fr/settings.json @@ -192,6 +192,13 @@ "chronological": "Chronologique (plus ancien en premier)", "reverseChronological": "Chronologique inverse (plus récent en premier)" }, + "gpuAcceleration": { + "label": "Accélération GPU", + "description": "Utiliser WebGL pour le rendu des terminaux (plus rapide avec plusieurs terminaux)", + "auto": "Auto (recommandé)", + "on": "Toujours activé", + "off": "Désactivé" + }, "general": { "otherAgentSettings": "Autres paramètres de l'agent", "otherAgentSettingsDescription": "Options de configuration supplémentaires de l'agent", diff --git a/apps/frontend/src/shared/types/settings.ts b/apps/frontend/src/shared/types/settings.ts index c6b03e52..77d3d6a3 100644 --- a/apps/frontend/src/shared/types/settings.ts +++ b/apps/frontend/src/shared/types/settings.ts @@ -294,8 +294,13 @@ export interface AppSettings { seenVersionWarnings?: string[]; // Sidebar collapsed state (icons only when true) sidebarCollapsed?: boolean; + // GPU acceleration for terminal rendering (WebGL) + gpuAcceleration?: GpuAcceleration; } +// GPU acceleration mode for terminal WebGL rendering +export type GpuAcceleration = 'auto' | 'on' | 'off'; + // Auto-Claude Source Environment Configuration (for auto-claude repo .env) export interface SourceEnvConfig { // Claude Authentication (required for ideation, roadmap generation, etc.) From 5495e6564eb3777fe043c699241d16d7b911776b Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 20:47:53 +0100 Subject: [PATCH 2/6] fix(terminal): wrap WebGL calls in try-catch and add crash diagnostics The WebGL register/acquire/unregister calls in useXterm were not wrapped in try-catch, meaning any failure during WebGL context acquisition (e.g., LRU eviction edge case, GPU memory pressure) would propagate as an uncaught exception and crash the renderer process. Also adds debug logging to terminal-session-store's pendingDelete mechanism to help diagnose a reported crash when spawning terminals after extended use with 5+ concurrent instances. The "Skipping save for deleted session" warning now includes the full pendingDelete state. Co-Authored-By: Claude Opus 4.6 --- .../src/main/terminal-session-store.ts | 9 ++++++++- .../renderer/components/terminal/useXterm.ts | 20 ++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/frontend/src/main/terminal-session-store.ts b/apps/frontend/src/main/terminal-session-store.ts index 3dcc598b..00e9f200 100644 --- a/apps/frontend/src/main/terminal-session-store.ts +++ b/apps/frontend/src/main/terminal-session-store.ts @@ -366,7 +366,9 @@ export class TerminalSessionStore { private updateSessionInMemory(session: TerminalSession): boolean { // Check if session was deleted - skip if pending deletion if (this.pendingDelete.has(session.id)) { - console.warn('[TerminalSessionStore] Skipping save for deleted session:', session.id); + debugLog('[TerminalSessionStore] Skipping save for deleted session:', session.id, + 'pendingDelete size:', this.pendingDelete.size, + 'all pending IDs:', [...this.pendingDelete].join(', ')); return false; } @@ -606,6 +608,9 @@ export class TerminalSessionStore { // Mark as pending delete BEFORE modifying data to prevent race condition // with in-flight saveSessionAsync() calls this.pendingDelete.add(sessionId); + debugLog('[TerminalSessionStore] removeSession: added to pendingDelete:', sessionId, + 'pendingDelete size:', this.pendingDelete.size, + 'all pending IDs:', [...this.pendingDelete].join(', ')); const todaySessions = this.getTodaysSessions(); if (todaySessions[projectPath]) { @@ -625,6 +630,8 @@ export class TerminalSessionStore { // Keep the ID in pendingDelete for a short time to handle any in-flight // async operations, then clean up to prevent memory leaks const timer = setTimeout(() => { + debugLog('[TerminalSessionStore] Cleanup timer fired for:', sessionId, + 'removing from pendingDelete. Remaining:', this.pendingDelete.size - 1); this.pendingDelete.delete(sessionId); this.pendingDeleteTimers.delete(sessionId); }, 5000); diff --git a/apps/frontend/src/renderer/components/terminal/useXterm.ts b/apps/frontend/src/renderer/components/terminal/useXterm.ts index 618723bc..c5a08709 100644 --- a/apps/frontend/src/renderer/components/terminal/useXterm.ts +++ b/apps/frontend/src/renderer/components/terminal/useXterm.ts @@ -121,10 +121,16 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea // WebGL acceleration: register terminal and acquire context based on user setting // Must happen AFTER xterm.open() — xterm.js requires the terminal to be mounted first - const gpuAcceleration = useSettingsStore.getState().settings.gpuAcceleration ?? 'auto'; - webglContextManager.register(terminalId, xterm); - if (gpuAcceleration !== 'off') { - webglContextManager.acquire(terminalId); + try { + const gpuAcceleration = useSettingsStore.getState().settings.gpuAcceleration ?? 'auto'; + debugLog(`[useXterm] WebGL init for ${terminalId}: gpuAcceleration=${gpuAcceleration}`); + webglContextManager.register(terminalId, xterm); + if (gpuAcceleration !== 'off') { + webglContextManager.acquire(terminalId); + } + } catch (error) { + // WebGL is a progressive enhancement — terminal works fine without it + debugError(`[useXterm] WebGL initialization failed for ${terminalId}, falling back to canvas renderer:`, error); } // Platform detection for copy/paste shortcuts @@ -563,7 +569,11 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea serializeBuffer(); // Release WebGL context before disposing addons and xterm - webglContextManager.unregister(terminalId); + try { + webglContextManager.unregister(terminalId); + } catch (error) { + debugError(`[useXterm] WebGL cleanup failed for ${terminalId}:`, error); + } // Dispose addons explicitly before disposing xterm // While xterm.dispose() handles loaded addons, explicit disposal ensures From d745aee48d67429bdb8485ec24f2eebf45dbb6a0 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 21:11:10 +0100 Subject: [PATCH 3/6] fix(terminal): resolve pendingDelete race and save contention causing crashes Two bugs in terminal session persistence caused crashes when terminals were destroyed and recreated with the same ID: 1. pendingDelete blocked legitimate terminal recreation: When a terminal exits and is recreated (worktree switch, shell restart), the 5-second pendingDelete window silently blocked all session saves for the new terminal, leaving it invisible to the session store. Added clearPendingDelete() to remove the guard when createTerminal() is called with a reused ID. 2. Sync save() and async saveAsync() raced on the same temp file: Both methods wrote to terminals.json.tmp without coordination, causing ENOENT errors when one renamed a file the other had already moved. save() now checks writeInProgress and defers to the async writer when a write is in-flight. Co-Authored-By: Claude Opus 4.6 --- .../src/main/terminal-session-store.ts | 33 +++++++++++++++++++ .../src/main/terminal/session-handler.ts | 12 +++++++ .../src/main/terminal/terminal-lifecycle.ts | 7 ++++ 3 files changed, 52 insertions(+) diff --git a/apps/frontend/src/main/terminal-session-store.ts b/apps/frontend/src/main/terminal-session-store.ts index 00e9f200..36e85168 100644 --- a/apps/frontend/src/main/terminal-session-store.ts +++ b/apps/frontend/src/main/terminal-session-store.ts @@ -195,8 +195,20 @@ export class TerminalSessionStore { * 1. Write to temp file * 2. Rotate current file to backup * 3. Rename temp to target (atomic on most filesystems) + * + * If an async write is in progress, defers to the async writer to avoid + * both operations competing for the same temp file (ENOENT race condition). */ private save(): void { + // If an async write is in progress, don't write synchronously — the async + // writer shares the same temp file path. Instead, mark a pending write so + // saveAsync() will re-save with the latest in-memory data when it finishes. + if (this.writeInProgress) { + this.writePending = true; + debugLog('[TerminalSessionStore] Deferring sync save — async write in progress'); + return; + } + try { const content = JSON.stringify(this.data, null, 2); @@ -598,6 +610,27 @@ export class TerminalSessionStore { return sessions.find(s => s.id === sessionId); } + /** + * Clear a session ID from pendingDelete, allowing saves to proceed. + * + * Called when a terminal is legitimately re-created with the same ID + * (e.g., worktree switching, terminal restart after exit). Without this, + * the 5-second pendingDelete window blocks session persistence for the + * new terminal. + */ + clearPendingDelete(sessionId: string): void { + if (this.pendingDelete.has(sessionId)) { + this.pendingDelete.delete(sessionId); + // Also clear the cleanup timer since we're explicitly clearing + const timer = this.pendingDeleteTimers.get(sessionId); + if (timer) { + clearTimeout(timer); + this.pendingDeleteTimers.delete(sessionId); + } + debugLog('[TerminalSessionStore] Cleared pendingDelete for re-created terminal:', sessionId); + } + } + /** * Remove a session (from today's sessions) * diff --git a/apps/frontend/src/main/terminal/session-handler.ts b/apps/frontend/src/main/terminal/session-handler.ts index f04e4a85..2be49c61 100644 --- a/apps/frontend/src/main/terminal/session-handler.ts +++ b/apps/frontend/src/main/terminal/session-handler.ts @@ -225,6 +225,18 @@ export function persistAllSessions(terminals: Map): voi }); } +/** + * Clear a terminal ID from pendingDelete, allowing session saves to proceed. + * + * Must be called when re-creating a terminal with a previously-used ID + * (e.g., worktree switching, terminal restart after shell exit). Without this, + * the pendingDelete guard blocks persistence for the new terminal. + */ +export function clearPendingDelete(terminalId: string): void { + const store = getTerminalSessionStore(); + store.clearPendingDelete(terminalId); +} + /** * Remove a session from persistent storage */ diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 0105f6ae..48107b0e 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -54,6 +54,13 @@ export async function createTerminal( return { success: true }; } + // Clear any pendingDelete for this terminal ID. This handles the case where + // a terminal is destroyed and immediately re-created with the same ID (e.g., + // worktree switching, terminal restart after shell exit). Without this, the + // pendingDelete guard (5-second window) blocks session persistence for the + // new terminal, causing it to be invisible to the session store. + SessionHandler.clearPendingDelete(id); + try { // For auth terminals, don't inject existing OAuth token - we want a fresh login const profileEnv = skipOAuthToken ? {} : PtyManager.getActiveProfileEnv(); From 9e276c4757d66e0eef2a4f03cdd1e13f961bee1a Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sun, 15 Feb 2026 16:12:20 +0100 Subject: [PATCH 4/6] fix(logging): enhance error handling in app-logger and terminal processes - Introduced safe logging functions to prevent crashes from console write failures and unhandled errors. - Updated error logging setup to use safeLogUnhandled for uncaught exceptions and unhandled rejections. - Added guards in terminal process management to avoid operations on exited PTYs, preventing potential crashes. - Improved WebGL context management in terminal rendering to ensure stability during GPU acceleration. This commit aims to enhance the robustness of the logging and terminal handling mechanisms, addressing potential crash scenarios and improving overall application stability. --- apps/frontend/src/main/app-logger.ts | 46 ++++- apps/frontend/src/main/index.ts | 12 +- .../claude-integration-handler.test.ts | 50 ++++- .../terminal/claude-integration-handler.ts | 190 ++++++++---------- .../frontend/src/main/terminal/pty-manager.ts | 47 ++++- .../main/terminal/terminal-event-handler.ts | 6 +- .../src/main/terminal/terminal-lifecycle.ts | 1 + apps/frontend/src/main/terminal/types.ts | 2 + .../components/settings/DisplaySettings.tsx | 2 +- .../__tests__/DisplaySettings.test.tsx | 10 +- .../terminal/__tests__/useXterm.test.ts | 39 +++- .../renderer/components/terminal/useXterm.ts | 50 +++-- .../src/renderer/lib/webgl-context-manager.ts | 3 + apps/frontend/src/shared/constants/config.ts | 6 +- .../src/shared/i18n/locales/en/settings.json | 6 +- .../src/shared/i18n/locales/fr/settings.json | 6 +- .../frontend/src/shared/utils/debug-logger.ts | 22 +- 17 files changed, 330 insertions(+), 168 deletions(-) diff --git a/apps/frontend/src/main/app-logger.ts b/apps/frontend/src/main/app-logger.ts index 07429c19..dc984d41 100644 --- a/apps/frontend/src/main/app-logger.ts +++ b/apps/frontend/src/main/app-logger.ts @@ -38,6 +38,18 @@ log.transports.file.fileName = 'main.log'; // Console transport - always show warnings and errors, debug only in dev mode log.transports.console.level = process.env.NODE_ENV === 'development' ? 'debug' : 'warn'; log.transports.console.format = '[{h}:{i}:{s}] [{level}] {text}'; +// Guard console transport writes so broken stdio streams do not crash the app. +{ + const originalConsoleWriteFn = log.transports.console.writeFn as (...args: unknown[]) => void; + log.transports.console.writeFn = (...args: unknown[]) => { + try { + originalConsoleWriteFn(...args); + } catch (error) { + const err = error instanceof Error ? `${error.name}: ${error.message}` : String(error); + safeStderrWrite(`[app-logger] console transport write failed: ${err}`); + } + }; +} // Determine if this is a beta version function isBetaVersion(): boolean { @@ -204,14 +216,44 @@ export const appLog = { log: (...args: unknown[]) => log.info(...args), }; +/** + * Best-effort stderr fallback used when electron-log itself throws (e.g. EIO). + * Must never throw, especially inside uncaught exception handlers. + */ +function safeStderrWrite(message: string): void { + try { + process.stderr.write(`${message}\n`); + } catch { + // Ignore - nothing else we can safely do here. + } +} + +/** + * Log an unhandled error without risking recursive crashes if logger transport fails. + */ +function safeLogUnhandled(prefix: string, value: unknown): void { + try { + log.error(prefix, value); + } catch (loggingError) { + const loggingFailure = loggingError instanceof Error + ? `${loggingError.name}: ${loggingError.message}` + : String(loggingError); + const original = value instanceof Error + ? (value.stack || `${value.name}: ${value.message}`) + : String(value); + safeStderrWrite(`[app-logger] ${prefix} (logger failed: ${loggingFailure})`); + safeStderrWrite(original); + } +} + // Log unhandled errors export function setupErrorLogging(): void { process.on('uncaughtException', (error) => { - log.error('Uncaught exception:', error); + safeLogUnhandled('Uncaught exception:', error); }); process.on('unhandledRejection', (reason) => { - log.error('Unhandled rejection:', reason); + safeLogUnhandled('Unhandled rejection:', reason); }); log.info('Error logging initialized'); diff --git a/apps/frontend/src/main/index.ts b/apps/frontend/src/main/index.ts index f98725d3..c8644ed8 100644 --- a/apps/frontend/src/main/index.ts +++ b/apps/frontend/src/main/index.ts @@ -49,7 +49,7 @@ import { initializeAppUpdater, stopPeriodicUpdates } from './app-updater'; import { DEFAULT_APP_SETTINGS, IPC_CHANNELS, SPELL_CHECK_LANGUAGE_MAP, DEFAULT_SPELL_CHECK_LANGUAGE, ADD_TO_DICTIONARY_LABELS } from '../shared/constants'; import { getAppLanguage, initAppLanguage } from './app-language'; import { readSettingsFile } from './settings-utils'; -import { setupErrorLogging } from './app-logger'; +import { appLog, setupErrorLogging } from './app-logger'; import { initSentryMain } from './sentry'; import { preWarmToolCache } from './cli-tool-manager'; import { initializeClaudeProfileManager, getClaudeProfileManager } from './claude-profile-manager'; @@ -143,6 +143,11 @@ let mainWindow: BrowserWindow | null = null; let agentManager: AgentManager | null = null; let terminalManager: TerminalManager | null = null; +// Capture child process exits (renderer/GPU/utility) for crash diagnostics. +app.on('child-process-gone', (_event, details) => { + appLog.error('[main] child-process-gone:', details); +}); + // Re-entrancy guard for before-quit handler. // The first before-quit call pauses quit for async cleanup, then calls app.quit() again. // The second call sees isQuitting=true and allows quit to proceed immediately. @@ -214,6 +219,11 @@ function createWindow(): void { mainWindow?.show(); }); + // Capture renderer process crashes/termination reasons for diagnostics. + mainWindow.webContents.on('render-process-gone', (_event, details) => { + appLog.error('[main] render-process-gone:', details); + }); + // Configure initial spell check languages with proper fallback logic // Uses shared constant for consistency with the IPC handler const defaultLanguage = 'en'; diff --git a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts index 5126fd60..7ed5d600 100644 --- a/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts +++ b/apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts @@ -109,7 +109,16 @@ function mockPlatform(platform: 'win32' | 'darwin' | 'linux') { /** * Helper to get platform-specific expectations for PATH prefix */ -function getPathPrefixExpectation(platform: 'win32' | 'darwin' | 'linux', pathValue: string): string { +function getPathPrefixExpectation( + platform: 'win32' | 'darwin' | 'linux', + pathValue: string, + command: string +): string { + // Absolute executable commands no longer need PATH prefix injection. + if (path.isAbsolute(command)) { + return ''; + } + if (platform === 'win32') { // Windows: set "PATH=value" && return `set "PATH=${pathValue}" && `; @@ -118,6 +127,20 @@ function getPathPrefixExpectation(platform: 'win32' | 'darwin' | 'linux', pathVa return `PATH='${pathValue}' `; } +function expectPathPrefix( + written: string, + platform: 'win32' | 'darwin' | 'linux', + pathValue: string, + command: string +): void { + const expectedPrefix = getPathPrefixExpectation(platform, pathValue, command); + if (expectedPrefix) { + expect(written).toContain(expectedPrefix); + } else { + expect(written).not.toContain('PATH='); + } +} + /** * Helper to get platform-specific expectations for command quoting */ @@ -241,7 +264,7 @@ describe('claude-integration-handler', () => { const written = mockWriteToPty.mock.calls[0][1] as string; expect(written).toContain(buildCdCommand('/tmp/project')); - expect(written).toContain(getPathPrefixExpectation(platform, '/opt/claude/bin:/usr/bin')); + expectPathPrefix(written, platform, '/opt/claude/bin:/usr/bin', "/opt/claude bin/claude's"); expect(written).toContain(getQuotedCommand(platform, "/opt/claude bin/claude's")); expect(mockReleaseSessionId).toHaveBeenCalledWith('term-1'); expect(mockPersistSession).toHaveBeenCalledWith(terminal); @@ -402,7 +425,7 @@ describe('claude-integration-handler', () => { expect(written).toContain(histPrefix); expect(written).toContain(configDir); - expect(written).toContain(getPathPrefixExpectation(platform, '/opt/claude/bin:/usr/bin')); + expectPathPrefix(written, platform, '/opt/claude/bin:/usr/bin', command); expect(written).toContain(getQuotedCommand(platform, command)); expect(written).toContain(clearCmd); expect(profileManager.getProfile).toHaveBeenCalledWith('prof-2'); @@ -436,7 +459,7 @@ describe('claude-integration-handler', () => { const written = mockWriteToPty.mock.calls[0][1] as string; expect(written).toContain(getQuotedCommand(platform, command)); - expect(written).toContain(getPathPrefixExpectation(platform, '/opt/claude/bin:/usr/bin')); + expectPathPrefix(written, platform, '/opt/claude/bin:/usr/bin', command); expect(profileManager.getProfile).toHaveBeenCalledWith('prof-3'); expect(profileManager.markProfileUsed).toHaveBeenCalledWith('prof-3'); expect(mockPersistSession).toHaveBeenCalledWith(terminal); @@ -460,7 +483,7 @@ describe('claude-integration-handler', () => { resumeClaude(terminal, 'abc123', () => null); const resumeCall = mockWriteToPty.mock.calls[0][1] as string; - expect(resumeCall).toContain(getPathPrefixExpectation(platform, '/opt/claude/bin:/usr/bin')); + expectPathPrefix(resumeCall, platform, '/opt/claude/bin:/usr/bin', '/opt/claude/bin/claude'); expect(resumeCall).toContain(getQuotedCommand(platform, '/opt/claude/bin/claude') + ' --continue'); expect(resumeCall).not.toContain('--resume'); // sessionId is cleared because --continue doesn't track specific sessions @@ -656,7 +679,7 @@ describe('invokeClaudeAsync', () => { const written = mockWriteToPty.mock.calls[0][1] as string; expect(written).toContain(buildCdCommand('/tmp/project')); - expect(written).toContain(getPathPrefixExpectation(platform, '/opt/claude/bin:/usr/bin')); + expectPathPrefix(written, platform, '/opt/claude/bin:/usr/bin', '/opt/claude/bin/claude'); expect(mockReleaseSessionId).toHaveBeenCalledWith('term-1'); expect(mockPersistSession).toHaveBeenCalledWith(terminal); expect(profileManager.markProfileUsed).toHaveBeenCalledWith('default'); @@ -914,7 +937,8 @@ describe('claude-integration-handler - Helper Functions', () => { // Use a default terminal name pattern so renaming logic kicks in const terminal = createMockTerminal({ title: 'Terminal 1' }); const mockWindow = { - webContents: { send: vi.fn() } + isDestroyed: () => false, + webContents: { send: vi.fn(), isDestroyed: () => false } }; finalizeClaudeInvoke( @@ -934,7 +958,8 @@ describe('claude-integration-handler - Helper Functions', () => { // Use a default terminal name pattern so renaming logic kicks in const terminal = createMockTerminal({ title: 'Terminal 2' }); const mockWindow = { - webContents: { send: vi.fn() } + isDestroyed: () => false, + webContents: { send: vi.fn(), isDestroyed: () => false } }; finalizeClaudeInvoke( @@ -955,7 +980,8 @@ describe('claude-integration-handler - Helper Functions', () => { const terminal = createMockTerminal({ title: 'Terminal 3' }); const mockSend = vi.fn(); const mockWindow = { - webContents: { send: mockSend } + isDestroyed: () => false, + webContents: { send: mockSend, isDestroyed: () => false } }; finalizeClaudeInvoke( @@ -980,7 +1006,8 @@ describe('claude-integration-handler - Helper Functions', () => { const terminal = createMockTerminal({ title: 'Claude' }); const mockSend = vi.fn(); const mockWindow = { - webContents: { send: mockSend } + isDestroyed: () => false, + webContents: { send: mockSend, isDestroyed: () => false } }; finalizeClaudeInvoke( @@ -1004,7 +1031,8 @@ describe('claude-integration-handler - Helper Functions', () => { const terminal = createMockTerminal({ title: 'My Custom Terminal' }); const mockSend = vi.fn(); const mockWindow = { - webContents: { send: mockSend } + isDestroyed: () => false, + webContents: { send: mockSend, isDestroyed: () => false } }; finalizeClaudeInvoke( diff --git a/apps/frontend/src/main/terminal/claude-integration-handler.ts b/apps/frontend/src/main/terminal/claude-integration-handler.ts index ef4c92b9..3f46397b 100644 --- a/apps/frontend/src/main/terminal/claude-integration-handler.ts +++ b/apps/frontend/src/main/terminal/claude-integration-handler.ts @@ -16,6 +16,7 @@ import { getEmailFromConfigDir } from '../claude-profile/profile-utils'; import * as OutputParser from './output-parser'; import * as SessionHandler from './session-handler'; import * as PtyManager from './pty-manager'; +import { safeSendToRenderer } from '../ipc-handlers/utils'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; import { escapeShellArg, escapeForWindowsDoubleQuote, buildCdCommand } from '../../shared/utils/shell-escape'; import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; @@ -116,6 +117,19 @@ function normalizePathForBash(envPath: string): string { return isWindows() ? envPath.replace(/;/g, ':') : envPath; } +/** + * Determine whether a command already resolves via an absolute executable path. + * + * When true, we should avoid prefixing PATH=... into the typed shell command because: + * 1) PATH is not needed to locate the executable + * 2) very long PATH prefixes create huge echoed command lines that can stress terminal rendering + */ +function isAbsoluteExecutableCommand(command: string): boolean { + const trimmed = command.trim(); + if (!trimmed) return false; + return path.isAbsolute(trimmed); +} + /** * Generate temp file content for OAuth token based on platform * @@ -380,11 +394,8 @@ export function finalizeClaudeInvoke( : 'Claude'; terminal.title = title; - // Notify renderer of title change - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, title); - } + // Notify renderer of title change (use safeSendToRenderer to prevent SIGABRT on disposed frame) + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, title); } // Persist session if project path is available @@ -434,18 +445,15 @@ export function handleRateLimit( const autoSwitchSettings = profileManager.getAutoSwitchSettings(); const bestProfile = profileManager.getBestAvailableProfile(currentProfileId); - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_RATE_LIMIT, { - terminalId: terminal.id, - resetTime, - detectedAt: new Date().toISOString(), - profileId: currentProfileId, - suggestedProfileId: bestProfile?.id, - suggestedProfileName: bestProfile?.name, - autoSwitchEnabled: autoSwitchSettings.autoSwitchOnRateLimit - } as RateLimitEvent); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_RATE_LIMIT, { + terminalId: terminal.id, + resetTime, + detectedAt: new Date().toISOString(), + profileId: currentProfileId, + suggestedProfileId: bestProfile?.id, + suggestedProfileName: bestProfile?.name, + autoSwitchEnabled: autoSwitchSettings.autoSwitchOnRateLimit + } as RateLimitEvent); if (autoSwitchSettings.enabled && autoSwitchSettings.autoSwitchOnRateLimit && bestProfile) { console.warn('[ClaudeIntegration] Auto-switching to profile:', bestProfile.name); @@ -535,19 +543,16 @@ export function handleOAuthToken( // Set flag to watch for Claude's ready state (onboarding complete) terminal.awaitingOnboardingComplete = true; - const win = getWindow(); - if (win) { - // needsOnboarding: true tells the UI to show "complete setup" message - // instead of "success" - user should finish Claude's onboarding before closing - win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { - terminalId: terminal.id, - profileId, - email: emailFromOutput || keychainCreds.email || profile?.email, - success: true, - needsOnboarding: true, - detectedAt: new Date().toISOString() - } as OAuthTokenEvent); - } + // needsOnboarding: true tells the UI to show "complete setup" message + // instead of "success" - user should finish Claude's onboarding before closing + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { + terminalId: terminal.id, + profileId, + email: emailFromOutput || keychainCreds.email || profile?.email, + success: true, + needsOnboarding: true, + detectedAt: new Date().toISOString() + } as OAuthTokenEvent); } else { // Token not in Keychain yet, but profile may still be authenticated via configDir // Check if profile has valid auth (credentials exist in configDir) @@ -559,19 +564,16 @@ export function handleOAuthToken( // Set flag to watch for Claude's ready state (onboarding complete) terminal.awaitingOnboardingComplete = true; - const win = getWindow(); - if (win) { - // needsOnboarding: true tells the UI to show "complete setup" message - // instead of "success" - user should finish Claude's onboarding before closing - win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { - terminalId: terminal.id, - profileId, - email: emailFromOutput || profile?.email, - success: true, - needsOnboarding: true, - detectedAt: new Date().toISOString() - } as OAuthTokenEvent); - } + // needsOnboarding: true tells the UI to show "complete setup" message + // instead of "success" - user should finish Claude's onboarding before closing + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { + terminalId: terminal.id, + profileId, + email: emailFromOutput || profile?.email, + success: true, + needsOnboarding: true, + detectedAt: new Date().toISOString() + } as OAuthTokenEvent); } else { console.warn('[ClaudeIntegration] Login successful but Keychain token not found and no credentials in configDir - user may need to complete authentication manually'); } @@ -622,16 +624,13 @@ export function handleOAuthToken( clearKeychainCache(profile.configDir); console.warn('[ClaudeIntegration] Profile credentials verified (not caching token):', profileId); - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { - terminalId: terminal.id, - profileId, - email, - success: true, - detectedAt: new Date().toISOString() - } as OAuthTokenEvent); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { + terminalId: terminal.id, + profileId, + email, + success: true, + detectedAt: new Date().toISOString() + } as OAuthTokenEvent); } else { console.error('[ClaudeIntegration] Profile not found for OAuth token:', profileId); } @@ -645,17 +644,14 @@ export function handleOAuthToken( // Defensive null check for active profile if (!activeProfile) { console.error('[ClaudeIntegration] Failed to update profile: no active profile found'); - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { - terminalId: terminal.id, - profileId: undefined, - email, - success: false, - message: 'No active profile found', - detectedAt: new Date().toISOString() - } as OAuthTokenEvent); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { + terminalId: terminal.id, + profileId: undefined, + email, + success: false, + message: 'No active profile found', + detectedAt: new Date().toISOString() + } as OAuthTokenEvent); return; } @@ -686,16 +682,13 @@ export function handleOAuthToken( clearKeychainCache(activeProfile.configDir); console.warn('[ClaudeIntegration] Active profile credentials verified (not caching token):', activeProfile.name); - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { - terminalId: terminal.id, - profileId: activeProfile.id, - email, - success: true, - detectedAt: new Date().toISOString() - } as OAuthTokenEvent); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { + terminalId: terminal.id, + profileId: activeProfile.id, + email, + success: true, + detectedAt: new Date().toISOString() + } as OAuthTokenEvent); } } @@ -785,14 +778,11 @@ export function handleOnboardingComplete( } } - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_ONBOARDING_COMPLETE, { - terminalId: terminal.id, - profileId, - detectedAt: new Date().toISOString() - } as OnboardingCompleteEvent); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_ONBOARDING_COMPLETE, { + terminalId: terminal.id, + profileId, + detectedAt: new Date().toISOString() + } as OnboardingCompleteEvent); // Trigger immediate usage fetch after successful re-authentication // This gives the user immediate feedback that their account is working @@ -845,10 +835,7 @@ export function handleClaudeSessionId( SessionHandler.updateClaudeSessionId(terminal.projectPath, terminal.id, sessionId); } - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminal.id, sessionId); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminal.id, sessionId); } /** @@ -879,10 +866,7 @@ export function handleClaudeExit( } // Notify renderer to update UI - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_EXIT, terminal.id); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_CLAUDE_EXIT, terminal.id); } /** @@ -1109,7 +1093,9 @@ export function invokeClaude( const cwdCommand = buildCdCommand(cwd, terminal.shellType); const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); const escapedClaudeCmd = escapeShellCommand(claudeCmd); - const pathPrefix = buildPathPrefix(claudeEnv.PATH || ''); + const pathPrefix = isAbsoluteExecutableCommand(claudeCmd) + ? '' + : buildPathPrefix(claudeEnv.PATH || ''); const needsEnvOverride: boolean = !!(profileId && profileId !== previousProfileId); debugLog('[ClaudeIntegration:invokeClaude] Environment override check:', { @@ -1196,7 +1182,9 @@ export function resumeClaude( const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); const escapedClaudeCmd = escapeShellCommand(claudeCmd); - const pathPrefix = buildPathPrefix(claudeEnv.PATH || ''); + const pathPrefix = isAbsoluteExecutableCommand(claudeCmd) + ? '' + : buildPathPrefix(claudeEnv.PATH || ''); // Always use --continue which resumes the most recent session in the current directory. // This is more reliable than --resume with session IDs since Auto Claude already restores @@ -1220,10 +1208,7 @@ export function resumeClaude( // This preserves user-customized names and prevents renaming on every resume if (shouldAutoRenameTerminal(terminal.title)) { terminal.title = 'Claude'; - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); } // Persist session @@ -1313,7 +1298,9 @@ export async function invokeClaudeAsync( }); const escapedClaudeCmd = escapeShellCommand(claudeCmd); - const pathPrefix = buildPathPrefix(claudeEnv.PATH || ''); + const pathPrefix = isAbsoluteExecutableCommand(claudeCmd) + ? '' + : buildPathPrefix(claudeEnv.PATH || ''); const needsEnvOverride: boolean = !!(profileId && profileId !== previousProfileId); debugLog('[ClaudeIntegration:invokeClaudeAsync] Environment override check:', { @@ -1409,7 +1396,9 @@ export async function resumeClaudeAsync( }); const escapedClaudeCmd = escapeShellCommand(claudeCmd); - const pathPrefix = buildPathPrefix(claudeEnv.PATH || ''); + const pathPrefix = isAbsoluteExecutableCommand(claudeCmd) + ? '' + : buildPathPrefix(claudeEnv.PATH || ''); // Always use --continue which resumes the most recent session in the current directory. // This is more reliable than --resume with session IDs since Auto Claude already restores @@ -1433,10 +1422,7 @@ export async function resumeClaudeAsync( // This preserves user-customized names and prevents renaming on every resume if (shouldAutoRenameTerminal(terminal.title)) { terminal.title = 'Claude'; - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); } // Persist session (async, fire-and-forget to prevent main process blocking) diff --git a/apps/frontend/src/main/terminal/pty-manager.ts b/apps/frontend/src/main/terminal/pty-manager.ts index 4a188e34..01bca38d 100644 --- a/apps/frontend/src/main/terminal/pty-manager.ts +++ b/apps/frontend/src/main/terminal/pty-manager.ts @@ -209,18 +209,25 @@ export function setupPtyHandlers( onExitCallback: (terminal: TerminalProcess) => void ): void { const { id, pty: ptyProcess } = terminal; + terminal.hasExited = false; // Handle data from terminal ptyProcess.onData((data) => { // Shutdown guard (GitHub #1469): skip processing to avoid accessing // destroyed BrowserWindow.webContents, which triggers pty.node SIGABRT if (isShuttingDown) return; + if (terminal.hasExited) return; // Append to output buffer (limit to 100KB) terminal.outputBuffer = (terminal.outputBuffer + data).slice(-100000); - // Call custom data handler - onDataCallback(terminal, data); + // Call custom data handler. This must never crash the main process: + // parser logic in higher layers can throw on unexpected output. + try { + onDataCallback(terminal, data); + } catch (error) { + debugError('[PtyManager] onData callback failed for terminal:', id, 'error:', error); + } // Send to renderer with isDestroyed() check to prevent crashes // when the window is closed during terminal activity @@ -229,6 +236,9 @@ export function setupPtyHandlers( // Handle terminal exit ptyProcess.onExit(({ exitCode }) => { + terminal.hasExited = true; + // Drop any queued writes for this terminal to avoid writing to dead PTYs. + pendingWrites.delete(id); debugLog('[PtyManager] Terminal exited:', id, 'code:', exitCode); // Always resolve pending exit promises, even during shutdown @@ -248,8 +258,13 @@ export function setupPtyHandlers( // when the window is closed during terminal exit safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_EXIT, id, exitCode); - // Call custom exit handler - onExitCallback(terminal); + // Call custom exit handler. Guard against unexpected exceptions so PTY exit + // handling remains robust and doesn't take down the main process. + try { + onExitCallback(terminal); + } catch (error) { + debugError('[PtyManager] onExit callback failed for terminal:', id, 'error:', error); + } // Only delete if this is the SAME terminal object (not a newly created one with same ID). // This prevents a race where destroyTerminal() awaits PTY exit, a new terminal is created @@ -285,6 +300,11 @@ const pendingWrites = new Map>(); */ function performWrite(terminal: TerminalProcess, data: string): Promise { return new Promise((resolve) => { + if (terminal.hasExited) { + resolve(); + return; + } + // For large commands, write in chunks to prevent blocking if (data.length > CHUNKED_WRITE_THRESHOLD) { debugLog('[PtyManager:writeToPty] Large write detected, using chunked write'); @@ -293,7 +313,7 @@ function performWrite(terminal: TerminalProcess, data: string): Promise { const writeChunk = () => { // Check if terminal is still valid before writing - if (!terminal.pty) { + if (!terminal.pty || terminal.hasExited) { debugError('[PtyManager:writeToPty] Terminal PTY no longer valid, aborting chunked write'); resolve(); return; @@ -322,6 +342,10 @@ function performWrite(terminal: TerminalProcess, data: string): Promise { setImmediate(writeChunk); } else { try { + if (terminal.hasExited) { + resolve(); + return; + } terminal.pty.write(data); debugLog('[PtyManager:writeToPty] Write completed successfully'); } catch (error) { @@ -339,6 +363,10 @@ function performWrite(terminal: TerminalProcess, data: string): Promise { */ export function writeToPty(terminal: TerminalProcess, data: string): void { debugLog('[PtyManager:writeToPty] About to write to pty, data length:', data.length); + if (terminal.hasExited) { + debugError('[PtyManager:writeToPty] Skipping write to exited terminal:', terminal.id); + return; + } // Get the previous write Promise for this terminal (if any) const previousWrite = pendingWrites.get(terminal.id) || Promise.resolve(); @@ -366,6 +394,11 @@ export function writeToPty(terminal: TerminalProcess, data: string): void { * @returns true if resize was successful, false otherwise */ export function resizePty(terminal: TerminalProcess, cols: number, rows: number): boolean { + if (terminal.hasExited) { + debugError('[PtyManager] Resize skipped for exited terminal:', terminal.id); + return false; + } + // Validate dimensions if (cols <= 0 || rows <= 0 || !Number.isFinite(cols) || !Number.isFinite(rows)) { debugError('[PtyManager] Invalid resize dimensions - terminal:', terminal.id, 'cols:', cols, 'rows:', rows); @@ -394,6 +427,10 @@ export function resizePty(terminal: TerminalProcess, cols: number, rows: number) export function killPty(terminal: TerminalProcess, waitForExit: true): Promise; export function killPty(terminal: TerminalProcess, waitForExit?: false): void; export function killPty(terminal: TerminalProcess, waitForExit?: boolean): Promise | void { + if (terminal.hasExited) { + return waitForExit ? Promise.resolve() : undefined; + } + if (waitForExit) { const exitPromise = waitForPtyExit(terminal.id); try { diff --git a/apps/frontend/src/main/terminal/terminal-event-handler.ts b/apps/frontend/src/main/terminal/terminal-event-handler.ts index d6d3ca2f..4f5569d8 100644 --- a/apps/frontend/src/main/terminal/terminal-event-handler.ts +++ b/apps/frontend/src/main/terminal/terminal-event-handler.ts @@ -7,6 +7,7 @@ import * as OutputParser from './output-parser'; import * as ClaudeIntegration from './claude-integration-handler'; import type { TerminalProcess, WindowGetter } from './types'; import { IPC_CHANNELS } from '../../shared/constants'; +import { safeSendToRenderer } from '../ipc-handlers/utils'; /** * Event handler callbacks @@ -109,10 +110,7 @@ export function createEventCallbacks( ClaudeIntegration.handleOnboardingComplete(terminal, data, getWindow); }, onClaudeBusyChange: (terminal, isBusy) => { - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_BUSY, terminal.id, isBusy); - } + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_CLAUDE_BUSY, terminal.id, isBusy); }, onClaudeExit: (terminal) => { ClaudeIntegration.handleClaudeExit(terminal, getWindow); diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 48107b0e..7573402f 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -108,6 +108,7 @@ export async function createTerminal( id, pty: ptyProcess, isClaudeMode: false, + hasExited: false, projectPath, cwd: terminalCwd, outputBuffer: '', diff --git a/apps/frontend/src/main/terminal/types.ts b/apps/frontend/src/main/terminal/types.ts index d68ca2c1..357c55b6 100644 --- a/apps/frontend/src/main/terminal/types.ts +++ b/apps/frontend/src/main/terminal/types.ts @@ -28,6 +28,8 @@ export interface TerminalProcess { shellType?: WindowsShellType; /** Whether this terminal is waiting for Claude onboarding to complete (login flow) */ awaitingOnboardingComplete?: boolean; + /** Whether PTY has emitted exit; used to avoid writes/resizes on dead PTYs */ + hasExited?: boolean; } /** diff --git a/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx b/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx index fd922e05..78b9ab55 100644 --- a/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx +++ b/apps/frontend/src/renderer/components/settings/DisplaySettings.tsx @@ -287,7 +287,7 @@ export function DisplaySettings({ settings, onSettingsChange }: DisplaySettingsP

+

+ {t('gpuAcceleration.helperText')} +

diff --git a/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx b/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx index cb4b4133..b19f6481 100644 --- a/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx +++ b/apps/frontend/src/renderer/components/settings/__tests__/DisplaySettings.test.tsx @@ -2,7 +2,7 @@ * @vitest-environment jsdom */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; import '../../../../shared/i18n'; import { DisplaySettings } from '../DisplaySettings'; @@ -15,20 +15,24 @@ vi.mock('../../../stores/settings-store', () => ({ })) })); -// Track onValueChange callbacks per Select instance -let selectCallbacks: Array<(v: string) => void> = []; +// Track onValueChange callbacks per Select instance, keyed by the SelectTrigger id +let selectCallbacks: Map void> = new Map(); +let currentSelectCallback: ((v: string) => void) | null = null; // Mock Radix Select to make it testable in jsdom (portals don't work in jsdom) vi.mock('../../ui/select', () => { return { Select: ({ value, onValueChange, children }: { value: string; onValueChange: (v: string) => void; children: React.ReactNode }) => { - selectCallbacks.push(onValueChange); - const idx = selectCallbacks.length - 1; - return
{children}
; + currentSelectCallback = onValueChange; + return
{children}
; + }, + SelectTrigger: ({ id, children }: { id?: string; className?: string; children: React.ReactNode }) => { + if (id && currentSelectCallback) { + selectCallbacks.set(id, currentSelectCallback); + currentSelectCallback = null; + } + return ; }, - SelectTrigger: ({ id, children }: { id?: string; className?: string; children: React.ReactNode }) => ( - - ), SelectValue: () => null, SelectContent: ({ children }: { className?: string; children: React.ReactNode }) => (
{children}
@@ -52,7 +56,8 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => { beforeEach(() => { vi.clearAllMocks(); - selectCallbacks = []; + selectCallbacks = new Map(); + currentSelectCallback = null; mockOnSettingsChange = vi.fn(); }); @@ -80,23 +85,25 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => { it('should display the current GPU acceleration value from settings', () => { const settingsWithOn: AppSettings = { ...defaultSettings, gpuAcceleration: 'on' }; - const { container } = render( + render( ); - // The GPU acceleration select is the second Select rendered (index 1, after logOrder) - const gpuSelect = container.querySelector('[data-testid="select-root-1"]'); + // The GPU acceleration select is identified by its trigger id + const gpuTrigger = screen.getByTestId('select-trigger-gpuAcceleration'); + const gpuSelect = gpuTrigger.closest('[data-value]'); expect(gpuSelect).toHaveAttribute('data-value', 'on'); }); it('should default to "off" when gpuAcceleration is not set', () => { const settingsWithoutGpu: AppSettings = { ...defaultSettings, gpuAcceleration: undefined }; - const { container } = render( + render( ); - const gpuSelect = container.querySelector('[data-testid="select-root-1"]'); + const gpuTrigger = screen.getByTestId('select-trigger-gpuAcceleration'); + const gpuSelect = gpuTrigger.closest('[data-value]'); expect(gpuSelect).toHaveAttribute('data-value', 'off'); }); @@ -105,8 +112,7 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => { ); - // selectCallbacks[1] is the GPU acceleration Select's onValueChange - selectCallbacks[1]('on'); + selectCallbacks.get('gpuAcceleration')!('on'); expect(mockOnSettingsChange).toHaveBeenCalledWith( expect.objectContaining({ gpuAcceleration: 'on' }) @@ -118,7 +124,7 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => { ); - selectCallbacks[1]('off'); + selectCallbacks.get('gpuAcceleration')!('off'); expect(mockOnSettingsChange).toHaveBeenCalledWith( expect.objectContaining({ gpuAcceleration: 'off' }) @@ -132,7 +138,7 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => { ); - selectCallbacks[1]('auto'); + selectCallbacks.get('gpuAcceleration')!('auto'); expect(mockOnSettingsChange).toHaveBeenCalledWith( expect.objectContaining({ gpuAcceleration: 'auto' }) diff --git a/apps/frontend/src/shared/i18n/locales/en/settings.json b/apps/frontend/src/shared/i18n/locales/en/settings.json index 2d3ed29c..bc7fd8fa 100644 --- a/apps/frontend/src/shared/i18n/locales/en/settings.json +++ b/apps/frontend/src/shared/i18n/locales/en/settings.json @@ -197,7 +197,8 @@ "description": "Use WebGL for terminal rendering (experimental, faster with many terminals)", "auto": "Auto (use WebGL when supported)", "on": "Always on", - "off": "Off (default)" + "off": "Off (default)", + "helperText": "Changes apply to new terminals only" }, "general": { "otherAgentSettings": "Other Agent Settings", diff --git a/apps/frontend/src/shared/i18n/locales/fr/settings.json b/apps/frontend/src/shared/i18n/locales/fr/settings.json index 85977492..8d506e90 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/settings.json +++ b/apps/frontend/src/shared/i18n/locales/fr/settings.json @@ -197,7 +197,8 @@ "description": "Utiliser WebGL pour le rendu des terminaux (expérimental, plus rapide avec plusieurs terminaux)", "auto": "Auto (utiliser WebGL si supporté)", "on": "Toujours activé", - "off": "Désactivé (par défaut)" + "off": "Désactivé (par défaut)", + "helperText": "Les modifications s'appliquent uniquement aux nouveaux terminaux" }, "general": { "otherAgentSettings": "Autres paramètres de l'agent", From 04618ccb08968e593d3f68915c2b61653bac3d19 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Tue, 17 Feb 2026 15:33:42 +0100 Subject: [PATCH 6/6] fix: move debug log after delete to fix off-by-one count in cleanup timer The cleanup timer debug log was computing pendingDelete.size - 1 before the actual delete call, logging an incorrect remaining count. Moved both delete calls before the debug log so it reports the accurate size. Co-Authored-By: Claude Opus 4.6 --- apps/frontend/src/main/terminal-session-store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/frontend/src/main/terminal-session-store.ts b/apps/frontend/src/main/terminal-session-store.ts index 46a2e861..317abf4b 100644 --- a/apps/frontend/src/main/terminal-session-store.ts +++ b/apps/frontend/src/main/terminal-session-store.ts @@ -664,9 +664,9 @@ export class TerminalSessionStore { // async operations, then clean up to prevent memory leaks const timer = setTimeout(() => { this.pendingDelete.delete(sessionId); + this.pendingDeleteTimers.delete(sessionId); debugLog('[TerminalSessionStore] Cleanup timer fired for:', sessionId, 'removing from pendingDelete. Remaining:', this.pendingDelete.size); - this.pendingDeleteTimers.delete(sessionId); }, 5000); this.pendingDeleteTimers.set(sessionId, timer); }