diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index ef5a340e02e..71d74396f8f 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -31409,6 +31409,41 @@ describe('OrcaRuntimeService', () => { ) }) + // Why (#10562): the renderer purges its state regardless of the sweep result, so + // revalidating against a cached scan (30s TTL) that still lists an already-deleted + // directory would strand those PTYs permanently — nothing asks a second time. + it('revalidates against a fresh scan instead of a warm worktree-scan cache', async () => { + const deletedId = `${TEST_REPO_ID}::/tmp/deleted` + const localProvider = { + listProcesses: vi.fn(async () => [ + { id: `${deletedId}@@deleted-session`, cwd: '/tmp/deleted', title: 'shell' } + ]), + shutdown: vi.fn(async () => {}) + } + const runtime = new OrcaRuntimeService(store, undefined, { + getLocalProvider: () => localProvider as never + }) + + // Warm the scan cache while the worktree still exists. + vi.mocked(listWorktrees).mockResolvedValueOnce([ + { path: '/tmp/deleted', head: 'abc', branch: 'd', isBare: false, isMainWorktree: false } + ]) + await runtime.teardownMissingManagedWorktreeTerminals(`id:${TEST_REPO_ID}`, [deletedId]) + expect(localProvider.shutdown).not.toHaveBeenCalled() + + // The worktree is now gone; the cached scan must not mask that. + vi.mocked(listWorktrees).mockResolvedValue([]) + const result = await runtime.teardownMissingManagedWorktreeTerminals(`id:${TEST_REPO_ID}`, [ + deletedId + ]) + + expect(result).toEqual({ stoppedWorktreeIds: [deletedId] }) + expect(localProvider.shutdown).toHaveBeenCalledWith( + `${deletedId}@@deleted-session`, + expect.objectContaining({ immediate: true }) + ) + }) + it('does not stop sessions after a non-authoritative revalidation', async () => { const localProvider = { listProcesses: vi.fn(async () => []), diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 92a4c3ee7f3..14cf1eaf807 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -18005,6 +18005,10 @@ export class OrcaRuntimeService { connectionId?: string | null ): Promise<{ stoppedWorktreeIds: string[] }> { const repo = await this.resolveRepoSelectorForConnection(repoSelector, connectionId) + // Why: killing PTYs must be proven against the host right now — a cached scan + // (30s TTL) can still list a directory git already dropped, and the renderer + // purges its state either way, so a stale miss strands those processes for good. + this.invalidateWorktreeScanCacheForRepo(repo.id) const detected = await this.listDetectedManagedWorktrees(`id:${repo.id}`, connectionId) if (!detected.authoritative) { return { stoppedWorktreeIds: [] } diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index ae8bf057aa8..3894e65384b 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -656,6 +656,82 @@ describe('fetchWorktrees', () => { expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) }) + // Why (#10562): the scan coalesces but each caller carries its own known-id + // snapshot, so a caller that joined an in-flight scan must still request + // teardown — otherwise it purges renderer state and strands live PTYs. + it('stops terminals for a caller that coalesced onto an in-flight scan', async () => { + const store = createTestStore() + const deleted = makeWorktree({ id: 'repo1::/p/deleted', repoId: 'repo1', path: '/p/deleted' }) + const surviving = makeWorktree({ id: 'repo1::/p/surv', repoId: 'repo1', path: '/p/surv' }) + + // Caller A starts before hydration, so its known-id snapshot is empty. + store.setState({ + repos: [{ id: 'repo1', path: '/p/repo1', displayName: 'R', badgeColor: '#000', addedAt: 0 }], + worktreesByRepo: {}, + detectedWorktreesByRepo: {}, + tabsByWorktree: {} + } as unknown as Partial) + + let releaseScan!: () => void + const scanStarted = new Promise((resolve) => { + mockApi.worktrees.listDetected.mockImplementationOnce(async ({ repoId }) => { + resolve() + await new Promise((release) => { + releaseScan = release + }) + return makeDetectedResult(repoId, [surviving]) + }) + }) + + const callerA = store.getState().fetchDetectedWorktrees('repo1') + await scanStarted + + // Hydration lands mid-scan: the renderer now owns `deleted` and its tabs. + store.setState({ + worktreesByRepo: { repo1: [deleted, surviving] }, + detectedWorktreesByRepo: { repo1: makeDetectedResult('repo1', [deleted, surviving]) }, + tabsByWorktree: { [deleted.id]: [{ id: 'tab-d', worktreeId: deleted.id }] } + } as unknown as Partial) + + const callerB = store.getState().fetchWorktrees('repo1') + releaseScan() + await Promise.all([callerA, callerB]) + + // One shared scan, but the coalesced caller still asked the host to sweep. + expect(mockApi.worktrees.listDetected).toHaveBeenCalledTimes(1) + expect(mockApi.runtime.call).toHaveBeenCalledWith({ + method: 'worktree.teardownMissingTerminals', + params: { repo: 'repo1', worktreeIds: [deleted.id], connectionId: null } + }) + expect(store.getState().tabsByWorktree[deleted.id]).toBeUndefined() + }) + + // Why: teardown rides outside the scan coalescer, so identical fan-out requests + // must share one host sweep instead of re-scanning the host per caller. + it('shares one teardown sweep across identical concurrent refreshes', async () => { + const store = createTestStore() + const deleted = makeWorktree({ id: 'repo1::/p/deleted', repoId: 'repo1', path: '/p/deleted' }) + const surviving = makeWorktree({ id: 'repo1::/p/surv', repoId: 'repo1', path: '/p/surv' }) + + store.setState({ + repos: [{ id: 'repo1', path: '/p/repo1', displayName: 'R', badgeColor: '#000', addedAt: 0 }], + worktreesByRepo: { repo1: [deleted, surviving] }, + detectedWorktreesByRepo: { repo1: makeDetectedResult('repo1', [deleted, surviving]) }, + tabsByWorktree: { [deleted.id]: [{ id: 'tab-d', worktreeId: deleted.id }] } + } as unknown as Partial) + + mockApi.worktrees.listDetected.mockResolvedValueOnce(makeDetectedResult('repo1', [surviving])) + + await Promise.all([ + store.getState().fetchWorktrees('repo1'), + store.getState().fetchDetectedWorktrees('repo1'), + store.getState().fetchWorktrees('repo1') + ]) + + expect(mockApi.runtime.call).toHaveBeenCalledTimes(1) + expect(store.getState().tabsByWorktree[deleted.id]).toBeUndefined() + }) + it('coalesces fetchDetectedWorktrees with a matching fetchWorktrees refresh', async () => { const store = createTestStore() const refreshed = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 9276b19ab98..319d48fad6a 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -633,6 +633,11 @@ function isRuntimeMethodNotFoundError(error: unknown): boolean { return error instanceof RuntimeRpcCallError && error.code === 'method_not_found' } +// Why: teardown cannot ride the scan's coalescing (each caller has its own known-id +// snapshot), so dedupe on the request it actually produces — identical fan-out +// requests share one host sweep instead of re-scanning per caller. +const missingWorktreeTeardownsInFlight = new Map>() + async function teardownMissingWorktreeTerminalsBestEffort( settings: AppState['settings'], repoId: string, @@ -648,16 +653,38 @@ async function teardownMissingWorktreeTerminalsBestEffort( if (missingIds.length === 0) { return } + const target = getActiveRuntimeTarget(settings) + const normalizedConnectionId = connectionId ?? null + const key = [ + target.kind === 'local' ? 'local' : `runtime:${target.environmentId}`, + repoId, + normalizedConnectionId ?? '', + [...missingIds].sort().join('\n') + ].join('\0') + const existing = missingWorktreeTeardownsInFlight.get(key) + if (existing) { + return existing + } + const teardown = (async () => { + try { + await callRuntimeRpc( + target, + 'worktree.teardownMissingTerminals', + { repo: repoId, worktreeIds: missingIds, connectionId: normalizedConnectionId }, + { timeoutMs: 30_000 } + ) + } catch (error) { + if (!isRuntimeMethodNotFoundError(error)) { + console.warn(`Failed to stop terminals for missing worktrees in repo ${repoId}:`, error) + } + } + })() + missingWorktreeTeardownsInFlight.set(key, teardown) try { - await callRuntimeRpc( - getActiveRuntimeTarget(settings), - 'worktree.teardownMissingTerminals', - { repo: repoId, worktreeIds: missingIds, connectionId: connectionId ?? null }, - { timeoutMs: 30_000 } - ) - } catch (error) { - if (!isRuntimeMethodNotFoundError(error)) { - console.warn(`Failed to stop terminals for missing worktrees in repo ${repoId}:`, error) + await teardown + } finally { + if (missingWorktreeTeardownsInFlight.get(key) === teardown) { + missingWorktreeTeardownsInFlight.delete(key) } } } @@ -1075,35 +1102,40 @@ async function listDetectedWorktreesForRepoCoalesced( ? getRuntimeEnvironmentConnectionGeneration(target.environmentId) : null const existing = detectedWorktreeRefreshesInFlight.get(key) - if (existing) { - return existing - } - // Why: startup/event fan-out can request the same repo/host refresh many times at once; share only the scan promise so state-merge semantics stay local. - const refresh = listDetectedWorktreesForRepo(settings, repoId, { - reuseRecentCompatibilityFailure: options.reuseRecentCompatibilityFailure - }) - detectedWorktreeRefreshesInFlight.set(key, refresh) - try { - const result = await refresh - if ( - target.kind === 'environment' && - (getEnvironmentSshStateGeneration(target.environmentId) !== connectionGeneration || - getRuntimeEnvironmentConnectionGeneration(target.environmentId) !== - runtimeConnectionGeneration) - ) { - throw new Error('runtime_environment_generation_changed') - } - await teardownMissingWorktreeTerminalsBestEffort( - settings, - repoId, - options.connectionId, - options.knownWorktreeIds, - result - ) - return result - } finally { - if (detectedWorktreeRefreshesInFlight.get(key) === refresh) { - detectedWorktreeRefreshesInFlight.delete(key) + // Why: the scan coalesces, but teardown must not — each caller carries its own + // known-id snapshot and purges its own state, so a caller that joined an + // in-flight scan would otherwise purge without ever stopping those terminals. + const result = existing ? await existing : await runCoalescedDetectedWorktreeScan() + await teardownMissingWorktreeTerminalsBestEffort( + settings, + repoId, + options.connectionId, + options.knownWorktreeIds, + result + ) + return result + + async function runCoalescedDetectedWorktreeScan(): Promise { + // Why: startup/event fan-out can request the same repo/host refresh many times at once; share only the scan promise so state-merge semantics stay local. + const refresh = listDetectedWorktreesForRepo(settings, repoId, { + reuseRecentCompatibilityFailure: options.reuseRecentCompatibilityFailure + }) + detectedWorktreeRefreshesInFlight.set(key, refresh) + try { + const scanned = await refresh + if ( + target.kind === 'environment' && + (getEnvironmentSshStateGeneration(target.environmentId) !== connectionGeneration || + getRuntimeEnvironmentConnectionGeneration(target.environmentId) !== + runtimeConnectionGeneration) + ) { + throw new Error('runtime_environment_generation_changed') + } + return scanned + } finally { + if (detectedWorktreeRefreshesInFlight.get(key) === refresh) { + detectedWorktreeRefreshesInFlight.delete(key) + } } } }