Guard renderer IPC sends during crash recovery

This commit is contained in:
iliya 2026-03-27 17:07:40 +02:00
parent 9f8287755c
commit e1413a32bd
9 changed files with 240 additions and 68 deletions

View file

@ -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<typeof setTimeout> | 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

View file

@ -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();

View file

@ -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);
});
}

View file

@ -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 {

View file

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

View file

@ -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);
}
}

View file

@ -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);
}
/**

View file

@ -0,0 +1,54 @@
import { createLogger } from '@shared/utils/logger';
import type { BrowserWindow } from 'electron';
const logger = createLogger('safeWebContentsSend');
const rendererAvailability = new WeakMap<BrowserWindow, boolean>();
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;
}
}

View file

@ -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);
});
});