fix(automations): retire owned desktop terminals (#9493)
* fix(automations): retire owned desktop terminals (#9479) * fix(automations): diagnose late settlement failures * fix(automations): clear retired terminal identity and guard finalize close - Clear the run's terminalSessionId/PaneKey/PtyId when finalize() actually retires the owned terminal, so 'View run' resolves to the workspace/snapshot instead of dead-ending on a now-closed terminal. - Guard finalize()'s closeTab in a try/catch; a throwing close reports not-closed so the run keeps its still-valid terminal identity and no stale clear runs. Review follow-ups from PR #9493 (does not attempt the durable cross-reload reconciliation that #9479 needs). Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
chsong
GitHub
parent
cc44acaaa3
commit
6c9be228ad
|
|
@ -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<typeof vi.fn>
|
||||
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<typeof vi.fn>
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string>()
|
||||
|
|
@ -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<void> => {
|
||||
const clearRetiredRunTerminalIdentity = async (): Promise<void> => {
|
||||
// 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<void> => {
|
||||
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>): 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',
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<OwnershipState>): 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<Parameters<typeof createAutomationTerminalOwnership>[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()
|
||||
})
|
||||
})
|
||||
|
|
@ -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<CreateAutomationTerminalOwnershipArgs, 'store' | 'runtimeKind'>
|
||||
): 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
|
||||
}
|
||||
|
|
@ -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
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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<typeof bindAutomationTerminal> = 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())
|
||||
|
|
|
|||
Loading…
Reference in New Issue