diff --git a/src/renderer/src/components/terminal-pane/pty-connection-spawn-left-pane-unbound.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-spawn-left-pane-unbound.test.ts new file mode 100644 index 00000000000..63778d251c0 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-spawn-left-pane-unbound.test.ts @@ -0,0 +1,226 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep, + requestTerminalPaneRecovery +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn(), + requestTerminalPaneRecovery: vi.fn(async () => true) +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync })) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +// Only the request is spied: connect still needs the real generation/instance registry. +vi.mock('./terminal-pane-recovery', async (importOriginal) => ({ + ...(await importOriginal>()), + requestTerminalPaneRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ toast: { info: toastInfo } })) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn( + (_environmentId: string, options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + } + ) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { ...actual, getEagerPtyBufferHandle: vi.fn(() => undefined) } +}) + +describe('fresh spawn leaves a local pane unbound', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) + } + + it('remounts the pane when the spawn resolves without a PTY id', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + // A spawn that produced nothing: no id returned and nothing bound after it. + transport.connect.mockImplementation(async () => null) + transportFactoryQueue.push(transport) + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ tabId: 'tab-unbound-spawn' }) as never + ) + await flushAsyncTicks(40) + + expect(transport.connect).toHaveBeenCalled() + expect(requestTerminalPaneRecovery).toHaveBeenCalledWith( + expect.objectContaining({ + tabId: 'tab-unbound-spawn', + ptyId: null, + reason: 'spawn-left-pane-unbound' + }) + ) + }) + + // The direct-SSH ledger runs its own retry; a second remount would race it. + it('leaves recovery to the direct SSH retry ledger', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockResolvedValueOnce(null) + transportFactoryQueue.push(transport) + const settleDirectSshPaneRetry = vi.fn() + const pendingRetry = { + attemptId: 'attempt-1', + authority: { targetId: 'target-a', providerEpoch: 'epoch-1', connectionGeneration: 3 }, + tabGeneration: 7, + startedAt: 1 + } + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null, generation: 7 }] }, + ptyIdsByTabId: { 'tab-1': [] }, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }], + sshConnectionStates: new Map([ + [ + 'target-a', + { + targetId: 'target-a', + status: 'connected', + providerEpoch: 'epoch-1', + connectionGeneration: 3 + } + ] + ]), + directSshPaneRetryByTabId: { 'tab-1': pendingRetry }, + settleDirectSshPaneRetry + } as StoreState + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + await flushAsyncTicks(40) + + // The lease settled, so the unbound branch ran and deliberately skipped recovery. + expect(settleDirectSshPaneRetry).toHaveBeenCalledWith( + expect.objectContaining({ status: 'failed', attemptId: 'attempt-1' }) + ) + expect(requestTerminalPaneRecovery).not.toHaveBeenCalledWith( + expect.objectContaining({ reason: 'spawn-left-pane-unbound' }) + ) + }) + + it('does not remount when the spawn bound a PTY', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-bound') + transportFactoryQueue.push(transport) + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ tabId: 'tab-bound-spawn' }) as never + ) + await flushAsyncTicks(40) + + expect(requestTerminalPaneRecovery).not.toHaveBeenCalledWith( + expect.objectContaining({ reason: 'spawn-left-pane-unbound' }) + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts index aefc798bb87..f3dab2d5425 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts @@ -2,6 +2,7 @@ import { useAppStore } from '@/store' import { hasPtySerializer } from '../pty-buffer-serializer' import { writeTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' +import { settleSpawnThatLeftPaneUnbound } from './unbound-pane-spawn-recovery' import { STARTUP_CWD_FALLBACK_NOTICE } from './startup-cwd-fallback-notice' import { pendingSpawnByPaneKey, pendingSpawnGenerationByPaneKey } from './pty-connect-limits' import { shouldWritePtyOutputForeground } from './foreground-output-scan' @@ -331,7 +332,7 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { ) { return } - session.settleDirectSshPaneRetryAttempt(session.directSshRetryAttempt, 'failed') + settleSpawnThatLeftPaneUnbound(session) }) }) // Why: split panes in the same tab can spawn concurrently. Key by pane diff --git a/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.test.ts b/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.test.ts new file mode 100644 index 00000000000..25bcd4fe1ed --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.test.ts @@ -0,0 +1,89 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { requestTerminalPaneRecovery } from '../terminal-pane-recovery' +import { settleSpawnThatLeftPaneUnbound } from './unbound-pane-spawn-recovery' + +vi.mock('../terminal-pane-recovery', () => ({ + requestTerminalPaneRecovery: vi.fn() +})) + +function buildSession(overrides: Record = {}): never { + return { + deps: { tabId: 'tab-1', worktreeId: 'wt-1', restoredLeafId: 'leaf-1' }, + pane: { id: 4, leafId: 'pane-leaf' }, + terminalRecoveryGeneration: 2, + terminalRecoveryInstance: { id: 3 }, + directSshRetryAttempt: undefined, + settleDirectSshPaneRetryAttempt: vi.fn(), + ...overrides + } as never +} + +describe('settleSpawnThatLeftPaneUnbound', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('remounts the tab so the pane rebinds over its live PTY', () => { + settleSpawnThatLeftPaneUnbound(buildSession()) + + expect(requestTerminalPaneRecovery).toHaveBeenCalledExactlyOnceWith({ + tabId: 'tab-1', + ptyId: null, + reason: 'spawn-left-pane-unbound', + terminalRecoveryGeneration: 2, + terminalRecoveryInstanceId: 3 + }) + }) + + it('leaves recovery to the direct SSH retry ledger when it holds a lease', () => { + const attempt = { attemptId: 'attempt-1' } + const settleDirectSshPaneRetryAttempt = vi.fn() + + settleSpawnThatLeftPaneUnbound( + buildSession({ directSshRetryAttempt: attempt, settleDirectSshPaneRetryAttempt }) + ) + + expect(settleDirectSshPaneRetryAttempt).toHaveBeenCalledExactlyOnceWith(attempt, 'failed') + expect(requestTerminalPaneRecovery).not.toHaveBeenCalled() + }) + + it('settles the spawn as failed before remounting', () => { + const settleDirectSshPaneRetryAttempt = vi.fn() + + settleSpawnThatLeftPaneUnbound( + buildSession({ deps: { tabId: 'tab-settle' }, settleDirectSshPaneRetryAttempt }) + ) + + expect(settleDirectSshPaneRetryAttempt).toHaveBeenCalledExactlyOnceWith(undefined, 'failed') + expect(requestTerminalPaneRecovery).toHaveBeenCalledOnce() + }) + + // Distinct ids per case: warnTerminalLifecycleAnomaly dedups on a module-global key. + it('prefers the restored leaf id when reporting the anomaly', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + settleSpawnThatLeftPaneUnbound( + buildSession({ deps: { tabId: 'tab-warn', worktreeId: 'wt-1', restoredLeafId: 'leaf-1' } }) + ) + + expect(warn).toHaveBeenCalledWith( + '[terminal-lifecycle] fresh spawn left the pane unbound', + expect.objectContaining({ leafId: 'leaf-1', paneId: 4, worktreeId: 'wt-1' }) + ) + warn.mockRestore() + }) + + it('falls back to the pane leaf id when no restored leaf exists', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + settleSpawnThatLeftPaneUnbound( + buildSession({ deps: { tabId: 'tab-2', worktreeId: 'wt-2', restoredLeafId: null } }) + ) + + expect(warn).toHaveBeenCalledWith( + '[terminal-lifecycle] fresh spawn left the pane unbound', + expect.objectContaining({ leafId: 'pane-leaf' }) + ) + warn.mockRestore() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.ts b/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.ts new file mode 100644 index 00000000000..698df12651c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection/unbound-pane-spawn-recovery.ts @@ -0,0 +1,40 @@ +import { warnTerminalLifecycleAnomaly } from '../terminal-lifecycle-diagnostics' +import { requestTerminalPaneRecovery } from '../terminal-pane-recovery' +import type { ConnectPanePtySession } from './connect-pane-pty-session' + +/** Settle a spawn that resolved without a PTY id, remounting the pane when + * nothing else owns its recovery. + * + * Why this is not self-correcting: the pane stays mounted with no transport + * binding, so `registerData` never runs. Main keeps pushing pty:data for the + * old id, the dispatcher finds no handler and buffers it in the pre-handler + * buffer — which claims no delivery credit, so the bytes are ACKed anyway and + * main's flow control reads healthy while the pane displays its last frame + * forever. The visibility reconciler skips unbound panes, so nothing else + * rebinds one. A remount reattaches over the still-live PTY and drains the + * buffer. + * + * A direct-SSH lease runs its own retry ledger, so it keeps ownership here and + * a second remount never races it. */ +export function settleSpawnThatLeftPaneUnbound(session: ConnectPanePtySession): void { + // Read before settling: the settle clears the lease this branch tests. + const directSshRetryOwnsRecovery = Boolean(session.directSshRetryAttempt) + session.settleDirectSshPaneRetryAttempt(session.directSshRetryAttempt, 'failed') + if (directSshRetryOwnsRecovery) { + return + } + warnTerminalLifecycleAnomaly('fresh spawn left the pane unbound', { + tabId: session.deps.tabId, + worktreeId: session.deps.worktreeId, + leafId: session.deps.restoredLeafId ?? session.pane.leafId, + paneId: session.pane.id, + ptyId: null + }) + void requestTerminalPaneRecovery({ + tabId: session.deps.tabId, + ptyId: null, + reason: 'spawn-left-pane-unbound', + terminalRecoveryGeneration: session.terminalRecoveryGeneration, + terminalRecoveryInstanceId: session.terminalRecoveryInstance.id + }) +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts index 9e2aa0f0d99..ff1f9843ede 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts @@ -30,6 +30,10 @@ export type TerminalPaneRecoveryReason = | 'reattach-unverifiable' // A restore was requested for a certified-dead pipeline (reveal path). | 'restore-blocked' + // A spawn resolved without a PTY id, so the pane is mounted with no transport + // binding. pty:data for the old id then lands in the pre-handler buffer, which + // ACKs it — main's delivery health stays green while the pane shows nothing. + | 'spawn-left-pane-unbound' type RecoveryRequest = { tabId: string