From 323abc701f7f131e8f1cf4ce24e05017f7e1d8ff Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sun, 30 Aug 2026 22:17:58 -0700 Subject: [PATCH] fix(pty): preserve teardown outcomes and history fences --- .../daemon/daemon-pty-session-shutdown.ts | 45 ++++++++++----- src/main/daemon/terminal-session-teardown.ts | 56 ++++++++++--------- 2 files changed, 60 insertions(+), 41 deletions(-) diff --git a/src/main/daemon/daemon-pty-session-shutdown.ts b/src/main/daemon/daemon-pty-session-shutdown.ts index 8a1c9bf531b..a06591b12fa 100644 --- a/src/main/daemon/daemon-pty-session-shutdown.ts +++ b/src/main/daemon/daemon-pty-session-shutdown.ts @@ -53,10 +53,26 @@ export abstract class DaemonPtySessionShutdown extends DaemonPtySessionSpawn { incarnationId?: string } ): Promise { - return await this.withHistorySpawnLock( + if (opts.keepHistory && this.disconnectOnlyPromise) { + throw new Error('Cannot keep history after daemon disconnect has started') + } + const shutdown = this.withHistorySpawnLock( id, () => this.shutdownWithHistoryLock(id, opts) as Promise ) + if (!opts.keepHistory) { + return await shutdown + } + const tracked = shutdown.then( + () => undefined, + () => undefined + ) + this.keepHistoryShutdowns.add(tracked) + try { + return await shutdown + } finally { + this.keepHistoryShutdowns.delete(tracked) + } } protected async shutdownWithHistoryLock( @@ -70,6 +86,8 @@ export abstract class DaemonPtySessionShutdown extends DaemonPtySessionSpawn { } ): Promise { await this.ensureConnected(opts.deadlineMs) + let coldRestore: ColdRestorePayload | null = null + let suspendHistory = false if (opts.keepHistory) { const committed = await this.runExclusiveCheckpoint( async () => { @@ -94,19 +112,11 @@ export abstract class DaemonPtySessionShutdown extends DaemonPtySessionSpawn { }) ?? '' } : null - const coldRestore = restoreInfo ? this.buildColdRestorePayload(restoreInfo) : null - if (coldRestore) { - this.coldRestoreCache.set(id, coldRestore) - if (this.coldRestoreCache.has(id)) { - this.sleepRestoreSessionIds.add(id) - } - this.historyManager?.suspendSession(id) - } else if ( - detection?.status === 'unreadable' || - (detection?.status === 'restored' && detection.hasUnreadableRecovery) - ) { - this.historyManager?.suspendSession(id) - } + coldRestore = restoreInfo ? this.buildColdRestorePayload(restoreInfo) : null + suspendHistory = + !coldRestore && + (detection?.status === 'unreadable' || + (detection?.status === 'restored' && detection.hasUnreadableRecovery)) } const result = await this.client.request( 'kill', @@ -121,6 +131,13 @@ export abstract class DaemonPtySessionShutdown extends DaemonPtySessionSpawn { if (isPtyShutdownFenceUnavailable(result)) { return result as PtyShutdownResult } + if (coldRestore) { + this.coldRestoreCache.set(id, coldRestore) + this.sleepRestoreSessionIds.add(id) + this.historyManager?.suspendSession(id) + } else if (suspendHistory) { + this.historyManager?.suspendSession(id) + } this.activeSessionIds.delete(id) this.clearSessionAwaitingDaemonRecovery(id) this.dirtySessionVersions.delete(id) diff --git a/src/main/daemon/terminal-session-teardown.ts b/src/main/daemon/terminal-session-teardown.ts index 3920f244b24..63a6c7516fd 100644 --- a/src/main/daemon/terminal-session-teardown.ts +++ b/src/main/daemon/terminal-session-teardown.ts @@ -4,7 +4,7 @@ import type { PtyKillIntent } from '../../shared/pty-kill-sessions' import type { PtyShutdownResult } from '../providers/pty-provider-contract' type AgentTeardownOperation = { - promise: Promise + promise: Promise immediate: boolean rootSignalled: boolean rootCompletion: Promise @@ -18,11 +18,11 @@ export class TerminalSessionTeardown { constructor(private sessions: ReadonlyMap) {} - get(sessionId: string): Promise | undefined { + get(sessionId: string): Promise | undefined { return this.operations.get(sessionId)?.promise } - requestImmediate(sessionId: string): Promise | undefined { + requestImmediate(sessionId: string): Promise | undefined { const pending = this.operations.get(sessionId) if (pending) { pending.immediate = true @@ -86,7 +86,7 @@ export class TerminalSessionTeardown { sessionId: string, session: Session, immediate: boolean - ): void | Promise { + ): void | Promise { const pending = this.operations.get(sessionId) if (pending) { // Why: an immediate caller is a stronger teardown request and must not @@ -114,33 +114,35 @@ export class TerminalSessionTeardown { rootCompletion: Promise.resolve(), session } - const sweep = Promise.resolve( - killWithDescendantSweep( - session.pid, - () => { - // Why: natural exit reaps the PID while ps is running. Never signal that - // stale numeric PID after the Session no longer represents a live root. - if (!session.isAlive) { - return - } - entry.rootSignalled = true - if (entry.immediate) { - entry.rootCompletion = session.forceKillAndWaitForExit() - } else { - session.signalTerminationRoot() - } - }, - { - // Why: the descendant rows are only authoritative while this exact - // Session still owns the root PID captured by ps. - ownsRoot: () => this.sessions.get(sessionId) === session && session.isAlive, - terminateOwnedTree: () => session.terminateOwnedTree() + const sweep = killWithDescendantSweep( + session.pid, + () => { + // Why: natural exit reaps the PID while ps is running. Never signal that + // stale numeric PID after the Session no longer represents a live root. + if (!session.isAlive) { + return } - ) + entry.rootSignalled = true + if (entry.immediate) { + entry.rootCompletion = session.forceKillAndWaitForExit() + } else { + session.signalTerminationRoot() + } + }, + { + // Why: the descendant rows are only authoritative while this exact + // Session still owns the root PID captured by ps. + ownsRoot: () => this.sessions.get(sessionId) === session && session.isAlive, + terminateOwnedTree: () => session.terminateOwnedTree() + } ) // Why: descendant capture completion only proves signals were requested; // destructive callers must retain the native owner until OS-confirmed exit. - const operation = sweep.then(() => entry.rootCompletion) + const operation = sweep.then((outcome) => + entry.rootCompletion.then(() => + outcome === 'tree_terminated' ? { outcome } : { outcome, treeUnverified: true as const } + ) + ) entry.promise = operation this.operations.set(sessionId, entry) const clearOperation = (): void => {