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 + } + }) + } })