diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index a585199df..48f4f919e 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -79,6 +79,17 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + it('does not create a second kill authority when a mounted pane detaches', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const kill = window.api.pty.kill as unknown as ReturnType + const transport = createIpcPtyTransport({}) + await transport.connect({ url: '', callbacks: {} }) + + transport.detach?.() + + expect(kill).not.toHaveBeenCalled() + }) + it('forwards requested environment deletions to the PTY spawn', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawn = window.api.pty.spawn as unknown as ReturnType diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index a2226262b..a051000b1 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -190,6 +190,28 @@ describe('shouldDetachPaneTransportOnUnmount', () => { }) ).toBe(false) }) + + it('detaches a removed automation pane after closeTab takes teardown authority', () => { + expect( + shouldDetachPaneTransportOnUnmount({ + tabStillExists: false, + tabId: 'automation-tab', + ptyId: 'automation-pty', + worktreeTabs: [ + { + id: 'unrelated-tab', + ptyId: 'unrelated-pty', + worktreeId: 'wt-1', + title: 'Terminal 1', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }) + ).toBe(true) + }) }) describe('mapRestoredPaneTitlesByPaneId', () => { diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts index b4c848f7e..562bbbbf2 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.test.ts @@ -10,6 +10,8 @@ const mockCreateWorktree = vi.fn() const mockMarkDispatchResult = vi.fn() const mockOnDispatchRequested = vi.fn() const mockRendererReady = vi.fn() +const mockFinalizeTerminalOwnership = vi.fn() +const mockReleaseTerminalOwnership = vi.fn() const setupLaunch = { runnerScriptPath: '/tmp/setup.sh', @@ -151,8 +153,13 @@ describe('useAutomationDispatchEvents setup launch', () => { mockLaunchWorktreeBackgroundTerminals.mockResolvedValue(undefined) mockLaunchAgentBackgroundSession.mockResolvedValue({ tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', ptyId: 'agent-pty', - startupPlan: {} + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } }) mockOnDispatchRequested.mockReturnValue(() => {}) vi.stubGlobal('window', { @@ -311,4 +318,204 @@ describe('useAutomationDispatchEvents setup launch', () => { }) ) }) + + it('finalizes a fresh non-reuse terminal only after completed result persistence', async () => { + const order: string[] = [] + let launchArgs: { onAgentStatus?: (payload: { state: string }) => void } = {} + mockMarkDispatchResult.mockImplementation( + async (result: { status: string; terminalPaneKey?: string | null }) => { + // The retirement clear reuses status 'completed' but nulls the terminal + // identity; label it distinctly so ordering stays legible. + order.push( + result.status === 'completed' && result.terminalPaneKey === null + ? 'clear-terminal-identity' + : `persist:${result.status}` + ) + } + ) + mockFinalizeTerminalOwnership.mockImplementation(() => { + order.push('finalize') + return true + }) + mockLaunchAgentBackgroundSession.mockImplementation(async (args) => { + launchArgs = args + return { + tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', + ptyId: 'agent-pty', + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } + } + }) + + await registerAndDispatch() + launchArgs.onAgentStatus?.({ state: 'done' }) + await vi.waitFor(() => expect(mockFinalizeTerminalOwnership).toHaveBeenCalledOnce()) + + expect(order).toEqual([ + 'persist:dispatched', + 'persist:completed', + 'finalize', + 'clear-terminal-identity' + ]) + expect(mockReleaseTerminalOwnership).not.toHaveBeenCalled() + // Why: the retired terminal is gone; the run must drop its pane/pty pointers + // so "View run" resolves to the workspace/snapshot, not an unavailable terminal. + expect(mockMarkDispatchResult).toHaveBeenLastCalledWith({ + runId: expect.any(String), + status: 'completed', + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null + }) + }) + + it('consumes duplicate done and zero-exit completion through one finalizer', async () => { + let launchArgs: { + onAgentStatus?: (payload: { state: string }) => void + onExit?: (ptyId: string, code: number) => void + } = {} + mockLaunchAgentBackgroundSession.mockImplementation(async (args) => { + launchArgs = args + return { + tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', + ptyId: 'agent-pty', + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } + } + }) + + await registerAndDispatch() + launchArgs.onAgentStatus?.({ state: 'done' }) + launchArgs.onExit?.('agent-pty', 0) + launchArgs.onAgentStatus?.({ state: 'done' }) + await vi.waitFor(() => expect(mockFinalizeTerminalOwnership).toHaveBeenCalledOnce()) + + expect( + mockMarkDispatchResult.mock.calls.filter( + ([result]) => result.status === 'completed' && result.terminalPaneKey !== null + ) + ).toHaveLength(1) + expect(mockReleaseTerminalOwnership).not.toHaveBeenCalled() + }) + + it('releases ownership on nonzero exit without finalizing the tab', async () => { + let onExit: ((ptyId: string, code: number) => void) | undefined + mockLaunchAgentBackgroundSession.mockImplementation(async (args) => { + onExit = args.onExit + return { + tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', + ptyId: 'agent-pty', + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } + } + }) + + await registerAndDispatch() + onExit?.('agent-pty', 9) + await vi.waitFor(() => expect(mockReleaseTerminalOwnership).toHaveBeenCalledOnce()) + + expect(mockFinalizeTerminalOwnership).not.toHaveBeenCalled() + expect(mockMarkDispatchResult).toHaveBeenCalledWith( + expect.objectContaining({ status: 'dispatch_failed' }) + ) + }) + + it('releases ownership when dispatched result persistence rejects', async () => { + mockMarkDispatchResult.mockRejectedValueOnce(new Error('persistence unavailable')) + + await registerAndDispatch() + + expect(mockReleaseTerminalOwnership).toHaveBeenCalledOnce() + expect(mockFinalizeTerminalOwnership).not.toHaveBeenCalled() + expect(mockMarkDispatchResult).toHaveBeenLastCalledWith( + expect.objectContaining({ status: 'dispatch_failed' }) + ) + }) + + it('releases ownership when completed result persistence rejects', async () => { + mockMarkDispatchResult + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('completion persistence unavailable')) + .mockResolvedValueOnce(undefined) + mockLaunchAgentBackgroundSession.mockImplementation(async (args) => { + args.onAgentStatus?.({ state: 'done' }) + return { + tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', + ptyId: 'agent-pty', + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } + } + }) + + await registerAndDispatch() + + expect(mockReleaseTerminalOwnership).toHaveBeenCalledOnce() + expect(mockFinalizeTerminalOwnership).not.toHaveBeenCalled() + expect(mockMarkDispatchResult).toHaveBeenLastCalledWith( + expect.objectContaining({ status: 'dispatch_failed' }) + ) + }) + + it('diagnoses a late completed-persistence rejection once without terminal cleanup', async () => { + let onAgentStatus: ((payload: { state: string }) => void) | undefined + const persistenceError = new Error('late completion persistence unavailable') + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + mockMarkDispatchResult.mockResolvedValueOnce(undefined).mockRejectedValueOnce(persistenceError) + mockLaunchAgentBackgroundSession.mockImplementation(async (args) => { + onAgentStatus = args.onAgentStatus + return { + tabId: 'agent-tab', + paneKey: 'agent-tab:7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d', + ptyId: 'agent-pty', + startupPlan: {}, + terminalOwnership: { + finalize: mockFinalizeTerminalOwnership, + release: mockReleaseTerminalOwnership + } + } + }) + + await registerAndDispatch() + onAgentStatus?.({ state: 'done' }) + onAgentStatus?.({ state: 'done' }) + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalledOnce()) + + expect(errorSpy).toHaveBeenCalledWith( + '[automations] Failed to persist late automation result:', + persistenceError + ) + expect(mockReleaseTerminalOwnership).toHaveBeenCalledOnce() + expect(mockFinalizeTerminalOwnership).not.toHaveBeenCalled() + expect( + mockMarkDispatchResult.mock.calls.filter( + ([result]) => result.status === 'completed' && result.terminalPaneKey !== null + ) + ).toHaveLength(1) + errorSpy.mockRestore() + }) + + it('preserves a fresh reuse-enabled session as the future reuse seed', async () => { + mockFindReusableAutomationSession.mockReturnValue(null) + + await registerAndDispatch(makeAutomation({ reuseSession: true })) + + expect(mockReleaseTerminalOwnership).toHaveBeenCalledOnce() + expect(mockFinalizeTerminalOwnership).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.ts index 2de085ada..1679aaf1a 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.ts @@ -23,6 +23,7 @@ import { } from '@/components/automations/automation-run-output-snapshot' import { translate } from '@/i18n/i18n' import { createBrowserUuid } from '@/lib/browser-uuid' +import type { AutomationTerminalOwnership } from '@/lib/automation-terminal-ownership' const AUTOMATIONS_CHANGED_EVENT = 'orca:automations-changed' const activeReuseDispatchTabIds = new Set() @@ -69,6 +70,17 @@ export function useAutomationDispatchEvents(): void { let dispatchWorkspaceDisplayName = automationWorktree?.displayName ?? run.workspaceDisplayName ?? null let precheckResult: AutomationPrecheckResult | null = null + let terminalOwnership: AutomationTerminalOwnership | null = null + const releaseTerminalOwnership = (): void => { + const ownership = terminalOwnership + terminalOwnership = null + ownership?.release() + } + const finalizeTerminalOwnership = (): boolean => { + const ownership = terminalOwnership + terminalOwnership = null + return ownership?.finalize() ?? false + } if (!repo) { await markDispatchResult({ @@ -275,26 +287,74 @@ export function useAutomationDispatchEvents(): void { } completionMarked = true cleanupRunObservers() - await markDispatchResult({ - runId: run.id, - status: 'completed', - workspaceId: worktree.id, - workspaceDisplayName: worktree.displayName, - outputSnapshot: getOutputSnapshot(), - precheckResult, - error: null - }) + try { + await markDispatchResult({ + runId: run.id, + status: 'completed', + workspaceId: worktree.id, + workspaceDisplayName: worktree.displayName, + outputSnapshot: getOutputSnapshot(), + precheckResult, + error: null + }) + } catch (error) { + releaseTerminalOwnership() + throw error + } + if (finalizeTerminalOwnership()) { + await clearRetiredRunTerminalIdentity() + } } - const markExitResult = (code: number): Promise => { + const clearRetiredRunTerminalIdentity = async (): Promise => { + // Why: the owned terminal was just retired, so the run's pane/pty + // pointers now reference a closed tab. Drop them (best-effort) so + // "View run" resolves to the workspace/snapshot instead of dead-ending + // on an unavailable terminal. + try { + await markDispatchResult({ + runId: run.id, + status: 'completed', + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null + }) + } catch (error) { + console.error('[automations] Failed to clear retired terminal identity:', error) + } + } + const markExitResult = async (code: number): Promise => { + if (completionMarked) { + return + } + completionMarked = true cleanupRunObservers() - return markDispatchResult({ - runId: run.id, - status: code === 0 ? 'completed' : 'dispatch_failed', - workspaceId: worktree.id, - workspaceDisplayName: worktree.displayName, - outputSnapshot: getOutputSnapshot(), - precheckResult, - error: code === 0 ? null : `Automation process exited with code ${code}.` + try { + await markDispatchResult({ + runId: run.id, + status: code === 0 ? 'completed' : 'dispatch_failed', + workspaceId: worktree.id, + workspaceDisplayName: worktree.displayName, + outputSnapshot: getOutputSnapshot(), + precheckResult, + error: code === 0 ? null : `Automation process exited with code ${code}.` + }) + } catch (error) { + releaseTerminalOwnership() + throw error + } + if (code === 0) { + if (finalizeTerminalOwnership()) { + await clearRetiredRunTerminalIdentity() + } + } else { + releaseTerminalOwnership() + } + } + const settleLateResult = (result: Promise): void => { + // Why: status/exit callbacks have no awaitable caller; the result + // path already releases ownership before propagating persistence errors. + void result.catch((error) => { + console.error('[automations] Failed to persist late automation result:', error) }) } const handleAgentDone = (): void => { @@ -305,7 +365,7 @@ export function useAutomationDispatchEvents(): void { pendingDone = true return } - void markCompletionResult() + settleLateResult(markCompletionResult()) } const observeAgentStatus = ( targetPaneKey: string, @@ -392,7 +452,7 @@ export function useAutomationDispatchEvents(): void { pendingExitCode = code return } - void markExitResult(code) + settleLateResult(markExitResult(code)) } }) observeAgentStatus(reusableSession.paneKey, reuseCompletionStartedAt, { @@ -449,12 +509,18 @@ export function useAutomationDispatchEvents(): void { pendingExitCode = code return } - void markExitResult(code) + settleLateResult(markExitResult(code)) } }) if (!result) { throw new Error('Unable to build an agent launch plan.') } + terminalOwnership = result.terminalOwnership + if (automation.reuseSession) { + // Why: the first fresh launch is the seed for later reuse and must + // survive completion under the same policy as an already-reused tab. + releaseTerminalOwnership() + } const launchedTabId = result.tabId observeAgentStatus(result.paneKey, dispatchStartedAt) try { @@ -494,6 +560,7 @@ export function useAutomationDispatchEvents(): void { currentState.setActiveTabType(focusBeforeDispatch.activeTabType) } } catch (error) { + releaseTerminalOwnership() await markDispatchResult({ runId: run.id, status: 'dispatch_failed', diff --git a/src/renderer/src/lib/agent-background-session-contract.ts b/src/renderer/src/lib/agent-background-session-contract.ts index 8b2715674..fe793aa24 100644 --- a/src/renderer/src/lib/agent-background-session-contract.ts +++ b/src/renderer/src/lib/agent-background-session-contract.ts @@ -2,6 +2,7 @@ import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-type import type { LaunchSource } from '../../../shared/telemetry-events' import type { TuiAgent } from '../../../shared/types' import type { AgentStartupPlan } from '@/lib/tui-agent-startup' +import type { AutomationTerminalOwnership } from '@/lib/automation-terminal-ownership' export type LaunchAgentBackgroundSessionArgs = { agent: TuiAgent @@ -19,4 +20,5 @@ export type LaunchAgentBackgroundSessionResult = { paneKey: string ptyId: string startupPlan: AgentStartupPlan + terminalOwnership: AutomationTerminalOwnership | null } diff --git a/src/renderer/src/lib/automation-terminal-ownership.test.ts b/src/renderer/src/lib/automation-terminal-ownership.test.ts new file mode 100644 index 000000000..9528bbdf3 --- /dev/null +++ b/src/renderer/src/lib/automation-terminal-ownership.test.ts @@ -0,0 +1,202 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '@/store/types' +import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' +import { + createAutomationTerminalOwnership, + type AutomationTerminalOwnershipStore +} from './automation-terminal-ownership' + +const WORKTREE_ID = 'worktree-1' +const TAB_ID = 'tab-1' +const LEAF_ID = '7c6fb4e5-3bf1-4ff4-8259-03f7ae81c40d' +const PANE_KEY = `${TAB_ID}:${LEAF_ID}` +const PTY_ID = 'pty-1' +const CREATED_AT = 100 + +type OwnershipState = Pick< + AppState, + | 'activeWorktreeId' + | 'activeTabId' + | 'activeTabType' + | 'tabsByWorktree' + | 'ptyIdsByTabId' + | 'terminalLayoutsByTabId' + | 'lastTerminalInputAtByPaneKey' + | 'closeTab' +> + +function createStore() { + const closeTab = vi.fn() + let state: OwnershipState = { + activeWorktreeId: 'other-worktree', + activeTabId: 'other-tab', + activeTabType: 'terminal' as const, + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: TAB_ID, + worktreeId: WORKTREE_ID, + ptyId: PTY_ID, + title: 'Automation', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: CREATED_AT + } + ] + }, + ptyIdsByTabId: { [TAB_ID]: [PTY_ID] }, + terminalLayoutsByTabId: { + [TAB_ID]: singlePaneLayoutSnapshot(LEAF_ID, PTY_ID) + }, + lastTerminalInputAtByPaneKey: {}, + closeTab + } + const listeners = new Set<(state: AppState, previousState: AppState) => void>() + const store: AutomationTerminalOwnershipStore = { + getState: () => state as unknown as AppState, + subscribe: (listener) => { + listeners.add(listener) + return () => listeners.delete(listener) + } + } + const update = (patch: Partial): void => { + const previousState = state + state = { ...state, ...patch } + for (const listener of listeners) { + listener(state as unknown as AppState, previousState as unknown as AppState) + } + } + return { closeTab, getState: () => state, store, update } +} + +function own( + store: AutomationTerminalOwnershipStore, + overrides: Partial[0]> = {} +) { + return createAutomationTerminalOwnership({ + store, + worktreeId: WORKTREE_ID, + tabId: TAB_ID, + paneKey: PANE_KEY, + ptyId: PTY_ID, + tabCreatedAt: CREATED_AT, + runtimeKind: 'desktop', + ...overrides + }) +} + +describe('automation terminal ownership', () => { + beforeEach(() => vi.clearAllMocks()) + + it('closes the exact fresh desktop tab once after the PTY exit binding is cleared', () => { + const { closeTab, store, update } = createStore() + const ownership = own(store) + update({ + ptyIdsByTabId: { [TAB_ID]: [] }, + tabsByWorktree: { + [WORKTREE_ID]: [{ ...store.getState().tabsByWorktree[WORKTREE_ID]![0]!, ptyId: null }] + } + }) + + expect(ownership.finalize()).toBe(true) + expect(ownership.finalize()).toBe(false) + expect(closeTab).toHaveBeenCalledTimes(1) + expect(closeTab).toHaveBeenCalledWith(TAB_ID, { + recordInteraction: false, + reason: 'cleanup' + }) + }) + + it('preserves a tab activated after launch even when focus later moves away', () => { + const { closeTab, store, update } = createStore() + const ownership = own(store) + + update({ activeWorktreeId: WORKTREE_ID, activeTabId: TAB_ID }) + update({ activeWorktreeId: 'other-worktree', activeTabId: 'other-tab' }) + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) + + it('preserves a tab that received user input after launch', () => { + const { closeTab, store, update } = createStore() + const ownership = own(store) + + update({ lastTerminalInputAtByPaneKey: { [PANE_KEY]: 200 } }) + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) + + it.each([ + ['tab PTY', { tabsByWorktree: undefined, ptyIdsByTabId: undefined, layoutPty: undefined }], + [ + 'PTY index', + { tabsByWorktree: null, ptyIdsByTabId: ['pty-replacement'], layoutPty: undefined } + ], + [ + 'pane layout', + { tabsByWorktree: null, ptyIdsByTabId: undefined, layoutPty: 'pty-replacement' } + ] + ])('refuses a replacement identity in the %s binding', (_label, drift) => { + const { closeTab, getState, store, update } = createStore() + const ownership = own(store) + const tab = getState().tabsByWorktree[WORKTREE_ID]![0]! + update({ + ...(drift.tabsByWorktree === undefined + ? { tabsByWorktree: { [WORKTREE_ID]: [{ ...tab, ptyId: 'pty-replacement' }] } } + : {}), + ...(drift.ptyIdsByTabId ? { ptyIdsByTabId: { [TAB_ID]: drift.ptyIdsByTabId } } : {}), + ...(drift.layoutPty + ? { + terminalLayoutsByTabId: { + [TAB_ID]: { + ...getState().terminalLayoutsByTabId[TAB_ID]!, + ptyIdsByLeafId: { [LEAF_ID]: drift.layoutPty } + } + } + } + : {}) + }) + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) + + it('refuses a tab recreated with the same id', () => { + const { closeTab, getState, store, update } = createStore() + const ownership = own(store) + update({ + tabsByWorktree: { + [WORKTREE_ID]: [ + { ...getState().tabsByWorktree[WORKTREE_ID]![0]!, createdAt: CREATED_AT + 1 } + ] + } + }) + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) + + it.each([ + ['remote runtime', { runtimeKind: 'environment' as const }], + ['remote PTY identity', { ptyId: 'remote:env-1@@terminal-1' }] + ])('never owns a %s terminal', (_label, overrides) => { + const { closeTab, store } = createStore() + const ownership = own(store, overrides) + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) + + it('release consumes ownership without closing the terminal', () => { + const { closeTab, store } = createStore() + const ownership = own(store) + + ownership.release() + + expect(ownership.finalize()).toBe(false) + expect(closeTab).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/automation-terminal-ownership.ts b/src/renderer/src/lib/automation-terminal-ownership.ts new file mode 100644 index 000000000..1f39a257a --- /dev/null +++ b/src/renderer/src/lib/automation-terminal-ownership.ts @@ -0,0 +1,162 @@ +import { useAppStore } from '@/store' +import type { AppState } from '@/store/types' +import type { TerminalTab } from '../../../shared/types' +import { parsePaneKey } from '../../../shared/stable-pane-id' +import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' + +export type AutomationTerminalOwnershipStore = { + getState: () => AppState + subscribe: (listener: (state: AppState, previousState: AppState) => void) => () => void +} + +export type AutomationTerminalOwnership = { + finalize: () => boolean + release: () => void +} + +type CreateAutomationTerminalOwnershipArgs = { + store: AutomationTerminalOwnershipStore + worktreeId: string + tabId: string + paneKey: string + ptyId: string + tabCreatedAt: number + runtimeKind: 'desktop' | 'environment' +} + +function isOwnedTabIdentityCurrent( + state: AppState, + args: Omit +): boolean { + const parsedPane = parsePaneKey(args.paneKey) + if (!parsedPane || parsedPane.tabId !== args.tabId) { + return false + } + const matchingTabs = Object.entries(state.tabsByWorktree).flatMap(([worktreeId, tabs]) => + tabs.filter((tab) => tab.id === args.tabId).map((tab) => ({ tab, worktreeId })) + ) + const match = matchingTabs[0] + if ( + matchingTabs.length !== 1 || + !match || + match.worktreeId !== args.worktreeId || + match.tab.worktreeId !== args.worktreeId || + match.tab.createdAt !== args.tabCreatedAt + ) { + return false + } + if (match.tab.ptyId !== null && match.tab.ptyId !== args.ptyId) { + return false + } + if ((state.ptyIdsByTabId[args.tabId] ?? []).some((ptyId) => ptyId !== args.ptyId)) { + return false + } + const layout = state.terminalLayoutsByTabId[args.tabId] + const root = layout?.root + const ptyIdsByLeafId = layout?.ptyIdsByLeafId + if ( + !layout || + !root || + root.type !== 'leaf' || + root.leafId !== parsedPane.leafId || + layout.activeLeafId !== parsedPane.leafId || + Object.keys(ptyIdsByLeafId ?? {}).some((leafId) => leafId !== parsedPane.leafId) + ) { + return false + } + const layoutPtyId = ptyIdsByLeafId?.[parsedPane.leafId] + return layoutPtyId === undefined || layoutPtyId === args.ptyId +} + +export function createAutomationTerminalOwnership( + args: CreateAutomationTerminalOwnershipArgs +): AutomationTerminalOwnership { + let consumed = false + let userTookOver = false + const inputAtLaunch = args.store.getState().lastTerminalInputAtByPaneKey[args.paneKey] + const observeTakeover = (): void => { + const state = args.store.getState() + if ( + (state.activeWorktreeId === args.worktreeId && + state.activeTabId === args.tabId && + state.activeTabType === 'terminal') || + state.lastTerminalInputAtByPaneKey[args.paneKey] !== inputAtLaunch + ) { + userTookOver = true + } + } + const unsubscribe = args.store.subscribe(observeTakeover) + observeTakeover() + + const release = (): void => { + if (consumed) { + return + } + consumed = true + unsubscribe() + } + + return { + release, + finalize: () => { + if (consumed) { + return false + } + // Why: completion and exit can race; consume before any identity check so + // only the first successful-result path can retire this exact session. + consumed = true + unsubscribe() + observeTakeover() + if (args.runtimeKind !== 'desktop' || args.ptyId.startsWith('remote:') || userTookOver) { + return false + } + const state = args.store.getState() + if (!isOwnedTabIdentityCurrent(state, args)) { + return false + } + try { + // Why: closeTab centrally owns provider shutdown and pane removal; a + // direct kill here would create a second teardown authority. + state.closeTab(args.tabId, { recordInteraction: false, reason: 'cleanup' }) + } catch (error) { + // Why: a throwing close leaves the terminal alive; report not-closed so + // the run keeps its (still-valid) terminal identity and no stale clear runs. + console.error('[automations] Failed to close owned automation terminal:', error) + return false + } + return true + } + } +} + +export function bindAutomationTerminal( + tab: TerminalTab, + paneKey: string, + ptyId: string, + runtimeKind: 'local' | 'environment', + title?: string +): AutomationTerminalOwnership | null { + const parsedPane = parsePaneKey(paneKey) + if (!parsedPane || parsedPane.tabId !== tab.id) { + throw new Error('Automation terminal pane identity is invalid.') + } + const store = useAppStore.getState() + if (title) { + store.setTabCustomTitle(tab.id, title, { recordInteraction: false }) + } + store.updateTabPtyId(tab.id, ptyId) + store.setTabLayout(tab.id, singlePaneLayoutSnapshot(parsedPane.leafId, ptyId)) + const ownership = + runtimeKind === 'local' + ? createAutomationTerminalOwnership({ + store: useAppStore, + worktreeId: tab.worktreeId, + tabId: tab.id, + paneKey, + ptyId, + tabCreatedAt: tab.createdAt, + runtimeKind: 'desktop' + }) + : null + return ownership +} diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index 3422f48f6..2bce1932d 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -38,8 +38,8 @@ function expectStablePaneSpawn(): string { } const state = { - activeRepoId: 'repo-1', activeWorktreeId: 'wt-1', + lastTerminalInputAtByPaneKey: {}, settings: { agentCmdOverrides: {}, activeRuntimeEnvironmentId: null as string | null, @@ -91,7 +91,8 @@ const state = { vi.mock('@/store', () => ({ useAppStore: { - getState: () => state + getState: () => state, + subscribe: vi.fn(() => () => {}) } })) @@ -127,8 +128,6 @@ describe('launchAgentBackgroundSession', () => { (args) => createCompatibleRuntimeStatusResponseIfNeeded(args) ?? mockRuntimeEnvironmentCall(args) ) - state.activeRepoId = 'repo-1' - state.activeWorktreeId = 'wt-1' state.settings = { agentCmdOverrides: {}, activeRuntimeEnvironmentId: null, @@ -854,7 +853,8 @@ describe('launchAgentBackgroundSession', () => { expect(result).toMatchObject({ tabId: 'tab-1', paneKey: `tab-1:${leafId}`, - ptyId: 'remote:env-1@@terminal-1' + ptyId: 'remote:env-1@@terminal-1', + terminalOwnership: null }) }) diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index 6ba76340c..15df8a92a 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -39,6 +39,7 @@ import { shouldUseShellReadyStartupDelivery } from '../../../shared/codex-startu import { isMainTerminalSideEffectAuthorityForPty } from '@/components/terminal-pane/terminal-side-effect-facts-handler' import { resolveLocalWindowsAgentStartupShell } from '../../../shared/windows-terminal-shell' import { runBestEffortAgentBackgroundCleanups } from '@/lib/agent-background-session-cleanup' +import { bindAutomationTerminal } from '@/lib/automation-terminal-ownership' import { createBackgroundAgentStatusConsumer } from '@/lib/background-agent-status-consumer' export async function launchAgentBackgroundSession( @@ -105,9 +106,6 @@ export async function launchAgentBackgroundSession( activate: false, recordInteraction: false }) - if (title) { - store.setTabCustomTitle(tab.id, title, { recordInteraction: false }) - } // Why: agent hook callbacks are keyed by pane, and background automation // tabs never mount a TerminalPane to inject this env for us. createBrowserUuid // (not crypto.randomUUID) because the latter is undefined in non-secure @@ -147,11 +145,12 @@ export async function launchAgentBackgroundSession( const runtimeTarget = getActiveRuntimeTarget( getSettingsForWorktreeRuntimeOwner(store, worktreeId) ) - let ptyId = '' - let runtimeTerminalHandle: string | null = null + let ptyId = '', + runtimeTerminalHandle: string | null = null let returnedLaunchConfig: typeof startupPlan.launchConfig | undefined - let exitHandled = false - let eagerPtyBuffer: EagerPtyHandle | null = null + let exitHandled = false, + eagerPtyBuffer: EagerPtyHandle | null = null + let terminalOwnership: ReturnType = null let unsubscribeExit = (): void => {}, unsubscribeData = (): void => {} const handleExit = (exitPtyId: string, code: number): void => { @@ -258,8 +257,7 @@ export async function launchAgentBackgroundSession( if (returnedLaunchConfig) { store.registerAgentLaunchConfig(paneKey, returnedLaunchConfig, launchRegistration) } - store.updateTabPtyId(tab.id, ptyId) - store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId, ptyId)) + terminalOwnership = bindAutomationTerminal(tab, paneKey, ptyId, runtimeTarget.kind, title) if (agent === 'command-code' && hasPrompt && !isFollowupPath) { // Why: Command Code does not expose a prompt-start hook; seed working for // hidden prompt launches so sidebar/activity surfaces do not stay idle. @@ -303,20 +301,20 @@ export async function launchAgentBackgroundSession( unsubscribeExit = subscribeToPtyExit(ptyId, (code) => handleExit(ptyId, code)) } - // Why: mount only after the explicit PTY is bound. Mounting at the earlier - // createTab boundary lets a slow SSH/remote spawn race TerminalPane's fresh - // spawn path and launch the agent twice. + // Why: bind the explicit PTY and ownership before mount; an earlier mount + // can double-spawn, while later tracking can miss user takeover. requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) if (pasteDraftAfterLaunch !== null) { scheduleAgentBackgroundDraft(tab.id, pasteDraftAfterLaunch, agent) } - return { tabId: tab.id, paneKey, ptyId, startupPlan } + return { tabId: tab.id, paneKey, ptyId, startupPlan, terminalOwnership } } catch (error) { // Why: terminal creation and stream subscription are separate remote calls. // A failure between them must not strand an invisible runtime terminal. exitHandled = true + terminalOwnership?.release() runBestEffortAgentBackgroundCleanups(unsubscribeExit, unsubscribeData) runBestEffortAgentBackgroundCleanups(() => eagerPtyBuffer?.dispose()) runBestEffortAgentBackgroundCleanups(() => sshStartupDelivery.clear())