* 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 <help@stably.ai> * 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 <help@stably.ai> * 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 <help@stably.ai> * 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 <help@stably.ai> * test(zombie-worktree): fix ptyIdsByTabId seed shape to match production type The purge unit test seeded ptyIdsByTabId as Record<string, string> when the runtime type is Record<string, string[]>. 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 <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
GitHub
parent
943f64a906
commit
66d596fe11
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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<string, (_event: unknown, args: unknown) => 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<unknown>
|
||||
).catch(() => {})
|
||||
|
||||
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects ask-policy creates before mutating git state when setup decision is missing', async () => {
|
||||
getEffectiveHooksMock.mockReturnValue({
|
||||
scripts: {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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<typeof vi.fn>
|
||||
attach: ReturnType<typeof vi.fn>
|
||||
write: ReturnType<typeof vi.fn>
|
||||
resize: ReturnType<typeof vi.fn>
|
||||
shutdown: ReturnType<typeof vi.fn>
|
||||
sendSignal: ReturnType<typeof vi.fn>
|
||||
getCwd: ReturnType<typeof vi.fn>
|
||||
getInitialCwd: ReturnType<typeof vi.fn>
|
||||
clearBuffer: ReturnType<typeof vi.fn>
|
||||
acknowledgeDataEvent: ReturnType<typeof vi.fn>
|
||||
hasChildProcesses: ReturnType<typeof vi.fn>
|
||||
getForegroundProcess: ReturnType<typeof vi.fn>
|
||||
serialize: ReturnType<typeof vi.fn>
|
||||
revive: ReturnType<typeof vi.fn>
|
||||
listProcesses: ReturnType<typeof vi.fn>
|
||||
getDefaultShell: ReturnType<typeof vi.fn>
|
||||
getProfiles: ReturnType<typeof vi.fn>
|
||||
onData: ReturnType<typeof vi.fn>
|
||||
onReplay: ReturnType<typeof vi.fn>
|
||||
onExit: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
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<typeof createProviderStub> = 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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string, Set<MessageWaiter>>()
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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<typeof vi.fn>).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<typeof killAllProcessesForWorktree>[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<typeof killAllProcessesForWorktree>[1]['runtime']
|
||||
|
||||
const localProvider = createProviderStub(async () => [])
|
||||
listRegisteredPtysMock.mockReturnValue([])
|
||||
|
||||
const result = await killAllProcessesForWorktree('w1', { runtime, localProvider })
|
||||
|
||||
expect(result.runtimeStopped).toBe(0)
|
||||
})
|
||||
})
|
||||
|
|
@ -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<WorktreeTeardownResult> {
|
||||
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<number> {
|
||||
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<number> {
|
||||
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
|
||||
}
|
||||
|
|
@ -34,7 +34,7 @@ export function attachMainWindowServices(
|
|||
prepareClaudeAuth?: () => Promise<ClaudeRuntimeAuthPreparation>
|
||||
): void {
|
||||
registerRepoHandlers(mainWindow, store)
|
||||
registerWorktreeHandlers(mainWindow, store)
|
||||
registerWorktreeHandlers(mainWindow, store, runtime)
|
||||
registerPtyHandlers(
|
||||
mainWindow,
|
||||
runtime,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -35,6 +35,15 @@ export type WorktreeSlice = {
|
|||
* agent output) count normally. Session-only; never persisted.
|
||||
*/
|
||||
everActivatedWorktreeIds: Set<string>
|
||||
/**
|
||||
* 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<void>
|
||||
fetchAllWorktrees: () => Promise<void>
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
store.getState().purgeWorktreeTerminalState([])
|
||||
|
||||
expect(store.getState().tabsByWorktree).toBe(before)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
deleteStateByWorktreeId: {},
|
||||
sortEpoch: 0,
|
||||
everActivatedWorktreeIds: new Set<string>(),
|
||||
hasHydratedWorktreePurge: false,
|
||||
|
||||
fetchWorktrees: async (repoId) => {
|
||||
try {
|
||||
|
|
@ -96,7 +97,68 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
|
||||
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<string>()
|
||||
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<AppState, [], [], WorktreeSlice>
|
|||
}
|
||||
},
|
||||
|
||||
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<string>()
|
||||
const removedFileIds = new Set<string>()
|
||||
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 = <T>(obj: Record<string, T>): Record<string, T> => {
|
||||
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 = <T>(obj: Record<string, T>): Record<string, T> => {
|
||||
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 = <T>(obj: Record<string, T>): Record<string, T> => {
|
||||
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
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue