diff --git a/src/main/index.ts b/src/main/index.ts index a6eeda5d..b6916518 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -84,6 +84,12 @@ import { TeamInboxReader } from './services/team/TeamInboxReader'; import { TeamSentMessagesStore } from './services/team/TeamSentMessagesStore'; import { getAppIconPath } from './utils/appIcon'; import { getProjectsBasePath, getTeamsBasePath, getTodosBasePath } from './utils/pathDecoder'; +import { + clearRendererAvailability, + markRendererReady, + markRendererUnavailable, + safeSendToRenderer, +} from './utils/safeWebContentsSend'; import { syncTelemetryFlag } from './sentry'; import { CliInstallerService, @@ -382,6 +388,8 @@ let httpServer: HttpServer; let schedulerService: SchedulerService; let skillsWatcherService: SkillsWatcherService | null = null; let teamBackupService: TeamBackupService | null = null; +let rendererRecoveryTimer: ReturnType | null = null; +let rendererRecoveryAttempts = 0; // File watcher event cleanup functions let fileChangeCleanup: (() => void) | null = null; @@ -472,9 +480,7 @@ function wireFileWatcherEvents(context: ServiceContext): void { // ignore } - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send('file-change', event); - } + safeSendToRenderer(mainWindow, 'file-change', event); httpServer?.broadcast('file-change', event); }; context.fileWatcher.on('file-change', fileChangeHandler); @@ -488,9 +494,7 @@ function wireFileWatcherEvents(context: ServiceContext): void { // Forward checklist-change events to renderer and HTTP SSE (mirrors file-change pattern above) const todoChangeHandler = (event: unknown): void => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send('todo-change', event); - } + safeSendToRenderer(mainWindow, 'todo-change', event); httpServer?.broadcast('todo-change', event); }; context.fileWatcher.on('todo-change', todoChangeHandler); @@ -498,9 +502,7 @@ function wireFileWatcherEvents(context: ServiceContext): void { // Forward team-change events to renderer and HTTP SSE const teamChangeHandler = (event: unknown): void => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(TEAM_CHANGE, event); - } + safeSendToRenderer(mainWindow, TEAM_CHANGE, event); httpServer?.broadcast('team-change', event); // Process inbox and task change events. @@ -648,13 +650,11 @@ function onContextSwitched(context: ServiceContext): void { rewireContextEvents(context); // Notify renderer of context change - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(SSH_STATUS, sshConnectionManager.getStatus()); - mainWindow.webContents.send(CONTEXT_CHANGED, { - id: context.id, - type: context.type, - }); - } + safeSendToRenderer(mainWindow, SSH_STATUS, sshConnectionManager.getStatus()); + safeSendToRenderer(mainWindow, CONTEXT_CHANGED, { + id: context.id, + type: context.type, + }); } /** @@ -835,30 +835,22 @@ function initializeServices(): void { // Allow TeamProvisioningService to trigger team refresh events (e.g. live lead replies). const teamChangeEmitter = (event: TeamChangeEvent): void => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(TEAM_CHANGE, event); - } + safeSendToRenderer(mainWindow, TEAM_CHANGE, event); httpServer?.broadcast('team-change', event); }; teamProvisioningService.setTeamChangeEmitter(teamChangeEmitter); // Allow SchedulerService to push schedule events to renderer schedulerService.setChangeEmitter((event) => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(SCHEDULE_CHANGE, event); - } + safeSendToRenderer(mainWindow, SCHEDULE_CHANGE, event); }); skillsWatcherService.setEmitter((event) => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(SKILLS_CHANGED, event); - } + safeSendToRenderer(mainWindow, SKILLS_CHANGED, event); }); teamProvisioningService.setToolApprovalEventEmitter((event) => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(TEAM_TOOL_APPROVAL_EVENT, event); - } + safeSendToRenderer(mainWindow, TEAM_TOOL_APPROVAL_EVENT, event); }); teamProvisioningService.setMainWindow(mainWindow); @@ -911,9 +903,7 @@ function initializeServices(): void { // Forward SSH state changes to renderer and HTTP SSE clients sshConnectionManager.on('state-change', (status: unknown) => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(SSH_STATUS, status); - } + safeSendToRenderer(mainWindow, SSH_STATUS, status); httpServer.broadcast('ssh:status', status); }); @@ -1061,7 +1051,35 @@ function syncTrafficLightPosition(win: BrowserWindow): void { if (process.platform === 'darwin') { win.setWindowButtonPosition(position); } - win.webContents.send(WINDOW_ZOOM_FACTOR_CHANGED_CHANNEL, zoomFactor); + safeSendToRenderer(win, WINDOW_ZOOM_FACTOR_CHANGED_CHANNEL, zoomFactor); +} + +function scheduleRendererRecovery(win: BrowserWindow): void { + if (rendererRecoveryTimer) { + return; + } + if (rendererRecoveryAttempts >= 2) { + logger.error('Renderer recovery limit reached; skipping automatic reload'); + return; + } + + rendererRecoveryAttempts += 1; + const delayMs = rendererRecoveryAttempts * 1000; + logger.warn(`Scheduling renderer recovery attempt ${rendererRecoveryAttempts} in ${delayMs}ms`); + + rendererRecoveryTimer = setTimeout(() => { + rendererRecoveryTimer = null; + if (!mainWindow || mainWindow !== win || win.isDestroyed()) { + return; + } + + markRendererUnavailable(win); + try { + win.webContents.reload(); + } catch (error) { + logger.error(`Renderer recovery reload failed: ${String(error)}`); + } + }, delayMs); } /** @@ -1090,6 +1108,7 @@ function createWindow(): void { ...(isMac && { trafficLightPosition: getTrafficLightPositionForZoom(1) }), title: 'Claude Agent Teams UI', }); + markRendererUnavailable(mainWindow); // In dev, forward selected renderer console warnings/errors to the main terminal. // Use the new single-argument event payload to avoid Electron deprecation warnings. @@ -1147,25 +1166,29 @@ function createWindow(): void { // Notify renderer when entering/leaving fullscreen (so traffic light padding can be removed) mainWindow.on('enter-full-screen', () => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(WINDOW_FULLSCREEN_CHANGED, true); - } + safeSendToRenderer(mainWindow, WINDOW_FULLSCREEN_CHANGED, true); }); mainWindow.on('leave-full-screen', () => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(WINDOW_FULLSCREEN_CHANGED, false); - } + safeSendToRenderer(mainWindow, WINDOW_FULLSCREEN_CHANGED, false); + }); + + mainWindow.webContents.on('did-start-loading', () => { + markRendererUnavailable(mainWindow); }); // Set traffic light position + notify renderer on first load, and auto-check for updates mainWindow.webContents.on('did-finish-load', () => { if (mainWindow && !mainWindow.isDestroyed()) { + markRendererReady(mainWindow); + rendererRecoveryAttempts = 0; + if (rendererRecoveryTimer) { + clearTimeout(rendererRecoveryTimer); + rendererRecoveryTimer = null; + } logger.warn('[startup] renderer did-finish-load'); syncTrafficLightPosition(mainWindow); setTimeout(() => { - if (mainWindow && !mainWindow.isDestroyed()) { - mainWindow.webContents.send(WINDOW_FULLSCREEN_CHANGED, mainWindow.isFullScreen()); - } + safeSendToRenderer(mainWindow, WINDOW_FULLSCREEN_CHANGED, mainWindow?.isFullScreen()); }, 0); // Start file watchers now that the window is visible and responsive. // Deferred from initializeServices() to avoid blocking window creation @@ -1234,7 +1257,7 @@ function createWindow(): void { // Prevent Cmd+N / Ctrl+N from opening new window; forward to renderer for review shortcuts if (isMod && input.key.toLowerCase() === 'n') { event.preventDefault(); - mainWindow.webContents.send('review:cmdN'); + safeSendToRenderer(mainWindow, 'review:cmdN'); return; } @@ -1264,6 +1287,11 @@ function createWindow(): void { }); mainWindow.on('closed', () => { + if (rendererRecoveryTimer) { + clearTimeout(rendererRecoveryTimer); + rendererRecoveryTimer = null; + } + clearRendererAvailability(mainWindow); mainWindow = null; // Clear main window references if (notificationManager) { @@ -1290,7 +1318,10 @@ function createWindow(): void { // Handle renderer process crashes (render-process-gone replaces deprecated 'crashed' event) mainWindow.webContents.on('render-process-gone', (_event, details) => { logger.error('Renderer process gone:', details.reason, details.exitCode); - // Could show an error dialog or attempt to reload the window + markRendererUnavailable(mainWindow); + const activeContext = contextRegistry.getActive(); + activeContext?.stopFileWatcher(); + scheduleRendererRecovery(mainWindow); }); // Set main window reference for notification manager and updater diff --git a/src/main/ipc/editor.ts b/src/main/ipc/editor.ts index 120df2ae..cd85dc94 100644 --- a/src/main/ipc/editor.ts +++ b/src/main/ipc/editor.ts @@ -7,6 +7,7 @@ import { getClaudeBasePath } from '@main/utils/pathDecoder'; import { isPathWithinRoot } from '@main/utils/pathValidation'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { EDITOR_CHANGE, EDITOR_CLOSE, @@ -367,9 +368,7 @@ async function handleEditorWatchDir( } // Forward event to renderer - if (mainWindowRef && !mainWindowRef.isDestroyed()) { - mainWindowRef.webContents.send(EDITOR_CHANGE, event); - } + safeSendToRenderer(mainWindowRef, EDITOR_CHANGE, event); }); } else { editorFileWatcher.stop(); diff --git a/src/main/ipc/review.ts b/src/main/ipc/review.ts index 7abb5ccc..a2acf0f4 100644 --- a/src/main/ipc/review.ts +++ b/src/main/ipc/review.ts @@ -8,6 +8,7 @@ import { createIpcWrapper } from '@main/ipc/ipcWrapper'; import { EditorFileWatcher } from '@main/services/editor'; import { ReviewDecisionStore } from '@main/services/team/ReviewDecisionStore'; import { validateFilePath } from '@main/utils/pathValidation'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { REVIEW_APPLY_DECISIONS, REVIEW_CHECK_CONFLICT, @@ -401,9 +402,7 @@ async function handleWatchReviewFiles( reviewFileWatcher.stop(); reviewWatcherProjectRoot = normalizedProjectPath; reviewFileWatcher.start(normalizedProjectPath, (event) => { - if (reviewMainWindowRef && !reviewMainWindowRef.isDestroyed()) { - reviewMainWindowRef.webContents.send(REVIEW_FILE_CHANGE, event); - } + safeSendToRenderer(reviewMainWindowRef, REVIEW_FILE_CHANGE, event); }); } diff --git a/src/main/services/infrastructure/CliInstallerService.ts b/src/main/services/infrastructure/CliInstallerService.ts index b8723175..4e4b0d8f 100644 --- a/src/main/services/infrastructure/CliInstallerService.ts +++ b/src/main/services/infrastructure/CliInstallerService.ts @@ -22,6 +22,7 @@ import { appendCliAuthDiag } from '@main/utils/cliAuthDiagLog'; import { buildEnrichedEnv } from '@main/utils/cliEnv'; import { buildMergedCliPath } from '@main/utils/cliPathMerge'; import { getClaudeBasePath, getHomeDir } from '@main/utils/pathDecoder'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { getCachedShellEnv, getShellPreferredHome, @@ -678,9 +679,7 @@ export class CliInstallerService { // --------------------------------------------------------------------------- private sendProgress(progress: CliInstallerProgress): void { - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.webContents.send(CLI_INSTALLER_PROGRESS_CHANNEL, progress); - } + safeSendToRenderer(this.mainWindow, CLI_INSTALLER_PROGRESS_CHANNEL, progress); } private detectPlatform(): CliPlatform { diff --git a/src/main/services/infrastructure/NotificationManager.ts b/src/main/services/infrastructure/NotificationManager.ts index c76543bd..4e3c1db7 100644 --- a/src/main/services/infrastructure/NotificationManager.ts +++ b/src/main/services/infrastructure/NotificationManager.ts @@ -17,6 +17,7 @@ import { getAppIconPath } from '@main/utils/appIcon'; import { getHomeDir } from '@main/utils/pathDecoder'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { stripMarkdown } from '@main/utils/textFormatting'; import { stripAgentBlocks } from '@shared/constants/agentBlocks'; import { createLogger } from '@shared/utils/logger'; @@ -481,7 +482,7 @@ export class NotificationManager extends EventEmitter { if (this.mainWindow && !this.mainWindow.isDestroyed()) { this.mainWindow.show(); this.mainWindow.focus(); - this.mainWindow.webContents.send('notification:clicked', stored); + safeSendToRenderer(this.mainWindow, 'notification:clicked', stored); } this.emit('notification-clicked', stored); } @@ -556,9 +557,7 @@ export class NotificationManager extends EventEmitter { * Emits a notification:new event to the renderer. */ private emitNewNotification(notification: StoredNotification): void { - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.webContents.send('notification:new', notification); - } + safeSendToRenderer(this.mainWindow, 'notification:new', notification); this.emit('notification-new', notification); } @@ -567,12 +566,10 @@ export class NotificationManager extends EventEmitter { * Emits a notification:updated event to the renderer. */ private emitNotificationUpdated(): void { - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.webContents.send('notification:updated', { - total: this.notifications.length, - unreadCount: this.getUnreadCountSync(), - }); - } + safeSendToRenderer(this.mainWindow, 'notification:updated', { + total: this.notifications.length, + unreadCount: this.getUnreadCountSync(), + }); this.emit('notification-updated', { total: this.notifications.length, diff --git a/src/main/services/infrastructure/PtyTerminalService.ts b/src/main/services/infrastructure/PtyTerminalService.ts index d1828bc9..89008836 100644 --- a/src/main/services/infrastructure/PtyTerminalService.ts +++ b/src/main/services/infrastructure/PtyTerminalService.ts @@ -14,6 +14,8 @@ import { TERMINAL_DATA, TERMINAL_EXIT } from '@preload/constants/ipcChannels'; import { createLogger } from '@shared/utils/logger'; import type { PtySpawnOptions } from '@shared/types/terminal'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; + import type { BrowserWindow } from 'electron'; const logger = createLogger('PtyTerminalService'); @@ -110,8 +112,6 @@ export class PtyTerminalService { } private send(channel: string, ...args: unknown[]): void { - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.webContents.send(channel, ...args); - } + safeSendToRenderer(this.mainWindow, channel, ...args); } } diff --git a/src/main/services/infrastructure/UpdaterService.ts b/src/main/services/infrastructure/UpdaterService.ts index 6cd0c7e1..170b0ca5 100644 --- a/src/main/services/infrastructure/UpdaterService.ts +++ b/src/main/services/infrastructure/UpdaterService.ts @@ -10,6 +10,7 @@ * artifacts for the current platform. */ +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { getErrorMessage } from '@shared/utils/errorHandling'; import { createLogger } from '@shared/utils/logger'; import electronUpdater from 'electron-updater'; @@ -132,9 +133,7 @@ export class UpdaterService { } private sendStatus(status: UpdaterStatus): void { - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.webContents.send('updater:status', status); - } + safeSendToRenderer(this.mainWindow, 'updater:status', status); } /** diff --git a/src/main/utils/safeWebContentsSend.ts b/src/main/utils/safeWebContentsSend.ts new file mode 100644 index 00000000..52d118ee --- /dev/null +++ b/src/main/utils/safeWebContentsSend.ts @@ -0,0 +1,54 @@ +import { createLogger } from '@shared/utils/logger'; + +import type { BrowserWindow } from 'electron'; + +const logger = createLogger('safeWebContentsSend'); +const rendererAvailability = new WeakMap(); + +export function markRendererReady(window: BrowserWindow | null | undefined): void { + if (!window || window.isDestroyed()) { + return; + } + rendererAvailability.set(window, true); +} + +export function markRendererUnavailable(window: BrowserWindow | null | undefined): void { + if (!window) { + return; + } + rendererAvailability.set(window, false); +} + +export function clearRendererAvailability(window: BrowserWindow | null | undefined): void { + if (!window) { + return; + } + rendererAvailability.delete(window); +} + +export function safeSendToRenderer( + window: BrowserWindow | null | undefined, + channel: string, + ...args: unknown[] +): boolean { + if (!window || window.isDestroyed()) { + return false; + } + + const contents = window.webContents; + if (!contents || contents.isDestroyed()) { + return false; + } + if (rendererAvailability.get(window) === false) { + return false; + } + + try { + contents.send(channel, ...args); + return true; + } catch (error) { + rendererAvailability.set(window, false); + logger.warn(`Failed to send "${channel}" to renderer: ${String(error)}`); + return false; + } +} diff --git a/test/main/utils/safeWebContentsSend.test.ts b/test/main/utils/safeWebContentsSend.test.ts new file mode 100644 index 00000000..b111af4b --- /dev/null +++ b/test/main/utils/safeWebContentsSend.test.ts @@ -0,0 +1,94 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { warn } = vi.hoisted(() => ({ + warn: vi.fn(), +})); + +vi.mock('@shared/utils/logger', () => ({ + createLogger: () => ({ + warn, + }), +})); + +import { + clearRendererAvailability, + markRendererReady, + markRendererUnavailable, + safeSendToRenderer, +} from '../../../src/main/utils/safeWebContentsSend'; + +import type { BrowserWindow } from 'electron'; + +function createWindow(options?: { + windowDestroyed?: boolean; + contentsDestroyed?: boolean; + sendImpl?: (...args: unknown[]) => void; +}): BrowserWindow { + return { + isDestroyed: vi.fn(() => options?.windowDestroyed ?? false), + webContents: { + isDestroyed: vi.fn(() => options?.contentsDestroyed ?? false), + send: vi.fn(options?.sendImpl ?? (() => undefined)), + }, + } as unknown as BrowserWindow; +} + +describe('safeSendToRenderer', () => { + beforeEach(() => { + warn.mockReset(); + }); + + it('sends IPC to a live renderer', () => { + const window = createWindow(); + + const result = safeSendToRenderer(window, 'test:channel', { ok: true }); + + expect(result).toBe(true); + expect(window.webContents.send).toHaveBeenCalledWith('test:channel', { ok: true }); + }); + + it('returns false when window is missing or destroyed', () => { + expect(safeSendToRenderer(null, 'test:channel')).toBe(false); + + const destroyedWindow = createWindow({ windowDestroyed: true }); + expect(safeSendToRenderer(destroyedWindow, 'test:channel')).toBe(false); + expect(destroyedWindow.webContents.send).not.toHaveBeenCalled(); + }); + + it('returns false when webContents is destroyed', () => { + const window = createWindow({ contentsDestroyed: true }); + + const result = safeSendToRenderer(window, 'test:channel'); + + expect(result).toBe(false); + expect(window.webContents.send).not.toHaveBeenCalled(); + }); + + it('swallows renderer disposal errors and logs a warning', () => { + const window = createWindow({ + sendImpl: () => { + throw new Error('Render frame was disposed before WebFrameMain could be accessed'); + }, + }); + + const result = safeSendToRenderer(window, 'test:channel', 123); + + expect(result).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain('test:channel'); + }); + + it('blocks sends while renderer is unavailable and resumes after ready', () => { + const window = createWindow(); + + markRendererUnavailable(window); + expect(safeSendToRenderer(window, 'test:channel', 'first')).toBe(false); + expect(window.webContents.send).not.toHaveBeenCalled(); + + markRendererReady(window); + expect(safeSendToRenderer(window, 'test:channel', 'second')).toBe(true); + expect(window.webContents.send).toHaveBeenCalledWith('test:channel', 'second'); + + clearRendererAvailability(window); + }); +});