From d745aee48d67429bdb8485ec24f2eebf45dbb6a0 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 21:11:10 +0100 Subject: [PATCH] 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();