diff --git a/apps/cli/src/agent/runtime/startupSideEffects.test.ts b/apps/cli/src/agent/runtime/startupSideEffects.test.ts index 2c6d90b1c..15c38be48 100644 --- a/apps/cli/src/agent/runtime/startupSideEffects.test.ts +++ b/apps/cli/src/agent/runtime/startupSideEffects.test.ts @@ -1,6 +1,16 @@ import { describe, expect, it, vi } from 'vitest'; - -import { primeAgentStateForUi, reportSessionToDaemonIfRunning } from '@/agent/runtime/startupSideEffects'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + persistTerminalAttachmentInfoIfNeeded, + primeAgentStateForUi, + reportSessionToDaemonIfRunning, +} from '@/agent/runtime/startupSideEffects'; +import { configuration } from '@/configuration'; +import { readTerminalAttachmentState } from '@/terminal/attachment/terminalAttachmentInfo'; +import { executeTerminalHostDisposition } from '@/terminal/attachment/terminalHostDisposition'; import type { Metadata } from '@/api/types'; const metadataStub = {} as Metadata; @@ -287,3 +297,42 @@ describe('startup side effects: daemon session reporting retry', () => { } }); }); + +describe('startup side effects: terminal attachment persistence', () => { + it('persists a bound v2 record for a tmux spawn that the stop-path disposition retires instead of parking as legacy', async () => { + const happyHomeDir = await mkdtemp(join(tmpdir(), 'happier-attachment-')); + const originalHappyHomeDir = configuration.happyHomeDir; + // Narrow test-harness override: happyHomeDir is resolved from env once at process start. + (configuration as { happyHomeDir: string }).happyHomeDir = happyHomeDir; + try { + const sessionId = 'sess-tmux-spawn-v2'; + await persistTerminalAttachmentInfoIfNeeded({ + sessionId, + terminal: { + mode: 'tmux', + requested: 'tmux', + tmux: { target: 'happier:happy-window-1', tmpDir: '/tmp/happier-tmux' }, + } as NonNullable, + }); + + const state = await readTerminalAttachmentState({ happyHomeDir, sessionId }); + if (state.status !== 'present' || state.info.version !== 2) { + throw new Error(`Expected a bound version-2 attachment record, got ${JSON.stringify(state)}`); + } + expect(state.info.handle.kind).toBe('tmux'); + expect(state.info.handle.attachmentId).toBe(state.info.attachmentId); + + const disposition = await executeTerminalHostDisposition({ + happyHomeDir, + sessionId, + expectedAttachmentId: state.info.attachmentId, + intent: { kind: 'retire_confirmed_dead_attachment', reason: 'positive_dead_recovery' }, + }); + expect(disposition).toEqual({ status: 'retired', attachmentId: state.info.attachmentId }); + await expect(readTerminalAttachmentState({ happyHomeDir, sessionId })).resolves.toEqual({ status: 'absent' }); + } finally { + (configuration as { happyHomeDir: string }).happyHomeDir = originalHappyHomeDir; + await rm(happyHomeDir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/cli/src/agent/runtime/startupSideEffects.ts b/apps/cli/src/agent/runtime/startupSideEffects.ts index ca3e9557a..900b0fe15 100644 --- a/apps/cli/src/agent/runtime/startupSideEffects.ts +++ b/apps/cli/src/agent/runtime/startupSideEffects.ts @@ -1,12 +1,12 @@ import type { ApiSessionClient } from '@/api/session/sessionClient'; import type { Metadata } from '@/api/types'; -import { configuration } from '@/configuration'; import { notifyDaemonSessionStarted } from '@/daemon/controlClient'; -import { writeTerminalAttachmentInfo } from '@/terminal/attachment/terminalAttachmentInfo'; import { buildTerminalFallbackMessage } from '@/terminal/attachment/terminalFallbackMessage'; import { logger } from '@/ui/logger'; import { updateAgentStateBestEffort } from '@/api/session/sessionWritesBestEffort'; +export { persistTerminalAttachmentInfoIfNeeded } from '@/agent/runtime/terminal/persistTerminalAttachmentInfo'; + type DaemonReportDeps = { notifyDaemonSessionStartedFn?: typeof notifyDaemonSessionStarted; sleepFn?: (ms: number) => Promise; @@ -67,22 +67,6 @@ export function primeAgentStateForUi(session: ApiSessionClient, logPrefix: strin ); } -export async function persistTerminalAttachmentInfoIfNeeded(opts: { - sessionId: string; - terminal: Metadata['terminal'] | undefined; -}): Promise { - if (!opts.terminal) return; - try { - await writeTerminalAttachmentInfo({ - happyHomeDir: configuration.happyHomeDir, - sessionId: opts.sessionId, - terminal: opts.terminal, - }); - } catch (error) { - logger.debug('[START] Failed to persist terminal attachment info', error); - } -} - export function sendTerminalFallbackMessageIfNeeded(opts: { session: ApiSessionClient; terminal: Metadata['terminal'] | undefined; diff --git a/apps/cli/src/agent/runtime/terminal/attachmentMetadata.test.ts b/apps/cli/src/agent/runtime/terminal/attachmentMetadata.test.ts index f1205191c..62ff4a2f9 100644 --- a/apps/cli/src/agent/runtime/terminal/attachmentMetadata.test.ts +++ b/apps/cli/src/agent/runtime/terminal/attachmentMetadata.test.ts @@ -81,6 +81,34 @@ describe('buildTerminalAttachmentMetadataFromHostHandle', () => { expect(legacyHandle?.socketDir).toBeUndefined(); }); + it('reconstructs a tmux handle with socketDir from persisted tmpDir', () => { + const handle = buildTerminalHostHandleFromAttachmentMetadata({ + mode: 'tmux', + tmux: { + target: 'happy:unified-window', + tmpDir: '/tmp/happier-tmux-root', + }, + }); + expect(handle).toMatchObject({ + kind: 'tmux', + sessionName: 'happy', + paneId: 'unified-window', + socketDir: '/tmp/happier-tmux-root', + }); + }); + + it('does not reconstruct a windows_console handle: persisted metadata carries no host identity', () => { + // The canonical PTY handle is keyed by the spawn-time session name (also its paneId), + // which windows_console terminal metadata does not persist. A fabricated identity would + // probe a nonexistent host, so reconstruction must refuse and leave the mode on the + // fail-closed legacy path. + expect(buildTerminalHostHandleFromAttachmentMetadata({ + mode: 'windows_console', + requested: 'console', + windows: { host: 'console' }, + })).toBeNull(); + }); + it('builds non-focusable Windows console metadata from a PTY host handle', () => { const handle: TerminalHostHandle = { kind: 'windows_console', diff --git a/apps/cli/src/agent/runtime/terminal/attachmentMetadata.ts b/apps/cli/src/agent/runtime/terminal/attachmentMetadata.ts index 17abc8359..df0c8938f 100644 --- a/apps/cli/src/agent/runtime/terminal/attachmentMetadata.ts +++ b/apps/cli/src/agent/runtime/terminal/attachmentMetadata.ts @@ -78,10 +78,12 @@ export function buildTerminalHostHandleFromAttachmentMetadata( const sessionName = separatorIndex >= 0 ? target.slice(0, separatorIndex).trim() : target; const paneId = separatorIndex >= 0 ? target.slice(separatorIndex + 1).trim() : ''; if (!sessionName) return null; + const socketDir = typeof terminal.tmux?.tmpDir === 'string' ? terminal.tmux.tmpDir.trim() : ''; return { kind: 'tmux', sessionName, ...(paneId ? { paneId } : {}), + ...(socketDir ? { socketDir } : {}), attachMetadata: { attachStrategy: 'terminal_host', topology: 'shared', @@ -93,5 +95,9 @@ export function buildTerminalHostHandleFromAttachmentMetadata( }; } + // windows_console is deliberately not reconstructable: the canonical PTY handle is keyed + // by the spawn-time session name (also its paneId), which this metadata does not persist. + // A fabricated identity would probe a nonexistent host, so that mode stays on the + // fail-closed legacy path until the metadata carries real host identity. return null; } diff --git a/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.test.ts b/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.test.ts new file mode 100644 index 000000000..42f442802 --- /dev/null +++ b/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { Metadata } from '@/api/types'; +import type { writeTerminalAttachmentInfo } from '@/terminal/attachment/terminalAttachmentInfo'; + +import { persistTerminalAttachmentInfoIfNeeded } from './persistTerminalAttachmentInfo'; + +type WriteInput = Parameters[0]; + +const tmuxTerminal = { + mode: 'tmux', + requested: 'tmux', + tmux: { target: 'happier:happy-window-1', tmpDir: '/tmp/happier-tmux' }, +} as NonNullable; + +describe('persistTerminalAttachmentInfoIfNeeded', () => { + it('binds tmux metadata as a version-2 write with a derived handle and attachment id', async () => { + const writes: WriteInput[] = []; + const writeAttachmentInfo = vi.fn(async (input: WriteInput) => { + writes.push(input); + }); + + await persistTerminalAttachmentInfoIfNeeded({ + sessionId: 'sess-bound-write', + terminal: tmuxTerminal, + writeAttachmentInfo, + }); + + expect(writes).toHaveLength(1); + expect(writes[0]?.attachmentId).toEqual(expect.any(String)); + expect(writes[0]?.handle).toMatchObject({ kind: 'tmux', sessionName: 'happier' }); + }); + + it('falls back to the unbound record when the bound write fails, so a record always exists', async () => { + const writes: WriteInput[] = []; + const writeAttachmentInfo = vi.fn(async (input: WriteInput) => { + writes.push(input); + if (input.attachmentId) { + throw new Error('Terminal attachment root does not match its bound host handle'); + } + }); + + await persistTerminalAttachmentInfoIfNeeded({ + sessionId: 'sess-fallback-write', + terminal: tmuxTerminal, + writeAttachmentInfo, + }); + + expect(writes).toHaveLength(2); + expect(writes[1]?.attachmentId).toBeUndefined(); + expect(writes[1]?.handle).toBeUndefined(); + expect(writes[1]?.terminal).toEqual(tmuxTerminal); + }); + + it('writes the unbound record directly for modes without reconstructable host identity', async () => { + const writes: WriteInput[] = []; + const writeAttachmentInfo = vi.fn(async (input: WriteInput) => { + writes.push(input); + }); + + await persistTerminalAttachmentInfoIfNeeded({ + sessionId: 'sess-windows-console', + terminal: { + mode: 'windows_console', + requested: 'console', + windows: { host: 'console' }, + } as NonNullable, + writeAttachmentInfo, + }); + + expect(writes).toHaveLength(1); + expect(writes[0]?.attachmentId).toBeUndefined(); + expect(writes[0]?.handle).toBeUndefined(); + }); +}); diff --git a/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.ts b/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.ts new file mode 100644 index 000000000..5b56aa48e --- /dev/null +++ b/apps/cli/src/agent/runtime/terminal/persistTerminalAttachmentInfo.ts @@ -0,0 +1,54 @@ +import type { Metadata } from '@/api/types'; +import { configuration } from '@/configuration'; +import { + createTerminalAttachmentId, + writeTerminalAttachmentInfo, +} from '@/terminal/attachment/terminalAttachmentInfo'; +import { logger } from '@/ui/logger'; + +import { buildTerminalHostHandleFromAttachmentMetadata } from './attachmentMetadata'; + +/** + * Persist the session's terminal attachment record from spawn-path terminal metadata. + * + * Modes whose metadata carries full host identity (tmux, zellij) are bound as version-2 + * records with an immutable attachment id. Other modes persist the version-1 record. + * A failed bound write falls back to the version-1 write so a readable record always + * exists whenever the filesystem write itself succeeds; the stop path fails closed on + * missing evidence, so silently persisting nothing would strand the session. + */ +export async function persistTerminalAttachmentInfoIfNeeded(opts: { + sessionId: string; + terminal: Metadata['terminal'] | undefined; + logPrefix?: string; + writeAttachmentInfo?: typeof writeTerminalAttachmentInfo; +}): Promise { + if (!opts.terminal) return; + const logPrefix = opts.logPrefix ?? '[START]'; + const writeAttachmentInfo = opts.writeAttachmentInfo ?? writeTerminalAttachmentInfo; + try { + const handle = buildTerminalHostHandleFromAttachmentMetadata(opts.terminal); + const attachmentId = handle ? createTerminalAttachmentId() : undefined; + if (handle && attachmentId) { + try { + await writeAttachmentInfo({ + happyHomeDir: configuration.happyHomeDir, + sessionId: opts.sessionId, + attachmentId, + handle, + terminal: opts.terminal, + }); + return; + } catch (error) { + logger.warn(`${logPrefix} Bound terminal attachment write failed; falling back to unbound record`, error); + } + } + await writeAttachmentInfo({ + happyHomeDir: configuration.happyHomeDir, + sessionId: opts.sessionId, + terminal: opts.terminal, + }); + } catch (error) { + logger.debug(`${logPrefix} Failed to persist terminal attachment info`, error); + } +} diff --git a/apps/cli/src/daemon/sessions/stopSession.test.ts b/apps/cli/src/daemon/sessions/stopSession.test.ts index 71de9627f..ef7512ac3 100644 --- a/apps/cli/src/daemon/sessions/stopSession.test.ts +++ b/apps/cli/src/daemon/sessions/stopSession.test.ts @@ -1,10 +1,32 @@ import { describe, expect, it, vi } from 'vitest'; +import type { TerminalHostAdapter } from '@/integrations/terminalHost/_types'; import type { BoundTerminalAttachmentInfo, TerminalAttachmentInfo, } from '@/terminal/attachment/terminalAttachmentInfo'; +/** Legacy-retirement tests only probe liveness; every other adapter surface must stay unreached. */ +function createLivenessProbeAdapterFixture( + kind: TerminalHostAdapter['kind'], + evaluateLiveness: TerminalHostAdapter['evaluateLiveness'], +): TerminalHostAdapter { + return { + kind, + createOrAttachHost: async () => { + throw new Error('createOrAttachHost must not be reached by legacy retirement'); + }, + injectUserPrompt: async () => { + throw new Error('injectUserPrompt must not be reached by legacy retirement'); + }, + interruptTurn: async () => { + throw new Error('interruptTurn must not be reached by legacy retirement'); + }, + evaluateLiveness, + dispose: async () => undefined, + }; +} + const { spawnSyncMock } = vi.hoisted(() => ({ spawnSyncMock: vi.fn(() => ({ status: 0, stdout: '', stderr: '' })), })); @@ -399,34 +421,47 @@ describe('createStopSession', () => { expect(pidToTrackedSession.has(333)).toBe(true); }); - it('parks a legacy zellij attachment without signaling either host or runner', async () => { + it('stops a legacy zellij v1 session when host is confirmed dead after runner exit', async () => { const { createStopSession } = await import('./stopSession'); - readTerminalAttachmentInfo.mockResolvedValueOnce({ - version: 1, - sessionId: 'sess-zellij', - terminal: { - mode: 'zellij', - zellij: { - sessionName: 'happier-claude-unified-123', - paneId: 'terminal_1', - }, + const terminal = { + mode: 'zellij' as const, + zellij: { + sessionName: 'happier-claude-unified-123', + paneId: 'terminal_1', }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-zellij', + terminal, updatedAt: 1, - }); + }; const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true as any); const pidToTrackedSession = new Map([ [333, { startedBy: 'terminal', pid: 333, happySessionId: 'sess-zellij', processCommandHash: 'h3' }], ]); - const stop = createStopSession({ pidToTrackedSession }); + const stop = createStopSession({ + pidToTrackedSession, + readAttachmentInfo: vi.fn(async () => legacyAttachment), + removeAttachmentInfo: vi.fn(async () => true), + waitForTrackedRunnersExit: vi.fn(async () => true), + terminalHostAdapters: { + zellij: createLivenessProbeAdapterFixture( + 'zellij', + vi.fn(async () => ({ paneAlive: false, paneDead: true, observedAt: Date.now() })), + ), + }, + }); const ok = await stop('sess-zellij'); - expect(ok).toEqual({ status: 'incomplete', reason: 'legacy_attachment' }); + expect(ok).toEqual({ status: 'stopped' }); expect(zellijKillSession).not.toHaveBeenCalled(); expect(zellijDeleteSession).not.toHaveBeenCalled(); - expect(killSpy).not.toHaveBeenCalled(); + // Runners ARE signaled (v1 no longer blocks pre-signaling) + expect(killSpy).toHaveBeenCalledWith(333, 'SIGTERM'); }); it('does not destroy an exact host when no tracked runner exit can be proven', async () => { @@ -1105,62 +1140,230 @@ describe('createStopSession', () => { expect(onExactTerminalAttachmentRetired).not.toHaveBeenCalled(); }); - it('parks a legacy attachment even when its old host is already missing', async () => { + it('stops a v1 tmux session whose runners have exited and attachment is confirmed dead', async () => { const { createStopSession } = await import('./stopSession'); - readTerminalAttachmentInfo.mockResolvedValueOnce({ - version: 1, - sessionId: 'sess-zellij', - terminal: { - mode: 'zellij', - zellij: { - sessionName: 'happier-claude-unified-123', - paneId: 'terminal_1', - }, - }, + const terminal = { + mode: 'tmux' as const, + tmux: { target: 'happy:legacy-window', tmpDir: '/tmp/happier-tmux' }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-legacy-tmux', + terminal, updatedAt: 1, + }; + const legacyReadAttachmentInfo = vi.fn(async () => legacyAttachment); + const legacyRemoveAttachmentInfo = vi.fn(async () => true); + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true as any); + + const pidToTrackedSession = new Map([ + [444, { startedBy: 'terminal', pid: 444, happySessionId: 'sess-legacy-tmux', processCommandHash: 'h4' }], + ]); + + const stop = createStopSession({ + pidToTrackedSession, + waitForTrackedRunnersExit: vi.fn(async () => true), + readAttachmentInfo: legacyReadAttachmentInfo, + removeAttachmentInfo: legacyRemoveAttachmentInfo, + terminalHostAdapters: { + tmux: createLivenessProbeAdapterFixture( + 'tmux', + vi.fn(async () => ({ paneAlive: false, paneDead: true, observedAt: Date.now() })), + ), + }, }); + const result = await stop('sess-legacy-tmux'); + + expect(result).toEqual({ status: 'stopped' }); + expect(killSpy).toHaveBeenCalledWith(444, 'SIGTERM'); + expect(legacyRemoveAttachmentInfo).toHaveBeenCalledWith(expect.objectContaining({ + legacyTerminalMetadataRemoval: true, + })); + }); + + it('refuses legacy v1 host retirement when the host is still alive', async () => { + const { createStopSession } = await import('./stopSession'); + + const terminal = { + mode: 'zellij' as const, + zellij: { + sessionName: 'happier-claude-unified-123', + paneId: 'terminal_1', + }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-zellij', + terminal, + updatedAt: 1, + }; const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true as any); const pidToTrackedSession = new Map([ [333, { startedBy: 'terminal', pid: 333, happySessionId: 'sess-zellij', processCommandHash: 'h3' }], ]); - const stop = createStopSession({ pidToTrackedSession }); + const stop = createStopSession({ + pidToTrackedSession, + waitForTrackedRunnersExit: vi.fn(async () => true), + readAttachmentInfo: vi.fn(async () => legacyAttachment), + terminalHostAdapters: { + zellij: createLivenessProbeAdapterFixture( + 'zellij', + vi.fn(async () => ({ paneAlive: true, observedAt: Date.now() })), + ), + }, + }); const ok = await stop('sess-zellij'); expect(ok).toEqual({ status: 'incomplete', reason: 'legacy_attachment' }); expect(zellijKillSession).not.toHaveBeenCalled(); - expect(killSpy).not.toHaveBeenCalled(); + // Runners ARE signaled (v1 no longer blocks pre-signaling) + expect(killSpy).toHaveBeenCalledWith(333, 'SIGTERM'); }); - it('does not fall through to runner signaling for a legacy zellij attachment', async () => { + it('refuses legacy v1 host retirement when liveness probe is inconclusive (not positively dead)', async () => { const { createStopSession } = await import('./stopSession'); - readTerminalAttachmentInfo.mockResolvedValueOnce({ - version: 1, - sessionId: 'sess-zellij', - terminal: { - mode: 'zellij', - zellij: { - sessionName: 'happier-claude-unified-123', - paneId: 'terminal_1', - }, - }, + const terminal = { + mode: 'tmux' as const, + tmux: { target: 'happy:legacy-window', tmpDir: '/tmp/happier-tmux' }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-inconclusive', + terminal, updatedAt: 1, + }; + const legacyRemoveAttachmentInfo = vi.fn(async () => true); + vi.spyOn(process, 'kill').mockImplementation(() => true as any); + + const pidToTrackedSession = new Map([ + [445, { startedBy: 'terminal', pid: 445, happySessionId: 'sess-inconclusive', processCommandHash: 'h4' }], + ]); + + // Both probes return inconclusive (paneAlive:false, probeInconclusive:true) + const evaluateLiveness = vi.fn(async () => ({ + paneAlive: false, + probeInconclusive: true, + observedAt: Date.now(), + })); + + const stop = createStopSession({ + pidToTrackedSession, + waitForTrackedRunnersExit: vi.fn(async () => true), + readAttachmentInfo: vi.fn(async () => legacyAttachment), + removeAttachmentInfo: legacyRemoveAttachmentInfo, + terminalHostAdapters: { + tmux: createLivenessProbeAdapterFixture('tmux', evaluateLiveness), + }, }); + const result = await stop('sess-inconclusive'); + + expect(result).toEqual({ status: 'incomplete', reason: 'legacy_attachment' }); + // The descriptor must NOT be removed + expect(legacyRemoveAttachmentInfo).not.toHaveBeenCalled(); + }); + + it('reports legacy_attachment when no adapter is available for legacy v1 host liveness probe', async () => { + const { createStopSession } = await import('./stopSession'); + + const terminal = { + mode: 'zellij' as const, + zellij: { + sessionName: 'happier-claude-unified-123', + paneId: 'terminal_1', + }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-zellij-no-adapter', + terminal, + updatedAt: 1, + }; const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true as any); const pidToTrackedSession = new Map([ - [333, { startedBy: 'terminal', pid: 333, happySessionId: 'sess-zellij', processCommandHash: 'h3' }], + [334, { startedBy: 'terminal', pid: 334, happySessionId: 'sess-zellij-no-adapter', processCommandHash: 'h3' }], ]); - const stop = createStopSession({ pidToTrackedSession }); - const ok = await stop('sess-zellij'); + const stop = createStopSession({ + pidToTrackedSession, + waitForTrackedRunnersExit: vi.fn(async () => true), + readAttachmentInfo: vi.fn(async () => legacyAttachment), + terminalHostAdapters: {}, + }); + const ok = await stop('sess-zellij-no-adapter'); expect(ok).toEqual({ status: 'incomplete', reason: 'legacy_attachment' }); - expect(zellijKillSession).not.toHaveBeenCalled(); - expect(killSpy).not.toHaveBeenCalled(); + expect(killSpy).toHaveBeenCalledWith(334, 'SIGTERM'); + }); + + it('retires a stale v1 tmux descriptor when zero runners are tracked and host is confirmed dead', async () => { + const { createStopSession } = await import('./stopSession'); + + const terminal = { + mode: 'tmux' as const, + tmux: { target: 'happy:stale-window', tmpDir: '/tmp/happier-tmux' }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-stale-tmux', + terminal, + updatedAt: 1, + }; + const legacyRemoveAttachmentInfo = vi.fn(async () => true); + + const stop = createStopSession({ + pidToTrackedSession: new Map(), + readAttachmentInfo: vi.fn(async () => legacyAttachment), + removeAttachmentInfo: legacyRemoveAttachmentInfo, + terminalHostAdapters: { + tmux: createLivenessProbeAdapterFixture( + 'tmux', + vi.fn(async () => ({ paneAlive: false, paneDead: true, observedAt: Date.now() })), + ), + }, + }); + const result = await stop('sess-stale-tmux'); + + expect(result).toEqual({ status: 'not_found' }); + expect(legacyRemoveAttachmentInfo).toHaveBeenCalledWith(expect.objectContaining({ + legacyTerminalMetadataRemoval: true, + })); + }); + + it('refuses stale v1 tmux descriptor retirement when zero runners are tracked and host is alive', async () => { + const { createStopSession } = await import('./stopSession'); + + const terminal = { + mode: 'tmux' as const, + tmux: { target: 'happy:stale-window', tmpDir: '/tmp/happier-tmux' }, + }; + const legacyAttachment = { + version: 1 as const, + sessionId: 'sess-stale-alive', + terminal, + updatedAt: 1, + }; + const legacyRemoveAttachmentInfo = vi.fn(async () => true); + + const stop = createStopSession({ + pidToTrackedSession: new Map(), + readAttachmentInfo: vi.fn(async () => legacyAttachment), + removeAttachmentInfo: legacyRemoveAttachmentInfo, + terminalHostAdapters: { + tmux: createLivenessProbeAdapterFixture( + 'tmux', + vi.fn(async () => ({ paneAlive: true, observedAt: Date.now() })), + ), + }, + }); + const result = await stop('sess-stale-alive'); + + expect(result).toEqual({ status: 'incomplete', reason: 'legacy_attachment' }); + expect(legacyRemoveAttachmentInfo).not.toHaveBeenCalled(); }); it('parks a tracked tmux host when no committed attachment identity exists', async () => { diff --git a/apps/cli/src/daemon/sessions/stopSession.ts b/apps/cli/src/daemon/sessions/stopSession.ts index 5aea86907..47b2b0a3d 100644 --- a/apps/cli/src/daemon/sessions/stopSession.ts +++ b/apps/cli/src/daemon/sessions/stopSession.ts @@ -7,9 +7,12 @@ import { readTerminalAttachmentState, removeTerminalAttachmentInfo, type BoundTerminalAttachmentInfo, + type LegacyTerminalAttachmentInfo, type TerminalAttachmentReadState, } from '@/terminal/attachment/terminalAttachmentInfo'; import { executeTerminalHostDisposition } from '@/terminal/attachment/terminalHostDisposition'; +import { buildTerminalHostHandleFromAttachmentMetadata } from '@/agent/runtime/terminal/attachmentMetadata'; +import { evaluateTerminalHostLivenessForRecovery } from '@/integrations/terminalHost/livenessPolicy'; import { configuration } from '@/configuration'; import { isPidSafeHappySessionProcess } from '../pidSafety'; @@ -80,6 +83,72 @@ async function taskkillWindowsDaemonChild(params: Readonly<{ return true; } +async function resolveLegacyHostAdapter( + attachmentInfo: LegacyTerminalAttachmentInfo, + terminalHostAdapters: TerminalHostRegistry | undefined, + loadTerminalHostAdapters: (() => Promise) | undefined, + logWarning: (message: string, ...args: unknown[]) => void, + sessionId: string, +): Promise { + const kind = attachmentInfo.terminal.mode === 'tmux' ? 'tmux' + : attachmentInfo.terminal.mode === 'zellij' ? 'zellij' + : attachmentInfo.terminal.mode === 'windows_console' ? 'windows_console' + : null; + if (!kind) return null; + const adapters = terminalHostAdapters + ?? await loadTerminalHostAdapters?.().catch((error) => { + logWarning(`[DAEMON RUN] Failed to acquire terminal host cleanup adapters for legacy session ${sessionId}`, error); + return null; + }); + return adapters?.[kind] ?? null; +} + +/** + * Attempt to retire a legacy v1 terminal host record by probing the canonical liveness policy. + * Only retires on positive death (`status === 'dead'`); alive and inconclusive refuse. + * Returns the disposition result on success, or an incomplete reason on refusal. + */ +async function attemptLegacyHostRetirement(input: Readonly<{ + attachmentInfo: LegacyTerminalAttachmentInfo; + adapter: TerminalHostAdapter; + normalizedSessionId: string; + readAttachmentInfo: typeof readTerminalAttachmentInfo; + removeAttachmentInfo: typeof removeTerminalAttachmentInfo; + logWarning: (message: string, ...args: unknown[]) => void; +}>): Promise { + const handle = buildTerminalHostHandleFromAttachmentMetadata(input.attachmentInfo.terminal); + if (!handle) return null; + + try { + const probeResult = await evaluateTerminalHostLivenessForRecovery(input.adapter, handle); + if (probeResult.status === 'dead') { + const disposition = await executeTerminalHostDisposition({ + happyHomeDir: configuration.happyHomeDir, + sessionId: input.normalizedSessionId, + expectedAttachmentId: 'legacy-v1-retirement', + intent: { kind: 'retire_confirmed_dead_attachment', reason: 'positive_dead_recovery' }, + provenDeadLegacyTerminal: input.attachmentInfo.terminal, + readAttachmentInfo: input.readAttachmentInfo, + removeAttachmentInfo: input.removeAttachmentInfo, + }); + if (disposition.status === 'retired_legacy') { + return { status: 'stopped' }; + } + input.logWarning(`[DAEMON RUN] Legacy attachment retirement failed for session ${input.normalizedSessionId}`); + return incompleteStopSession('legacy_attachment'); + } + if (probeResult.status === 'alive') { + input.logWarning(`[DAEMON RUN] Legacy terminal host is still alive for session ${input.normalizedSessionId}; refusing retirement`); + } else { + input.logWarning(`[DAEMON RUN] Legacy terminal host liveness probe was inconclusive for session ${input.normalizedSessionId}; refusing retirement`); + } + return incompleteStopSession('legacy_attachment'); + } catch (error) { + input.logWarning(`[DAEMON RUN] Failed to probe legacy terminal host liveness for session ${input.normalizedSessionId}`, error); + return incompleteStopSession('legacy_attachment'); + } +} + export function createStopSession(params: Readonly<{ pidToTrackedSession: Map; logPidReuseRefusal?: (message: string) => void; @@ -185,14 +254,12 @@ export function createStopSession(params: Readonly<{ logWarning(`[DAEMON RUN] Terminal attachment retired but provider artifacts could not be cleaned for session ${normalizedSessionId}`, error); }); }; + // Track whether a v1 legacy record with a terminal host needs dead-host retirement after runner exit. + const legacyHostRetirementNeeded = attachmentInfo?.version === 1 + && (attachmentInfo.terminal.mode === 'tmux' + || attachmentInfo.terminal.mode === 'zellij' + || attachmentInfo.terminal.mode === 'windows_console'); if (!isPidFallback) { - if (attachmentInfo) { - if (attachmentInfo.version !== 2) { - logWarning(`[DAEMON RUN] Refusing to destroy legacy terminal attachment without immutable identity for session ${normalizedSessionId}`); - return incompleteStopSession('legacy_attachment'); - } - } - const terminalModes = pidsToStop.map((pid) => { const provenTerminalHostKind = params.provenTerminalHostKindsByPid?.get(pid); if (provenTerminalHostKind) return provenTerminalHostKind; @@ -233,6 +300,33 @@ export function createStopSession(params: Readonly<{ return incompleteStopSession('missing_topology_proof'); } } + // A stale v1 record can outlive its runners; retire it only on a provably dead host. + if (legacyHostRetirementNeeded && attachmentInfo?.version === 1) { + const legacyAdapter = await resolveLegacyHostAdapter( + attachmentInfo, params.terminalHostAdapters, params.loadTerminalHostAdapters, logWarning, normalizedSessionId, + ); + if (legacyAdapter) { + const retirementResult = await attemptLegacyHostRetirement({ + attachmentInfo, + adapter: legacyAdapter, + normalizedSessionId, + readAttachmentInfo, + removeAttachmentInfo: params.removeAttachmentInfo ?? removeTerminalAttachmentInfo, + logWarning, + }); + if (retirementResult) { + // On successful retirement, return not_found (truthful: daemon never tracked these runners). + // On refusal, the retirementResult already carries the incomplete reason. + return retirementResult.status === 'stopped' + ? { status: 'not_found' } + : retirementResult; + } + } + // Cannot probe: refuse with legacy_attachment + logWarning(`[DAEMON RUN] Cannot probe legacy terminal host for untracked session ${normalizedSessionId}; refusing retirement`); + return incompleteStopSession('legacy_attachment'); + } + logger.debug(`[DAEMON RUN] Session ${normalizedSessionId} not found`); return { status: 'not_found' }; } @@ -418,6 +512,27 @@ export function createStopSession(params: Readonly<{ ? incompleteStopSession(mapDispositionFailureReason(disposition.reason)) : incompleteStopSession('destroy_failed'); } + + if (legacyHostRetirementNeeded && attachmentInfo?.version === 1) { + // After runners have exited, attempt to retire the legacy v1 terminal host record. + const legacyAdapter = await resolveLegacyHostAdapter( + attachmentInfo, params.terminalHostAdapters, params.loadTerminalHostAdapters, logWarning, normalizedSessionId, + ); + if (legacyAdapter) { + const result = await attemptLegacyHostRetirement({ + attachmentInfo, + adapter: legacyAdapter, + normalizedSessionId, + readAttachmentInfo, + removeAttachmentInfo: params.removeAttachmentInfo ?? removeTerminalAttachmentInfo, + logWarning, + }); + if (result) return result; + } + logWarning(`[DAEMON RUN] Cannot probe legacy terminal host for session ${normalizedSessionId}; refusing retirement`); + return incompleteStopSession('legacy_attachment'); + } + return { status: 'stopped' }; }; } diff --git a/apps/cli/src/daemon/startDaemon.spawnResume.integration.test.ts b/apps/cli/src/daemon/startDaemon.spawnResume.integration.test.ts index 197e6a9f4..310a643bc 100644 --- a/apps/cli/src/daemon/startDaemon.spawnResume.integration.test.ts +++ b/apps/cli/src/daemon/startDaemon.spawnResume.integration.test.ts @@ -4118,7 +4118,7 @@ describe('startDaemon spawn resume wiring (integration)', () => { } }); - it('fences explicit Resume before spawn when preserved terminal topology is unreadable or legacy', async () => { + it('keeps fencing explicit Resume when preserved legacy terminal topology cannot be provably retired', async () => { const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); const refreshEnvOriginal = process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED = 'false'; @@ -4131,6 +4131,7 @@ describe('startDaemon spawn resume wiring (integration)', () => { unresolvedTerminalHostSessionIds: ['sess_unresolved_terminal_topology'], connectedServiceRestartIntents: [], }); + stopSessionMocks.stopSession.mockResolvedValue({ status: 'incomplete', reason: 'legacy_attachment' }); const { startDaemon } = await import('./startDaemon'); run = startDaemon(); @@ -4151,6 +4152,7 @@ describe('startDaemon spawn resume wiring (integration)', () => { errorCode: SPAWN_SESSION_ERROR_CODES.UNEXPECTED, errorMessage: 'This session has preserved terminal topology that is unreadable or legacy. Repair or migrate that topology before trying Resume again.', }); + expect(stopSessionMocks.stopSession).toHaveBeenCalledWith('sess_unresolved_terminal_topology'); expect(spawnHappyCLI).not.toHaveBeenCalled(); harness.requestShutdown('happier-cli'); @@ -4172,6 +4174,140 @@ describe('startDaemon spawn resume wiring (integration)', () => { } }); + it('repairs provably dead preserved legacy topology through the stop path and resumes', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); + const refreshEnvOriginal = process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; + process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED = 'false'; + let run: Promise | null = null; + + try { + const reattachModule = await import('./sessions/reattachFromMarkers'); + vi.mocked(reattachModule.reattachTrackedSessionsFromMarkers).mockResolvedValue({ + orphanedDeadDaemonSessions: [], + unresolvedTerminalHostSessionIds: ['sess_plain'], + connectedServiceRestartIntents: [], + }); + // The stop path retires a legacy v1 record only on a provably dead host and, with no + // tracked runners, truthfully reports not_found. That is the repaired-topology signal. + stopSessionMocks.stopSession.mockResolvedValue({ status: 'not_found' }); + + const { startDaemon } = await import('./startDaemon'); + run = startDaemon(); + let spawnSession = harness.getSpawnSession(); + for (let attempt = 0; attempt < 20 && !spawnSession; attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, 0)); + spawnSession = harness.getSpawnSession(); + } + if (!spawnSession) throw new Error('Expected spawnSession to be registered'); + + const result = await spawnSession({ + directory: '/tmp', + backendTarget: { kind: 'builtInAgent', agentId: 'codex' }, + existingSessionId: 'sess_plain', + token: 'token-from-spawn-options', + codexBackendMode: 'acp', + }); + + expect(stopSessionMocks.stopSession).toHaveBeenCalledWith('sess_plain'); + expect(result).toMatchObject({ type: 'success' }); + expect(spawnHappyCLI).toHaveBeenCalledTimes(1); + const firstCall = spawnHappyCLI.mock.calls[0]; + if (!firstCall) throw new Error('Expected spawnHappyCLI to be called'); + expect(firstCall[0]).toEqual(expect.arrayContaining(['--existing-session', 'sess_plain'])); + + harness.requestShutdown('happier-cli'); + await run; + run = null; + } finally { + if (run) { + harness.requestShutdown('happier-cli'); + await run; + } + const reattachModule = await import('./sessions/reattachFromMarkers'); + vi.mocked(reattachModule.reattachTrackedSessionsFromMarkers).mockResolvedValue({ + orphanedDeadDaemonSessions: [], + connectedServiceRestartIntents: [], + }); + if (refreshEnvOriginal === undefined) delete process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; + else process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED = refreshEnvOriginal; + exitSpy.mockRestore(); + } + }); + + it('joins a concurrent Stop to an in-flight Resume topology repair instead of running it twice', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); + const refreshEnvOriginal = process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; + process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED = 'false'; + let run: Promise | null = null; + + try { + const reattachModule = await import('./sessions/reattachFromMarkers'); + vi.mocked(reattachModule.reattachTrackedSessionsFromMarkers).mockResolvedValue({ + orphanedDeadDaemonSessions: [], + unresolvedTerminalHostSessionIds: ['sess_plain'], + connectedServiceRestartIntents: [], + }); + let releaseRepair!: (result: StopSessionResult) => void; + const repairGate = new Promise((resolve) => { + releaseRepair = resolve; + }); + stopSessionMocks.stopSession.mockImplementation(async () => await repairGate); + + const { startDaemon } = await import('./startDaemon'); + run = startDaemon(); + let spawnSession = harness.getSpawnSession(); + for (let attempt = 0; attempt < 20 && (!spawnSession || !harness.getStopSession()); attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, 0)); + spawnSession = harness.getSpawnSession(); + } + const stopHandler = harness.getStopSession(); + if (!spawnSession || !stopHandler) throw new Error('Expected spawnSession and stopSession to be registered'); + + const resumePromise = spawnSession({ + directory: '/tmp', + backendTarget: { kind: 'builtInAgent', agentId: 'codex' }, + existingSessionId: 'sess_plain', + token: 'token-from-spawn-options', + codexBackendMode: 'acp', + }); + for (let attempt = 0; attempt < 50 && stopSessionMocks.stopSession.mock.calls.length === 0; attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, 0)); + } + expect(stopSessionMocks.stopSession).toHaveBeenCalledTimes(1); + + const concurrentStop = stopHandler('sess_plain'); + // Give a (wrongly) unserialized concurrent stop the chance to start a second core stop. + await new Promise((resolve) => setTimeout(resolve, 10)); + + releaseRepair({ status: 'not_found' }); + const [resumeResult, stopResult] = await Promise.all([resumePromise, concurrentStop]); + + expect(stopSessionMocks.stopSession).toHaveBeenCalledTimes(1); + expect(stopResult).toEqual({ status: 'not_found' }); + expect(resumeResult).toMatchObject({ type: 'success' }); + expect(spawnHappyCLI).toHaveBeenCalledTimes(1); + + harness.requestShutdown('happier-cli'); + await run; + run = null; + } finally { + if (run) { + harness.requestShutdown('happier-cli'); + await run; + } + const reattachModule = await import('./sessions/reattachFromMarkers'); + vi.mocked(reattachModule.reattachTrackedSessionsFromMarkers).mockResolvedValue({ + orphanedDeadDaemonSessions: [], + connectedServiceRestartIntents: [], + }); + stopSessionMocks.stopSession.mockReset(); + stopSessionMocks.stopSession.mockResolvedValue({ status: 'stopped' }); + if (refreshEnvOriginal === undefined) delete process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; + else process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED = refreshEnvOriginal; + exitSpy.mockRestore(); + } + }); + it('fences duplicate resume when process liveness is known but exact-session serviceability is unknown', async () => { const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); const refreshEnvOriginal = process.env.HAPPIER_CONNECTED_SERVICES_REFRESH_ENABLED; diff --git a/apps/cli/src/daemon/startDaemon.ts b/apps/cli/src/daemon/startDaemon.ts index c2a6bca47..4223b0921 100644 --- a/apps/cli/src/daemon/startDaemon.ts +++ b/apps/cli/src/daemon/startDaemon.ts @@ -401,7 +401,8 @@ import type { RuntimeAccountIdentitySelectionInput } from './connectedServices/q import { decodeJwtPayload } from '@/cloud/decodeJwtPayload'; import { parseBooleanEnv, resolveConnectedServicesProviderStateSharingPolicyV1, type AccountSettings, type BackendTargetRefV1, type ConnectedServiceId } from '@happier-dev/protocol'; import type { CatalogAgentId, ConnectedServiceSwitchEffectiveBinding } from '@/backends/types'; -import { readTerminalAttachmentInfo, writeTerminalAttachmentInfo } from '@/terminal/attachment/terminalAttachmentInfo'; +import { readTerminalAttachmentInfo } from '@/terminal/attachment/terminalAttachmentInfo'; +import { persistTerminalAttachmentInfoIfNeeded } from '@/agent/runtime/terminal/persistTerminalAttachmentInfo'; import { isAccountSettingsVersionAtLeast, normalizeAccountSettingsVersionHint, @@ -2967,14 +2968,37 @@ export async function startDaemon(options: Readonly<{ takeover?: boolean }> = {} // completed Stop make a racing or subsequently failed stop look successful. completedStopSessionIds.delete(normalizedExistingSessionId); if (unresolvedTerminalHostSessionIds.has(normalizedExistingSessionId)) { - logger.warn('[DAEMON RUN] Refusing Resume while preserved terminal topology is unreadable or legacy', { - sessionId: normalizedExistingSessionId, - }); - return { - type: 'error', - errorCode: SPAWN_SESSION_ERROR_CODES.UNEXPECTED, - errorMessage: 'This session has preserved terminal topology that is unreadable or legacy. Repair or migrate that topology before trying Resume again.', - }; + // Resume is the user's explicit intent to relaunch this session, so attempt the + // canonical stop-path repair first: it retires a preserved legacy (v1) terminal + // record only when its host is provably dead and fails closed otherwise. Cold + // startup deliberately never probes terminal hosts; this is the probe point. + // Register the repair under the canonical in-flight key so a concurrent Stop + // joins this operation instead of racing it on the same session. + const repairOperation = stopSessionCore(normalizedExistingSessionId); + stopSessionInFlightBySessionId.set(normalizedExistingSessionId, repairOperation); + let topologyRepair: StopSessionResult; + try { + topologyRepair = await repairOperation; + } finally { + if (stopSessionInFlightBySessionId.get(normalizedExistingSessionId) === repairOperation) { + stopSessionInFlightBySessionId.delete(normalizedExistingSessionId); + } + } + if (topologyRepair.status === 'stopped' || topologyRepair.status === 'not_found') { + unresolvedTerminalHostSessionIds.delete(normalizedExistingSessionId); + logger.debug('[DAEMON RUN] Retired preserved legacy terminal topology before Resume', { + sessionId: normalizedExistingSessionId, + }); + } else { + logger.warn('[DAEMON RUN] Refusing Resume while preserved terminal topology is unreadable or legacy', { + sessionId: normalizedExistingSessionId, + }); + return { + type: 'error', + errorCode: SPAWN_SESSION_ERROR_CODES.UNEXPECTED, + errorMessage: 'This session has preserved terminal topology that is unreadable or legacy. Repair or migrate that topology before trying Resume again.', + }; + } } const disconnectedHostCandidate = disconnectedTerminalHostCandidates.find( (candidate) => candidate.sessionId === normalizedExistingSessionId @@ -4015,15 +4039,11 @@ export async function startDaemon(options: Readonly<{ takeover?: boolean }> = {} const resolvedSessionId = typeof resolved.sessionId === 'string' ? resolved.sessionId.trim() : ''; if (resolvedSessionId) { - try { - await writeTerminalAttachmentInfo({ - happyHomeDir: configuration.happyHomeDir, - sessionId: resolvedSessionId, - terminal: params.terminal, - }); - } catch (error) { - logger.debug('[DAEMON RUN] Failed to persist Windows terminal attachment info', error); - } + await persistTerminalAttachmentInfoIfNeeded({ + sessionId: resolvedSessionId, + terminal: params.terminal, + logPrefix: '[DAEMON RUN]', + }); try { await publishCurrentTerminalControlServiceability({ credentials, diff --git a/apps/cli/src/terminal/attachment/terminalAttachmentInfo.test.ts b/apps/cli/src/terminal/attachment/terminalAttachmentInfo.test.ts index 60fcf13ee..563d4b804 100644 --- a/apps/cli/src/terminal/attachment/terminalAttachmentInfo.test.ts +++ b/apps/cli/src/terminal/attachment/terminalAttachmentInfo.test.ts @@ -244,6 +244,53 @@ describe('terminalAttachmentInfo', () => { } }); + it('removes a legacy v1 attachment by terminal metadata match when explicitly requested', async () => { + const dir = tmp.dirSync({ unsafeCleanup: true }); + try { + const sessionId = 'sess_legacy_retire'; + const terminal = { mode: 'tmux', tmux: { target: 'happy:legacy-window', tmpDir: '/tmp/tmux-root' } } as const; + await writeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId, + terminal, + }); + + await expect(removeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId, + expectedTerminal: terminal, + legacyTerminalMetadataRemoval: true, + })).resolves.toBe(true); + await expect(readTerminalAttachmentInfo({ happyHomeDir: dir.name, sessionId })).resolves.toBeNull(); + } finally { + dir.removeCallback(); + } + }); + + it('refuses legacy v1 removal when terminal metadata does not match', async () => { + const dir = tmp.dirSync({ unsafeCleanup: true }); + try { + const sessionId = 'sess_legacy_mismatch'; + await writeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId, + terminal: { mode: 'tmux', tmux: { target: 'happy:original' } }, + }); + + await expect(removeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId, + expectedTerminal: { mode: 'tmux', tmux: { target: 'happy:different' } }, + legacyTerminalMetadataRemoval: true, + })).resolves.toBe(false); + await expect(readTerminalAttachmentInfo({ happyHomeDir: dir.name, sessionId })).resolves.toMatchObject({ + version: 1, + }); + } finally { + dir.removeCallback(); + } + }); + it('parks legacy v1 attachment removal even when its terminal metadata matches', async () => { const dir = tmp.dirSync({ unsafeCleanup: true }); try { diff --git a/apps/cli/src/terminal/attachment/terminalAttachmentInfo.ts b/apps/cli/src/terminal/attachment/terminalAttachmentInfo.ts index a9f919dcd..db656285b 100644 --- a/apps/cli/src/terminal/attachment/terminalAttachmentInfo.ts +++ b/apps/cli/src/terminal/attachment/terminalAttachmentInfo.ts @@ -118,13 +118,26 @@ async function removeTerminalAttachmentInfoPath(params: { sessionId: string; expectedAttachmentId?: TerminalAttachmentId | string | undefined; expectedTerminal?: NonNullable | undefined; + /** Allow removing a v1 record by terminal metadata match only (no attachmentId CAS). */ + legacyTerminalMetadataRemoval?: boolean; }): Promise { try { const raw = await readFile(params.path, 'utf8'); const parsed = parseTerminalAttachmentInfo(raw, params.sessionId); + if (!parsed) return false; + + if (params.legacyTerminalMetadataRemoval && parsed.version === 1) { + // For v1 (legacy) records: allow removal by terminal metadata deep-equality only. + // This is deliberately narrow — the caller must prove the host is dead before using this. + if (!params.expectedTerminal) return false; + if (!terminalMatchesExpected(parsed.terminal, params.expectedTerminal)) return false; + await unlink(params.path); + return true; + } + if (!params.expectedAttachmentId) return false; - if (parsed?.version !== 2 || parsed.attachmentId !== params.expectedAttachmentId) return false; - if (!parsed || !terminalMatchesExpected(parsed.terminal, params.expectedTerminal)) return false; + if (parsed.version !== 2 || parsed.attachmentId !== params.expectedAttachmentId) return false; + if (!terminalMatchesExpected(parsed.terminal, params.expectedTerminal)) return false; await unlink(params.path); return true; } catch { @@ -179,6 +192,8 @@ export async function removeTerminalAttachmentInfo(params: { sessionId: string; expectedAttachmentId?: TerminalAttachmentId | string | undefined; expectedTerminal?: NonNullable | undefined; + /** Allow removing a v1 record by terminal metadata match only (no attachmentId CAS). */ + legacyTerminalMetadataRemoval?: boolean; }): Promise { const encodedPath = sessionFilePath(params.happyHomeDir, params.sessionId); if (await removeTerminalAttachmentInfoPath({ @@ -186,6 +201,7 @@ export async function removeTerminalAttachmentInfo(params: { sessionId: params.sessionId, expectedAttachmentId: params.expectedAttachmentId, expectedTerminal: params.expectedTerminal, + legacyTerminalMetadataRemoval: params.legacyTerminalMetadataRemoval, })) { return true; } @@ -197,6 +213,7 @@ export async function removeTerminalAttachmentInfo(params: { sessionId: params.sessionId, expectedAttachmentId: params.expectedAttachmentId, expectedTerminal: params.expectedTerminal, + legacyTerminalMetadataRemoval: params.legacyTerminalMetadataRemoval, }); } diff --git a/apps/cli/src/terminal/attachment/terminalHostDisposition.test.ts b/apps/cli/src/terminal/attachment/terminalHostDisposition.test.ts index a70e09d7b..50e1942cc 100644 --- a/apps/cli/src/terminal/attachment/terminalHostDisposition.test.ts +++ b/apps/cli/src/terminal/attachment/terminalHostDisposition.test.ts @@ -232,7 +232,35 @@ describe('executeTerminalHostDisposition', () => { } }); - it('parks legacy and shared missing-pane attachments without physical destruction', async () => { + it('retires a confirmed-dead v1 legacy attachment by terminal metadata match', async () => { + const dir = tmp.dirSync({ unsafeCleanup: true }); + try { + const terminal = { + mode: 'tmux' as const, + tmux: { target: 'happy:legacy-window', tmpDir: '/tmp/happier-tmux-root' }, + }; + await writeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'session-legacy-dead', + terminal, + }); + + await expect(executeTerminalHostDisposition({ + happyHomeDir: dir.name, + sessionId: 'session-legacy-dead', + expectedAttachmentId: 'attachment-unused', + intent: { kind: 'retire_confirmed_dead_attachment', reason: 'positive_dead_recovery' }, + })).resolves.toMatchObject({ status: 'retired_legacy' }); + await expect(readTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'session-legacy-dead', + })).resolves.toBeNull(); + } finally { + dir.removeCallback(); + } + }); + + it('retires a legacy v1 attachment on confirmed-dead intent without physical destruction', async () => { const dir = tmp.dirSync({ unsafeCleanup: true }); try { await writeTerminalAttachmentInfo({ @@ -248,6 +276,64 @@ describe('executeTerminalHostDisposition', () => { expectedAttachmentId: 'attachment-guessed', intent: { kind: 'retire_confirmed_dead_attachment', reason: 'positive_dead_recovery' }, adapter: buildAdapter(dispose), + })).resolves.toMatchObject({ status: 'retired_legacy' }); + expect(dispose).not.toHaveBeenCalled(); + await expect(readTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'legacy-session', + })).resolves.toBeNull(); + } finally { + dir.removeCallback(); + } + }); + + it('parks legacy v1 retirement when on-disk terminal differs from proven terminal', async () => { + const dir = tmp.dirSync({ unsafeCleanup: true }); + try { + // Write a v1 record with terminal A + await writeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'legacy-race', + terminal: { mode: 'tmux', tmux: { target: 'happy:original-window' } }, + }); + + // Caller proves terminal B is dead (different from what's on disk now) + const provenTerminal = { mode: 'tmux' as const, tmux: { target: 'happy:probed-window' } }; + + await expect(executeTerminalHostDisposition({ + happyHomeDir: dir.name, + sessionId: 'legacy-race', + expectedAttachmentId: 'unused', + intent: { kind: 'retire_confirmed_dead_attachment', reason: 'positive_dead_recovery' }, + provenDeadLegacyTerminal: provenTerminal, + })).resolves.toMatchObject({ status: 'parked', reason: 'legacy_attachment' }); + + // The on-disk record must survive + await expect(readTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'legacy-race', + })).resolves.not.toBeNull(); + } finally { + dir.removeCallback(); + } + }); + + it('parks a legacy v1 attachment on destroy intent (no immutable identity)', async () => { + const dir = tmp.dirSync({ unsafeCleanup: true }); + try { + await writeTerminalAttachmentInfo({ + happyHomeDir: dir.name, + sessionId: 'legacy-destroy', + terminal: { mode: 'tmux', tmux: { target: 'happy' } }, + }); + const dispose = vi.fn(async () => undefined); + + await expect(executeTerminalHostDisposition({ + happyHomeDir: dir.name, + sessionId: 'legacy-destroy', + expectedAttachmentId: 'attachment-guessed', + intent: { kind: 'destroy_owned_host', reason: 'explicit_user_stop' }, + adapter: buildAdapter(dispose), })).resolves.toMatchObject({ status: 'parked', reason: 'legacy_attachment' }); expect(dispose).not.toHaveBeenCalled(); } finally { diff --git a/apps/cli/src/terminal/attachment/terminalHostDisposition.ts b/apps/cli/src/terminal/attachment/terminalHostDisposition.ts index 907906c9a..03c6ecbca 100644 --- a/apps/cli/src/terminal/attachment/terminalHostDisposition.ts +++ b/apps/cli/src/terminal/attachment/terminalHostDisposition.ts @@ -24,6 +24,7 @@ export type TerminalHostDispositionIntent = export type TerminalHostDispositionResult = | Readonly<{ status: 'preserved'; attachmentId: TerminalAttachmentId }> | Readonly<{ status: 'retired'; attachmentId: TerminalAttachmentId }> + | Readonly<{ status: 'retired_legacy' }> | Readonly<{ status: 'destroyed'; attachmentId: TerminalAttachmentId; @@ -43,12 +44,19 @@ export async function executeTerminalHostDisposition(input: Readonly<{ expectedAttachmentId: TerminalAttachmentId | string; intent: TerminalHostDispositionIntent; adapter?: TerminalHostAdapter; + /** + * For legacy v1 retirement: the exact terminal metadata the caller probed and confirmed dead. + * The on-disk record is only removed when it deep-equals this value, preventing removal of + * a concurrently rewritten record whose liveness was never verified. + */ + provenDeadLegacyTerminal?: TerminalAttachmentInfo['terminal']; readAttachmentInfo?: (input: Readonly<{ happyHomeDir: string; sessionId: string }>) => Promise; removeAttachmentInfo?: (input: Readonly<{ happyHomeDir: string; sessionId: string; expectedAttachmentId: TerminalAttachmentId | string; expectedTerminal: TerminalAttachmentInfo['terminal']; + legacyTerminalMetadataRemoval?: boolean; }>) => Promise; /** Runs after physical retirement is proven and before the local retry identity is removed. */ beforeDescriptorRetirement?: (input: Readonly<{ @@ -63,9 +71,30 @@ export async function executeTerminalHostDisposition(input: Readonly<{ happyHomeDir: input.happyHomeDir, sessionId: input.sessionId, }); - if (!attachmentInfo || attachmentInfo.version === 1) { + if (!attachmentInfo) { return { status: 'parked', reason: 'legacy_attachment' }; } + if (attachmentInfo.version === 1) { + // Legacy v1 records can only be retired when the host is confirmed dead. + // For destroy or preserve intents, we cannot safely proceed without an immutable identity. + if (input.intent.kind !== 'retire_confirmed_dead_attachment') { + return { status: 'parked', reason: 'legacy_attachment' }; + } + // Remove the v1 descriptor by terminal metadata match (no attachmentId CAS). + // Compare against the caller's proven-dead terminal, not the fresh read's own terminal, + // so a concurrent rewrite with different metadata is not silently removed. + const legacyExpectedTerminal = input.provenDeadLegacyTerminal ?? attachmentInfo.terminal; + const removed = await removeAttachment({ + happyHomeDir: input.happyHomeDir, + sessionId: input.sessionId, + expectedAttachmentId: input.expectedAttachmentId, + expectedTerminal: legacyExpectedTerminal, + legacyTerminalMetadataRemoval: true, + }); + return removed + ? { status: 'retired_legacy' } + : { status: 'parked', reason: 'legacy_attachment' }; + } if (attachmentInfo.attachmentId !== input.expectedAttachmentId) { return { status: 'parked', reason: 'attachment_mismatch' }; }