-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(terminal): centralized auto-resume queue for terminal recovery #1835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 5 commits
1a11eb0
f53e2e0
5461377
b2cda4f
e332416
e36163f
8dc2a60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -76,6 +76,110 @@ export function writeToTerminal(terminalId: string, data: string): void { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| // === Auto-Resume Queue Coordinator === | ||||||
| // Coordinates staggered auto-resume of non-active terminals after app restart. | ||||||
| // Each terminal enqueues itself when its PTY is confirmed ready (onCreated). | ||||||
| // A single coordinator processes the queue with stagger delays. | ||||||
|
|
||||||
| const AUTO_RESUME_INITIAL_DELAY_MS = 1500; | ||||||
| const AUTO_RESUME_STAGGER_MS = 500; | ||||||
|
|
||||||
| // Auto-resume queue state (single-threaded JS assumption - see processAutoResumeQueue) | ||||||
| let autoResumeQueue: string[] = []; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For managing a collection of unique terminal IDs, using a While the performance difference is negligible with a maximum of 12 terminals, using a To implement this, you would change this line and update the related functions:
Suggested change
|
||||||
| let autoResumeTimer: ReturnType<typeof setTimeout> | null = null; | ||||||
| let autoResumeProcessing = false; | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| let autoResumeGeneration = 0; // Generation counter to abort stale processing runs | ||||||
| let isResumingAll = false; // Concurrency guard for resumeAllPendingClaude | ||||||
|
|
||||||
| export function enqueueAutoResume(terminalId: string): void { | ||||||
| if (autoResumeQueue.includes(terminalId)) return; | ||||||
| autoResumeQueue.push(terminalId); | ||||||
| debugLog(`[AutoResume] Enqueued terminal: ${terminalId}, queue size: ${autoResumeQueue.length}`); | ||||||
|
|
||||||
| // Start initial delay timer on first enqueue only | ||||||
| if (autoResumeTimer === null && !autoResumeProcessing) { | ||||||
| debugLog(`[AutoResume] Starting initial delay (${AUTO_RESUME_INITIAL_DELAY_MS}ms)`); | ||||||
| autoResumeTimer = setTimeout(() => { | ||||||
| autoResumeTimer = null; | ||||||
| processAutoResumeQueue(); | ||||||
| }, AUTO_RESUME_INITIAL_DELAY_MS); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export function dequeueAutoResume(terminalId: string): void { | ||||||
| const idx = autoResumeQueue.indexOf(terminalId); | ||||||
| if (idx !== -1) { | ||||||
| autoResumeQueue.splice(idx, 1); | ||||||
| debugLog(`[AutoResume] Dequeued terminal: ${terminalId}, queue size: ${autoResumeQueue.length}`); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export function clearAutoResumeQueue(): void { | ||||||
| autoResumeQueue = []; | ||||||
| if (autoResumeTimer !== null) { | ||||||
| clearTimeout(autoResumeTimer); | ||||||
| autoResumeTimer = null; | ||||||
| } | ||||||
| // Don't reset autoResumeProcessing here - the generation counter is sufficient | ||||||
| // to abort stale processing runs. Resetting the flag here creates a race condition | ||||||
| // where clearAutoResumeQueue() could allow a new processAutoResumeQueue() to start | ||||||
| // before the aborted one has fully exited. | ||||||
| autoResumeGeneration++; // Increment generation to abort any in-flight processing | ||||||
| debugLog('[AutoResume] Queue cleared'); | ||||||
|
Comment on lines
118
to
128
This comment was marked as outdated.
Sorry, something went wrong. |
||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Shared helper to resume a terminal's Claude session with consistent behavior. | ||||||
| * Clears the pending flag and triggers IPC activation. | ||||||
| */ | ||||||
| function resumeTerminalClaudeSession(terminalId: string): void { | ||||||
| useTerminalStore.getState().setPendingClaudeResume(terminalId, false); | ||||||
| window.electronAPI.activateDeferredClaudeResume(terminalId); | ||||||
| } | ||||||
|
|
||||||
| async function processAutoResumeQueue(): Promise<void> { | ||||||
| if (autoResumeProcessing) return; | ||||||
| autoResumeProcessing = true; | ||||||
| const generation = autoResumeGeneration; // Capture generation to detect cancellation | ||||||
| debugLog(`[AutoResume] Processing queue (generation ${generation}), ${autoResumeQueue.length} terminals`); | ||||||
|
|
||||||
| while (autoResumeQueue.length > 0) { | ||||||
| // Check if this processing run has been cancelled | ||||||
| if (generation !== autoResumeGeneration) { | ||||||
| debugLog(`[AutoResume] Generation mismatch (${generation} !== ${autoResumeGeneration}) — aborting stale processing`); | ||||||
| return; // Don't reset autoResumeProcessing — the new run owns it | ||||||
| } | ||||||
|
|
||||||
| const terminalId = autoResumeQueue.shift()!; | ||||||
|
|
||||||
| // Check if terminal still needs resume | ||||||
| const terminal = useTerminalStore.getState().terminals.find(t => t.id === terminalId); | ||||||
| if (!terminal?.pendingClaudeResume) { | ||||||
| debugLog(`[AutoResume] Skipping ${terminalId} — no longer pending`); | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| debugLog(`[AutoResume] Resuming terminal: ${terminalId}`); | ||||||
| resumeTerminalClaudeSession(terminalId); | ||||||
|
|
||||||
| // Stagger delay between resumes | ||||||
| if (autoResumeQueue.length > 0) { | ||||||
| await new Promise(resolve => setTimeout(resolve, AUTO_RESUME_STAGGER_MS)); | ||||||
| // Re-check generation after await (may have been cancelled during stagger delay) | ||||||
| if (generation !== autoResumeGeneration) { | ||||||
| debugLog(`[AutoResume] Generation mismatch after stagger — aborting stale processing`); | ||||||
| return; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Only reset processing flag if this is still the current generation | ||||||
| if (generation === autoResumeGeneration) { | ||||||
| autoResumeProcessing = false; | ||||||
| } | ||||||
| debugLog('[AutoResume] Queue processing complete'); | ||||||
| } | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| export type TerminalStatus = 'idle' | 'running' | 'claude-active' | 'exited'; | ||||||
|
|
||||||
| export interface Terminal { | ||||||
|
|
@@ -434,35 +538,51 @@ export const useTerminalStore = create<TerminalState>((set, get) => ({ | |||||
| }, | ||||||
|
|
||||||
| resumeAllPendingClaude: async () => { | ||||||
| const state = get(); | ||||||
|
|
||||||
| // Filter terminals with pending Claude resume | ||||||
| const pendingTerminals = state.terminals.filter(t => t.pendingClaudeResume === true); | ||||||
|
|
||||||
| if (pendingTerminals.length === 0) { | ||||||
| debugLog('[TerminalStore] No terminals with pending Claude resume'); | ||||||
| // Concurrency guard - prevent multiple simultaneous executions | ||||||
| if (isResumingAll) { | ||||||
| debugLog('[TerminalStore] Resume All already in progress — skipping'); | ||||||
| return; | ||||||
| } | ||||||
| isResumingAll = true; | ||||||
|
|
||||||
| debugLog(`[TerminalStore] Resuming ${pendingTerminals.length} pending Claude sessions with 500ms stagger`); | ||||||
| try { | ||||||
| // Clear auto-resume queue to prevent redundant processing | ||||||
| clearAutoResumeQueue(); | ||||||
|
Comment on lines
+557
to
+562
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixMove the line Prompt for AI Agent |
||||||
|
|
||||||
| // Iterate through terminals with staggered delays | ||||||
| for (let i = 0; i < pendingTerminals.length; i++) { | ||||||
| const terminal = pendingTerminals[i]; | ||||||
| // Clear the pending flag BEFORE IPC call to prevent race condition | ||||||
| // with auto-resume effect in Terminal.tsx (which checks this flag on a 100ms timeout) | ||||||
| get().setPendingClaudeResume(terminal.id, false); | ||||||
| const state = get(); | ||||||
|
|
||||||
| debugLog(`[TerminalStore] Activating deferred Claude resume for terminal: ${terminal.id}`); | ||||||
| window.electronAPI.activateDeferredClaudeResume(terminal.id); | ||||||
| // Filter terminals with pending Claude resume | ||||||
| const pendingTerminals = state.terminals.filter(t => t.pendingClaudeResume === true); | ||||||
|
|
||||||
| // Wait 500ms before processing next terminal (staggered delay) | ||||||
| if (i < pendingTerminals.length - 1) { | ||||||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||||||
| if (pendingTerminals.length === 0) { | ||||||
| debugLog('[TerminalStore] No terminals with pending Claude resume'); | ||||||
| return; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| debugLog('[TerminalStore] Completed resuming all pending Claude sessions'); | ||||||
| debugLog(`[TerminalStore] Resuming ${pendingTerminals.length} pending Claude sessions with ${AUTO_RESUME_STAGGER_MS}ms stagger`); | ||||||
|
|
||||||
| // Iterate through terminals with staggered delays | ||||||
| for (let i = 0; i < pendingTerminals.length; i++) { | ||||||
| const terminal = pendingTerminals[i]; | ||||||
|
|
||||||
| try { | ||||||
| debugLog(`[TerminalStore] Activating deferred Claude resume for terminal: ${terminal.id}`); | ||||||
| resumeTerminalClaudeSession(terminal.id); | ||||||
| } catch (error) { | ||||||
| // Log error and continue processing remaining terminals | ||||||
| debugError(`[TerminalStore] Error resuming terminal ${terminal.id}:`, error); | ||||||
| } | ||||||
|
|
||||||
| // Wait before processing next terminal (staggered delay) | ||||||
| if (i < pendingTerminals.length - 1) { | ||||||
| await new Promise(resolve => setTimeout(resolve, AUTO_RESUME_STAGGER_MS)); | ||||||
| } | ||||||
| } | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| debugLog('[TerminalStore] Completed resuming all pending Claude sessions'); | ||||||
| } finally { | ||||||
| isResumingAll = false; | ||||||
| } | ||||||
| }, | ||||||
|
Comment on lines
548
to
616
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial
If a project switch triggers Consider reusing the generation counter (or a dedicated one) to allow early exit, matching the pattern in Sketch: add generation-based cancellation resumeAllPendingClaude: async () => {
if (isResumingAll) {
debugLog('[TerminalStore] Resume All already in progress — skipping');
return;
}
isResumingAll = true;
+ const gen = autoResumeGeneration; // capture before clearAutoResumeQueue bumps it
try {
clearAutoResumeQueue();
// ...
for (let i = 0; i < pendingTerminals.length; i++) {
+ if (gen !== autoResumeGeneration) {
+ debugLog('[TerminalStore] Generation mismatch in resumeAll — aborting');
+ return;
+ }
// ... existing re-check + resume + stagger ...
}
} finally {
isResumingAll = false;
}
},Note: capture 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| getTerminal: (id: string) => { | ||||||
|
|
@@ -507,6 +627,7 @@ export async function restoreTerminalSessions(projectPath: string): Promise<void | |||||
| return; | ||||||
| } | ||||||
| restoringProjects.add(projectPath); | ||||||
| clearAutoResumeQueue(); | ||||||
|
|
||||||
| try { | ||||||
| const store = useTerminalStore.getState(); | ||||||
|
|
@@ -579,3 +700,7 @@ export async function restoreTerminalSessions(projectPath: string): Promise<void | |||||
| restoringProjects.delete(projectPath); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // NOTE: HMR cleanup for auto-resume queue state would be beneficial during development | ||||||
| // to clear timers on hot reload, but requires augmenting ImportMeta types in vite-env.d.ts. | ||||||
| // The generation counter provides sufficient protection against stale processing runs. | ||||||
Uh oh!
There was an error while loading. Please reload this page.