-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
auto-claude: 229-implement-account-aware-terminal-session-persisten #1819
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 19 commits
c2adf89
d78fac5
2c63663
839893c
4d74fd1
96a2648
531f7f8
aef8a68
1d06ad1
2062c4f
ad2a319
2874cd0
dea5aeb
962cfd8
a4047ed
5f66bde
e7f9908
93d3389
70af403
0d613f4
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 |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ import type { | |
| TerminalProcess, | ||
| WindowGetter, | ||
| TerminalOperationResult, | ||
| TerminalProfileChangeInfo | ||
| TerminalProfileChangeInfo, | ||
| } from './types'; | ||
| import * as PtyManager from './pty-manager'; | ||
| import * as SessionHandler from './session-handler'; | ||
|
|
@@ -26,6 +26,8 @@ export class TerminalManager { | |
| private saveTimer: NodeJS.Timeout | null = null; | ||
| private lastNotifiedRateLimitReset: Map<string, string> = new Map(); | ||
| private eventCallbacks: TerminalEventHandler.EventHandlerCallbacks; | ||
| /** Server-side storage for YOLO mode flags during profile migration (sessionId → flag) */ | ||
| private migratedSessionFlags: Map<string, boolean> = new Map(); | ||
|
|
||
| constructor(getWindow: WindowGetter) { | ||
| this.getWindow = getWindow; | ||
|
|
@@ -101,6 +103,12 @@ export class TerminalManager { | |
| * Destroy a terminal process | ||
| */ | ||
| async destroy(id: string): Promise<TerminalOperationResult> { | ||
| // Clean up migrated session flags if this terminal has a pending migrated session | ||
| const terminal = this.terminals.get(id); | ||
| if (terminal?.claudeSessionId) { | ||
| this.migratedSessionFlags.delete(terminal.claudeSessionId); | ||
| } | ||
|
Comment on lines
+106
to
+110
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: During a profile swap, Suggested FixThe flag should not be deleted from Prompt for AI Agent |
||
|
|
||
| return TerminalLifecycle.destroyTerminal( | ||
| id, | ||
| this.terminals, | ||
|
|
@@ -114,6 +122,7 @@ export class TerminalManager { | |
| * Kill all terminal processes | ||
| */ | ||
| async killAll(): Promise<void> { | ||
| this.migratedSessionFlags.clear(); | ||
| this.saveTimer = await TerminalLifecycle.destroyAllTerminals( | ||
| this.terminals, | ||
| this.saveTimer | ||
|
|
@@ -223,13 +232,32 @@ export class TerminalManager { | |
| /** | ||
| * Resume Claude in a terminal asynchronously (non-blocking) | ||
| */ | ||
| async resumeClaudeAsync(id: string, sessionId?: string): Promise<void> { | ||
| async resumeClaudeAsync(id: string, sessionId?: string, options?: { migratedSession?: boolean }): Promise<void> { | ||
| const terminal = this.terminals.get(id); | ||
| if (!terminal) { | ||
| return; | ||
| } | ||
|
|
||
| await ClaudeIntegration.resumeClaudeAsync(terminal, sessionId, this.getWindow); | ||
| // For migrated sessions, restore YOLO mode from server-side storage | ||
| // (set during profile change in storeMigratedSessionFlag) | ||
| if (options?.migratedSession && sessionId) { | ||
| const storedFlag = this.migratedSessionFlags.get(sessionId); | ||
| if (storedFlag !== undefined) { | ||
| terminal.dangerouslySkipPermissions = storedFlag; | ||
| this.migratedSessionFlags.delete(sessionId); | ||
| } | ||
| } | ||
|
|
||
| await ClaudeIntegration.resumeClaudeAsync(terminal, sessionId, this.getWindow, options); | ||
| } | ||
|
|
||
| /** | ||
| * Store YOLO mode flag for a session being migrated during profile swap. | ||
| * Called from the profile change handler before the renderer recreates terminals. | ||
| * The flag is consumed by resumeClaudeAsync when the new terminal resumes. | ||
| */ | ||
| storeMigratedSessionFlag(sessionId: string, dangerouslySkipPermissions: boolean): void { | ||
| this.migratedSessionFlags.set(sessionId, dangerouslySkipPermissions); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -387,7 +415,8 @@ export class TerminalManager { | |
| projectPath: terminal.projectPath, | ||
| claudeSessionId: terminal.claudeSessionId, | ||
| claudeProfileId: terminal.claudeProfileId, | ||
| isClaudeMode: terminal.isClaudeMode | ||
| isClaudeMode: terminal.isClaudeMode, | ||
| dangerouslySkipPermissions: terminal.dangerouslySkipPermissions | ||
| }); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,7 @@ export interface TerminalAPI { | |
| rows?: number | ||
| ) => Promise<IPCResult<import('../../shared/types').TerminalRestoreResult>>; | ||
| clearTerminalSessions: (projectPath: string) => Promise<IPCResult>; | ||
| resumeClaudeInTerminal: (id: string, sessionId?: string) => void; | ||
| resumeClaudeInTerminal: (id: string, sessionId?: string, options?: { migratedSession?: boolean }) => void; | ||
|
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.
The interface declares 🐛 Proposed fix- resumeClaudeInTerminal: (id: string, sessionId?: string, options?: { migratedSession?: boolean }) => void;
+ resumeClaudeInTerminal: (id: string, sessionId?: string, options?: { migratedSession?: boolean; dangerouslySkipPermissions?: boolean }) => void;🤖 Prompt for AI Agents |
||
| activateDeferredClaudeResume: (id: string) => void; | ||
| getTerminalSessionDates: (projectPath?: string) => Promise<IPCResult<import('../../shared/types').SessionDateInfo[]>>; | ||
| getTerminalSessionsForDate: ( | ||
|
|
@@ -166,8 +166,8 @@ export const createTerminalAPI = (): TerminalAPI => ({ | |
| clearTerminalSessions: (projectPath: string): Promise<IPCResult> => | ||
| ipcRenderer.invoke(IPC_CHANNELS.TERMINAL_CLEAR_SESSIONS, projectPath), | ||
|
|
||
| resumeClaudeInTerminal: (id: string, sessionId?: string): void => | ||
| ipcRenderer.send(IPC_CHANNELS.TERMINAL_RESUME_CLAUDE, id, sessionId), | ||
| resumeClaudeInTerminal: (id: string, sessionId?: string, options?: { migratedSession?: boolean }): void => | ||
| ipcRenderer.send(IPC_CHANNELS.TERMINAL_RESUME_CLAUDE, id, sessionId, options), | ||
|
|
||
| activateDeferredClaudeResume: (id: string): void => | ||
| ipcRenderer.send(IPC_CHANNELS.TERMINAL_ACTIVATE_DEFERRED_RESUME, id), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor:
migratedSessionFlagsentries may linger if a migrated session is never resumed.Entries are consumed in
resumeClaudeAsync(line 241) and bulk-cleared inkillAll(line 119), but if a terminal is destroyed individually before resuming (e.g., user closes it), the stale entry persists until the nextkillAll. Practically this is a handful ofMapentries at most, so low impact, but consider also clearing entries indestroy()if the terminal had a pending flag.🤖 Prompt for AI Agents