From 66d596fe11e7e27cc7374c9a03479b2fefb94a0f Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 3 May 2026 15:54:45 -0700 Subject: [PATCH] fix: zombie worktree PTY cleanup (issue #218) (#1376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(runtime): kill all PTYs for a worktree on removal (design §4.3) Worktree deletion only shut down renderer-tracked terminals, so PTYs owned by background tabs, split panes, or pre-reload sessions survived the removal and kept leaking memory. Introduce killAllProcessesForWorktree with three sweeps (runtime leaves, provider-prefix scan of daemon session ids, pty-registry by worktreeId) and wire it into both teardown paths: the CLI-initiated removeManagedWorktree and the renderer-initiated worktrees:remove IPC handler. OrcaRuntimeService gets a lazy getLocalProvider thunk so construction order stays robust. Co-authored-by: Orca * fix(renderer): purge worktree-scoped state on removal + hydration (design §4.4) When a worktree is deleted, ~25 worktree-scoped maps (tabsByWorktree, git caches, browser state, split-tab models, per-file editor drafts, etc.) kept references to the gone worktree, so SessionsStatusSegment kept mis-classifying orphaned PTYs as bound and dropdowns rendered stale ids. Add purgeWorktreeTerminalState as a single atomic action that wipes every scoped map plus cascades top-level actives. Fire it from the worktrees:changed listener on the set-diff of removed ids, and once more at hydration via fetchAllWorktrees to clean up persisted entries from pre-fix sessions. The hydration-time purge is gated behind a per-repo success check: a single transient IPC error or an all-empty fetch defers the purge so a degraded launch cannot wipe legitimate persisted state. Co-authored-by: Orca * test(zombie-worktree): regression coverage for design §4.5 Adds tests for every layer of the zombie-worktree fix: - worktree-teardown: unit coverage of the three-sweep helper including best-effort error swallowing across provider/registry. - orca-runtime: RPC-initiated removeManagedWorktree kills PTYs before any git mutation + verifies the lazy getLocalProvider thunk resolves on each call. - worktrees IPC: renderer-initiated remove kills PTYs before git and skips the kill helper for SSH-backed repos. - renderer slice: fetchAllWorktrees defers the purge when any sibling repo fetch fails or every repo returns empty (F1 regression); happy-path fires the purge once and does not re-run on subsequent calls. Direct purgeWorktreeTerminalState coverage pins the cascade across worktree-keyed, tab-id-keyed, and file-id-keyed maps. Co-authored-by: Orca * fix(runtime): log worktree-teardown kill counts (design §4.4 observability) Breadcrumb lets ops distinguish a renderer-state-induced leak (diff-path purge non-empty) from a backend-induced one (nothing to kill but memory still pinned). Emit only when the sweep actually shut anything down so steady-state logs stay quiet. Added at both call sites — removeManagedWorktree (CLI path) and the worktrees:remove IPC handler. Co-authored-by: Orca * test(zombie-worktree): fix ptyIdsByTabId seed shape to match production type The purge unit test seeded ptyIdsByTabId as Record when the runtime type is Record. The unit tests passed because they never hit the UI renderer, but live e2e surfaced a TypeError: (ptyIdsByTabId[tabId] ?? []).some is not a function. Corrected to arrays; 21/21 tests still pass. Co-authored-by: Orca --------- Co-authored-by: Orca --- src/main/index.ts | 11 +- src/main/ipc/pty.ts | 17 +- src/main/ipc/worktrees-windows.test.ts | 2 +- src/main/ipc/worktrees.test.ts | 79 ++++++- src/main/ipc/worktrees.ts | 34 ++- src/main/runtime/orca-runtime.test.ts | 119 ++++++++++ src/main/runtime/orca-runtime.ts | 51 ++++- src/main/runtime/worktree-teardown.test.ts | 155 +++++++++++++ src/main/runtime/worktree-teardown.ts | 93 ++++++++ .../window/attach-main-window-services.ts | 2 +- src/renderer/src/hooks/useIpcEvents.ts | 27 ++- .../src/store/slices/worktree-helpers.ts | 15 ++ .../src/store/slices/worktrees.test.ts | 214 ++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 197 +++++++++++++++- 14 files changed, 997 insertions(+), 19 deletions(-) create mode 100644 src/main/runtime/worktree-teardown.test.ts create mode 100644 src/main/runtime/worktree-teardown.ts diff --git a/src/main/index.ts b/src/main/index.ts index 618f2f505..3535d8151 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -44,7 +44,7 @@ import { claudeHookService } from './claude/hook-service' import { codexHookService } from './codex/hook-service' import { geminiHookService } from './gemini/hook-service' import { cursorHookService } from './cursor/hook-service' -import { getPtyIdForPaneKey, registerPaneKeyTeardownListener } from './ipc/pty' +import { getPtyIdForPaneKey, registerPaneKeyTeardownListener, getLocalPtyProvider } from './ipc/pty' import { AgentBrowserBridge } from './browser/agent-browser-bridge' import { browserManager } from './browser/browser-manager' @@ -396,7 +396,14 @@ app.whenReady().then(async () => { .filter((account) => account.id !== settings.activeCodexManagedAccountId) .map((account) => ({ id: account.id, managedHomePath: account.managedHomePath })) }) - runtime = new OrcaRuntimeService(store, stats) + runtime = new OrcaRuntimeService(store, stats, { + // Why: resolve the PTY provider lazily. initDaemonPtyProvider() runs later + // inside attachMainWindowServices and calls setLocalPtyProvider(routedAdapter) + // to swap the in-process provider for the daemon-routed one. Capturing the + // provider reference eagerly here would freeze the pre-daemon LocalPtyProvider + // and defeat the teardown helper's prefix sweep (design §4.3 wire-up). + getLocalProvider: () => getLocalPtyProvider() + }) starNag = new StarNagService(store, stats) starNag.start() starNag.registerIpcHandlers() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 90b673f98..4519c99ac 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -228,13 +228,16 @@ export function getSshPtyProvider(connectionId: string): IPtyProvider | undefine return sshProviders.get(connectionId) } -/** Get the local PTY provider (for direct access in tests/runtime). */ -export function getLocalPtyProvider(): LocalPtyProvider { - // Why: callers that need LocalPtyProvider-specific methods (killOrphanedPtys, - // advanceGeneration, getPtyProcess) can only work with the local provider. - // Daemon mode replaces it with an adapter, so callers must use this only when - // they know the concrete local provider is installed. - return localProvider as LocalPtyProvider +/** Get the installed PTY provider (for direct access in tests/runtime). + * + * Returns the installed PTY provider — after `setLocalPtyProvider()` runs + * during daemon init this may be the routed adapter (specifically either + * `DaemonPtyAdapter` or its `DaemonPtyRouter` wrapper). Callers needing + * `LocalPtyProvider`-specific methods (`killOrphanedPtys`, + * `advanceGeneration`, `getPtyProcess`) must type-narrow or import the + * concrete class directly. */ +export function getLocalPtyProvider(): IPtyProvider { + return localProvider } /** Replace the local PTY provider with a daemon-backed one. diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 9d35ecb57..34ab0f830 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -184,7 +184,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard') listWorktreesMock.mockResolvedValue([]) - registerWorktreeHandlers(mainWindow as never, store as never) + registerWorktreeHandlers(mainWindow as never, store as never, {} as never) }) it('accepts a newly created Windows worktree when git lists the same path with different separators', async () => { diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 53057e826..7377f15cc 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -104,6 +104,19 @@ vi.mock('../terminal-history', () => ({ deleteWorktreeHistoryDir: deleteWorktreeHistoryDirMock })) +const { killAllProcessesForWorktreeMock, getLocalPtyProviderMock } = vi.hoisted(() => ({ + killAllProcessesForWorktreeMock: vi.fn(), + getLocalPtyProviderMock: vi.fn() +})) + +vi.mock('../runtime/worktree-teardown', () => ({ + killAllProcessesForWorktree: killAllProcessesForWorktreeMock +})) + +vi.mock('./pty', () => ({ + getLocalPtyProvider: getLocalPtyProviderMock +})) + import { registerWorktreeHandlers } from './worktrees' type HandlerMap = Record unknown> @@ -155,10 +168,18 @@ describe('registerWorktreeHandlers', () => { store.getSettings, store.getWorktreeMeta, store.setWorktreeMeta, - store.removeWorktreeMeta + store.removeWorktreeMeta, + killAllProcessesForWorktreeMock, + getLocalPtyProviderMock ]) { m.mockReset() } + killAllProcessesForWorktreeMock.mockResolvedValue({ + runtimeStopped: 0, + providerStopped: 0, + registryStopped: 0 + }) + getLocalPtyProviderMock.mockReturnValue({} as never) for (const key of Object.keys(handlers)) { delete handlers[key] @@ -230,7 +251,7 @@ describe('registerWorktreeHandlers', () => { ensurePathWithinWorkspaceMock.mockImplementation((targetPath: string) => targetPath) listWorktreesMock.mockResolvedValue([]) - registerWorktreeHandlers(mainWindow as never, store as never) + registerWorktreeHandlers(mainWindow as never, store as never, {} as never) }) it('auto-suffixes the branch name when the first choice collides with a remote branch', async () => { @@ -750,6 +771,60 @@ describe('registerWorktreeHandlers', () => { ) }) + it('IPC-initiated delete kills PTYs BEFORE git-level removal (design §4.3)', async () => { + listWorktreesMock.mockResolvedValue([]) + getEffectiveHooksMock.mockReturnValue(null) + const callOrder: string[] = [] + killAllProcessesForWorktreeMock.mockImplementation(async () => { + callOrder.push('kill') + return { runtimeStopped: 1, providerStopped: 0, registryStopped: 0 } + }) + removeWorktreeMock.mockImplementation(async () => { + callOrder.push('git') + }) + + await handlers['worktrees:remove'](null, { + worktreeId: 'repo-1::/workspace/feature-wt' + }) + + expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith( + 'repo-1::/workspace/feature-wt', + expect.objectContaining({ + localProvider: expect.anything() + }) + ) + expect(removeWorktreeMock).toHaveBeenCalled() + expect(callOrder).toEqual(['kill', 'git']) + }) + + it('skips the PTY teardown for SSH-backed repos (design §6 out-of-scope)', async () => { + // Why: SSH-backed PTYs live on the remote host and are handled by the + // remote provider's own teardown. The local-host helper must not run for + // SSH repos, because it would sweep registry entries for other worktrees. + const repo = { + id: 'repo-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1', + worktreeBaseRef: null + } + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + + // The test can't easily mock the SSH provider without more plumbing — the + // call will throw about 'no git provider for connection'. What matters + // here is that the kill helper was NOT called for the SSH branch. + await ( + handlers['worktrees:remove'](null, { + worktreeId: 'repo-ssh::/remote/feature-wt' + }) as Promise + ).catch(() => {}) + + expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled() + }) + it('rejects ask-policy creates before mutating git state when setup decision is missing', async () => { getEffectiveHooksMock.mockReturnValue({ scripts: { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 141b4c155..6789f441e 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -34,8 +34,15 @@ import { notifyWorktreesChanged } from './worktree-remote' import { rebuildAuthorizedRootsCache, ensureAuthorizedRootsCache } from './filesystem-auth' +import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import { killAllProcessesForWorktree } from '../runtime/worktree-teardown' +import { getLocalPtyProvider } from './pty' -export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store): void { +export function registerWorktreeHandlers( + mainWindow: BrowserWindow, + store: Store, + runtime: OrcaRuntimeService +): void { // Remove any previously registered handlers so we can re-register them // (e.g. when macOS re-activates the app and creates a new window). ipcMain.removeHandler('worktrees:listAll') @@ -259,6 +266,31 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store throw new Error('Folder mode does not support deleting worktrees.') } + // Why: kill every PTY belonging to this worktree BEFORE git-level + // removal. The renderer pre-kills via shutdownWorktreeTerminals, but + // defensive teardown here protects against: (a) a future renderer bug, + // (b) a disconnected window, (c) an out-of-band window.api.worktrees.remove + // caller. Placement is before the SSH early-return so local-host PTYs + // are still reaped for local repos; SSH-backed PTYs are handled by the + // remote provider's own teardown (design §4.3, §6). + if (!repo.connectionId) { + await killAllProcessesForWorktree(args.worktreeId, { + runtime, + localProvider: getLocalPtyProvider() + }) + .then((r) => { + const total = r.runtimeStopped + r.providerStopped + r.registryStopped + if (total > 0) { + console.info( + `[worktree-teardown] ${args.worktreeId} killed runtime=${r.runtimeStopped} provider=${r.providerStopped} registry=${r.registryStopped}` + ) + } + }) + .catch((err) => { + console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err) + }) + } + if (repo.connectionId) { const provider = getSshGitProvider(repo.connectionId) if (!provider) { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e25338cac..744568a7a 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1336,4 +1336,123 @@ describe('OrcaRuntimeService', () => { expect(getRegisteredTabsMock).toHaveBeenCalledWith(`${TEST_REPO_ID}::/tmp/worktree-b`) }) }) + + describe('removeManagedWorktree PTY teardown (design §4.3)', () => { + function createProviderStub( + listProcesses: () => Promise<{ id: string; cwd: string; title: string }[]> + ): { + spawn: ReturnType + attach: ReturnType + write: ReturnType + resize: ReturnType + shutdown: ReturnType + sendSignal: ReturnType + getCwd: ReturnType + getInitialCwd: ReturnType + clearBuffer: ReturnType + acknowledgeDataEvent: ReturnType + hasChildProcesses: ReturnType + getForegroundProcess: ReturnType + serialize: ReturnType + revive: ReturnType + listProcesses: ReturnType + getDefaultShell: ReturnType + getProfiles: ReturnType + onData: ReturnType + onReplay: ReturnType + onExit: ReturnType + } { + return { + spawn: vi.fn(), + attach: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn().mockResolvedValue(undefined), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + listProcesses: vi.fn(listProcesses), + getDefaultShell: vi.fn(), + getProfiles: vi.fn(), + onData: vi.fn().mockReturnValue(() => {}), + onReplay: vi.fn().mockReturnValue(() => {}), + onExit: vi.fn().mockReturnValue(() => {}) + } + } + + it('RPC-initiated delete kills matching PTYs before git', async () => { + // Seed the runtime with a live leaf whose worktreeId matches the target. + const killSpy = vi.fn().mockReturnValue(true) + const localProvider = createProviderStub(async () => []) + const callOrder: string[] = [] + vi.mocked(removeWorktree).mockImplementation(async () => { + callOrder.push('git-removeWorktree') + }) + + const runtime = new OrcaRuntimeService(store, undefined, { + getLocalProvider: () => { + callOrder.push('getLocalProvider') + return localProvider as never + } + }) + runtime.setPtyController({ + write: () => true, + kill: (id) => { + callOrder.push(`kill:${id}`) + return killSpy(id) as boolean + }, + getForegroundProcess: async () => null + }) + syncSinglePty(runtime, 'pty-1') + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + expect(killSpy).toHaveBeenCalledWith('pty-1') + // The provider-prefix sweep and the git removal must happen AFTER the + // runtime-graph kill. Git removal must NOT happen before any kill. + const killIdx = callOrder.indexOf('kill:pty-1') + const gitIdx = callOrder.indexOf('git-removeWorktree') + expect(killIdx).toBeGreaterThanOrEqual(0) + expect(gitIdx).toBeGreaterThan(killIdx) + }) + + it('thunk resolves the installed provider lazily, not at construction time', async () => { + // Simulates the daemon adapter being installed AFTER OrcaRuntimeService + // construction (setLocalPtyProvider(routedAdapter) in daemon-init). + // A capture-at-construction refactor would break this test. + const preDaemonProvider = createProviderStub(async () => [ + { id: '1', cwd: '/tmp', title: 'shell' }, + { id: '2', cwd: '/tmp', title: 'shell' } + ]) + const postDaemonProvider = createProviderStub(async () => [ + { id: `${TEST_WORKTREE_ID}@@aaaaaaaa`, cwd: '/tmp', title: 'shell' } + ]) + let currentProvider: ReturnType = preDaemonProvider + + const runtime = new OrcaRuntimeService(store, undefined, { + getLocalProvider: () => currentProvider as never + }) + vi.mocked(removeWorktree).mockResolvedValue(undefined) + + // Simulate daemon-init swapping the provider after construction. + currentProvider = postDaemonProvider + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + // The post-daemon provider's prefix-matching session must have been + // shut down, proving the thunk resolved lazily at call time. + expect(postDaemonProvider.shutdown).toHaveBeenCalledWith( + `${TEST_WORKTREE_ID}@@aaaaaaaa`, + true + ) + // The pre-daemon provider must not have been consulted for the kill. + expect(preDaemonProvider.shutdown).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index cd4a9ea96..3ba6281df 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -105,6 +105,8 @@ import { areWorktreePathsEqual } from '../ipc/worktree-logic' import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth' +import { killAllProcessesForWorktree } from './worktree-teardown' +import type { IPtyProvider } from '../providers/types' type RuntimeStore = { getRepos: Store['getRepos'] @@ -240,12 +242,28 @@ export class OrcaRuntimeService { private agentDetector: AgentDetector | null = null private _orchestrationDb: OrchestrationDb | null = null private messageWaitersByHandle = new Map>() + private readonly getLocalProviderFn: (() => IPtyProvider) | null - constructor(store: RuntimeStore | null = null, stats?: StatsCollector) { + constructor( + store: RuntimeStore | null = null, + stats?: StatsCollector, + deps?: { getLocalProvider?: () => IPtyProvider } + ) { this.store = store if (stats) { this.agentDetector = new AgentDetector(stats) } + // Why: the daemon adapter is installed via `setLocalPtyProvider()` during + // attachMainWindowServices, AFTER this service is constructed. Capturing + // `getLocalPtyProvider()` at construction time would freeze a reference to + // the pre-daemon `LocalPtyProvider` and miss the routed adapter. Resolve + // lazily via thunk so teardown always sees the currently-installed + // provider (design §4.3 wire-up). + this.getLocalProviderFn = deps?.getLocalProvider ?? null + } + + getLocalProvider(): IPtyProvider | null { + return this.getLocalProviderFn ? this.getLocalProviderFn() : null } // Why: lazy initialization — the DB path depends on Electron's userData @@ -1128,6 +1146,37 @@ export class OrcaRuntimeService { throw new Error('Folder mode does not support deleting worktrees.') } + // Why: kill every PTY belonging to this worktree BEFORE the git-level + // removal. Some shells keep the worktree directory busy, and `git worktree + // remove` throws a confusing error if PTYs still hold it open. This also + // closes the headless-CLI leak (design §2a/§2b): without this call, the + // CLI path runs git removal and never touches PTYs, leaving zombies + // behind. Best-effort: any failure here must not prevent git removal — + // the worst case without the call is the status quo. + const localProvider = this.getLocalProvider() + if (localProvider) { + await killAllProcessesForWorktree(worktree.id, { + runtime: this, + localProvider + }) + .then((r) => { + const total = r.runtimeStopped + r.providerStopped + r.registryStopped + if (total > 0) { + // Why (design §4.4 observability): breadcrumb lets ops + // distinguish a renderer-state-induced leak (diff-path purge + // non-empty) from a backend-induced one (nothing to kill but + // memory still pinned). Emit only when the sweep actually did + // work so steady-state logs stay quiet. + console.info( + `[worktree-teardown] ${worktree.id} killed runtime=${r.runtimeStopped} provider=${r.providerStopped} registry=${r.registryStopped}` + ) + } + }) + .catch((err) => { + console.warn(`[worktree-teardown] failed for ${worktree.id}:`, err) + }) + } + const hooks = getEffectiveHooks(repo) let warning: string | undefined if (hooks?.scripts.archive && runHooks) { diff --git a/src/main/runtime/worktree-teardown.test.ts b/src/main/runtime/worktree-teardown.test.ts new file mode 100644 index 000000000..6fcfd9c79 --- /dev/null +++ b/src/main/runtime/worktree-teardown.test.ts @@ -0,0 +1,155 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { listRegisteredPtysMock } = vi.hoisted(() => ({ + listRegisteredPtysMock: vi.fn() +})) + +vi.mock('../memory/pty-registry', () => ({ + listRegisteredPtys: listRegisteredPtysMock +})) + +import { killAllProcessesForWorktree } from './worktree-teardown' +import type { IPtyProvider } from '../providers/types' + +function createProviderStub( + listProcesses: () => Promise<{ id: string; cwd: string; title: string }[]> +): IPtyProvider { + return { + spawn: vi.fn(), + attach: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn().mockResolvedValue(undefined), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + listProcesses: vi.fn(listProcesses), + getDefaultShell: vi.fn(), + getProfiles: vi.fn(), + onData: vi.fn().mockReturnValue(() => {}), + onReplay: vi.fn().mockReturnValue(() => {}), + onExit: vi.fn().mockReturnValue(() => {}) + } as unknown as IPtyProvider +} + +describe('killAllProcessesForWorktree', () => { + beforeEach(() => { + listRegisteredPtysMock.mockReset() + }) + + it('reaches daemon sessions and registry entries without a runtime', async () => { + // Simulate headless-CLI: no renderer, so `runtime` is undefined. + const localProvider = createProviderStub(async () => [ + { id: 'w1@@abcd1234', cwd: '/tmp/w1', title: 'shell' }, + { id: 'w2@@efef5678', cwd: '/tmp/w2', title: 'shell' } + ]) + listRegisteredPtysMock.mockReturnValue([ + { ptyId: 'w1-registry-1', worktreeId: 'w1', sessionId: null, paneKey: null, pid: 100 }, + { ptyId: 'w2-registry-2', worktreeId: 'w2', sessionId: null, paneKey: null, pid: 101 } + ]) + + const result = await killAllProcessesForWorktree('w1', { localProvider }) + + expect(result.runtimeStopped).toBe(0) + expect(result.providerStopped).toBe(1) + expect(result.registryStopped).toBe(1) + + expect(localProvider.shutdown).toHaveBeenCalledWith('w1@@abcd1234', true) + expect(localProvider.shutdown).toHaveBeenCalledWith('w1-registry-1', true) + expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2@@efef5678', true) + expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2-registry-2', true) + }) + + it('skips the daemon prefix sweep safely when the provider uses numeric ids', async () => { + // LocalPtyProvider shape: numeric ids that cannot match `${worktreeId}@@`. + const localProvider = createProviderStub(async () => [ + { id: '1', cwd: '/tmp/w1', title: 'shell' }, + { id: '2', cwd: '/tmp/w2', title: 'shell' } + ]) + listRegisteredPtysMock.mockReturnValue([ + { ptyId: '1', worktreeId: 'w1', sessionId: null, paneKey: null, pid: 200 } + ]) + + const result = await killAllProcessesForWorktree('w1', { localProvider }) + + // Prefix sweep must kill nothing; registry sweep must still fire. + expect(result.providerStopped).toBe(0) + expect(result.registryStopped).toBe(1) + expect(localProvider.shutdown).toHaveBeenCalledWith('1', true) + expect(localProvider.shutdown).toHaveBeenCalledTimes(1) + }) + + it('best-effort: swallows errors from listProcesses and shutdown', async () => { + const localProvider = createProviderStub(() => Promise.reject(new Error('boom'))) + listRegisteredPtysMock.mockReturnValue([ + { ptyId: 'x', worktreeId: 'w1', sessionId: null, paneKey: null, pid: 10 } + ]) + ;(localProvider.shutdown as unknown as ReturnType).mockRejectedValue( + new Error('already dead') + ) + + const result = await killAllProcessesForWorktree('w1', { localProvider }) + + // listProcesses rejected → provider sweep returns 0; registry shutdown + // rejected → counted as not-killed (registry sweep currently swallows). + expect(result.providerStopped).toBe(0) + expect(result.registryStopped).toBe(0) + }) + + it('does not carry state between successive calls with distinct providers', async () => { + // Guards against a future refactor that memoises provider or registry + // reads inside the helper. + const providerA = createProviderStub(async () => [ + { id: 'w1@@aaaa', cwd: '/tmp', title: 'shell' } + ]) + const providerB = createProviderStub(async () => [ + { id: 'w1@@bbbb', cwd: '/tmp', title: 'shell' } + ]) + listRegisteredPtysMock.mockReturnValue([]) + + const r1 = await killAllProcessesForWorktree('w1', { localProvider: providerA }) + expect(providerA.shutdown).toHaveBeenCalledWith('w1@@aaaa', true) + expect(providerB.shutdown).not.toHaveBeenCalled() + expect(r1.providerStopped).toBe(1) + + const r2 = await killAllProcessesForWorktree('w1', { localProvider: providerB }) + expect(providerB.shutdown).toHaveBeenCalledWith('w1@@bbbb', true) + expect(providerB.shutdown).toHaveBeenCalledTimes(1) + expect(r2.providerStopped).toBe(1) + }) + + it('invokes runtime.stopTerminalsForWorktree when runtime is provided', async () => { + const stopTerminalsForWorktree = vi.fn().mockResolvedValue({ stopped: 3 }) + const runtime = { + stopTerminalsForWorktree + } as unknown as Parameters[1]['runtime'] + + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + + const result = await killAllProcessesForWorktree('w1', { runtime, localProvider }) + + expect(stopTerminalsForWorktree).toHaveBeenCalledWith('w1') + expect(result.runtimeStopped).toBe(3) + }) + + it('tolerates runtime.stopTerminalsForWorktree throwing (headless assertGraphReady reject)', async () => { + const stopTerminalsForWorktree = vi.fn().mockRejectedValue(new Error('graph not ready')) + const runtime = { + stopTerminalsForWorktree + } as unknown as Parameters[1]['runtime'] + + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + + const result = await killAllProcessesForWorktree('w1', { runtime, localProvider }) + + expect(result.runtimeStopped).toBe(0) + }) +}) diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts new file mode 100644 index 000000000..a25d46ec4 --- /dev/null +++ b/src/main/runtime/worktree-teardown.ts @@ -0,0 +1,93 @@ +import type { IPtyProvider } from '../providers/types' +import type { OrcaRuntimeService } from './orca-runtime' +import { listRegisteredPtys } from '../memory/pty-registry' + +export type WorktreeTeardownDeps = { + runtime?: OrcaRuntimeService + localProvider: IPtyProvider +} + +export type WorktreeTeardownResult = { + runtimeStopped: number + providerStopped: number + registryStopped: number +} + +/** + * Kills every PTY we can prove belongs to `worktreeId`, across all three + * registration surfaces (renderer graph, installed PTY provider session list, + * local pty-registry). + * + * Why all three: + * - runtime.leaves is authoritative when the renderer is attached, but is + * empty in the headless-CLI case (see design §2b). + * - The installed provider's listProcesses() surfaces daemon sessions by + * the `${worktreeId}@@` session-id contract (§3.1). Because daemon-init + * installs the daemon adapter AS the localProvider via + * setLocalPtyProvider(), a single call reaches the right backend in both + * daemon-on and daemon-off configurations. LocalPtyProvider uses numeric + * ids, so the prefix filter is a safe no-op when the daemon is absent. + * - pty-registry covers the fallback local provider case and is the + * canonical source for memory attribution; it also redundantly backstops + * daemon spawns. + * + * Best-effort throughout: each sweep catches its own errors. The caller + * (removeManagedWorktree, worktrees:remove IPC) must run the git-level + * removal regardless of what this returns. + */ +export async function killAllProcessesForWorktree( + worktreeId: string, + deps: WorktreeTeardownDeps +): Promise { + const result: WorktreeTeardownResult = { + runtimeStopped: 0, + providerStopped: 0, + registryStopped: 0 + } + + if (deps.runtime) { + const r = await deps.runtime.stopTerminalsForWorktree(worktreeId).catch(() => ({ stopped: 0 })) + result.runtimeStopped = r.stopped + } + + result.providerStopped = await sweepProviderByPrefix(worktreeId, deps.localProvider) + result.registryStopped = await sweepRegistryForWorktree(worktreeId, deps.localProvider) + + return result +} + +async function sweepProviderByPrefix(worktreeId: string, provider: IPtyProvider): Promise { + const prefix = `${worktreeId}@@` + const sessions = await provider.listProcesses().catch(() => []) + let killed = 0 + for (const s of sessions) { + if (!s.id.startsWith(prefix)) { + continue + } + try { + await provider.shutdown(s.id, true) + killed += 1 + } catch { + // Already dead, or the backend dropped the session — treat as success. + killed += 1 + } + } + return killed +} + +async function sweepRegistryForWorktree( + worktreeId: string, + localProvider: IPtyProvider +): Promise { + const entries = listRegisteredPtys().filter((r) => r.worktreeId === worktreeId) + let killed = 0 + for (const entry of entries) { + try { + await localProvider.shutdown(entry.ptyId, true) + killed += 1 + } catch { + /* ignore — best-effort */ + } + } + return killed +} diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 3ec378783..c5ba11733 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -34,7 +34,7 @@ export function attachMainWindowServices( prepareClaudeAuth?: () => Promise ): void { registerRepoHandlers(mainWindow, store) - registerWorktreeHandlers(mainWindow, store) + registerWorktreeHandlers(mainWindow, store, runtime) registerPtyHandlers( mainWindow, runtime, diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 29496a806..6269f2ce0 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -34,8 +34,31 @@ export function useIpcEvents(): void { ) unsubs.push( - window.api.worktrees.onChanged((data: { repoId: string }) => { - useAppStore.getState().fetchWorktrees(data.repoId) + window.api.worktrees.onChanged(async (data: { repoId: string }) => { + // Why: diff before vs. after fetchWorktrees to detect server-side + // deletions (CLI `orca worktree rm`, other window, out-of-band RPC) + // and purge worktree-scoped state for removed ids. Without this, + // `ptyIdsByTabId` would retain entries for tabs whose worktree is + // gone, and SessionsStatusSegment's `boundPtyIds` set would keep + // misclassifying the zombie as bound (design §2c, §4.4). + const state = useAppStore.getState() + const before = new Set((state.worktreesByRepo[data.repoId] ?? []).map((w) => w.id)) + await state.fetchWorktrees(data.repoId) + const afterState = useAppStore.getState() + const after = new Set((afterState.worktreesByRepo[data.repoId] ?? []).map((w) => w.id)) + const removed: string[] = [] + for (const id of before) { + if (!after.has(id)) { + removed.push(id) + } + } + if (removed.length > 0) { + console.warn( + `[worktree-purge] diff-based purge removing state for ${removed.length} worktree(s):`, + removed + ) + afterState.purgeWorktreeTerminalState(removed) + } }) ) diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 7449ff88b..9a310162d 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -35,6 +35,15 @@ export type WorktreeSlice = { * agent output) count normally. Session-only; never persisted. */ everActivatedWorktreeIds: Set + /** + * Guards the one-shot hydration-time purge in `fetchAllWorktrees`. Set to + * `true` only after the first launch where every repo's `worktrees.list` IPC + * call succeeded AND at least one repo returned a non-empty result — at that + * moment the renderer has enough signal to treat the union of fetched ids as + * authoritative and purge stale `tabsByWorktree` keys left behind by pre-fix + * sessions (design §4.4). Session-only; never persisted. + */ + hasHydratedWorktreePurge: boolean fetchWorktrees: (repoId: string) => Promise fetchAllWorktrees: () => Promise createWorktree: ( @@ -58,6 +67,12 @@ export type WorktreeSlice = { bumpWorktreeActivity: (worktreeId: string) => void setActiveWorktree: (worktreeId: string | null) => void allWorktrees: () => Worktree[] + /** + * Wipes every terminal- and worktree-scoped map entry for each given id. + * Called by the `worktrees:changed` listener on server-side deletions and + * one-shot at hydration time. See design §4.4. + */ + purgeWorktreeTerminalState: (worktreeIds: string[]) => void } export function findWorktreeById( diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 232ce7a10..4f5f0b370 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -504,3 +504,217 @@ describe('worktree unread (show-until-interact)', () => { expect(mockApi.worktrees.updateMeta).not.toHaveBeenCalled() }) }) + +// Why: design §4.4 — the hydration-time purge must be gated behind a +// per-repo success check (F1 regression) so a transient git error on one +// repo cannot silently wipe every persisted tabsByWorktree entry for that +// repo. An empty-but-successful fetch from a newly-cloned sibling repo is +// also unsafe to treat as authoritative on its own. Pins both halves of +// that contract plus the happy-path purge. +describe('fetchAllWorktrees hydration-time purge (design §4.4)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + const repoA = { + id: 'repoA', + path: '/repos/a', + displayName: 'a', + badgeColor: '#000', + addedAt: 0 + } + const repoB = { + id: 'repoB', + path: '/repos/b', + displayName: 'b', + badgeColor: '#111', + addedAt: 0 + } + + it('defers the purge when a sibling repo fetch fails (F1 regression)', async () => { + const store = createTestStore() + const wtA = makeWorktree({ id: 'repoA::/a/wt1', repoId: 'repoA', path: '/a/wt1' }) + const wtB = makeWorktree({ id: 'repoB::/b/wt1', repoId: 'repoB', path: '/b/wt1' }) + + // Stub: repoA succeeds; repoB throws. Stale tabsByWorktree entry for + // repoA::/a/stale must NOT be purged while any repo fetch is degraded. + mockApi.worktrees.list.mockImplementation(async ({ repoId }: { repoId: string }) => { + if (repoId === 'repoA') { + return [wtA] + } + throw new Error('git error') + }) + + store.setState({ + repos: [repoA, repoB], + worktreesByRepo: { repoB: [wtB] }, + tabsByWorktree: { + 'repoA::/a/stale': [{ id: 'tab-A-stale', worktreeId: 'repoA::/a/stale' }], + 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] + } + } as unknown as Partial) + + await store.getState().fetchAllWorktrees() + + expect(store.getState().hasHydratedWorktreePurge).toBe(false) + expect(store.getState().tabsByWorktree).toEqual({ + 'repoA::/a/stale': [{ id: 'tab-A-stale', worktreeId: 'repoA::/a/stale' }], + 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] + }) + + // After repoB recovers, the deferred purge fires for genuinely stale ids. + mockApi.worktrees.list.mockImplementation(async ({ repoId }: { repoId: string }) => { + if (repoId === 'repoA') { + return [wtA] + } + return [wtB] + }) + + await store.getState().fetchAllWorktrees() + + expect(store.getState().hasHydratedWorktreePurge).toBe(true) + expect(store.getState().tabsByWorktree).toEqual({ + 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] + }) + }) + + it('defers the purge when every repo succeeds but none returns worktrees (empty-sibling safety)', async () => { + const store = createTestStore() + + // Both repos succeed but legitimately return empty (newly-cloned). The + // union of valid ids would be empty — declaring that authoritative + // would wipe every persisted tabsByWorktree entry. Must defer instead. + mockApi.worktrees.list.mockResolvedValue([]) + + store.setState({ + repos: [repoA, repoB], + tabsByWorktree: { + 'repoA::/a/wt1': [{ id: 'tab-A', worktreeId: 'repoA::/a/wt1' }] + } + } as unknown as Partial) + + await store.getState().fetchAllWorktrees() + + expect(store.getState().hasHydratedWorktreePurge).toBe(false) + expect(store.getState().tabsByWorktree).toEqual({ + 'repoA::/a/wt1': [{ id: 'tab-A', worktreeId: 'repoA::/a/wt1' }] + }) + }) + + it('fires the purge once when every repo returns successfully with ≥1 worktree', async () => { + const store = createTestStore() + const wtA = makeWorktree({ id: 'repoA::/a/wt1', repoId: 'repoA', path: '/a/wt1' }) + const wtB = makeWorktree({ id: 'repoB::/b/wt1', repoId: 'repoB', path: '/b/wt1' }) + + mockApi.worktrees.list.mockImplementation(async ({ repoId }: { repoId: string }) => + repoId === 'repoA' ? [wtA] : [wtB] + ) + + store.setState({ + repos: [repoA, repoB], + tabsByWorktree: { + 'repoA::/a/wt1': [{ id: 'tab-A', worktreeId: 'repoA::/a/wt1' }], + 'repoA::/a/zombie': [{ id: 'tab-zombie', worktreeId: 'repoA::/a/zombie' }], + 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] + } + } as unknown as Partial) + + await store.getState().fetchAllWorktrees() + + expect(store.getState().hasHydratedWorktreePurge).toBe(true) + expect(store.getState().tabsByWorktree).toEqual({ + 'repoA::/a/wt1': [{ id: 'tab-A', worktreeId: 'repoA::/a/wt1' }], + 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] + }) + + // Second call must not re-run the purge even if new stale ids appear. + store.setState({ + tabsByWorktree: { + ...store.getState().tabsByWorktree, + 'repoA::/a/new-zombie': [{ id: 'tab-new-zombie', worktreeId: 'repoA::/a/new-zombie' }] + } + } as unknown as Partial) + + await store.getState().fetchAllWorktrees() + + expect(store.getState().tabsByWorktree['repoA::/a/new-zombie']).toBeDefined() + }) +}) + +// Why: design §4.4 — purgeWorktreeTerminalState wipes every worktree-scoped +// map symmetrically so a single removed-worktree event cannot leave +// per-worktree entries stranded. The full cascade is already covered by +// removeWorktree tests above; this block exercises the action directly to +// confirm cross-map coverage (worktree key, tab id, file id, and top-level +// actives). +describe('purgeWorktreeTerminalState direct (design §4.4)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('wipes tab-id-keyed maps (terminalLayoutsByTabId, ptyIdsByTabId) and clears actives', () => { + const store = createTestStore() + + store.setState({ + tabsByWorktree: { + 'repoA::/a/wt1': [ + { id: 'tab-1', worktreeId: 'repoA::/a/wt1' }, + { id: 'tab-2', worktreeId: 'repoA::/a/wt1' } + ], + 'repoA::/a/wt2': [{ id: 'tab-3', worktreeId: 'repoA::/a/wt2' }] + }, + terminalLayoutsByTabId: { + 'tab-1': { panes: [] }, + 'tab-2': { panes: [] }, + 'tab-3': { panes: [] } + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'], 'tab-2': ['pty-2'], 'tab-3': ['pty-3'] }, + runtimePaneTitlesByTabId: { 'tab-1': 'claude', 'tab-3': 'bash' }, + openFiles: [ + { + id: 'file-1', + worktreeId: 'repoA::/a/wt1', + filePath: '/a/wt1/a.ts', + relativePath: 'a.ts', + language: 'typescript', + isDirty: false, + isPreview: false, + mode: 'edit' as const + } + ], + editorDrafts: { 'file-1': 'draft', 'file-99': 'other' }, + activeWorktreeId: 'repoA::/a/wt1', + activeFileId: 'file-1', + activeTabId: 'tab-1', + activeTabType: 'editor' as const + } as unknown as Partial) + + store.getState().purgeWorktreeTerminalState(['repoA::/a/wt1']) + + const s = store.getState() + expect(s.tabsByWorktree).toEqual({ + 'repoA::/a/wt2': [{ id: 'tab-3', worktreeId: 'repoA::/a/wt2' }] + }) + expect(s.terminalLayoutsByTabId).toEqual({ 'tab-3': { panes: [] } }) + expect(s.ptyIdsByTabId).toEqual({ 'tab-3': ['pty-3'] }) + expect(s.runtimePaneTitlesByTabId).toEqual({ 'tab-3': 'bash' }) + expect(s.openFiles).toEqual([]) + expect(s.editorDrafts).toEqual({ 'file-99': 'other' }) + expect(s.activeWorktreeId).toBeNull() + expect(s.activeFileId).toBeNull() + expect(s.activeTabId).toBeNull() + expect(s.activeTabType).toBe('terminal') + }) + + it('is a no-op when the id list is empty', () => { + const store = createTestStore() + const before = { + 'repoA::/a/wt1': [{ id: 'tab-1', worktreeId: 'repoA::/a/wt1' }] + } + store.setState({ tabsByWorktree: before } as unknown as Partial) + + store.getState().purgeWorktreeTerminalState([]) + + expect(store.getState().tabsByWorktree).toBe(before) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 3417764cb..69c6f95ce 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -62,6 +62,7 @@ export const createWorktreeSlice: StateCreator deleteStateByWorktreeId: {}, sortEpoch: 0, everActivatedWorktreeIds: new Set(), + hasHydratedWorktreePurge: false, fetchWorktrees: async (repoId) => { try { @@ -96,7 +97,68 @@ export const createWorktreeSlice: StateCreator fetchAllWorktrees: async () => { const { repos } = get() - await Promise.all(repos.map((r) => get().fetchWorktrees(r.id))) + + // Why: once the one-shot hydration-time purge has fired, subsequent + // calls just need to refresh each repo's cached list. No need to + // double-probe the IPC for the per-repo success signal. + if (get().hasHydratedWorktreePurge) { + await Promise.all(repos.map((r) => get().fetchWorktrees(r.id))) + return + } + + // Why: users upgrading from a pre-fix build may have persisted + // tabsByWorktree entries for worktrees that were deleted in the previous + // session. Without the hydration-time purge below those entries would + // keep zombie PTYs misclassified as "bound" in SessionsStatusSegment + // (design §2c), which means the user would still need a second restart + // post-upgrade to reclaim memory. + // + // Safety gate: fetchWorktrees swallows IPC errors (catch at :93-95) + // and short-circuits on empty-replace when cached data exists + // (empty-guard at :82-84). Neither signal bubbles up to the caller, so + // we can't distinguish "threw" from "returned []" from "returned ≥1" + // by inspecting post-state alone. If we declared the union of + // worktreesByRepo authoritative without confirming every repo + // succeeded, a single transient git error at launch would wipe every + // tabsByWorktree entry for the affected repo — the exact data-loss + // class the empty-guard exists to prevent. Probe the IPC directly to + // get the precise per-repo result, and defer the purge until every + // repo returns success AND at least one has >0 worktrees. In steady + // state this fires on the first fully-successful launch; in the + // degraded state it simply waits. + const results = await Promise.all( + repos.map(async (r) => { + try { + const list = await window.api.worktrees.list({ repoId: r.id }) + await get().fetchWorktrees(r.id) + return { repoId: r.id, ok: list.length > 0 } + } catch (err) { + console.error(`Failed to fetch worktrees for repo ${r.id}:`, err) + return { repoId: r.id, ok: false as const } + } + }) + ) + + const allSucceeded = results.length > 0 && results.every((r) => r.ok) + if (!allSucceeded) { + // Defer; try again on the next fetchAllWorktrees call. + return + } + const validIds = new Set() + for (const list of Object.values(get().worktreesByRepo)) { + for (const w of list) { + validIds.add(w.id) + } + } + const stale = Object.keys(get().tabsByWorktree).filter((id) => !validIds.has(id)) + if (stale.length > 0) { + console.warn( + `[worktree-purge] hydration-time purge removing stale state for ${stale.length} worktree(s):`, + stale + ) + get().purgeWorktreeTerminalState(stale) + } + set({ hasHydratedWorktreePurge: true }) }, createWorktree: async (repoId, name, baseBranch, setupDecision = 'inherit', sparseCheckout) => { @@ -697,5 +759,136 @@ export const createWorktreeSlice: StateCreator } }, - allWorktrees: () => Object.values(get().worktreesByRepo).flat() + allWorktrees: () => Object.values(get().worktreesByRepo).flat(), + + purgeWorktreeTerminalState: (worktreeIds: string[]) => { + if (worktreeIds.length === 0) { + return + } + set((s) => { + const worktreeIdSet = new Set(worktreeIds) + + // Collect every tab id (and removed file id) we are about to orphan. + const doomedTabIds = new Set() + const removedFileIds = new Set() + for (const id of worktreeIdSet) { + for (const tab of s.tabsByWorktree[id] ?? []) { + doomedTabIds.add(tab.id) + } + } + for (const file of s.openFiles) { + if (worktreeIdSet.has(file.worktreeId)) { + removedFileIds.add(file.id) + } + } + + const omitByWorktree = (obj: Record): Record => { + let changed = false + const out = { ...obj } + for (const id of worktreeIdSet) { + if (id in out) { + delete out[id] + changed = true + } + } + return changed ? out : obj + } + const omitByTabId = (obj: Record): Record => { + let changed = false + const out = { ...obj } + for (const tabId of doomedTabIds) { + if (tabId in out) { + delete out[tabId] + changed = true + } + } + return changed ? out : obj + } + const omitByFileId = (obj: Record): Record => { + let changed = false + const out = { ...obj } + for (const fileId of removedFileIds) { + if (fileId in out) { + delete out[fileId] + changed = true + } + } + return changed ? out : obj + } + + const nextOpenFiles = s.openFiles.some((f) => worktreeIdSet.has(f.worktreeId)) + ? s.openFiles.filter((f) => !worktreeIdSet.has(f.worktreeId)) + : s.openFiles + + const removedActive = s.activeWorktreeId != null && worktreeIdSet.has(s.activeWorktreeId) + const activeFileCleared = s.activeFileId != null && removedFileIds.has(s.activeFileId) + const activeTabCleared = s.activeTabId != null && doomedTabIds.has(s.activeTabId) + + const nextEverActivatedWorktreeIds = (() => { + let hit = false + for (const id of worktreeIdSet) { + if (s.everActivatedWorktreeIds.has(id)) { + hit = true + break + } + } + if (!hit) { + return s.everActivatedWorktreeIds + } + const next = new Set(s.everActivatedWorktreeIds) + for (const id of worktreeIdSet) { + next.delete(id) + } + return next + })() + + return { + // Worktree-scoped terminal/tab state + tabsByWorktree: omitByWorktree(s.tabsByWorktree), + terminalLayoutsByTabId: omitByTabId(s.terminalLayoutsByTabId), + ptyIdsByTabId: omitByTabId(s.ptyIdsByTabId), + runtimePaneTitlesByTabId: omitByTabId(s.runtimePaneTitlesByTabId), + // Delete state + deleteStateByWorktreeId: omitByWorktree(s.deleteStateByWorktreeId), + // File search + fileSearchStateByWorktree: omitByWorktree(s.fileSearchStateByWorktree), + // Browser state + browserTabsByWorktree: omitByWorktree(s.browserTabsByWorktree), + recentlyClosedBrowserTabsByWorktree: omitByWorktree(s.recentlyClosedBrowserTabsByWorktree), + activeBrowserTabIdByWorktree: omitByWorktree(s.activeBrowserTabIdByWorktree), + // Editor state + activeFileIdByWorktree: omitByWorktree(s.activeFileIdByWorktree), + activeTabTypeByWorktree: omitByWorktree(s.activeTabTypeByWorktree), + activeTabIdByWorktree: omitByWorktree(s.activeTabIdByWorktree), + tabBarOrderByWorktree: omitByWorktree(s.tabBarOrderByWorktree), + pendingReconnectTabByWorktree: omitByWorktree(s.pendingReconnectTabByWorktree), + // Split-tab / unified tab state + unifiedTabsByWorktree: omitByWorktree(s.unifiedTabsByWorktree), + groupsByWorktree: omitByWorktree(s.groupsByWorktree), + layoutByWorktree: omitByWorktree(s.layoutByWorktree), + activeGroupIdByWorktree: omitByWorktree(s.activeGroupIdByWorktree), + // Git status caches + gitStatusByWorktree: omitByWorktree(s.gitStatusByWorktree), + gitConflictOperationByWorktree: omitByWorktree(s.gitConflictOperationByWorktree), + trackedConflictPathsByWorktree: omitByWorktree(s.trackedConflictPathsByWorktree), + gitBranchChangesByWorktree: omitByWorktree(s.gitBranchChangesByWorktree), + gitBranchCompareSummaryByWorktree: omitByWorktree(s.gitBranchCompareSummaryByWorktree), + gitBranchCompareRequestKeyByWorktree: omitByWorktree( + s.gitBranchCompareRequestKeyByWorktree + ), + expandedDirs: omitByWorktree(s.expandedDirs), + // Per-file editor state for removed files + editorDrafts: omitByFileId(s.editorDrafts), + markdownViewMode: omitByFileId(s.markdownViewMode), + // Top-level actives + openFiles: nextOpenFiles, + everActivatedWorktreeIds: nextEverActivatedWorktreeIds, + activeWorktreeId: removedActive ? null : s.activeWorktreeId, + activeFileId: activeFileCleared ? null : s.activeFileId, + activeBrowserTabId: removedActive ? null : s.activeBrowserTabId, + activeTabId: activeTabCleared ? null : s.activeTabId, + activeTabType: removedActive || activeFileCleared ? 'terminal' : s.activeTabType + } + }) + } })