fix: address PR #1831 review findings for webgl-context-management

- Remove unused fireEvent import from DisplaySettings test
- Add i18n helper text below GPU acceleration dropdown (en/fr)
- Replace positional selectCallbacks array with Map keyed by Select id
- Move debugLog after delete in terminal-session-store cleanup timer
- Remove redundant hasExited guard in pty-manager synchronous write path

Co-Authored-By: Claude Opus 4.6 <[email protected]>

apps/frontend/src/main/terminal-session-store.ts

@@ -663,9 +663,9 @@ 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);
+      debugLog('[TerminalSessionStore] Cleanup timer fired for:', sessionId,
+        'removing from pendingDelete. Remaining:', this.pendingDelete.size);
       this.pendingDeleteTimers.delete(sessionId);
     }, 5000);
     this.pendingDeleteTimers.set(sessionId, timer);

apps/frontend/src/main/terminal/pty-manager.ts

@@ -342,10 +342,6 @@ function performWrite(terminal: TerminalProcess, data: string): Promise<void> {
       setImmediate(writeChunk);
     } else {
       try {
-        if (terminal.hasExited) {
-          resolve();
-          return;
-        }
         terminal.pty.write(data);
         debugLog('[PtyManager:writeToPty] Write completed successfully');
       } catch (error) {

apps/frontend/src/renderer/components/settings/DisplaySettings.tsx

@@ -309,6 +309,9 @@ export function DisplaySettings({ settings, onSettingsChange }: DisplaySettingsP
               </SelectContent>
             </Select>
           </div>
+          <p className="text-xs text-muted-foreground">
+            {t('gpuAcceleration.helperText')}
+          </p>
         </div>
       </div>
     </SettingsSection>

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<string, (v: string) => 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 <div data-testid={`select-root-${idx}`} data-value={value}>{children}</div>;
+      currentSelectCallback = onValueChange;
+      return <div data-value={value}>{children}</div>;
+    },
+    SelectTrigger: ({ id, children }: { id?: string; className?: string; children: React.ReactNode }) => {
+      if (id && currentSelectCallback) {
+        selectCallbacks.set(id, currentSelectCallback);
+        currentSelectCallback = null;
+      }
+      return <button data-testid={`select-trigger-${id || 'unknown'}`}>{children}</button>;
     },
-    SelectTrigger: ({ id, children }: { id?: string; className?: string; children: React.ReactNode }) => (
-      <button data-testid={`select-trigger-${id || 'unknown'}`}>{children}</button>
-    ),
     SelectValue: () => null,
     SelectContent: ({ children }: { className?: string; children: React.ReactNode }) => (
       <div data-testid="select-content">{children}</div>
@@ -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(
       <DisplaySettings settings={settingsWithOn} onSettingsChange={mockOnSettingsChange} />
     );
 
-    // 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(
       <DisplaySettings settings={settingsWithoutGpu} onSettingsChange={mockOnSettingsChange} />
     );
 
-    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', () => {
       <DisplaySettings settings={defaultSettings} onSettingsChange={mockOnSettingsChange} />
     );
 
-    // 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', () => {
       <DisplaySettings settings={defaultSettings} onSettingsChange={mockOnSettingsChange} />
     );
 
-    selectCallbacks[1]('off');
+    selectCallbacks.get('gpuAcceleration')!('off');
 
     expect(mockOnSettingsChange).toHaveBeenCalledWith(
       expect.objectContaining({ gpuAcceleration: 'off' })
@@ -132,7 +138,7 @@ describe('DisplaySettings - GPU Acceleration Dropdown', () => {
       <DisplaySettings settings={settingsWithOff} onSettingsChange={mockOnSettingsChange} />
     );
 
-    selectCallbacks[1]('auto');
+    selectCallbacks.get('gpuAcceleration')!('auto');
 
     expect(mockOnSettingsChange).toHaveBeenCalledWith(
       expect.objectContaining({ gpuAcceleration: 'auto' })

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",

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",