fix(terminal): sync worktree config after PTY creation to fix first-attempt failure (#1213)
* fix(terminal): sync worktree config after PTY creation to fix first-attempt failure When selecting a worktree immediately after app launch, the terminal would fail to spawn on the first attempt but succeed on the second. Root cause: IPC calls to setTerminalWorktreeConfig and setTerminalTitle happened before the terminal existed in the main process, so the config wasn't persisted. On recreation, the new PTY was created but without the worktree association. Changes: - Add pendingWorktreeConfigRef to store config during recreation - Re-sync worktree config to main process in onCreated callback - Increase MAX_RECREATION_RETRIES from 10 to 30 (1s → 3s) to handle slow app startup scenarios where xterm dimensions take longer Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(terminal): address PR review findings for worktree config race conditions - Clear pendingWorktreeConfigRef on PTY creation error to prevent stale config - Add try/catch error handling for IPC calls in onCreated callback - Extract duplicated worktree recreation logic into applyWorktreeConfig helper --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -51,6 +51,10 @@ export const Terminal = forwardRef<TerminalHandle, TerminalProps>(function Termi
|
||||
// Track deliberate terminal recreation (e.g., worktree switching)
|
||||
// This prevents exit handlers from triggering auto-removal during controlled recreation
|
||||
const isRecreatingRef = useRef(false);
|
||||
// Store pending worktree config during recreation to sync after PTY creation
|
||||
// This fixes a race condition where IPC calls to set worktree config happen before
|
||||
// the terminal exists in main process, causing the config to not be persisted
|
||||
const pendingWorktreeConfigRef = useRef<TerminalWorktreeConfig | null>(null);
|
||||
|
||||
// Worktree dialog state
|
||||
const [showWorktreeDialog, setShowWorktreeDialog] = useState(false);
|
||||
@@ -165,8 +169,24 @@ export const Terminal = forwardRef<TerminalHandle, TerminalProps>(function Termi
|
||||
isRecreatingRef,
|
||||
onCreated: () => {
|
||||
isCreatedRef.current = true;
|
||||
// If there's a pending worktree config from a recreation attempt,
|
||||
// sync it to main process now that the terminal exists.
|
||||
// This fixes the race condition where IPC calls happen before terminal creation.
|
||||
if (pendingWorktreeConfigRef.current) {
|
||||
const config = pendingWorktreeConfigRef.current;
|
||||
try {
|
||||
window.electronAPI.setTerminalWorktreeConfig(id, config);
|
||||
window.electronAPI.setTerminalTitle(id, config.name);
|
||||
} catch (error) {
|
||||
console.error('Failed to sync worktree config after PTY creation:', error);
|
||||
}
|
||||
pendingWorktreeConfigRef.current = null;
|
||||
}
|
||||
},
|
||||
onError: (error) => {
|
||||
// Clear pending config on error to prevent stale config from being applied
|
||||
// if PTY is recreated later (fixes potential race condition on failed recreation)
|
||||
pendingWorktreeConfigRef.current = null;
|
||||
writeln(`\r\n\x1b[31mError: ${error}\x1b[0m`);
|
||||
},
|
||||
});
|
||||
@@ -298,23 +318,28 @@ Please confirm you're ready by saying: I'm ready to work on ${selectedTask.title
|
||||
setShowWorktreeDialog(true);
|
||||
}, []);
|
||||
|
||||
const handleWorktreeCreated = useCallback(async (config: TerminalWorktreeConfig) => {
|
||||
const applyWorktreeConfig = useCallback(async (config: TerminalWorktreeConfig) => {
|
||||
// IMPORTANT: Set isRecreatingRef BEFORE destruction to signal deliberate recreation
|
||||
// This prevents exit handlers from triggering auto-removal during controlled recreation
|
||||
isRecreatingRef.current = true;
|
||||
|
||||
// Store pending config to be synced after PTY creation succeeds
|
||||
// This fixes race condition where IPC calls happen before terminal exists in main process
|
||||
pendingWorktreeConfigRef.current = config;
|
||||
|
||||
// Set isCreatingRef BEFORE updating the store to prevent race condition
|
||||
// This prevents the PTY effect from running before destroyTerminal completes
|
||||
prepareForRecreate();
|
||||
|
||||
// Update terminal store with worktree config
|
||||
setWorktreeConfig(id, config);
|
||||
// Sync to main process so worktree config persists across hot reloads
|
||||
// Try to sync to main process (may be ignored if terminal doesn't exist yet)
|
||||
// The onCreated callback will re-sync using pendingWorktreeConfigRef
|
||||
window.electronAPI.setTerminalWorktreeConfig(id, config);
|
||||
|
||||
// Update terminal title and cwd to worktree path
|
||||
updateTerminal(id, { title: config.name, cwd: config.worktreePath });
|
||||
// Sync to main process so title persists across hot reloads
|
||||
// Try to sync to main process (may be ignored if terminal doesn't exist yet)
|
||||
window.electronAPI.setTerminalTitle(id, config.name);
|
||||
|
||||
// Destroy current PTY - a new one will be created in the worktree directory
|
||||
@@ -327,30 +352,13 @@ Please confirm you're ready by saying: I'm ready to work on ${selectedTask.title
|
||||
resetForRecreate();
|
||||
}, [id, setWorktreeConfig, updateTerminal, prepareForRecreate, resetForRecreate]);
|
||||
|
||||
const handleWorktreeCreated = useCallback(async (config: TerminalWorktreeConfig) => {
|
||||
await applyWorktreeConfig(config);
|
||||
}, [applyWorktreeConfig]);
|
||||
|
||||
const handleSelectWorktree = useCallback(async (config: TerminalWorktreeConfig) => {
|
||||
// IMPORTANT: Set isRecreatingRef BEFORE destruction to signal deliberate recreation
|
||||
// This prevents exit handlers from triggering auto-removal during controlled recreation
|
||||
isRecreatingRef.current = true;
|
||||
|
||||
// Set isCreatingRef BEFORE updating the store to prevent race condition
|
||||
prepareForRecreate();
|
||||
|
||||
// Same logic as handleWorktreeCreated - attach terminal to existing worktree
|
||||
setWorktreeConfig(id, config);
|
||||
// Sync to main process so worktree config persists across hot reloads
|
||||
window.electronAPI.setTerminalWorktreeConfig(id, config);
|
||||
updateTerminal(id, { title: config.name, cwd: config.worktreePath });
|
||||
// Sync to main process so title persists across hot reloads
|
||||
window.electronAPI.setTerminalTitle(id, config.name);
|
||||
|
||||
// Destroy current PTY - a new one will be created in the worktree directory
|
||||
if (isCreatedRef.current) {
|
||||
await window.electronAPI.destroyTerminal(id);
|
||||
isCreatedRef.current = false;
|
||||
}
|
||||
|
||||
resetForRecreate();
|
||||
}, [id, setWorktreeConfig, updateTerminal, prepareForRecreate, resetForRecreate]);
|
||||
await applyWorktreeConfig(config);
|
||||
}, [applyWorktreeConfig]);
|
||||
|
||||
const handleOpenInIDE = useCallback(async () => {
|
||||
const worktreePath = terminal?.worktreeConfig?.worktreePath;
|
||||
|
||||
@@ -2,7 +2,9 @@ import { useEffect, useRef, useCallback, useState, type RefObject } from 'react'
|
||||
import { useTerminalStore } from '../../stores/terminal-store';
|
||||
|
||||
// Maximum retry attempts for recreation when dimensions aren't ready
|
||||
const MAX_RECREATION_RETRIES = 10;
|
||||
// Increased from 10 to 30 (3 seconds total) to handle slow app startup scenarios
|
||||
// where xterm dimensions may take longer to stabilize
|
||||
const MAX_RECREATION_RETRIES = 30;
|
||||
// Delay between retry attempts in ms
|
||||
const RECREATION_RETRY_DELAY = 100;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user