diff --git a/src/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsx b/src/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsx index 7a76434ce2f..ec897d479da 100644 --- a/src/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsx +++ b/src/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsx @@ -3,15 +3,16 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' import { useTerminalWatcherEffects } from '../use-terminal-watcher-effects' -import type { TerminalColdActivationController } from '../terminal-cold-activation' const mocks = vi.hoisted(() => ({ gate: vi.fn(), + resume: vi.fn(), + authority: 'none', launchStatus: vi.fn((_worktreeId: string, _provider: string): string => 'idle'), createTab: vi.fn() })) vi.mock('@/store', () => ({ - useAppStore: Object.assign(() => 'none', { + useAppStore: Object.assign(() => mocks.authority, { getState: () => ({ activeWorktreeId: 'wt-1' }) }) })) @@ -22,7 +23,7 @@ vi.mock('@/lib/structured-agent-session-launch', () => ({ getStructuredAgentLaunchStatus: mocks.launchStatus })) vi.mock('@/lib/resume-sleeping-agent-session', () => ({ - resumeSleepingAgentSessionsForWorktree: vi.fn() + resumeSleepingAgentSessionsForWorktree: mocks.resume })) vi.mock('@/lib/workspace-terminal-host-authority', () => ({ createWorkspaceTerminalHostAuthoritySelector: () => () => 'none' @@ -34,23 +35,48 @@ vi.mock('../terminal-pane/terminal-parked-tab-watchers', () => ({ disposeAllParkedTerminalWatchers: vi.fn() })) -;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }) let root: Root | undefined afterEach(async () => { await act(async () => root?.unmount()) vi.clearAllMocks() + mocks.authority = 'none' }) -function Watcher(): null { +function Watcher({ restored = true, hydrated = false, worktreeId = 'wt-1' } = {}): null { useTerminalWatcherEffects({ - activeWorktreeId: 'wt-1', + activeWorktreeId: worktreeId, workspaceSessionReady: true, - terminalStartupRestorationReady: true, + terminalStartupRestorationReady: restored, + hydrationSucceeded: hydrated, workspaceSurfaceIds: [], tabsByWorktree: {}, createTab: mocks.createTab, - reconcileWorktreeTabModel: () => ({ renderableTabCount: 0 }) - } as unknown as TerminalColdActivationController) + reconcileWorktreeTabModel: () => ({ + renderableTabCount: 0, + activeRenderableTabId: null + }), + activationDeferredMountTabIdsByWorktreeRef: { current: new Map() }, + activeTabId: null, + activeTabIdByWorktree: {}, + activeView: 'terminal', + activityTerminalPortals: [], + anyMountedWorktreeHasLayout: false, + backgroundMountRevision: 0, + effectiveParkedTerminalWorktreeIds: new Set(), + evictionExemptTerminalTabIds: new Set(), + getEffectiveLayoutForWorktree: () => undefined, + groupsByWorktree: {}, + measurableBackgroundWorktreeIdsRef: { current: new Set() }, + mountedWorktreeIdsRef: { current: new Set() }, + pairedRuntimeParkingEnvironmentIds: new Set(), + pendingStartupByTabId: {}, + renderedActiveWorktreeId: worktreeId, + terminalParkingEnabled: false, + terminalProviderSnapshotCapabilityRevision: 0, + terminalSshParkingEnabled: false, + terminalTitleSnapshotAuthorityEnabled: false + }) return null } @@ -81,3 +107,51 @@ describe('passive terminal seeding during native chat creation', () => { expect(mocks.createTab).toHaveBeenCalledTimes(expectedTabs) }) }) + +describe('startup agent recovery host inventory', () => { + it('keeps recovery available until the execution host answers', async () => { + mocks.authority = 'unverifiable' + mocks.gate.mockResolvedValue('adopted') + root = createRoot(document.createElement('div')) + await act(async () => root?.render()) + expect(mocks.gate).not.toHaveBeenCalled() + expect(mocks.resume).not.toHaveBeenCalled() + mocks.authority = 'live' + await act(async () => root?.render()) + expect(mocks.gate).toHaveBeenCalledTimes(1) + await act(async () => root?.render()) + expect(mocks.gate).toHaveBeenCalledTimes(1) + }) + + it('waits for terminal restoration, then uses the activation gate', async () => { + mocks.authority = 'live' + mocks.gate.mockResolvedValue('adopted') + root = createRoot(document.createElement('div')) + await act(async () => root?.render()) + expect(mocks.resume).not.toHaveBeenCalled() + expect(mocks.gate).not.toHaveBeenCalled() + await act(async () => root?.render()) + expect(mocks.gate).toHaveBeenCalledWith('wt-1') + expect(mocks.resume).not.toHaveBeenCalled() + }) + + it.each(['blocked', 'rejected'])( + 'retries a %s startup after leaving and returning', + async (outcome) => { + mocks.authority = 'live' + if (outcome === 'rejected') { + mocks.gate.mockRejectedValue(new Error('host unavailable')) + } else { + mocks.gate.mockResolvedValue('blocked') + } + root = createRoot(document.createElement('div')) + await act(async () => root?.render()) + expect(mocks.gate).toHaveBeenCalledTimes(1) + await act(async () => root?.render()) + mocks.gate.mockResolvedValue('adopted') + await act(async () => root?.render()) + expect(mocks.gate.mock.calls.map(([id]) => id)).toEqual(['wt-1', 'wt-2', 'wt-1']) + expect(mocks.resume).not.toHaveBeenCalled() + } + ) +}) diff --git a/src/renderer/src/components/use-terminal-watcher-effects.ts b/src/renderer/src/components/use-terminal-watcher-effects.ts index 3fae0571342..e840c3f9ba0 100644 --- a/src/renderer/src/components/use-terminal-watcher-effects.ts +++ b/src/renderer/src/components/use-terminal-watcher-effects.ts @@ -11,7 +11,6 @@ import { } from './terminal-pane/terminal-parked-tab-watchers' import { useAppStore } from '@/store' import { gateWorktreeAgentActivation } from '@/lib/worktree-agent-activation-gate' -import { resumeSleepingAgentSessionsForWorktree } from '@/lib/resume-sleeping-agent-session' import { createWorkspaceTerminalHostAuthoritySelector } from '@/lib/workspace-terminal-host-authority' import { getStructuredAgentLaunchStatus } from '@/lib/structured-agent-session-launch' import { AGENT_SESSION_PROVIDER_HANDLE_PROVIDERS } from '../../../shared/agent-session-provider-handle' @@ -251,7 +250,12 @@ export function useTerminalWatcherEffects(controller: TerminalWatcherController) const startupResumeWorktreeIdsRef = useRef(new Set()) useEffect(() => { - if (!workspaceSessionReady || !hydrationSucceeded || !activeWorktreeId) { + if ( + !workspaceSessionReady || + !terminalStartupRestorationReady || + !hydrationSucceeded || + !activeWorktreeId + ) { return } if (startupResumeWorktreeIdsRef.current.has(activeWorktreeId)) { @@ -263,7 +267,20 @@ export function useTerminalWatcherEffects(controller: TerminalWatcherController) return } startupResumeWorktreeIdsRef.current.add(activeWorktreeId) - // Why: startup hydration restores the worktree without activateAndRevealWorktree, so orphaned live/quit records need a terminal-surface pass after cold restore. - resumeSleepingAgentSessionsForWorktree(activeWorktreeId) - }, [activeWorktreeId, activeWorktreeHostAuthority, hydrationSucceeded, workspaceSessionReady]) + // Startup recovery needs the same host census and in-flight gate as explicit activation. + void gateWorktreeAgentActivation(activeWorktreeId).then( + (outcome) => { + if (outcome === 'blocked') { + startupResumeWorktreeIdsRef.current.delete(activeWorktreeId) + } + }, + () => startupResumeWorktreeIdsRef.current.delete(activeWorktreeId) + ) + }, [ + activeWorktreeId, + activeWorktreeHostAuthority, + hydrationSucceeded, + terminalStartupRestorationReady, + workspaceSessionReady + ]) } diff --git a/src/renderer/src/lib/worktree-activation-pty-inventory.test.ts b/src/renderer/src/lib/worktree-activation-pty-inventory.test.ts index da717ad50e7..8c41bf66fa8 100644 --- a/src/renderer/src/lib/worktree-activation-pty-inventory.test.ts +++ b/src/renderer/src/lib/worktree-activation-pty-inventory.test.ts @@ -105,9 +105,7 @@ describe('activation inventory census', () => { expect(listSessions).toHaveBeenCalledExactlyOnceWith({ connectionId: 'box' }) }) - // A paired peer's PTYs never enter this client's registry, so refusing to answer would strand the - // workspace with no surface at all; the unscoped inventory is the shipped answer for it. - it('falls back to the unscoped inventory for a workspace it cannot scope', async () => { + it('rejects paired ownership without consulting the client inventory', async () => { const listSessions = stubListSessions(async () => []) await expect( listActivationPtySessions( @@ -125,11 +123,11 @@ describe('activation inventory census', () => { }, worktreeId ) - ).resolves.toEqual([]) - expect(listSessions).toHaveBeenCalledExactlyOnceWith() + ).rejects.toThrow('Activation PTY inventory is unverifiable') + expect(listSessions).not.toHaveBeenCalled() }) - it('retries unscoped when the selected relay is detached, and only then', async () => { + it('propagates detached and unavailable host errors without a client fallback', async () => { const detached = stubListSessions(async (scope) => { if (scope) { throw new Error( @@ -140,8 +138,8 @@ describe('activation inventory census', () => { }) await expect( listActivationPtySessions({ repos: [{ id: 'repo', executionHostId: 'ssh:box' }] }, worktreeId) - ).resolves.toEqual([{ id: 'local-1' }]) - expect(detached.mock.calls).toEqual([[{ connectionId: 'box' }], []]) + ).rejects.toThrow('No PTY provider for connection') + expect(detached.mock.calls).toEqual([[{ connectionId: 'box' }]]) const refused = stubListSessions(async () => { throw new Error('relay unavailable') diff --git a/src/renderer/src/lib/worktree-activation-pty-inventory.ts b/src/renderer/src/lib/worktree-activation-pty-inventory.ts index 26b701938de..ce211a5b63c 100644 --- a/src/renderer/src/lib/worktree-activation-pty-inventory.ts +++ b/src/renderer/src/lib/worktree-activation-pty-inventory.ts @@ -12,19 +12,7 @@ import { type WorktreeOperationRouteState } from './worktree-operation-route' -/** Main rejects a scoped list with this prefix when the relay is detached (pty/provider/registry.ts). */ -const DETACHED_PROVIDER_REJECTION = 'No PTY provider for connection' - -/** - * The one provider that owns this workspace's PTYs, or `undefined` when the client cannot name a - * provider it could reach. - * - * Why `undefined` rather than a throw: a paired-runtime workspace is never in this client's PTY - * registry at all, so refusing to answer turns a healthy peer workspace into a `blocked` gate, and - * activation then leaves it with no surface whatsoever. Falling back to the unscoped diagnostic - * inventory reproduces the shipped answer for exactly those workspaces while the scoped fast path - * still covers local, folder and attached-SSH ones. - */ +/** Only a reachable execution-owner route can authorize activation from its inventory. */ export function resolveActivationPtyListScope( state: WorktreeOperationRouteState, worktreeId: string @@ -61,27 +49,14 @@ export function resolveActivationPtyListScope( return { connectionId: host.kind === 'ssh' ? host.targetId : null } } -/** - * Activation's PTY census, scoped to the owning host whenever the client can name one. - * - * A detached relay is loss of contact, not evidence about the host, and it must not strand the - * workspace: fall back to the same unscoped inventory that shipped so the gate still reaches a - * verdict. Every other rejection is a real answer from the selected host and propagates. - */ +/** An unavailable execution host cannot be replaced with the client's diagnostic inventory. */ export async function listActivationPtySessions( state: WorktreeOperationRouteState, worktreeId: string ): Promise { const scope = resolveActivationPtyListScope(state, worktreeId) if (!scope) { - return window.api.pty.listSessions() - } - try { - return await window.api.pty.listSessions(scope) - } catch (error) { - if (!String((error as Error)?.message ?? error).includes(DETACHED_PROVIDER_REJECTION)) { - throw error - } - return window.api.pty.listSessions() + throw new Error('Activation PTY inventory is unverifiable: no execution-owner route') } + return window.api.pty.listSessions(scope) } diff --git a/src/renderer/src/lib/worktree-agent-activation-gate.test.ts b/src/renderer/src/lib/worktree-agent-activation-gate.test.ts index f80c1e50337..5209d9b134d 100644 --- a/src/renderer/src/lib/worktree-agent-activation-gate.test.ts +++ b/src/renderer/src/lib/worktree-agent-activation-gate.test.ts @@ -415,15 +415,49 @@ describe('worktree agent activation gate', () => { }) }) + it.each(['present', 'unknown'] as const)( + 'does not resume OMP when a surfaced %s agent lacks conversation ownership', + async (agentOwnership) => { + const record = { + ...sleepingRecord('old-tab', DEAD_LEAF_ID, 'omp-session'), + agent: 'omp' as const + } + const ptyId = `${WORKTREE_ID}@@current-omp` + const { deps, resume, createTab } = testDeps({ + sessions: [{ ...listed(ptyId), title: 'OMP', agentOwnership }], + sleeping: [record], + surfaceOwners: new Map([ + [ + ptyId, + { + ptyId, + tabId: 'current-tab', + paneKey: `current-tab:${LIVE_LEAF_ID}` + } + ] + ]) + }) + seedExistingSurface(deps.getState(), { + tabId: 'current-tab', + leafId: LIVE_LEAF_ID, + boundPtyId: ptyId + }) + await expect(runWorktreeAgentActivationGate(WORKTREE_ID, deps)).resolves.toBe('blocked') + expect(resume).not.toHaveBeenCalled() + expect(createTab).not.toHaveBeenCalled() + expect(deps.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toEqual(record) + } + ) + it('does not use an ambiguous tab binding as a live session claim', async () => { const live = sleepingRecord('tab-live', LIVE_LEAF_ID, 'live-session') const livePtyId = `${WORKTREE_ID}@@live-agent` const { deps, resume } = testDeps({ sessions: [listed(livePtyId)], sleeping: [live] }) deps.getState().ptyIdsByTabId['tab-live'] = [livePtyId, `${WORKTREE_ID}@@other-agent`] - await expect(runWorktreeAgentActivationGate(WORKTREE_ID, deps)).resolves.toBe('resumed') + await expect(runWorktreeAgentActivationGate(WORKTREE_ID, deps)).resolves.toBe('blocked') - expect(resume).toHaveBeenCalledWith(WORKTREE_ID, { skipClaimKeys: new Set() }) + expect(resume).not.toHaveBeenCalled() }) it('blocks when a structured TUI owner is absent from live inventory', async () => { diff --git a/src/renderer/src/lib/worktree-agent-activation-gate.ts b/src/renderer/src/lib/worktree-agent-activation-gate.ts index 6ae016512ec..ae650c0e118 100644 --- a/src/renderer/src/lib/worktree-agent-activation-gate.ts +++ b/src/renderer/src/lib/worktree-agent-activation-gate.ts @@ -103,13 +103,14 @@ function sessionBelongsToWorkspace(sessionId: string, worktreeId: string): boole ) } -function liveSleepingAgentClaimKeys( +function liveSleepingAgentClaims( store: ActivationStore, worktreeId: string, livePtyIds: ReadonlySet, structuredInventory: StructuredActivationInventory | null -): Set { +): { keys: Set; claimedPtyIds: Set } { const keys = new Set() + const claimedPtyIds = new Set() for (const record of Object.values(store.sleepingAgentSessionsByPaneKey)) { if (record.worktreeId !== worktreeId) { continue @@ -134,10 +135,11 @@ function liveSleepingAgentClaimKeys( const persistedPtyId = layoutPtyId ?? (tabPtyIds?.length === 1 ? tabPtyIds[0] : undefined) ?? structuredOwnerPtyId if (persistedPtyId && livePtyIds.has(persistedPtyId)) { + claimedPtyIds.add(persistedPtyId) keys.add(getProviderSessionClaimKey(record)) } } - return keys + return { keys, claimedPtyIds } } export async function runWorktreeAgentActivationGate( @@ -243,14 +245,27 @@ export async function runWorktreeAgentActivationGate( if (structured && !workspaceHasSleepingAgentSessions(deps.getState(), worktreeId)) { return 'structured' } - const launched = deps.resume(worktreeId, { - skipClaimKeys: liveSleepingAgentClaimKeys( - deps.getState(), - worktreeId, - liveWorkspacePtyIds, - structuredInventory + const store = deps.getState() + const claims = liveSleepingAgentClaims( + store, + worktreeId, + liveWorkspacePtyIds, + structuredInventory + ) + const hasUnclaimedRecovery = Object.values(store.sleepingAgentSessionsByPaneKey).some( + (record) => + record.worktreeId === worktreeId && !claims.keys.has(getProviderSessionClaimKey(record)) + ) + // A surfaced PTY without a conversation claim may still own the sleeping session. + if ( + hasUnclaimedRecovery && + liveWorkspaceSessions.some( + (session) => session.agentOwnership !== 'absent' && !claims.claimedPtyIds.has(session.id) ) - }) + ) { + return 'blocked' + } + const launched = deps.resume(worktreeId, { skipClaimKeys: claims.keys }) // 'empty' is the caller's directive — "this gate produced no surface, seed one" — not a // claim the host had nothing; the callers re-check their own seeding guards first. return launched > 0 diff --git a/src/renderer/src/lib/worktree-agent-activation-seam.test.ts b/src/renderer/src/lib/worktree-agent-activation-seam.test.ts index 60644670b53..6bf8139cb0d 100644 --- a/src/renderer/src/lib/worktree-agent-activation-seam.test.ts +++ b/src/renderer/src/lib/worktree-agent-activation-seam.test.ts @@ -6,6 +6,7 @@ import type { RuntimeMobileSessionTabsResult, RuntimeTerminalSummary } from '../../../shared/runtime-types' +import * as sleepingResume from './resume-sleeping-agent-session' import { activateAndRevealWorktree } from './worktree-activation' import { waitForWorktreeAgentActivationGateForTests } from './worktree-agent-activation-gate' import { makeCreatedAgentWorktree as makeWorktree } from './worktree-activation-created-agent-test-state' @@ -158,6 +159,7 @@ function stubInventory(args?: { } afterEach(() => { + vi.restoreAllMocks() vi.unstubAllGlobals() useAppStore.setState(initialState, true) }) @@ -302,9 +304,7 @@ describe('worktree agent activation seam', () => { }) }) - // A peer owns its own PTYs, so this client can never scope an inventory at it. Scoping must not - // turn that into a refusal: 'blocked' would also skip the sleeping-agent resume below. - it('still reaches a verdict for a paired-runtime-owned workspace', async () => { + it('does not authorize client-side recovery for a paired-runtime-owned workspace', async () => { const worktree = makeWorktree() useAppStore.setState({ ...baseState(), @@ -313,14 +313,11 @@ describe('worktree agent activation seam', () => { const { listSessions } = stubInventory() expect(activateAndRevealWorktree(worktree.id)).toEqual({ primaryTabId: null }) - await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('empty') - expect(listSessions).toHaveBeenCalledExactlyOnceWith() + await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('blocked') + expect(listSessions).not.toHaveBeenCalled() }) - // Loss of contact with the relay is not evidence about the host, and once the sync has stopped - // without an answer the bounded floor in workspace-terminal-host-authority.ts hands seeding back - // to this client — a detached provider must not turn that into a permanently empty workspace. - it('still seeds a pane when the selected SSH relay is detached', async () => { + it('blocks seeding while detached and allows activation after the owning relay answers', async () => { const worktree = makeWorktree() useAppStore.setState({ ...baseState(), @@ -336,11 +333,58 @@ describe('worktree agent activation seam', () => { }) expect(activateAndRevealWorktree(worktree.id)).toEqual({ primaryTabId: null }) + await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('blocked') + expect(listSessions.mock.calls).toEqual([[{ connectionId: 'box' }]]) + expect(useAppStore.getState().tabsByWorktree[worktree.id] ?? []).toHaveLength(0) + + listSessions.mockResolvedValue([]) + activateAndRevealWorktree(worktree.id) await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('empty') - expect(listSessions.mock.calls).toEqual([[{ connectionId: 'box' }], []]) + expect(listSessions.mock.calls).toEqual([[{ connectionId: 'box' }], [{ connectionId: 'box' }]]) await vi.waitFor(() => expect(useAppStore.getState().tabsByWorktree[worktree.id] ?? []).toHaveLength(1) ) expect(useAppStore.getState().tabsByWorktree[worktree.id]?.[0]?.ptyId).toBeNull() }) + + it('preserves a sleeping OMP session until its execution host can answer', async () => { + const worktree = makeWorktree() + const record = { + paneKey: 'saved-tab:11111111-1111-4111-8111-111111111111', + tabId: 'saved-tab', + worktreeId: worktree.id, + agent: 'omp' as const, + providerSession: { key: 'session_id' as const, id: 'saved-omp-session' }, + prompt: 'resume', + state: 'working' as const, + capturedAt: 1, + updatedAt: 1 + } + useAppStore.setState({ + ...baseState(), + worktreesByRepo: { [worktree.repoId]: [{ ...worktree, hostId: 'ssh:box' }] }, + sleepingAgentSessionsByPaneKey: { [record.paneKey]: record } + }) + const { listSessions } = stubInventory() + const resume = vi + .spyOn(sleepingResume, 'resumeSleepingAgentSessionsForWorktree') + .mockReturnValue(1) + listSessions.mockImplementation(async (scope?: unknown) => { + if (scope) { + throw new Error('No PTY provider for connection "box": the SSH relay is not attached') + } + return [] + }) + + activateAndRevealWorktree(worktree.id) + await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('blocked') + expect(resume).not.toHaveBeenCalled() + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toEqual(record) + + listSessions.mockResolvedValue([]) + activateAndRevealWorktree(worktree.id) + await expect(waitForWorktreeAgentActivationGateForTests(worktree.id)).resolves.toBe('resumed') + expect(resume).toHaveBeenCalledExactlyOnceWith(worktree.id, { skipClaimKeys: new Set() }) + expect(listSessions.mock.calls).toEqual([[{ connectionId: 'box' }], [{ connectionId: 'box' }]]) + }) })