diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 34f399d8c38..cbf6cd9a965 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -7,6 +7,7 @@ import { RESET_TERMINAL_CURSOR_STYLE } from './layout-serialization' import type * as UseNotificationDispatchModule from './use-notification-dispatch' +import { getEagerPtyBufferHandle } from './pty-dispatcher' import { makePaneKey } from '../../../../shared/stable-pane-id' import type { TerminalLayoutSnapshot } from '../../../../shared/types' @@ -214,6 +215,18 @@ vi.mock('./remote-runtime-pty-transport', () => ({ ) })) +// Why: the adopt-vs-reattach decision consults the eager-PTY buffer registry to +// tell a still-live locally-spawned PTY (attach + replay) from a daemon session +// to re-connect. Keep the real module but stub the lookup so tests can simulate +// a live eager buffer without standing up the real IPC dispatcher. +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + function createMockTransport(initialPtyId: string | null = null): MockTransport { let ptyId = initialPtyId const transport = { @@ -2559,6 +2572,96 @@ describe('connectPanePty', () => { expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(2, 'leaf-pty-2') }) + it('adopts a still-live background PTY via attach instead of reattaching when an eager buffer exists', async () => { + // Why: background automation tabs spawn the agent PTY eagerly and register an + // eager buffer, then never mount until opened. On first mount the restored + // ptyId equals the tab ptyId, so without this guard the pane mis-routes into + // the daemon-reattach branch (connect) and spawns a fresh shell, orphaning the + // live agent PTY. A live eager buffer means "attach + replay", not "reattach". + const eagerPtyId = 'auto-eager-pty' + vi.mocked(getEagerPtyBufferHandle).mockImplementation((ptyId: string) => + ptyId === eagerPtyId ? { flush: () => '', dispose: () => {} } : undefined + ) + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: eagerPtyId }] }, + ptyIdsByTabId: { 'tab-1': [eagerPtyId] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_1 }, + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: eagerPtyId } + } + } + } as StoreState + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: eagerPtyId } + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks() + + expect(transport.attach).toHaveBeenCalledWith( + expect.objectContaining({ existingPtyId: eagerPtyId }) + ) + expect(transport.connect).not.toHaveBeenCalledWith( + expect.objectContaining({ sessionId: eagerPtyId }) + ) + const { hasPtySerializer } = await import('./pty-buffer-serializer') + expect(hasPtySerializer(eagerPtyId)).toBe(true) + }) + + it('does not adopt another tab live eager PTY from a stale restored leaf binding', async () => { + // Why: restored leaf bindings can outlive tab ownership. A global eager + // buffer only proves the PTY is alive; ptyIdsByTabId proves this tab owns it. + const otherTabPtyId = 'other-tab-eager-pty' + vi.mocked(getEagerPtyBufferHandle).mockImplementation((ptyId: string) => + ptyId === otherTabPtyId ? { flush: () => '', dispose: () => {} } : undefined + ) + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async (opts: { sessionId?: string }) => { + if (opts.sessionId) { + return { id: opts.sessionId } + } + return 'fresh-pty' + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [ + { id: 'tab-1', ptyId: 'tab-pty' }, + { id: 'tab-2', ptyId: otherTabPtyId } + ] + }, + ptyIdsByTabId: { + 'tab-1': ['tab-pty'], + 'tab-2': [otherTabPtyId] + } + } as StoreState + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: otherTabPtyId } + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks() + + expect(transport.attach).not.toHaveBeenCalledWith( + expect.objectContaining({ existingPtyId: otherTabPtyId }) + ) + expect(transport.connect).not.toHaveBeenCalledWith( + expect.objectContaining({ sessionId: otherTabPtyId }) + ) + expect(deps.updateTabPtyId).not.toHaveBeenCalledWith('tab-1', otherTabPtyId) + }) + it('spawns a fresh PTY when a restored daemon split session cannot reattach', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 5d178070b19..31db682e583 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -75,6 +75,7 @@ import { } from './terminal-bracketed-paste' import { createCommandCodeOutputStatusDetector } from './command-code-output-status' import type { PtyDataMeta } from './pty-dispatcher' +import { getEagerPtyBufferHandle } from './pty-dispatcher' import { createTerminalGitHubPRLinkDetector } from '@/lib/terminal-github-pr-link-detector' import { installConptyDeviceAttributesHandler } from './terminal-conpty-device-attributes' import { @@ -3031,6 +3032,24 @@ export function connectPanePty( restoredSessionId && restoredSessionId !== detachedLivePtyId ? restoredSessionId : detachedLivePtyId + const currentTabLivePtyIds = storeSnapshot.ptyIdsByTabId[deps.tabId] ?? [] + const candidateHasEagerBuffer = Boolean( + candidateReattachSessionId && + !isRemoteRuntimePtyId(candidateReattachSessionId) && + getEagerPtyBufferHandle(candidateReattachSessionId) + ) + // Why: a still-live locally-spawned PTY (e.g. a background automation agent + // launched before its tab mounts) keeps an eager buffer until a pane adopts + // it. Such a PTY must be adopted via attach()+replay, not re-connected as a + // daemon session — connect({ sessionId }) on a non-session ptyId spawns a + // fresh shell and orphans the live agent. Presence of an eager buffer plus + // current-tab live ownership is the discriminator; route these to attach. + const eagerLivePtyId = + candidateReattachSessionId && + candidateHasEagerBuffer && + currentTabLivePtyIds.includes(candidateReattachSessionId) + ? candidateReattachSessionId + : null // Why: daemon session IDs encode `${worktreeId}@@${uuid}`. After a daemon // crash + cold restore, corrupted or stale session-to-tab mappings can // cause a tab in workspace A to hold a ptyId from workspace B. Restoring @@ -3039,6 +3058,7 @@ export function connectPanePty( const deferredReattachSessionId = candidateReattachSessionId && !isRemoteRuntimePtyId(candidateReattachSessionId) && + !candidateHasEagerBuffer && isSessionOwnedByWorktree(candidateReattachSessionId, deps.worktreeId) ? candidateReattachSessionId : null @@ -3131,11 +3151,14 @@ export function connectPanePty( prepareColdRestoreAgentResumeCommand() startFreshSpawn() }) - } else if (detachedRemoteLeafPtyId || detachedLivePtyId) { + } else if (detachedRemoteLeafPtyId || detachedLivePtyId || eagerLivePtyId) { // Why: mirrored web terminal layouts mount one pane per host leaf. // Later leaves already have a pane transport, but must still attach to // their exact remote PTY instead of spawning replacement host tabs. - const attachPtyId = detachedRemoteLeafPtyId ?? detachedLivePtyId! + // eagerLivePtyId covers a still-live background PTY (e.g. an automation + // agent) whose restored id may not equal the tab ptyId yet still has a + // live eager buffer to adopt. + const attachPtyId = detachedRemoteLeafPtyId ?? detachedLivePtyId ?? eagerLivePtyId! recordPtyConnectDiagnostic(`pane=${pane.id} -> ATTACH detached=${attachPtyId}`) allowInitialIdleCacheSeed = false // Why: surface synchronous attach failures (e.g., the PTY died between @@ -3161,6 +3184,9 @@ export function connectPanePty( deps.syncPanePtyLayoutBinding(pane.id, attachPtyId) deps.updateTabPtyId(deps.tabId, attachPtyId) agentCompletionCoordinator.startProcessTracking() + if (attachPtyId === eagerLivePtyId) { + registerPaneSerializerFor(attachPtyId) + } } catch (err) { reportError(err instanceof Error ? err.message : String(err)) deps.clearTabPtyId(deps.tabId, attachPtyId) diff --git a/src/renderer/src/runtime/sync-runtime-graph-automation-leaf.test.ts b/src/renderer/src/runtime/sync-runtime-graph-automation-leaf.test.ts new file mode 100644 index 00000000000..8bab08206b2 --- /dev/null +++ b/src/renderer/src/runtime/sync-runtime-graph-automation-leaf.test.ts @@ -0,0 +1,124 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeSyncWindowGraph } from '../../../shared/runtime-types' +import type { AppState } from '../store/types' +import type { TerminalTab } from '../../../shared/types' + +// Why: Part B publishes never-mounted background automation tabs into the +// runtime graph, gated on a live eager buffer. Stub the eager-buffer lookup so +// the test can flip "still-live unmounted PTY" on and off without the real IPC +// dispatcher, and spy on the anomaly warning to prove it stays scoped to mounted +// tabs. +const { warnTerminalLifecycleAnomaly } = vi.hoisted(() => ({ + warnTerminalLifecycleAnomaly: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { ...actual, getEagerPtyBufferHandle: vi.fn(() => undefined) } +}) +vi.mock('@/components/terminal-pane/terminal-lifecycle-diagnostics', async (importOriginal) => { + const actual = await importOriginal>() + return { ...actual, warnTerminalLifecycleAnomaly } +}) + +import { getEagerPtyBufferHandle } from '@/components/terminal-pane/pty-dispatcher' +import { setRuntimeGraphStoreStateGetter, setRuntimeGraphSyncEnabled } from './sync-runtime-graph' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const AUTO_PTY = 'auto-bg-pty' + +function makeState(overrides: Partial = {}): AppState { + return { + tabsByWorktree: {}, + terminalLayoutsByTabId: {} as AppState['terminalLayoutsByTabId'], + runtimePaneTitlesByTabId: {} as AppState['runtimePaneTitlesByTabId'], + groupsByWorktree: {}, + activeGroupIdByWorktree: {}, + layoutByWorktree: {}, + unifiedTabsByWorktree: {}, + tabBarOrderByWorktree: {}, + activeFileId: null, + activeFileIdByWorktree: {}, + openFiles: [], + editorDrafts: {}, + activeTabId: null, + ...overrides + } as AppState +} + +function makeAutomationTab(): TerminalTab { + return { + id: 'auto-tab-1', + ptyId: AUTO_PTY, + worktreeId: 'wt-1', + title: 'Generate PO review prep brief', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function automationState(): AppState { + return makeState({ + tabsByWorktree: { 'wt-1': [makeAutomationTab()] } as AppState['tabsByWorktree'], + terminalLayoutsByTabId: { + 'auto-tab-1': { + root: { type: 'leaf', leafId: LEAF }, + activeLeafId: LEAF, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF]: AUTO_PTY } + } + } as AppState['terminalLayoutsByTabId'] + }) +} + +async function flushMicrotasks(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +afterEach(() => { + setRuntimeGraphSyncEnabled(false) + setRuntimeGraphStoreStateGetter(null) + vi.mocked(getEagerPtyBufferHandle).mockReturnValue(undefined) + warnTerminalLifecycleAnomaly.mockClear() + vi.unstubAllGlobals() +}) + +async function captureGraph(): Promise { + const syncWindowGraph = vi.fn().mockResolvedValue(undefined) + vi.stubGlobal('window', { api: { runtime: { syncWindowGraph } } }) + vi.stubGlobal('HTMLElement', class HTMLElement {}) + setRuntimeGraphStoreStateGetter(() => automationState()) + setRuntimeGraphSyncEnabled(true) + await flushMicrotasks() + expect(syncWindowGraph).toHaveBeenCalledTimes(1) + return syncWindowGraph.mock.calls[0]![0] as RuntimeSyncWindowGraph +} + +describe('syncRuntimeGraph background automation tabs', () => { + it('publishes an unmounted automation tab leaf when its PTY is still live (eager buffer present)', async () => { + vi.mocked(getEagerPtyBufferHandle).mockImplementation((ptyId: string) => + ptyId === AUTO_PTY ? { flush: () => '', dispose: () => {} } : undefined + ) + + const graph = await captureGraph() + + expect(graph.leaves).toContainEqual( + expect.objectContaining({ tabId: 'auto-tab-1', leafId: LEAF, ptyId: AUTO_PTY }) + ) + expect(graph.tabs).toContainEqual(expect.objectContaining({ tabId: 'auto-tab-1' })) + // Why: the no-live-transport anomaly must stay scoped to mounted tabs; an + // unmounted background tab legitimately has no live transport yet. + expect(warnTerminalLifecycleAnomaly).not.toHaveBeenCalled() + }) + + it('does not publish an unmounted tab whose saved PTY is no longer live (no eager buffer)', async () => { + vi.mocked(getEagerPtyBufferHandle).mockReturnValue(undefined) + + const graph = await captureGraph() + + expect(graph.leaves).not.toContainEqual(expect.objectContaining({ tabId: 'auto-tab-1' })) + expect(graph.tabs).not.toContainEqual(expect.objectContaining({ tabId: 'auto-tab-1' })) + }) +}) diff --git a/src/renderer/src/runtime/sync-runtime-graph.ts b/src/renderer/src/runtime/sync-runtime-graph.ts index 112f60a3bb4..80a8b27b70a 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.ts @@ -5,6 +5,7 @@ import { normalizeTerminalLayoutSnapshot } from '@/components/terminal-pane/layout-serialization' import { warnTerminalLifecycleAnomaly } from '@/components/terminal-pane/terminal-lifecycle-diagnostics' +import { getEagerPtyBufferHandle } from '@/components/terminal-pane/pty-dispatcher' import { createBrowserUuid } from '@/lib/browser-uuid' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { resolveLeafIdForManager } from '@/lib/pane-manager/pane-key-resolution' @@ -570,6 +571,55 @@ async function syncRuntimeGraph(): Promise { } } + // Why: background automation tabs spawn their agent PTY eagerly and are created + // inactive, so they never mount a TerminalPane and never enter `registeredTabs`. + // Without this pass their leaf+ptyId is never published, so the runtime treats + // the live agent PTY as orphaned (surfaced as a synthetic `pty:` terminal) + // and `orca terminal list` / session-reuse can't see the real tab. Publish them + // from the persisted layout, gated on a live eager buffer so we only adopt a + // still-running unmounted PTY (never a stale saved ptyId). + for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { + for (const tab of tabs) { + if (registeredTabs.has(tab.id) || isWebOnlyMirroredTerminalTab(state, tab)) { + continue + } + const layout = state.terminalLayoutsByTabId[tab.id] + const savedPtyIdsByLeafId = layout?.ptyIdsByLeafId + if (!savedPtyIdsByLeafId) { + continue + } + const liveLeaves = Object.entries(savedPtyIdsByLeafId).filter( + ([leafId, ptyId]) => + typeof ptyId === 'string' && + ptyId.length > 0 && + isTerminalLeafId(leafId) && + Boolean(getEagerPtyBufferHandle(ptyId)) + ) + if (liveLeaves.length === 0) { + continue + } + const title = resolveRuntimeTerminalTitle(tab, generatedTitlesEnabled) + graph.tabs.push({ + tabId: tab.id, + worktreeId, + title, + activeLeafId: layout?.activeLeafId ?? liveLeaves[0][0], + layout: layout?.root ?? fallbackLayoutForLeafIds(liveLeaves.map(([leafId]) => leafId)) + }) + liveLeaves.forEach(([leafId, ptyId], index) => { + graph.leaves.push({ + tabId: tab.id, + worktreeId, + leafId, + paneRuntimeId: index + 1, + ptyId, + paneTitle: null, + title + }) + }) + } + } + try { const result = await window.api.runtime.syncWindowGraph(graph) getStoreState()?.setRuntimeAgentOrchestrationByPaneKey?.(