Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions apps/frontend/src/main/app-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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');
Expand Down
12 changes: 11 additions & 1 deletion apps/frontend/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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';
Expand Down
42 changes: 41 additions & 1 deletion apps/frontend/src/main/terminal-session-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -366,7 +378,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(', '));
Comment on lines +381 to +383
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Debug logging spreads pendingDelete on every invocation.

[...this.pendingDelete].join(', ') allocates an array and builds a string on every updateSessionInMemory and removeSession call, even if debug logging is effectively a no-op at runtime. If debugLog is not gated behind a compile-time or lazy-evaluation check, this is wasted work on a hot path (updateSessionInMemory is called by the periodic saveSessionAsync).

Consider deferring the serialization behind a condition or using a lazy callback pattern if debugLog supports one.

Also applies to: 611-613

🤖 Prompt for AI Agents
In `@apps/frontend/src/main/terminal-session-store.ts` around lines 369 - 371, The
debug logging currently eagerly serializes this.pendingDelete with
[...this.pendingDelete].join(', ') on every call in
updateSessionInMemory/removeSession, causing allocations on a hot path; change
the call so serialization only happens when debug logging is active — either
wrap the serialization in a conditional that checks the logger's enabled flag
before constructing the string, or switch to a lazy callback pattern if debugLog
supports a thunk (e.g., pass () => [...this.pendingDelete].join(', ')); update
the debugLog invocation sites around the debug message in TerminalSessionStore
(references: debugLog, pendingDelete, updateSessionInMemory, removeSession)
accordingly.

return false;
}

Expand Down Expand Up @@ -596,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)
*
Expand All @@ -606,6 +641,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]) {
Expand All @@ -627,6 +665,8 @@ export class TerminalSessionStore {
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);
}, 5000);
this.pendingDeleteTimers.set(sessionId, timer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}" && `;
Expand All @@ -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
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
Loading
Loading