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:
Andy
2026-01-17 22:27:43 +01:00
committed by GitHub
parent ba089c5b0c
commit 39236f185c
2 changed files with 37 additions and 27 deletions
@@ -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;