From 3e7730ebb67e8d2259f944eaf13a749f51ea262b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:11:13 -0700 Subject: [PATCH] fix(hibernation): prove local ownership before restored reap --- .../restored-subagent-liveness-sweep.test.ts | 146 ++++++++++++++---- .../restored-subagent-liveness-sweep.ts | 116 ++++++++++---- src/main/agent-hooks/server.ts | 11 +- src/main/index.ts | 59 +++---- 4 files changed, 236 insertions(+), 96 deletions(-) diff --git a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts index 40824f1ee9f..1708de63424 100644 --- a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts +++ b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts @@ -8,6 +8,8 @@ import { toAppSshPtyId } from '../../shared/ssh-pty-id' import { AgentHookServer } from './server' import { indexPersistedPaneKeyPtyIds, + isLocalExecutionHost, + resolveAgentWorkspaceExecutionHostId, sweepRestoredSubagentsWithoutLiveAgent } from './restored-subagent-liveness-sweep' @@ -62,18 +64,22 @@ async function restartWithInFlightSubagent(options?: { function sweepWith( server: AgentHookServer, overrides: { - livePtyIds?: readonly string[] | null + hasLiveLocalPty?: (ptyId: string) => boolean | null + executionHostId?: string | null boundPtyIdByPaneKey?: Record persistedPtyIdByPaneKey?: Record } = {} -): Promise { +): number { return sweepRestoredSubagentsWithoutLiveAgent({ - listLiveLocalPtyIds: async () => - overrides.livePtyIds === undefined ? [] : overrides.livePtyIds, + hasLiveLocalPty: overrides.hasLiveLocalPty ?? (() => false), + isLocalExecutionHost: () => + isLocalExecutionHost( + overrides.executionHostId === undefined ? 'local' : overrides.executionHostId + ), getBoundPtyIdForPaneKey: (paneKey) => overrides.boundPtyIdByPaneKey?.[paneKey], getPersistedPtyIdForPaneKey: (paneKey) => overrides.persistedPtyIdByPaneKey?.[paneKey], - reap: (isLocalPaneAgentLive) => - server.reapRestoredClaudeSubagentsWithoutLiveAgent(isLocalPaneAgentLive) + reap: (isLocalHost, isLocalPaneAgentLive) => + server.reapRestoredClaudeSubagentsWithoutLiveAgent(isLocalHost, isLocalPaneAgentLive) }) } @@ -91,7 +97,7 @@ describe('restored subagent liveness sweep', () => { try { expect(paneStatus(server)).toEqual({ state: 'working', subagents: [WORKING_CHILD] }) - await expect(sweepWith(server, { persistedPtyIdByPaneKey: { [PANE]: PTY } })).resolves.toBe(1) + expect(sweepWith(server, { persistedPtyIdByPaneKey: { [PANE]: PTY } })).toBe(1) expect(paneStatus(server)).toEqual({ state: 'done', subagents: undefined }) } finally { @@ -102,9 +108,12 @@ describe('restored subagent liveness sweep', () => { it('keeps a seed whose pane still has a live local PTY', async () => { const server = await restartWithInFlightSubagent() try { - await expect( - sweepWith(server, { livePtyIds: [PTY], persistedPtyIdByPaneKey: { [PANE]: PTY } }) - ).resolves.toBe(0) + expect( + sweepWith(server, { + hasLiveLocalPty: (ptyId) => ptyId === PTY, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) + ).toBe(0) expect(paneStatus(server)).toEqual({ state: 'working', subagents: [WORKING_CHILD] }) } finally { @@ -112,13 +121,13 @@ describe('restored subagent liveness sweep', () => { } }) - it('keeps a seed whose PTY is bound in this session but not listed yet', async () => { + it('uses targeted liveness for a PTY bound in this runtime', async () => { const server = await restartWithInFlightSubagent() try { - await expect( - sweepWith(server, { livePtyIds: [PTY], boundPtyIdByPaneKey: { [PANE]: PTY } }) - ).resolves.toBe(0) + const hasLiveLocalPty = vi.fn((ptyId: string) => ptyId === PTY) + expect(sweepWith(server, { hasLiveLocalPty, boundPtyIdByPaneKey: { [PANE]: PTY } })).toBe(0) + expect(hasLiveLocalPty).toHaveBeenCalledExactlyOnceWith(PTY) expect(paneStatus(server).state).toBe('working') } finally { server.stop() @@ -129,9 +138,12 @@ describe('restored subagent liveness sweep', () => { const sshPtyId = toAppSshPtyId('conn-1', PTY) const server = await restartWithInFlightSubagent() try { - await expect( - sweepWith(server, { livePtyIds: [], persistedPtyIdByPaneKey: { [PANE]: sshPtyId } }) - ).resolves.toBe(0) + expect( + sweepWith(server, { + executionHostId: 'ssh:conn-1', + persistedPtyIdByPaneKey: { [PANE]: sshPtyId } + }) + ).toBe(0) expect(paneStatus(server)).toEqual({ state: 'working', subagents: [WORKING_CHILD] }) } finally { @@ -142,7 +154,7 @@ describe('restored subagent liveness sweep', () => { it('never reaps a relay-owned pane even with no local PTY at all', async () => { const server = await restartWithInFlightSubagent({ connectionId: 'conn-1' }) try { - await expect(sweepWith(server)).resolves.toBe(0) + expect(sweepWith(server)).toBe(0) expect(paneStatus(server).state).toBe('working') } finally { @@ -150,10 +162,34 @@ describe('restored subagent liveness sweep', () => { } }) - it('does nothing when the PTY inventory cannot be read', async () => { + it('never reaps a runtime-hosted pane from the desktop local provider', async () => { + const server = await restartWithInFlightSubagent() + const hasLiveLocalPty = vi.fn(() => false) + try { + expect( + sweepWith(server, { + executionHostId: 'runtime:ephemeral-vm-1', + hasLiveLocalPty, + persistedPtyIdByPaneKey: { [PANE]: 'remote:ephemeral-vm-1@@pty-1' } + }) + ).toBe(0) + + expect(hasLiveLocalPty).not.toHaveBeenCalled() + expect(paneStatus(server).state).toBe('working') + } finally { + server.stop() + } + }) + + it('does nothing when targeted PTY liveness is unknown', async () => { const server = await restartWithInFlightSubagent() try { - await expect(sweepWith(server, { livePtyIds: null })).resolves.toBe(0) + expect( + sweepWith(server, { + hasLiveLocalPty: () => null, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) + ).toBe(0) expect(paneStatus(server).state).toBe('working') } finally { @@ -176,7 +212,7 @@ describe('restored subagent liveness sweep', () => { } }) - await expect(sweepWith(server)).resolves.toBe(0) + expect(sweepWith(server)).toBe(0) expect(paneStatus(server).state).toBe('working') } finally { server.stop() @@ -195,7 +231,7 @@ describe('restored subagent liveness sweep', () => { payload: { state: 'working', prompt: 'still going', agentType: 'claude' } }) - await sweepWith(server) + sweepWith(server) expect(paneStatus(server, makePaneKey('tab-2', LEAF)).state).toBe('working') } finally { @@ -203,20 +239,18 @@ describe('restored subagent liveness sweep', () => { } }) - it('reports zero and leaves state alone when the listing throws', async () => { + it('reports zero and leaves state alone when targeted liveness throws', async () => { const server = await restartWithInFlightSubagent() - const listLiveLocalPtyIds = vi.fn(async () => { + const hasLiveLocalPty = vi.fn(() => { throw new Error('daemon unreachable') }) try { - await expect( - sweepRestoredSubagentsWithoutLiveAgent({ - listLiveLocalPtyIds, - getBoundPtyIdForPaneKey: () => undefined, - getPersistedPtyIdForPaneKey: () => undefined, - reap: (probe) => server.reapRestoredClaudeSubagentsWithoutLiveAgent(probe) + expect( + sweepWith(server, { + hasLiveLocalPty, + persistedPtyIdByPaneKey: { [PANE]: PTY } }) - ).rejects.toThrow('daemon unreachable') + ).toBe(0) expect(paneStatus(server).state).toBe('working') } finally { @@ -236,3 +270,53 @@ describe('indexPersistedPaneKeyPtyIds', () => { ).toEqual(new Map([[PANE, PTY]])) }) }) + +describe('resolveAgentWorkspaceExecutionHostId', () => { + const localRepo = { + id: 'local-repo', + connectionId: null, + executionHostId: 'local' as const + } + const runtimeRepo = { + id: 'runtime-repo', + connectionId: null, + executionHostId: 'runtime:ephemeral-vm-1' as const + } + const futureHostRepo = { + id: 'future-repo', + connectionId: null, + executionHostId: 'container:future-host' + } + const deps = { + getRepo: (repoId: string) => + [localRepo, runtimeRepo, futureHostRepo].find((candidate) => candidate.id === repoId), + getFolderWorkspace: (id: string) => + id === 'folder-runtime' ? { projectGroupId: 'group-runtime', connectionId: null } : undefined, + getProjectGroups: () => [ + { + id: 'group-runtime', + connectionId: null, + executionHostId: 'runtime:ephemeral-vm-1' + } + ] + } + + it('positively identifies local ownership and rejects runtime hosts', () => { + expect(resolveAgentWorkspaceExecutionHostId('local-repo::/repo', deps)).toBe('local') + expect(resolveAgentWorkspaceExecutionHostId('runtime-repo::/repo', deps)).toBe( + 'runtime:ephemeral-vm-1' + ) + expect(resolveAgentWorkspaceExecutionHostId('folder:folder-runtime', deps)).toBe( + 'runtime:ephemeral-vm-1' + ) + expect(resolveAgentWorkspaceExecutionHostId('future-repo::/repo', deps)).toBeNull() + expect(isLocalExecutionHost('local')).toBe(true) + expect(isLocalExecutionHost('runtime:ephemeral-vm-1')).toBe(false) + }) + + it('treats missing workspace provenance as unknown', () => { + expect(resolveAgentWorkspaceExecutionHostId('missing::/repo', deps)).toBeNull() + expect(resolveAgentWorkspaceExecutionHostId(undefined, deps)).toBeNull() + expect(isLocalExecutionHost(null)).toBe(false) + }) +}) diff --git a/src/main/agent-hooks/restored-subagent-liveness-sweep.ts b/src/main/agent-hooks/restored-subagent-liveness-sweep.ts index 04cb1f65c3e..bf43bcb5043 100644 --- a/src/main/agent-hooks/restored-subagent-liveness-sweep.ts +++ b/src/main/agent-hooks/restored-subagent-liveness-sweep.ts @@ -1,44 +1,48 @@ -import { parseAppSshPtyId } from '../../shared/ssh-pty-id' - -/** How long after startup the sweep runs. Terminal restore binds panes to their - * daemon sessions in a burst right after launch; waiting keeps a pane that is - * simply not reattached yet from reading as one whose agent process is gone. */ -export const RESTORED_SUBAGENT_LIVENESS_SWEEP_DELAY_MS = 2 * 60 * 1000 +import { + LOCAL_EXECUTION_HOST_ID, + parseExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../shared/execution-host' +import type { FolderWorkspace, ProjectGroup } from '../../shared/types' +import { getRepoIdFromWorktreeId } from '../../shared/worktree-id' +import { parseWorkspaceKey } from '../../shared/workspace-scope' export type RestoredSubagentLivenessSweepDeps = { - /** Live local PTY session ids, or null when the inventory could not be read — - * an unavailable listing is not evidence that anything exited. */ - listLiveLocalPtyIds: () => Promise + /** Targeted provider liveness, or null when the provider cannot prove either state. */ + hasLiveLocalPty: (ptyId: string) => boolean | null + isLocalExecutionHost: (worktreeId: string | undefined) => boolean /** PTY bound to this pane in the current session, if it has one. */ getBoundPtyIdForPaneKey: (paneKey: string) => string | undefined /** PTY this pane was bound to when the session was last persisted; covers panes * whose surviving daemon session has not been reattached yet. */ getPersistedPtyIdForPaneKey: (paneKey: string) => string | undefined - reap: (isLocalPaneAgentLive: (paneKey: string) => boolean) => number + reap: ( + isLocalExecutionHost: (worktreeId: string | undefined) => boolean, + isLocalPaneAgentLive: (paneKey: string) => boolean + ) => number } -/** Cross-check restored Claude subagent rows against the live local PTY inventory - * and drop the ones no agent process can still be running. Panes launched over - * SSH resolve as live unconditionally: their agent runs on the remote host and - * can never appear here, so scanning for it would prune every live remote row. */ -export async function sweepRestoredSubagentsWithoutLiveAgent( +/** Drop restored rows only when the owning host is local and its provider proves + * the exact PTY absent. */ +export function sweepRestoredSubagentsWithoutLiveAgent( deps: RestoredSubagentLivenessSweepDeps -): Promise { - const liveIds = await deps.listLiveLocalPtyIds() - if (!liveIds) { - return 0 - } - const livePtyIds = new Set(liveIds) - return deps.reap((paneKey) => { - const ptyId = deps.getBoundPtyIdForPaneKey(paneKey) ?? deps.getPersistedPtyIdForPaneKey(paneKey) - if (!ptyId) { - return false +): number { + return deps.reap( + (worktreeId) => deps.isLocalExecutionHost(worktreeId), + (paneKey) => { + const ptyId = + deps.getBoundPtyIdForPaneKey(paneKey) ?? deps.getPersistedPtyIdForPaneKey(paneKey) + if (!ptyId) { + return false + } + try { + return deps.hasLiveLocalPty(ptyId) !== false + } catch { + return true + } } - if (parseAppSshPtyId(ptyId)) { - return true - } - return livePtyIds.has(ptyId) - }) + ) } /** Index the persisted terminal layouts as `paneKey -> ptyId`. Layout leaves are @@ -57,3 +61,55 @@ export function indexPersistedPaneKeyPtyIds( } return byPaneKey } + +type AgentWorkspaceExecutionHostDeps = { + getRepo: (repoId: string) => ExecutionHostOwner | null | undefined + getFolderWorkspace: ( + folderWorkspaceId: string + ) => Pick | null | undefined + getProjectGroups: () => readonly Pick[] +} + +type ExecutionHostOwner = { + connectionId?: string | null + executionHostId?: string | null +} + +function resolveDeclaredExecutionHost(owner: ExecutionHostOwner): ExecutionHostId | null { + if (owner.executionHostId?.trim()) { + return parseExecutionHostId(owner.executionHostId)?.id ?? null + } + const connectionId = owner.connectionId?.trim() + return connectionId ? toSshExecutionHostId(connectionId) : LOCAL_EXECUTION_HOST_ID +} + +/** Resolve persisted workspace ownership; unknown provenance is not local authority. */ +export function resolveAgentWorkspaceExecutionHostId( + workspaceId: string | undefined, + deps: AgentWorkspaceExecutionHostDeps +): ExecutionHostId | null { + if (!workspaceId) { + return null + } + const scope = parseWorkspaceKey(workspaceId) + if (scope?.type === 'folder') { + const workspace = deps.getFolderWorkspace(scope.folderWorkspaceId) + const group = workspace + ? deps.getProjectGroups().find((candidate) => candidate.id === workspace.projectGroupId) + : undefined + if (!workspace || !group) { + return null + } + return resolveDeclaredExecutionHost({ + connectionId: workspace.connectionId ?? group.connectionId, + executionHostId: group.executionHostId + }) + } + const worktreeId = scope?.type === 'worktree' ? scope.worktreeId : workspaceId + const repo = deps.getRepo(getRepoIdFromWorktreeId(worktreeId)) + return repo ? resolveDeclaredExecutionHost(repo) : null +} + +export function isLocalExecutionHost(hostId: string | null | undefined): boolean { + return parseExecutionHostId(hostId)?.kind === 'local' +} diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index f2b2a2ad06c..ef1fc224eec 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -1929,13 +1929,11 @@ export class AgentHookServer { * gates the pane 'working' for the rest of its life and hibernation, which * requires 'done', can never reclaim the agent's heap. * - * `isLocalPaneAgentLive` is only consulted for panes whose binding is local - * (`connectionId === null`): a remote/SSH agent runs on the far host and can - * never appear in a local process index, so scanning for it would prune every - * live remote binding. Panes that have produced hook traffic in this runtime are - * skipped as well — their rows are backed by the process that is talking to us. - * Returns the number of panes changed. */ + * Both the execution host and relay binding must prove local ownership before + * targeted PTY liveness is consulted. Panes that reported in this runtime are + * also skipped. Returns the number of panes changed. */ reapRestoredClaudeSubagentsWithoutLiveAgent( + isLocalExecutionHost: (worktreeId: string | undefined) => boolean, isLocalPaneAgentLive: (paneKey: string) => boolean ): number { let changedPanes = 0 @@ -1944,6 +1942,7 @@ export class AgentHookServer { if ( enriched.payload.agentType !== 'claude' || enriched.connectionId !== null || + !isLocalExecutionHost(enriched.worktreeId) || this.runtimeObservedStatusPaneKeys.has(paneKey) || isLocalPaneAgentLive(paneKey) ) { diff --git a/src/main/index.ts b/src/main/index.ts index ef20d12df0e..8dc5a8865e1 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -40,7 +40,8 @@ import { classifyError } from './telemetry/classify-error' import { runManagedHookInstallers } from './agent-hooks/install-telemetry' import { indexPersistedPaneKeyPtyIds, - RESTORED_SUBAGENT_LIVENESS_SWEEP_DELAY_MS, + isLocalExecutionHost, + resolveAgentWorkspaceExecutionHostId, sweepRestoredSubagentsWithoutLiveAgent } from './agent-hooks/restored-subagent-liveness-sweep' import { @@ -741,33 +742,33 @@ ipcMain.handle( /** A PTY that dies while Orca is down never runs the teardown that clears pane * state, so hydrate can rebuild a Claude subagent roster that no later hook can * retire — pinning the pane 'working' and locking its agent out of hibernation - * for good. One post-restore pass drops those rows once the live PTY inventory - * proves no agent process is left behind them. */ -function scheduleRestoredSubagentLivenessSweep(): void { - const timer = setTimeout(() => { - const persistedPtyIdByPaneKey = indexPersistedPaneKeyPtyIds( - store?.getWorkspaceSession?.()?.terminalLayoutsByTabId ?? {} - ) - void sweepRestoredSubagentsWithoutLiveAgent({ - listLiveLocalPtyIds: async () => { - try { - const sessions = await getLocalPtyProvider().listProcesses() - return sessions.map((session) => session.id) - } catch (err) { - console.warn('[agent-hooks] restored-subagent sweep skipped; PTY listing failed:', err) - return null - } - }, - getBoundPtyIdForPaneKey: getPtyIdForPaneKey, - getPersistedPtyIdForPaneKey: (paneKey) => persistedPtyIdByPaneKey.get(paneKey), - reap: (isLocalPaneAgentLive) => - agentHookServer.reapRestoredClaudeSubagentsWithoutLiveAgent(isLocalPaneAgentLive) - }).catch((err) => { - console.error('[agent-hooks] restored-subagent liveness sweep failed:', err) - }) - }, RESTORED_SUBAGENT_LIVENESS_SWEEP_DELAY_MS) - // Why: recovery bookkeeping must never hold the process open at quit. - timer.unref?.() + * for good. Once provider and hook hydration settle, targeted PTY liveness can + * retire only rows whose local owner is proven gone. */ +function reapRestoredSubagentsWithoutLiveAgent(): void { + const currentStore = store + if (!currentStore) { + return + } + const provider = getLocalPtyProvider() + const persistedPtyIdByPaneKey = indexPersistedPaneKeyPtyIds( + currentStore.getWorkspaceSession().terminalLayoutsByTabId ?? {} + ) + sweepRestoredSubagentsWithoutLiveAgent({ + hasLiveLocalPty: (ptyId) => provider.hasPty?.(ptyId) ?? null, + isLocalExecutionHost: (worktreeId) => + isLocalExecutionHost( + resolveAgentWorkspaceExecutionHostId(worktreeId, { + getRepo: (repoId) => currentStore.getRepo(repoId), + getFolderWorkspace: (folderWorkspaceId) => + currentStore.getFolderWorkspace(folderWorkspaceId), + getProjectGroups: () => currentStore.getProjectGroups() + }) + ), + getBoundPtyIdForPaneKey: getPtyIdForPaneKey, + getPersistedPtyIdForPaneKey: (paneKey) => persistedPtyIdByPaneKey.get(paneKey), + reap: (isLocalHost, isLocalPaneAgentLive) => + agentHookServer.reapRestoredClaudeSubagentsWithoutLiveAgent(isLocalHost, isLocalPaneAgentLive) + }) } function startTerminalRuntimeStartupServices(): Promise { @@ -815,10 +816,10 @@ function startTerminalRuntimeStartupServices(): Promise { localPtyProviderStartupReady = startupServices.localPtyProviderReady void firstWindowStartupServicesReady.then(() => { logStartupMilestone('first-window-startup-services-ready') - scheduleRestoredSubagentLivenessSweep() }) void localPtyStartupReady.then(() => { logStartupMilestone('local-pty-startup-ready') + reapRestoredSubagentsWithoutLiveAgent() }) return firstWindowStartupServicesReady }