From 869f6bbef71bd3353531ee69e76bbaf48310cee8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 13 Nov 2026 20:50:00 -0800 Subject: [PATCH] Restore workspace deletion on server hosts (#41858) * Fix worktree deletion on server hosts * Finish workspace removal cancellation and final saves * Wait for WSL workspace deletion to finish * Capture CLI deletion errors in the server integration test * test: canonicalize removal fixture and expose serve stop phases * test: align serve fixture versions and report workspace paths * test: isolate removal lifetimes and resolve native fixture paths * Type-check cancellation assertions on the shared removal mock --- .../process-capture/worker-exec-file.test.ts | 64 ++++ ...rktree-scan-cache-annotation-reuse.test.ts | 66 +++- .../worktrees/catalog/worktree-scan-cache.ts | 35 ++- .../worktrees/removal/remove-worktree.test.ts | 72 +++++ .../windows-worktree-authorization.test.ts | 191 ++++++++++++ .../removal/windows-worktree-removal.ts | 65 +++- .../git/worktrees/removal/worktree-removal.ts | 61 +++- .../ipc/worktrees-remove-preflight.test.ts | 253 +++++++++++++++- .../remove-registered-local-worktree.ts | 188 ++++++------ .../orcad-lifecycle-host.test.ts | 66 +++- .../orcad/host-lifecycle/orcad-lifecycle.ts | 23 +- .../host-lifecycle/orcad-lock-yield.test.ts | 2 + src/main/orcad/host-runtime/orcad-entry.ts | 48 ++- .../host-runtime/orcad-profile-shutdown.ts | 24 ++ .../orcad/host-runtime/orcad-runtime-rpc.ts | 27 ++ .../orcad-serve-parity.integration.test.ts | 209 ++++++++++++- .../profile-shutdown-checkpoint.ts | 15 + ...removal-and-reconciliation-part-02.spec.ts | 10 +- ...removal-and-reconciliation-part-04.spec.ts | 1 - ...interrupted-local-worktree-removal.test.ts | 35 +++ ...time-interrupted-local-worktree-removal.ts | 51 +++- ...ntime-registered-local-worktree-removal.ts | 111 ++++--- ...ime-removal-recovery-scan-contract.test.ts | 285 ++++++++++++++++++ .../runtime-worktree-push-target-cleanup.ts | 9 + ...-process-ready-persistence-cleanup.test.ts | 154 +++++++++- .../startup/main-process-ready-runtime.ts | 7 +- src/main/startup/main-process-ready.ts | 32 +- src/main/startup/main-process-state.ts | 1 + .../main-process-quit-pending-writer.test.ts | 164 +++++++++- src/main/startup/quit/main-process-quit.ts | 56 ++-- .../quit/window-all-closed-quit-handler.ts | 19 ++ .../worktree-removal-records-load.ts | 18 +- .../local-orphaned-worktree-cleanup.test.ts | 62 ++++ .../local-orphaned-worktree-cleanup.ts | 26 +- .../removal/local-worktree-filesystem.ts | 2 + .../local-worktree-removal-recovery.test.ts | 109 +++++++ .../local-worktree-removal-recovery.ts | 8 +- .../worktree-background-removal.test.ts | 1 + .../removal/worktree-background-removal.ts | 61 ++-- .../removal/worktree-removal-leftover.ts | 29 +- ...worktree-removal-lifetime.test-fixtures.ts | 80 +++++ .../removal/worktree-removal-lifetime.test.ts | 208 +++++++++++++ .../removal/worktree-removal-lifetime.ts | 83 +++++ .../removal/worktree-removal-records-load.ts | 32 ++ .../removal/worktree-removal-table.ts | 15 +- 45 files changed, 2728 insertions(+), 350 deletions(-) create mode 100644 src/main/git/worktrees/removal/windows-worktree-authorization.test.ts create mode 100644 src/main/orcad/host-runtime/orcad-profile-shutdown.ts create mode 100644 src/main/orcad/host-runtime/orcad-runtime-rpc.ts create mode 100644 src/main/persistence/profile-shutdown-checkpoint.ts create mode 100644 src/main/runtime/workspaces/removal/runtime-removal-recovery-scan-contract.test.ts create mode 100644 src/main/runtime/workspaces/removal/runtime-worktree-push-target-cleanup.ts create mode 100644 src/main/startup/quit/window-all-closed-quit-handler.ts create mode 100644 src/main/workspaces/removal/local-orphaned-worktree-cleanup.test.ts create mode 100644 src/main/workspaces/removal/worktree-removal-lifetime.test-fixtures.ts create mode 100644 src/main/workspaces/removal/worktree-removal-lifetime.test.ts create mode 100644 src/main/workspaces/removal/worktree-removal-lifetime.ts create mode 100644 src/main/workspaces/removal/worktree-removal-records-load.ts diff --git a/src/main/git/command-runner/process-capture/worker-exec-file.test.ts b/src/main/git/command-runner/process-capture/worker-exec-file.test.ts index 73bbe5e2773..0fb4eece4dc 100644 --- a/src/main/git/command-runner/process-capture/worker-exec-file.test.ts +++ b/src/main/git/command-runner/process-capture/worker-exec-file.test.ts @@ -1,3 +1,14 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { removeWorktree } from '../../worktrees/removal/worktree-removal' +import { prepareWorktreeRemovalsForStore } from '../../../workspaces/removal/worktree-removal-records-load' +import { + startBackgroundWorktreeRemoval, + _resetPendingWorktreeRemovalsForTests +} from '../../../workspaces/removal/worktree-background-removal' +import { worktreeRemovalLifetime } from '../../../workspaces/removal/worktree-removal-lifetime' +import { readWorktreeRemovalRecords } from '../../../workspaces/removal/worktree-removal-records' import { setImmediate as nextTurn } from 'node:timers/promises' import { Worker } from 'node:worker_threads' import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' @@ -142,6 +153,59 @@ describe.skipIf(process.platform !== 'darwin')('macOS worker capture cancellatio await vi.waitFor(() => expect(terminated).toHaveBeenCalledOnce()) }) + it('keeps removal ownership and durable intent until an aborted worker child is actually terminated', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-removal-worker-close-')) + const gate = new SharedArrayBuffer(4) + await createHost(gate) + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => [{ id: 'repo' }] + }) + const childStarted = Promise.withResolvers() + let closing: Promise | undefined + try { + const result = startBackgroundWorktreeRemoval({ + removal: { + worktreeId: 'repo::feature', + repoId: 'repo', + repoPath: directory, + worktree: { path: join(directory, 'missing-checkout'), branch: 'feature', head: 'abc' }, + force: false, + deleteBranch: false + }, + publish: () => {}, + run: (signal) => + removeWorktree(directory, join(directory, 'missing-checkout'), false, { + knownRemovedWorktree: { branch: 'feature', head: 'abc' }, + deleteBranch: false, + checkoutDeleteSignal: signal, + observeChildTermination: () => { + childStarted.resolve() + } + }) + }) + void result.catch(() => {}) + await childStarted.promise + let closed = false + closing = worktreeRemovalLifetime.close().then(() => { + closed = true + }) + await nextTurn() + expect(closed).toBe(false) + expect(await readWorktreeRemovalRecords(directory)).toHaveLength(1) + Atomics.store(new Int32Array(gate), 0, 1) + Atomics.notify(new Int32Array(gate), 0) + await closing + expect(await readWorktreeRemovalRecords(directory)).toHaveLength(1) + } finally { + Atomics.store(new Int32Array(gate), 0, 1) + Atomics.notify(new Int32Array(gate), 0) + await closing + _resetPendingWorktreeRemovalsForTests() + await rm(directory, { recursive: true, force: true }) + } + }) + it('does not infer child termination from worker loss before a spawn acknowledgement', async () => { const gate = new SharedArrayBuffer(4) await createHost(gate) diff --git a/src/main/git/worktrees/catalog/worktree-scan-cache-annotation-reuse.test.ts b/src/main/git/worktrees/catalog/worktree-scan-cache-annotation-reuse.test.ts index b3ba536a06c..42abd2f94ab 100644 --- a/src/main/git/worktrees/catalog/worktree-scan-cache-annotation-reuse.test.ts +++ b/src/main/git/worktrees/catalog/worktree-scan-cache-annotation-reuse.test.ts @@ -25,7 +25,10 @@ vi.mock('./worktree-list-reader', () => ({ import { _resetWorktreeScanCacheForTests, listWorktreeGraph, - listWorktrees + listWorktrees, + listWorktreesSharedStrict, + listWorktreeGraphSharedStrict, + bumpWorktreeScanGeneration } from './worktree-scan-cache' import { __resetSparseCheckoutStateCacheForTests } from '../sparse-checkout/worktree-sparse-checkout-cache' @@ -67,3 +70,64 @@ describe('graph and annotated worktree scans', () => { expect((await annotatedScan)[0]?.isSparse).toBe(true) }) }) + +it('shares only matching live cancellation scopes and preserves generation freshness', async () => { + _resetWorktreeScanCacheForTests() + readWorktreeListMock.mockReset() + __resetSparseCheckoutStateCacheForTests() + const pending = Promise.withResolvers() + readWorktreeListMock.mockReturnValue(pending.promise) + detectSparseCheckoutMock.mockResolvedValue(false) + const first = new AbortController() + const other = new AbortController() + const reads = [ + listWorktreesSharedStrict('/repo', { signal: first.signal }), + listWorktreesSharedStrict('/repo', { signal: first.signal }), + listWorktreesSharedStrict('/repo', { signal: other.signal }), + listWorktreesSharedStrict('/repo') + ] + expect(readWorktreeListMock).toHaveBeenCalledTimes(3) + bumpWorktreeScanGeneration('/repo') + reads.push(listWorktreesSharedStrict('/repo', { signal: first.signal })) + expect(readWorktreeListMock).toHaveBeenCalledTimes(4) + first.abort() + await expect(listWorktreesSharedStrict('/repo', { signal: first.signal })).rejects.toThrow() + expect(readWorktreeListMock).toHaveBeenCalledTimes(4) + pending.resolve([]) + const outcomes = await Promise.allSettled(reads) + expect(outcomes.map((outcome) => outcome.status)).toEqual([ + 'rejected', + 'rejected', + 'fulfilled', + 'fulfilled', + 'rejected' + ]) +}) + +it('keeps strict all-row graphs separate, preserves errors and refreshes in the same cancellation scope', async () => { + _resetWorktreeScanCacheForTests() + readTranslatedWorktreeGraphMock.mockReset() + const pending = Promise.withResolvers() + readTranslatedWorktreeGraphMock.mockReturnValue(pending.promise) + const signal = new AbortController().signal + const scans = [ + listWorktreeGraphSharedStrict('/repo', { signal }), + listWorktreeGraphSharedStrict('/repo', { signal }) + ] + expect(readTranslatedWorktreeGraphMock).toHaveBeenCalledOnce() + scans.push(listWorktreeGraph('/repo', { signal })) + expect(readTranslatedWorktreeGraphMock).toHaveBeenCalledTimes(2) + bumpWorktreeScanGeneration('/repo') + scans.push(listWorktreeGraphSharedStrict('/repo', { signal })) + expect(readTranslatedWorktreeGraphMock).toHaveBeenCalledTimes(3) + const nested = { + ...ROW, + path: '/repo/checkout/.orca-worktree-preparation', + branch: 'refs/heads/orca-create-preparation/test' + } + pending.resolve([ROW, nested]) + expect((await Promise.all(scans))[0]).toEqual([ROW, nested]) + const failure = new Error('inventory unavailable') + readTranslatedWorktreeGraphMock.mockRejectedValueOnce(failure) + await expect(listWorktreeGraphSharedStrict('/repo', { signal })).rejects.toBe(failure) +}) diff --git a/src/main/git/worktrees/catalog/worktree-scan-cache.ts b/src/main/git/worktrees/catalog/worktree-scan-cache.ts index fa2cfe03e97..8256dabed2d 100644 --- a/src/main/git/worktrees/catalog/worktree-scan-cache.ts +++ b/src/main/git/worktrees/catalog/worktree-scan-cache.ts @@ -1,3 +1,5 @@ +import { readTranslatedWorktreeGraph } from './worktree-list-reader' +import { abortSignalReason } from '../../../../shared/abort-signal-reason' import type { GitWorktreeInfo } from '../../../../shared/worktree/types' import { annotateSparseCheckoutStatus, @@ -10,9 +12,12 @@ import { WORKTREE_LIST_TIMEOUT_MS } from '../worktree-operation-options' import { resolveGitAdmissionTier } from '../../command-runner/admission/git-operation-executor' import { withRepositoryGitCause } from '../../repository-git-cause' +let signalIdentities = new WeakMap() +let nextSignalIdentity = 0 + const inFlightWorktreeScans = new Map>() -type WorktreeScanKind = 'graph' | 'lenient' | 'strict' | 'strict-true-empty' +type WorktreeScanKind = 'graph' | 'strict-graph' | 'lenient' | 'strict' | 'strict-true-empty' const worktreeScanGenerations = new Map() @@ -62,6 +67,8 @@ export function _getWorktreeScanCacheSizesForTests(): { inFlight: number; genera } export function _resetWorktreeScanCacheForTests(): void { + signalIdentities = new WeakMap() + nextSignalIdentity = 0 inFlightWorktreeScans.clear() worktreeScanGenerations.clear() } @@ -73,18 +80,18 @@ function shareWorktreeScan( kind: WorktreeScanKind, run: (repoPath: string, options: GitWorktreeExecOptions) => Promise ): Promise { + if (options.signal?.aborted) { + return Promise.reject(abortSignalReason(options.signal)) + } + let signalIdentity = 0 if (options.signal) { - return withRepositoryGitCause( - repoPath, - options, - { family: 'catalog', reason: 'worktree-scan', cache: 'bypass-signal' }, - () => run(repoPath, options) - ) + signalIdentity = signalIdentities.get(options.signal) ?? ++nextSignalIdentity + signalIdentities.set(options.signal, signalIdentity) } const generation = worktreeScanGenerations.get(repoPath) ?? 0 const timeout = options.timeout ?? WORKTREE_LIST_TIMEOUT_MS // Separate deadlines, failure contracts and admission tiers so joiners cannot inherit weaker scans. - const key = `${repoPath}\0${options.wslDistro ?? ''}\0${timeout}\0${options.includeCreatePreparations === true}\0${generation}\0${kind}\0${resolveGitAdmissionTier(options.admissionTier)}` + const key = `${repoPath}\0${options.wslDistro ?? ''}\0${timeout}\0${options.includeCreatePreparations === true}\0${generation}\0${kind}\0${resolveGitAdmissionTier(options.admissionTier)}\0${signalIdentity}` const inFlight = inFlightWorktreeScans.get(key) if (inFlight) { return inFlight @@ -113,7 +120,7 @@ async function runAnnotatedWorktreeScan( return annotateSparseCheckoutStatus(repoPath, worktrees, options) } -/** Git failures return []; matching callers coalesce, while AbortSignal callers stay isolated. */ +/** Git failures return []; matching callers coalesce, while different cancellation scopes stay isolated. */ export function listWorktrees( repoPath: string, options: GitWorktreeExecOptions = {} @@ -121,7 +128,7 @@ export function listWorktrees( return shareWorktreeScan(repoPath, options, 'lenient', runAnnotatedWorktreeScan) } -/** Skip sparse probes while retaining generation fencing and AbortSignal isolation. */ +/** Skip sparse probes while retaining generation fencing and cancellation-scope isolation. */ export function listWorktreeGraph( repoPath: string, options: GitWorktreeExecOptions = {} @@ -149,3 +156,11 @@ export function listWorktreesSharedStrictAllowingTrueEmpty( listWorktreesStrictAllowingTrueEmptyUnshared ) } + +/** Strict authority includes every registration, without preparation filtering or sparse probes. */ +export function listWorktreeGraphSharedStrict( + repoPath: string, + options: GitWorktreeExecOptions = {} +): Promise { + return shareWorktreeScan(repoPath, options, 'strict-graph', readTranslatedWorktreeGraph) +} diff --git a/src/main/git/worktrees/removal/remove-worktree.test.ts b/src/main/git/worktrees/removal/remove-worktree.test.ts index eba05bac7e1..e97dd501c75 100644 --- a/src/main/git/worktrees/removal/remove-worktree.test.ts +++ b/src/main/git/worktrees/removal/remove-worktree.test.ts @@ -56,6 +56,7 @@ import { } from './remove-worktree-test-harness' import { removeWorktree } from './worktree-removal' +import * as nativeRemoval from './windows-worktree-removal' import { __getSparseCheckoutStateCacheSizeForTests } from '../sparse-checkout/worktree-sparse-checkout-cache' const mockGitCommands = createGitCommandMocker(gitExecFileAsyncMock) @@ -85,6 +86,35 @@ describe('removeWorktree', () => { }) }) + it('keeps an aborted removal pending until the mutation child termination is confirmed', async () => { + const termination = Promise.withResolvers() + const started = Promise.withResolvers() + const abort = new Error('checkout delete stopped') + gitExecFileAsyncMock.mockImplementation(async (args, options) => { + if (args.includes('remove')) { + expect(options.spawnOnWorkerThread).toBe(true) + options.observeChildTermination(termination.promise, true) + started.resolve() + throw abort + } + return { stdout: '', stderr: '' } + }) + let settled = false + const result = removeWorktree('/repo', '/repo/feature', false, { + knownRemovedWorktree: { branch: 'feature', head: 'abc' }, + deleteBranch: false + }) + const outcome = result.catch((error: unknown) => { + settled = true + return error + }) + await started.promise + await Promise.resolve() + expect(settled).toBe(false) + termination.resolve() + expect(await outcome).toBe(abort) + }) + it('preserves the branch when requested for a pre-existing local branch checkout', async () => { mockGitCommands({ 'git worktree list --porcelain': { @@ -250,3 +280,45 @@ locked active agent session expect(getGitCalls()).not.toContain('git worktree remove --force /repo-feature') }) }) + +it('finishes native Git registration and branch cleanup when checkout stop follows physical success', async () => { + platformSpy.mockReturnValue('win32') + const directory = await mkdtemp(join(tmpdir(), 'orca-native-finalization-')) + const checkout = join(directory, 'feature') + await mkdir(checkout) + const controller = new AbortController() + const physical = vi.spyOn(nativeRemoval, 'removeWindowsCheckout').mockImplementation(async () => { + await rm(checkout, { recursive: true }) + controller.abort(new Error('host stopping after checkout completion')) + }) + let registered = true + gitExecFileAsyncMock.mockImplementation(async (args: string[], options) => { + if (options.signal?.aborted) { + throw options.signal.reason + } + if (args.includes('list')) { + return { + stdout: registered ? `worktree ${checkout}\nHEAD abc\nbranch refs/heads/feature\n` : '', + stderr: '' + } + } + if (args.includes('remove')) { + registered = false + } + return { stdout: '', stderr: '' } + }) + try { + await expect( + removeWorktree(directory, checkout, false, { + knownRemovedWorktree: { branch: 'feature', head: 'abc' }, + checkoutDeleteSignal: controller.signal + }) + ).resolves.toEqual({}) + expect(existsSync(checkout)).toBe(false) + expect(registered).toBe(false) + expect(getGitCalls()).toContain('git branch -d -- feature') + } finally { + physical.mockRestore() + await rm(directory, { recursive: true, force: true }) + } +}) diff --git a/src/main/git/worktrees/removal/windows-worktree-authorization.test.ts b/src/main/git/worktrees/removal/windows-worktree-authorization.test.ts new file mode 100644 index 00000000000..3847b06772a --- /dev/null +++ b/src/main/git/worktrees/removal/windows-worktree-authorization.test.ts @@ -0,0 +1,191 @@ +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { + throwIfSignalAborted, + waitForPromiseWithSignal +} from '../../../../shared/abort-signal-reason' +import type { GitWorktreeInfo } from '../../../../shared/worktree/types' + +const { graph, stat, names, removeChild, unlink, rmdir, clean } = vi.hoisted(() => ({ + graph: vi.fn(), + stat: vi.fn(), + names: vi.fn(), + removeChild: vi.fn(), + unlink: vi.fn(), + rmdir: vi.fn(), + clean: vi.fn() +})) +vi.mock('../catalog/worktree-list-reader', async (original) => ({ + ...(await original>()), + readTranslatedWorktreeGraph: graph +})) +vi.mock('../../../filesystem/asar-transparent-fs', () => ({ + lstat: stat, + readdir: names, + rm: removeChild, + unlink, + rmdir +})) +vi.mock('node:fs/promises', async (original) => ({ + ...(await original>()), + readFile: async () => 'gitdir: C:/repo/.git/worktrees/feature' +})) +vi.mock('../../../workspaces/removal/worktree-removal-safety', async (original) => ({ + ...(await original>()), + canSafelyRemoveOrphanedWorktreeDirectory: async () => true +})) +vi.mock('./worktree-removal-preflight', () => ({ assertWorktreeCleanForRemoval: clean })) + +import { authorizeWindowsCheckout, removeWindowsCheckout } from './windows-worktree-removal' +import { _resetWorktreeScanCacheForTests } from '../catalog/worktree-scan-cache' + +const originalPlatform = process.platform +const root = 'C:/checkout/feature' +const row: GitWorktreeInfo = { + path: root, + branch: 'refs/heads/feature', + head: 'abc', + isBare: false, + isMainWorktree: false +} +const missing = Object.assign(new Error('missing'), { code: 'ENOENT' }) + +beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + _resetWorktreeScanCacheForTests() + graph.mockResolvedValue([row]) + stat.mockImplementation(async (path: string) => { + if (String(path).endsWith('locked')) { + throw missing + } + return { isDirectory: () => true, isSymbolicLink: () => false } + }) + names.mockResolvedValue(['held', '.git']) + removeChild.mockResolvedValue(undefined) + unlink.mockResolvedValue(undefined) + rmdir.mockResolvedValue(undefined) + clean.mockResolvedValue(undefined) +}) +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + vi.resetAllMocks() + _resetWorktreeScanCacheForTests() +}) + +it.each([10, 100])( + 'shares one native eligibility graph for %i equivalent overlapping authorizations', + async (population) => { + const reading = Promise.withResolvers() + graph.mockReturnValueOnce(reading.promise) + const signal = new AbortController().signal + const checks = Array.from({ length: population }, () => + authorizeWindowsCheckout('C:/repo', root, true, { checkoutDeleteSignal: signal }) + ) + expect(graph).toHaveBeenCalledOnce() + reading.resolve([row]) + expect(await Promise.all(checks)).toEqual(Array.from({ length: population }, () => true)) + expect(graph.mock.calls[0]?.[1].signal).toBe(signal) + } +) + +it('refuses nested preparation registrations and strict inventory errors', async () => { + graph.mockResolvedValueOnce([row, { ...row, path: `${root}/.orca-worktree-preparation` }]) + await expect(authorizeWindowsCheckout('C:/repo', root, true, {})).rejects.toThrow( + 'another registered worktree' + ) + const failure = new Error('inventory failed') + graph.mockRejectedValueOnce(failure) + await expect(authorizeWindowsCheckout('C:/repo', root, true, {})).rejects.toBe(failure) + expect(removeChild).not.toHaveBeenCalled() +}) + +it.each(['status', 'recursive'] as const)( + 'cancels the later %s read with the original checkout stop reason', + async (stage) => { + const controller = new AbortController() + const held = Promise.withResolvers() + const entered = Promise.withResolvers() + if (stage === 'status') { + clean.mockImplementation(async (_root, _force, options) => { + entered.resolve() + await waitForPromiseWithSignal(held.promise, options.signal) + }) + } else { + graph + .mockImplementationOnce(async () => [row]) + .mockImplementationOnce(async (_repo, options) => { + entered.resolve() + await waitForPromiseWithSignal(held.promise, options.signal) + return [row] + }) + } + const reason = new Error('owner stopped') + const pending = authorizeWindowsCheckout('C:/repo', root, false, { + checkoutDeleteSignal: controller.signal + }) + const rejected = expect(pending).rejects.toBe(reason) + await entered.promise + controller.abort(reason) + await rejected + expect(unlink).not.toHaveBeenCalled() + } +) + +it('joins held children before cancelling the real repeated delete-time authorization', async () => { + const controller = new AbortController() + const sibling = Promise.withResolvers() + const childrenStarted = Promise.withResolvers() + const repeated = Promise.withResolvers() + let settled = false + removeChild.mockImplementation(async () => { + childrenStarted.resolve() + await sibling.promise + }) + // Initial clean authorization and its recursive proof, then the post-child recheck. + graph + .mockImplementationOnce(async () => [row]) + .mockImplementationOnce(async () => [row]) + .mockImplementationOnce(async (_repo, options) => { + repeated.resolve() + await waitForPromiseWithSignal(new Promise(() => {}), options.signal) + throwIfSignalAborted(options.signal) + return [row] + }) + const pending = removeWindowsCheckout('C:/repo', root, false, { + checkoutDeleteSignal: controller.signal + }) + const outcome = pending + .finally(() => { + settled = true + }) + .catch((error: unknown) => error) + await childrenStarted.promise + expect(settled).toBe(false) + expect(unlink).not.toHaveBeenCalled() + sibling.resolve() + await repeated.promise + const reason = new Error('owner stopped during repeated proof') + controller.abort(reason) + expect(await outcome).toBe(reason) + expect(unlink).not.toHaveBeenCalled() + expect(rmdir).not.toHaveBeenCalled() +}) + +it('honors distinct read and checkout signals without replacing either caller option', async () => { + const read = new AbortController() + const checkout = new AbortController() + const entered = Promise.withResolvers() + graph.mockImplementation(async (_repo, options) => { + entered.resolve() + return waitForPromiseWithSignal(new Promise(() => {}), options.signal) + }) + const options = { signal: read.signal, checkoutDeleteSignal: checkout.signal } + const reason = new Error('checkout closing') + const rejected = expect(authorizeWindowsCheckout('C:/repo', root, true, options)).rejects.toBe( + reason + ) + await entered.promise + checkout.abort(reason) + await rejected + expect(options.signal).toBe(read.signal) + expect(options.checkoutDeleteSignal).toBe(checkout.signal) +}) diff --git a/src/main/git/worktrees/removal/windows-worktree-removal.ts b/src/main/git/worktrees/removal/windows-worktree-removal.ts index 24f3b1232ee..7c7455398e5 100644 --- a/src/main/git/worktrees/removal/windows-worktree-removal.ts +++ b/src/main/git/worktrees/removal/windows-worktree-removal.ts @@ -1,3 +1,4 @@ +import { throwIfSignalAborted } from '../../../../shared/abort-signal-reason' import { join, resolve } from 'node:path' import { readFile } from 'node:fs/promises' import { lstat, readdir } from '../../../filesystem/asar-transparent-fs' @@ -15,7 +16,7 @@ import { isDangerousWorktreeRemovalPath, ORPHANED_WORKTREE_DIRECTORY_MESSAGE } from '../../../workspaces/removal/worktree-removal-safety' -import { readTranslatedWorktreeGraph } from '../catalog/worktree-list-reader' +import { listWorktreeGraphSharedStrict } from '../catalog/worktree-scan-cache' import { assertWorktreeCleanForRemoval } from './worktree-removal-preflight' import { getErrorCode, type RemoveWorktreeOptions } from '../worktree-operation-options' @@ -83,7 +84,42 @@ export async function authorizeWindowsCheckout( beforeTeardown = false, checkClean = true ): Promise { - const worktrees = await readTranslatedWorktreeGraph(repo, options) + const signal = + options.signal && + options.checkoutDeleteSignal && + options.signal !== options.checkoutDeleteSignal + ? AbortSignal.any([options.signal, options.checkoutDeleteSignal]) + : (options.checkoutDeleteSignal ?? options.signal) + options = { ...options, signal, checkoutDeleteSignal: signal } + const checkStopped = () => throwIfSignalAborted(signal) + checkStopped() + try { + return await authorizeWindowsCheckoutReads( + repo, + root, + force, + options, + beforeTeardown, + checkClean + ) + } catch (error) { + checkStopped() + throw error + } +} + +async function authorizeWindowsCheckoutReads( + repo: string, + root: string, + force: boolean, + options: RemoveWorktreeOptions & Parameters[2], + beforeTeardown: boolean, + checkClean: boolean +): Promise { + const checkStopped = () => throwIfSignalAborted(options.signal) + checkStopped() + const worktrees = await listWorktreeGraphSharedStrict(repo, options) + checkStopped() const current = findRegisteredDeletableWorktree(repo, root, worktrees, CLIENT_REMOVAL_HOME) if (isDangerousWorktreeRemovalPath(root, repo, CLIENT_REMOVAL_HOME)) { throw new Error(`Refusing to delete protected worktree path: ${root}`) @@ -107,7 +143,9 @@ export async function authorizeWindowsCheckout( let entry try { entry = await lstat(root) + checkStopped() } catch (error) { + checkStopped() if (getErrorCode(error) === 'ENOENT') { return false } @@ -116,14 +154,22 @@ export async function authorizeWindowsCheckout( if (!entry.isDirectory() || entry.isSymbolicLink()) { throw new Error(`Refusing to delete unregistered worktree path: ${root}`) } - if (await nativeWindowsMarkerMissing(root, options)) { + const markerMissing = await nativeWindowsMarkerMissing(root, options) + checkStopped() + if (markerMissing) { // Reject only; an empty observation never authorizes recursive deletion after teardown. - if (beforeTeardown && (await readdir(root)).length > 0) { - throw unregisteredFolderRefusal(root) + if (beforeTeardown) { + const names = await readdir(root) + checkStopped() + if (names.length > 0) { + throw unregisteredFolderRefusal(root) + } } return false } - if (!(await canSafelyRemoveOrphanedWorktreeDirectory(root, repo, CLIENT_REMOVAL_HOME))) { + const safe = await canSafelyRemoveOrphanedWorktreeDirectory(root, repo, CLIENT_REMOVAL_HOME) + checkStopped() + if (!safe) { throw new Error(`Refusing to delete unregistered worktree path: ${root}`) } if (!current && !force) { @@ -137,17 +183,22 @@ export async function authorizeWindowsCheckout( .trim() .replace(/^gitdir:\s*/i, '') ) + checkStopped() try { await lstat(join(admin, 'locked')) + checkStopped() throw new Error('Cannot remove a locked worktree. Unlock it before deleting.') } catch (error) { + checkStopped() if (getErrorCode(error) !== 'ENOENT') { throw error } } if (checkClean && !force) { await assertWorktreeCleanForRemoval(root, force, options, admin) - return authorizeWindowsCheckout(repo, root, force, options, beforeTeardown, false) + checkStopped() + return authorizeWindowsCheckoutReads(repo, root, force, options, beforeTeardown, false) } + checkStopped() return true } diff --git a/src/main/git/worktrees/removal/worktree-removal.ts b/src/main/git/worktrees/removal/worktree-removal.ts index 8d921aaca22..3371abd5af9 100644 --- a/src/main/git/worktrees/removal/worktree-removal.ts +++ b/src/main/git/worktrees/removal/worktree-removal.ts @@ -37,7 +37,17 @@ export async function removeWorktree( // Reserve forceBranchDelete for failed-creation rollback so user deletes preserve unmerged commits. options: RemoveWorktreeOptions = {} ): Promise { - options = removalWorkerOptions(options) + const writers: Promise[] = [] + const observe = options.observeChildTermination + options = removalWorkerOptions({ + ...options, + observeChildTermination: (terminated, writes) => { + observe?.(terminated, writes) + if (writes) { + writers.push(terminated) + } + } + }) try { // Wait out ref packing before deleting branches; concurrent pruning can finish safely. return await withRepoRefMaintenancePaused('worktree-remove', () => @@ -46,6 +56,7 @@ export async function removeWorktree( ) ) } finally { + await Promise.all(writers) invalidateWslLinkedWorktreeGitRouting(worktreePath) invalidateSparseCheckoutState(repoPath, worktreePath) bumpWorktreeScanGeneration(repoPath) @@ -72,12 +83,15 @@ async function performRemoveWorktree( // Avoid default deadlines during large deletes; preserve Windows long-path support used at creation. const longPathArgs = windowsLongPathGitArgs(repoPath) - const execOptions = { + const finalizationOptions = { ...gitExecOptions(repoPath, options), - ...(options.checkoutDeleteSignal ? { signal: options.checkoutDeleteSignal } : {}), ...gitEnvWithoutYesNoPrompt(), admissionExempt: true as const } + const execOptions = { + ...finalizationOptions, + ...(options.checkoutDeleteSignal ? { signal: options.checkoutDeleteSignal } : {}) + } const args = [...longPathArgs, 'worktree', 'remove'] if (force) { args.push('--force') @@ -102,7 +116,7 @@ async function performRemoveWorktree( } await gitExecFileAsync( [...longPathArgs, 'worktree', 'remove', '--force', '--', worktreePath], - execOptions + finalizationOptions ) } } else { @@ -143,12 +157,26 @@ export async function finishUnregisteredWorktreeRemoval( assertCheckoutGone: () => Promise, options: RemoveWorktreeOptions = {} ): Promise { - options = removalWorkerOptions(options) - await assertCheckoutGone() - await pruneGitWorktrees(gitExecOptions(repoPath, options)).catch((error: unknown) => - console.warn(`[git] worktree prune failed in ${repoPath}`, error) - ) - return deleteBranchOfUnregisteredWorktree(repoPath, worktreePath, branch, options) + const writers: Promise[] = [] + const observe = options.observeChildTermination + options = removalWorkerOptions({ + ...options, + observeChildTermination: (terminated, writes) => { + observe?.(terminated, writes) + if (writes) { + writers.push(terminated) + } + } + }) + try { + await assertCheckoutGone() + await pruneGitWorktrees(gitExecOptions(repoPath, options)).catch((error: unknown) => + console.warn(`[git] worktree prune failed in ${repoPath}`, error) + ) + return await deleteBranchOfUnregisteredWorktree(repoPath, worktreePath, branch, options) + } finally { + await Promise.all(writers) + } } /** Clean up an unregistered checkout's branch using normal preservation rules, without deleting files. */ @@ -158,7 +186,17 @@ export async function deleteBranchOfUnregisteredWorktree( branch: { name: string; head: string } | null, options: RemoveWorktreeOptions = {} ): Promise { - options = removalWorkerOptions(options) + const writers: Promise[] = [] + const observe = options.observeChildTermination + options = removalWorkerOptions({ + ...options, + observeChildTermination: (terminated, writes) => { + observe?.(terminated, writes) + if (writes) { + writers.push(terminated) + } + } + }) try { if (!branch?.name || !(await localBranchExists(repoPath, branch.name, options))) { return {} @@ -167,6 +205,7 @@ export async function deleteBranchOfUnregisteredWorktree( deleteBranchOfRemovedWorktree(repoPath, branch.name, branch.head, options) ) } finally { + await Promise.all(writers) invalidateSparseCheckoutState(repoPath, worktreePath) bumpWorktreeScanGeneration(repoPath) } diff --git a/src/main/ipc/worktrees-remove-preflight.test.ts b/src/main/ipc/worktrees-remove-preflight.test.ts index 07258baad9a..d07d86c703d 100644 --- a/src/main/ipc/worktrees-remove-preflight.test.ts +++ b/src/main/ipc/worktrees-remove-preflight.test.ts @@ -1,5 +1,20 @@ import type * as WorktreeScanCacheModule from '../git/worktrees/catalog/worktree-scan-cache' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + _resetWorktreeRemovalLifetimeForTests, + worktreeRemovalLifetime +} from '../workspaces/removal/worktree-removal-lifetime' +import { prepareWorktreeRemovalsForStore } from '../workspaces/removal/worktree-removal-records-load' +import { readWorktreeRemovalRecords } from '../workspaces/removal/worktree-removal-records' +import { + waitForWorktreeRemovalAcceptance, + _resetPendingWorktreeRemovalsForTests +} from '../workspaces/removal/worktree-background-removal' +import { removeRuntimeRegisteredLocalWorktree } from '../runtime/workspaces/removal/runtime-registered-local-worktree-removal' +import type { RuntimeStore } from '../runtime/construction/runtime-store-contract' import * as scopedRootsInvalidation from './worktrees/removal/registered-worktree-roots-scoped-invalidation' import * as authorizedRootsCache from './worktrees/removal/registered-worktree-roots-cache' import { @@ -12,7 +27,11 @@ import { killAllProcessesForWorktreeMock } from './worktrees-test-module-mocks' import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness' -import { mockKnownFeatureWorktree } from './worktrees-test-fixtures' +import { mockKnownFeatureWorktree, mockSelectedWslProjectRuntime } from './worktrees-test-fixtures' + +afterEach(() => { + _resetWorktreeRemovalLifetimeForTests() +}) vi.mock('../git/worktrees/catalog/worktree-scan-cache', async (importOriginal) => { return { @@ -222,3 +241,233 @@ describe('registerWorktreeHandlers', () => { expect(removeWorktreeMock).not.toHaveBeenCalled() }) }) + +function deferred() { + let resolve!: () => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +it.each(['ipc', 'runtime'] as const)( + '%s refuses after a held preflight loses to committed shutdown, before stopping terminals', + async (entry) => { + const runtime = setupWorktreeHandlers() + const acquireWatcherRemoval = vi.fn(async (_path: string) => ({ + finish: async (_removed: boolean) => {} + })) + runtime.acquireFileWatcherRemoval = acquireWatcherRemoval + const rows = mockKnownFeatureWorktree() + const entered = deferred() + const preflight = deferred() + assertWorktreeCleanForRemovalMock.mockImplementation(async () => { + entered.resolve() + await preflight.promise + }) + const invoke = () => + entry === 'ipc' + ? Promise.resolve( + handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' }) + ) + : removeRuntimeRegisteredLocalWorktree({ + repo: { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '', + addedAt: 0 + }, + target: { + id: 'repo-1::/workspace/feature-wt', + repoId: 'repo-1', + path: '/workspace/feature-wt' + }, + registeredWorktree: rows[1], + removedPushTarget: undefined, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this path refuses before consuming store mutations. + store: store as unknown as RuntimeStore, + localOptions: {}, + hasLocalOptions: false, + force: false, + runHooks: false, + allowFailedArchiveHook: false, + allowUnverifiedPtyStop: false, + deleteBranch: true, + acquireWatcherRemoval, + stopPtys: async () => { + await killAllProcessesForWorktreeMock() + }, + closeWatchers: async () => {}, + preserveBranchHead: (result) => result ?? {}, + finishRemoval: () => {}, + onRemoved: () => {}, + publish: () => {} + }) + const request = invoke() + const refused = expect(request).rejects.toThrow('Orca is stopping') + await entered.promise + await worktreeRemovalLifetime.close() + preflight.resolve() + await refused + expect(runtime.acquireFileWatcherRemoval).not.toHaveBeenCalled() + expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled() + expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled() + expect(removeWorktreeMock).not.toHaveBeenCalled() + } +) + +it.each(['ipc', 'runtime'] as const)( + '%s finishes actual admission when teardown owned preparation before shutdown', + async (entry) => { + const runtime = setupWorktreeHandlers() + const acquireWatcherRemoval = vi.fn(async (_path: string) => ({ + finish: async (_removed: boolean) => {} + })) + runtime.acquireFileWatcherRemoval = acquireWatcherRemoval + const rows = mockKnownFeatureWorktree() + const directory = await mkdtemp(join(tmpdir(), 'orca-removal-boundary-')) + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => [{ id: 'repo-1' }] + }) + const entered = deferred() + const stopped = deferred() + killAllProcessesForWorktreeMock.mockImplementation(async () => { + entered.resolve() + await stopped.promise + return { runtimeStopped: 1, providerStopped: 0, registryStopped: 0 } + }) + try { + const request = + entry === 'ipc' + ? Promise.resolve( + handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' }) + ) + : removeRuntimeRegisteredLocalWorktree({ + repo: { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '', + addedAt: 0 + }, + target: { + id: 'repo-1::/workspace/feature-wt', + repoId: 'repo-1', + path: '/workspace/feature-wt' + }, + registeredWorktree: rows[1], + removedPushTarget: undefined, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: cleanup is aborted before store mutations; admission uses the real journal. + store: store as unknown as RuntimeStore, + localOptions: {}, + hasLocalOptions: false, + force: false, + runHooks: false, + allowFailedArchiveHook: false, + allowUnverifiedPtyStop: false, + deleteBranch: true, + acquireWatcherRemoval, + stopPtys: async () => { + await killAllProcessesForWorktreeMock() + }, + closeWatchers: async () => {}, + preserveBranchHead: (result) => result ?? {}, + finishRemoval: () => {}, + onRemoved: () => {}, + publish: () => {} + }) + await entered.promise + const closing = worktreeRemovalLifetime.close() + stopped.resolve() + if (entry === 'ipc') { + void request.catch(() => {}) + await waitForWorktreeRemovalAcceptance('repo-1::/workspace/feature-wt') + } else { + await expect(request).resolves.toMatchObject({ removing: true }) + } + await closing + expect(removeWorktreeMock).not.toHaveBeenCalled() + expect(await readWorktreeRemovalRecords(directory)).toEqual([ + expect.objectContaining({ + worktreeId: 'repo-1::/workspace/feature-wt', + deleteBranch: true, + force: false + }) + ]) + } finally { + _resetPendingWorktreeRemovalsForTests() + await rm(directory, { recursive: true, force: true }) + } + } +) + +it.each(['ipc', 'runtime'] as const)( + '%s synchronous WSL removal retains main shutdown behavior without a journal lease', + async (entry) => { + const runtime = setupWorktreeHandlers() + mockSelectedWslProjectRuntime() + const rows = mockKnownFeatureWorktree() + const finish = vi.fn(async () => {}) + const gate = { finish: vi.fn(async () => {}) } + const acquireWatcherRemoval = vi.fn(async (_path: string) => gate) + runtime.acquireFileWatcherRemoval = acquireWatcherRemoval + const signal = worktreeRemovalLifetime.signal + removeWorktreeMock.mockImplementation(async (_repo, _path, _force, options) => { + expect(options).not.toHaveProperty('checkoutDeleteSignal', expect.anything()) + expect(options).not.toHaveProperty('signal', expect.anything()) + await worktreeRemovalLifetime.close() + expect(signal.aborted).toBe(true) + return {} + }) + const result = + entry === 'ipc' + ? Promise.resolve( + handlers['worktrees:remove'](null, { + worktreeId: 'repo-1::/workspace/feature-wt', + deleteBranch: false + }) + ) + : removeRuntimeRegisteredLocalWorktree({ + repo: { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '', + addedAt: 0 + }, + target: { + id: 'repo-1::/workspace/feature-wt', + repoId: 'repo-1', + path: '/workspace/feature-wt' + }, + registeredWorktree: rows[1], + removedPushTarget: undefined, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the existing store fixture owns all metadata consumed by this real removal boundary. + store: store as unknown as RuntimeStore, + localOptions: { wslDistro: 'Ubuntu' }, + hasLocalOptions: true, + force: false, + runHooks: false, + allowFailedArchiveHook: false, + allowUnverifiedPtyStop: false, + deleteBranch: false, + acquireWatcherRemoval, + stopPtys: async () => {}, + closeWatchers: async () => {}, + preserveBranchHead: (value) => value ?? {}, + finishRemoval: finish, + onRemoved: () => {}, + publish: () => {} + }) + await expect(result).resolves.toMatchObject({}) + if (entry === 'runtime') { + expect(finish).toHaveBeenCalledOnce() + } else { + expect(store.removeWorktreeMeta).toHaveBeenCalled() + } + expect(gate.finish).toHaveBeenCalledWith(true) + expect(removeWorktreeMock).toHaveBeenCalledOnce() + } +) diff --git a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts index a142f28f03c..da20793a6d6 100644 --- a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts +++ b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts @@ -1,3 +1,4 @@ +import { worktreeRemovalLifetime } from '../../../workspaces/removal/worktree-removal-lifetime' import { assertRegisteredLocalWorktreePreflight } from './registered-local-worktree-preflight' import type { Repo } from '../../../../shared/repo-types' import { @@ -37,11 +38,7 @@ import { stopPtysForDestructiveWorktreeRemoval } from './worktree-removal-ownership' import { preservedBranchCleanupScopeKey } from '../../../../shared/git/branches/preserved-branch-cleanup' -import { - removesInBackground, - startBackgroundWorktreeRemoval, - waitForWorktreeRemovalAcceptance -} from '../../../workspaces/removal/worktree-background-removal' +import * as backgroundRemoval from '../../../workspaces/removal/worktree-background-removal' import { worktreeHostIdentity } from '../../../persistence/worktree-meta-host-identity' export async function removeRegisteredLocalWorktree( @@ -56,7 +53,6 @@ export async function removeRegisteredLocalWorktree( hasLocalWorktreeGitOptions: boolean, deleteBranch: boolean ): Promise { - const { runtime } = context const refreshedWorktrees = hasLocalWorktreeGitOptions ? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions) : await listGitWorktreesStrict(repo.path) @@ -101,86 +97,92 @@ export async function removeRegisteredLocalWorktree( markerMissing ) - const removalGate = await withWorktreeRemoveStageSpan('watcher_gate', 'local', async () => - runtime.acquireFileWatcherRemoval(canonicalWorktreePath) + const background = backgroundRemoval.removesInBackground( + canonicalWorktreePath, + localWorktreeGitOptions ) - let accepted = false - try { - // Fence watcher/spawn installs through linked-path cleanup, Git removal and recursive fallback; PTYs must release handles first. - await withWorktreeRemoveStageSpan('pty_sweep', 'local', async () => { - await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId, { - allowUnverifiedStop: args.allowUnverifiedPtyStop - }) - }) - - if (!markerMissing && linkedPaths.length > 0) { - await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) - } - accepted = true - } finally { - if (!accepted) { - await removalGate.finish(false) - } - } - - const cleanupPushTarget = () => - cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - context.store, - localWorktreeGitOptions + const prepare = async (): Promise => { + const removalGate = await withWorktreeRemoveStageSpan('watcher_gate', 'local', async () => + context.runtime.acquireFileWatcherRemoval(canonicalWorktreePath) ) - // Read before the purge drops the row; listeners hear `removed` after it. - const removedIdentity = worktreeHostIdentity(context.store, args.worktreeId, removalHostId) - const finish = (checkoutDeleteSignal?: AbortSignal): Promise => - finishLocalWorktreeRemoval({ - context, - args, - repo, - repoId, - canonicalWorktreePath, - removalHostId, - removedPushTarget, - localWorktreeGitOptions, - hasLocalWorktreeGitOptions, - deleteBranch, - refreshedRegisteredWorktree, - removalGate, - checkoutDeleteSignal, - cleanupPushTarget - }) - if (!removesInBackground(canonicalWorktreePath, localWorktreeGitOptions)) { - const result = await finish() - runtime.publishWorktreeRemovalChange(repoId) - return result - } - // After acceptance, deletion must survive request cancellation; other views observe the host's removing marker. - void startBackgroundWorktreeRemoval({ - removal: { - worktreeId: args.worktreeId, - repoId, - repoPath: repo.path, - worktree: refreshedRegisteredWorktree, - deleteBranch, - force: args.force ?? false - }, - run: async (stopSignal) => { - const result = await finish(stopSignal) - context.options?.onWorktreeLifecycle?.({ - kind: 'removed', - worktreeId: args.worktreeId, - path: canonicalWorktreePath, - ...removedIdentity + let accepted = false + try { + // Fence watcher/spawn installs through linked-path cleanup, Git removal and recursive fallback; PTYs must release handles first. + await withWorktreeRemoveStageSpan('pty_sweep', 'local', async () => { + await stopPtysForDestructiveWorktreeRemoval(context.runtime, args.worktreeId, { + allowUnverifiedStop: args.allowUnverifiedPtyStop + }) }) + + if (!markerMissing && linkedPaths.length > 0) { + await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) + } + accepted = true + } finally { + if (!accepted) { + await removalGate.finish(false) + } + } + + const cleanupPushTarget = () => + cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + context.store, + localWorktreeGitOptions + ) + // Read before the purge drops the row; listeners hear `removed` after it. + const removedIdentity = worktreeHostIdentity(context.store, args.worktreeId, removalHostId) + const finish = (checkoutDeleteSignal?: AbortSignal): Promise => + finishLocalWorktreeRemoval({ + context, + args, + repo, + repoId, + canonicalWorktreePath, + removalHostId, + removedPushTarget, + localWorktreeGitOptions, + deleteBranch, + refreshedRegisteredWorktree, + removalGate, + checkoutDeleteSignal, + cleanupPushTarget + }) + if (!background) { + const result = await finish() + context.runtime.publishWorktreeRemovalChange(repoId) return result - }, - publish: () => runtime.publishWorktreeRemovalChange(repoId), - admissionFailed: () => removalGate.finish(false), - cleanupPushTargetRemote: cleanupPushTarget - }) - await waitForWorktreeRemovalAcceptance(args.worktreeId) - return { removing: true } + } + // After acceptance, deletion must survive request cancellation; other views observe the host's removing marker. + void backgroundRemoval.startBackgroundWorktreeRemoval({ + removal: { + worktreeId: args.worktreeId, + repoId, + repoPath: repo.path, + worktree: refreshedRegisteredWorktree, + deleteBranch, + force: args.force ?? false + }, + run: async (stopSignal) => { + const result = await finish(stopSignal) + context.options?.onWorktreeLifecycle?.({ + kind: 'removed', + worktreeId: args.worktreeId, + path: canonicalWorktreePath, + ...removedIdentity + }) + return result + }, + publish: () => context.runtime.publishWorktreeRemovalChange(repoId), + admissionFailed: () => removalGate.finish(false), + cleanupPushTargetRemote: cleanupPushTarget + }) + await backgroundRemoval.waitForWorktreeRemovalAcceptance(args.worktreeId) + return { removing: true } + } + return background ? worktreeRemovalLifetime.withPreparation(prepare) : prepare() } async function finishLocalWorktreeRemoval({ @@ -192,7 +194,6 @@ async function finishLocalWorktreeRemoval({ removalHostId, removedPushTarget, localWorktreeGitOptions, - hasLocalWorktreeGitOptions, deleteBranch, refreshedRegisteredWorktree, removalGate, @@ -207,14 +208,12 @@ async function finishLocalWorktreeRemoval({ removalHostId: ExecutionHostId removedPushTarget: GitPushTarget | undefined localWorktreeGitOptions: LocalProjectWorktreeGitOptions - hasLocalWorktreeGitOptions: boolean deleteBranch: boolean refreshedRegisteredWorktree: GitWorktreeInfo removalGate: { finish: (removed: boolean) => Promise } checkoutDeleteSignal?: AbortSignal cleanupPushTarget: () => Promise }): Promise { - const { store, runtime } = context let removalResult: RemoveWorktreeResult | undefined let removalCompleted = false try { @@ -223,7 +222,7 @@ async function finishLocalWorktreeRemoval({ ...(!deleteBranch ? { deleteBranch } : {}), // Reuse the authoritative listing instead of rescanning siblings during deletion. knownRemovedWorktree: refreshedRegisteredWorktree, - ...(hasLocalWorktreeGitOptions ? localWorktreeGitOptions : {}), + ...localWorktreeGitOptions, checkoutDeleteSignal } removalResult = preserveBranchHeadFallback( @@ -245,7 +244,7 @@ async function finishLocalWorktreeRemoval({ localWorktreeGitOptions, registeredWorktree: refreshedRegisteredWorktree, deleteBranch, - closeWatcher: (worktreePath) => runtime.closeFileWatchersForRemoval(worktreePath), + closeWatcher: (worktreePath) => context.runtime.closeFileWatchersForRemoval(worktreePath), signal: checkoutDeleteSignal }) if (recoveredRemovalResult) { @@ -259,14 +258,15 @@ async function finishLocalWorktreeRemoval({ repo.path, canonicalWorktreePath, localWorktreeGitOptions, - (path) => runtime.closeFileWatchersForRemoval(path) + (path) => context.runtime.closeFileWatchersForRemoval(path), + checkoutDeleteSignal ) // Failed removal may retain registration and branch locks; prune stale tracking. await pruneGitWorktrees({ cwd: repo.path, ...localWorktreeGitOptions }).catch(() => {}) await cleanupPushTarget() - runtime.clearOptimisticReconcileToken(args.worktreeId) + context.runtime.clearOptimisticReconcileToken(args.worktreeId) removeWorktreeMetadataAndTransientState( - { store, runtime }, + context, args.worktreeId, removalHostId, args.snapshotPruneBatchId @@ -276,7 +276,7 @@ async function finishLocalWorktreeRemoval({ hostId: removalHostId }) preservedBranchCleanupByScope.delete(cleanupScopeKey) - invalidateAuthorizedRootsCacheForRepo(store, repoId) + invalidateAuthorizedRootsCacheForRepo(context.store, repoId) removalCompleted = true return {} } else { @@ -299,17 +299,17 @@ async function finishLocalWorktreeRemoval({ refreshedRegisteredWorktree.head, removedPushTarget ) - runtime.clearOptimisticReconcileToken(args.worktreeId) + context.runtime.clearOptimisticReconcileToken(args.worktreeId) await withWorktreeRemoveStageSpan('metadata_purge', 'local', async () => { removeWorktreeMetadataAndTransientState( - { store, runtime }, + context, args.worktreeId, removalHostId, args.snapshotPruneBatchId ) }) await withWorktreeRemoveStageSpan('cache_invalidation', 'local', async () => { - invalidateAuthorizedRootsCacheForRepo(store, repoId) + invalidateAuthorizedRootsCacheForRepo(context.store, repoId) }) return removalResult ?? {} } diff --git a/src/main/orcad/host-lifecycle/orcad-lifecycle-host.test.ts b/src/main/orcad/host-lifecycle/orcad-lifecycle-host.test.ts index b28c26c5697..636743f5324 100644 --- a/src/main/orcad/host-lifecycle/orcad-lifecycle-host.test.ts +++ b/src/main/orcad/host-lifecycle/orcad-lifecycle-host.test.ts @@ -1,3 +1,14 @@ +import { OrcadProfileShutdown } from '../host-runtime/orcad-profile-shutdown' +import { Store } from '../../persistence/loading-store/store' +import { + closeTestStores, + createSqliteTestStore, + readPersistedStateJson +} from '../../persistence/persistence-test-harness' +import { readFile } from 'node:fs/promises' +import { holdRemovalMetadata } from '../../workspaces/removal/worktree-removal-lifetime.test-fixtures' +import { readWorktreeRemovalRecords } from '../../workspaces/removal/worktree-removal-records' +import { _resetPendingWorktreeRemovalsForTests } from '../../workspaces/removal/worktree-background-removal' import { existsSync, mkdtempSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -8,10 +19,15 @@ vi.mock('../browser-providers/orcad-browser-startup', () => ({ })) import { ORCAD_LOCK_FILE_NAME } from '../instance-lock/orcad-instance-lock' +import { worktreeRemovalLifetime } from '../../workspaces/removal/worktree-removal-lifetime' import { startOrcadWithHost } from './orcad-lifecycle' const roots: string[] = [] -afterEach(() => { +afterEach(async () => { + await closeTestStores() + vi.restoreAllMocks() + vi.useRealTimers() + _resetPendingWorktreeRemovalsForTests() for (const root of roots.splice(0)) { rmSync(root, { recursive: true, force: true }) } @@ -41,3 +57,51 @@ describe('startOrcadWithHost', () => { expect(existsSync(join(root, ORCAD_LOCK_FILE_NAME))).toBe(true) }) }) + +it('joins successful removal metadata and journal before profile ownership is released', async () => { + vi.useFakeTimers() + const root = dataRoot() + const profile = new OrcadProfileShutdown() + const dataFile = join(root, 'orca-data.json') + const store = createSqliteTestStore(Store, { dataFile }) + profile.store = store + await profile.store.flushPendingOrThrowAsync({ fullCheckpoint: true }) + store.getWorkspaceSession().activeTabId = 'before-stop' + const savedTab = () => JSON.parse(readPersistedStateJson(dataFile)).workspaceSession.activeTabId + let pending!: Awaited> + let transportStopped!: () => void + const transport = new Promise((resolve) => { + transportStopped = resolve + }) + let checkpointing: Promise | undefined + const handle = await startOrcadWithHost( + root, + async (registerCleanup) => { + registerCleanup(profile.finalize) + registerCleanup(() => worktreeRemovalLifetime.close()) + pending = await holdRemovalMetadata(root, () => { + store.getWorkspaceSession().activeTabId = 'after-delete' + }) + registerCleanup(async () => { + transportStopped() + }) + return {} + }, + () => {}, + () => (checkpointing = profile.checkpoint()) + ) + const stopping = handle.stop() + await transport + expect(() => worktreeRemovalLifetime.acquirePreparation()).toThrow('Orca is stopping') + expect(existsSync(join(root, ORCAD_LOCK_FILE_NAME))).toBe(true) + expect(await readWorktreeRemovalRecords(root)).toHaveLength(1) + await checkpointing + expect(savedTab()).toBe('before-stop') + pending.release() + await stopping + await pending.result + expect(await readFile(join(root, 'metadata-finished'), 'utf8')).toBe('removed') + expect(await readWorktreeRemovalRecords(root)).toEqual([]) + expect(savedTab()).toBe('after-delete') + expect(existsSync(join(root, ORCAD_LOCK_FILE_NAME))).toBe(false) +}) diff --git a/src/main/orcad/host-lifecycle/orcad-lifecycle.ts b/src/main/orcad/host-lifecycle/orcad-lifecycle.ts index 884cab84148..b39a81dfdda 100644 --- a/src/main/orcad/host-lifecycle/orcad-lifecycle.ts +++ b/src/main/orcad/host-lifecycle/orcad-lifecycle.ts @@ -1,3 +1,4 @@ +import { worktreeRemovalLifetime } from '../../workspaces/removal/worktree-removal-lifetime' import { setRuntimeBrowserCommandsFactory } from '../../runtime/browser/installation/runtime-browser-commands-factory' import { markHostShuttingDown } from '../../startup/quit/host-shutdown-state' import { setDesktopBrowserBridgeHostEnabled } from '../../runtime/desktop-browser-bridge/desktop-browser-bridge-registry' @@ -76,6 +77,7 @@ export function installOrcadShutdownSignals( } stopping = true markHostShuttingDown() + void worktreeRemovalLifetime.close().catch(() => {}) holdOrcadTerminalAdmissionUntilExit() exitCode = stoppedExitCode retractClean = onFailed @@ -117,7 +119,8 @@ export async function stopOrcadStructuredAgentSessions(): Promise { export async function startOrcadWithLifecycle( start: (registerRuntimeCleanup: (cleanup: BackendServiceCleanup) => void) => Promise, - cleanupHost: (runtimeCleanupSucceeded: boolean) => Promise + cleanupHost: (runtimeCleanupSucceeded: boolean) => Promise, + checkpoint: () => Promise = async () => {} ): Promise }> { const runtime = new BackendServiceLifetime() const cleanup = createIdempotentOrcadCleanup(async () => { @@ -125,11 +128,23 @@ export async function startOrcadWithLifecycle( recordAgentSessionRuntimeEnd('quit') // Startup failures reach cleanup without shutdown admission while RPC may already be serving. markHostShuttingDown() + void worktreeRemovalLifetime.close().catch(() => {}) + let checkpointError: unknown + const checkpointing = Promise.resolve() + .then(checkpoint) + .catch((error: unknown) => { + checkpointError = error + }) let runtimeCleanupSucceeded = false try { await runtime.stop() + await checkpointing + if (checkpointError) { + throw checkpointError + } runtimeCleanupSucceeded = true } finally { + await checkpointing await cleanupHost(runtimeCleanupSucceeded) } }) @@ -151,7 +166,8 @@ export async function startOrcadWithLifecycle( export async function startOrcadWithHost( userDataPath: string, start: (registerCleanup: (cleanup: BackendServiceCleanup) => void) => Promise, - runQuitHandlers: () => void | Promise + runQuitHandlers: () => void | Promise, + checkpoint: () => Promise = async () => {} ): Promise; instance: OrcadManagedStopInstance }> { const instanceLock = acquireOrcadInstanceLock(userDataPath, { // Yield the data root to the process that took its lock; supervisors stand by on this code. @@ -201,7 +217,8 @@ export async function startOrcadWithHost( host.add(() => setDesktopBrowserBridgeHostEnabled(false)) host.add(() => browserProvider?.stop()) await host.stop() - } + }, + checkpoint ) } diff --git a/src/main/orcad/host-lifecycle/orcad-lock-yield.test.ts b/src/main/orcad/host-lifecycle/orcad-lock-yield.test.ts index ac51d380859..a3354a130a3 100644 --- a/src/main/orcad/host-lifecycle/orcad-lock-yield.test.ts +++ b/src/main/orcad/host-lifecycle/orcad-lock-yield.test.ts @@ -16,12 +16,14 @@ import { installOrcadShutdownSignals, startOrcadWithHost } from './orcad-lifecyc import { ORCAD_LOCK_HEARTBEAT_MS } from '../instance-lock/orcad-lock-clone-machine' import { orcadRunFailed } from './orcad-unexpected-exit' import { SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE } from '../../../shared/single-instance-exit-code' +import { _resetPendingWorktreeRemovalsForTests } from '../../workspaces/removal/worktree-background-removal' const roots: string[] = [] afterEach(() => { // Drops the shutdown deadline timer so it cannot call the real process.exit later. vi.useRealTimers() vi.restoreAllMocks() + _resetPendingWorktreeRemovalsForTests() for (const root of roots.splice(0)) { rmSync(root, { recursive: true, force: true }) } diff --git a/src/main/orcad/host-runtime/orcad-entry.ts b/src/main/orcad/host-runtime/orcad-entry.ts index 008d8d4ba70..c1830f19a53 100644 --- a/src/main/orcad/host-runtime/orcad-entry.ts +++ b/src/main/orcad/host-runtime/orcad-entry.ts @@ -1,3 +1,6 @@ +import { OrcadProfileShutdown } from './orcad-profile-shutdown' +import { prepareWorktreeRemovalsForStore } from '../../workspaces/removal/worktree-removal-records-load' +import { worktreeRemovalLifetime } from '../../workspaces/removal/worktree-removal-lifetime' // Compose the Node-host runtime; paired desktop pages work even without a local browser engine. import process from 'node:process' import { getAppEnvironment } from '../../../shared/app-environment' @@ -7,11 +10,9 @@ import { installOrcadHostAdapters, runOrcadQuitHandlers } from './orcad-host-ada import { prepareOrcadSecretStoreAndMigrate } from '../credentials/orcad-secret-store-startup' import { describeOrcadBindExposure, resolveOrcadBindHost } from '../../../shared/orcad-bind-address' import { - flushOrcadProfileStoreForShutdown, startOrcadWithHost, stopOrcadStructuredAgentSessions } from '../host-lifecycle/orcad-lifecycle' -import { resolveOrcadWebClientRoot } from '../serve-startup/orcad-command-arguments' import type { BackendServiceCleanup } from '../../backend/service-composition' import { bindOrcadStopTerminalAdmission } from '../host-lifecycle/managed-stop/orcad-stop-terminal-admission' import type { OrcadHandle, OrcadOptions } from '../serve-startup/orcad-start-contract' @@ -23,7 +24,6 @@ import { } from '../../../shared/ai-vault/search/ai-vault-search-settings' import { stampRuntimeSourceEnv } from '../../../shared/runtime-source-env' import { adoptWslHostLoginEnvironmentLogged } from './orcad-wsl-host-login-environment' -import { orcadWsPortOptions } from '../serve-startup/orcad-ws-port-policy' import { createOrcadStartupTiming } from '../diagnostics/orcad-startup-timing' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../shared/layout-owner-switch' import { restoreOrcadTerminalAuthority } from './orcad-restored-terminal-authority' @@ -42,13 +42,14 @@ export async function startOrcad(options: OrcadOptions = {}): Promise activateBackendServices({ supervised: process.env[ORCA_SERVE_SUPERVISED_ENV] === '1', registerCleanup, - activate: (cleanup) => activateOrcadRuntime(options, cleanup) + activate: (cleanup) => activateOrcadRuntime(options, cleanup, profileShutdown) }), async () => { try { @@ -59,7 +60,8 @@ export async function startOrcad(options: OrcadOptions = {}): Promise Promise.resolve() await closing } - } + }, + profileShutdown.checkpoint ) const version = getAppEnvironment().getVersion() return { readiness, managedStop: { version, runtimeId: readiness.runtimeId, instance }, stop } @@ -67,7 +69,8 @@ export async function startOrcad(options: OrcadOptions = {}): Promise void + registerCleanup: (cleanup: BackendServiceCleanup) => void, + profileShutdown: OrcadProfileShutdown ): Promise> { const { time: timed, report: reportStartupTiming } = createOrcadStartupTiming() // Overlap login-shell probing with imports and store startup. @@ -83,7 +86,7 @@ async function activateOrcadRuntime( // in-flight spawn settles; a failed one leaves only the disconnect to do. registerCleanup(() => daemonStartup.catch(() => {}).then(() => stopOrcadDaemon())) const { OrcaRuntimeService } = await import('../../runtime/orca-runtime') - const { OrcaRuntimeRpcServer } = await import('../../runtime/rpc/server/runtime-rpc') + const { createOrcadRuntimeRpcServer } = await import('./orcad-runtime-rpc') const { registerHeadlessPtyRuntime } = await import('../../ipc/pty/register-headless-runtime') const { getLocalPtyProvider } = await import('../../ipc/pty/provider/registry') const { installOrcadObservability } = await import('../diagnostics/orcad-observability') @@ -111,8 +114,6 @@ async function activateOrcadRuntime( const { disposeWatcherProcessAndWait } = await import('../../ipc/filesystem/watcher-process/parcel-watcher-process') - let paneGridsForShutdown: { flushPaneGrids(): Promise | null } | null = null - let profileStoreForShutdown: Parameters[0] | undefined let uninstallHookStatusRepublish = (): void => {} let uninstallObservedStatusIdentity = (): void => {} // Cleanups run in reverse: RPC, then recovery and watchers, then the final flush; the daemon @@ -120,12 +121,7 @@ async function activateOrcadRuntime( registerCleanup(() => agentHookServer.stop()) registerCleanup(() => uninstallHookStatusRepublish()) registerCleanup(() => uninstallObservedStatusIdentity()) - registerCleanup(async () => { - // A SQLite-backed orcad has no JSON mirror to absorb a debounced write after SIGTERM. - if (profileStoreForShutdown) { - await flushOrcadProfileStoreForShutdown(profileStoreForShutdown, paneGridsForShutdown) - } - }) + registerCleanup(profileShutdown.finalize) registerCleanup(stopOrcadStructuredAgentSessions) registerCleanup(() => disposeWatcherProcessAndWait()) const { DesktopPushService } = await import('../../runtime/push/desktop-push-service') @@ -145,9 +141,11 @@ async function activateOrcadRuntime( const { store: profileStore, authority: profileStateAuthority } = await timed('profile', () => createOrcadProfileStateStartup(runtimeUserDataPath) ) + profileShutdown.store = profileStore + registerCleanup(() => worktreeRemovalLifetime.close()) + await timed('worktree-removals', () => prepareWorktreeRemovalsForStore(profileStore)) const observedPaneIdentities = new AgentStatusObservedPaneIdentities() const observedStatusCapture = new AgentStatusObservedPaneIdentityCapture(observedPaneIdentities) - profileStoreForShutdown = profileStore uninstallObservedStatusIdentity = agentHookServer.subscribeEnrichedStatus((enriched) => observedStatusCapture.observe(enriched) ) @@ -224,7 +222,7 @@ async function activateOrcadRuntime( } } }) - paneGridsForShutdown = runtime + profileShutdown.paneGrids = runtime accountLaunch?.attachToRuntime(runtime) const { wireGitHubIssueLinkTitleFill } = @@ -309,25 +307,13 @@ async function activateOrcadRuntime( const bindHost = resolveOrcadBindHost(options.bind) bindOrcadStopTerminalAdmission(runtime.terminalUpdateAdmission) - const rpc = new OrcaRuntimeRpcServer({ - runtime, - userDataPath: runtimeUserDataPath, - enableWebSocket: true, - // Pin operator exposure across restarts; pairing must not silently widen a loopback bind. - pinnedBindHost: bindHost, - serveNetworkSources: { - allowPublic: options.allowPublic === true, - refusePublic: options.refusePublic === true, - pairingAddress: options.pairingAddress ?? null - }, - webClientRoot: resolveOrcadWebClientRoot(options.webClientRoot), - ...orcadWsPortOptions(options.port) - }) + const rpc = createOrcadRuntimeRpcServer(runtime, runtimeUserDataPath, options, bindHost) pairedEnvironments.bindRpc(rpc) runtime.setAgentSenderPeerKey(() => rpc.getE2EEPublicKey()) // Stop RPC before cleanup can tear down writers and runtime state. registerCleanup(() => rpc.stop()) await timed('rpc', () => rpc.start()) + worktreeRemovalLifetime.resume(() => runtime.finishInterruptedWorktreeRemovals()) // The daemon has no window to wait for; its CI-line watches start with the runtime. runtime.startPullRequestWatches() startOrcadSchedulers(runtime, profileStore, registerCleanup) diff --git a/src/main/orcad/host-runtime/orcad-profile-shutdown.ts b/src/main/orcad/host-runtime/orcad-profile-shutdown.ts new file mode 100644 index 00000000000..5e6797f03c5 --- /dev/null +++ b/src/main/orcad/host-runtime/orcad-profile-shutdown.ts @@ -0,0 +1,24 @@ +import type { Store } from '../../persistence/loading-store/store' +import { beginProfileShutdownCheckpoint } from '../../persistence/profile-shutdown-checkpoint' +import { flushOrcadProfileStoreForShutdown } from '../host-lifecycle/orcad-lifecycle' + +/** Capture acquired profile resources even when activation fails before runtime construction. */ +export class OrcadProfileShutdown { + store?: Pick + paneGrids?: { flushPaneGrids(): Promise | null } + + private checkpointing?: Promise + + readonly checkpoint = (): Promise => + (this.checkpointing ??= beginProfileShutdownCheckpoint( + this.store, + this.paneGrids?.flushPaneGrids() + )) + + readonly finalize = async (): Promise => { + await this.checkpointing?.catch(() => {}) + if (this.store) { + await flushOrcadProfileStoreForShutdown(this.store, this.paneGrids) + } + } +} diff --git a/src/main/orcad/host-runtime/orcad-runtime-rpc.ts b/src/main/orcad/host-runtime/orcad-runtime-rpc.ts new file mode 100644 index 00000000000..ed6d2611126 --- /dev/null +++ b/src/main/orcad/host-runtime/orcad-runtime-rpc.ts @@ -0,0 +1,27 @@ +import { OrcaRuntimeRpcServer } from '../../runtime/rpc/server/runtime-rpc' +import type { OrcaRuntimeService } from '../../runtime/orca-runtime' +import type { OrcadOptions } from '../serve-startup/orcad-start-contract' +import { resolveOrcadWebClientRoot } from '../serve-startup/orcad-command-arguments' +import { orcadWsPortOptions } from '../serve-startup/orcad-ws-port-policy' + +export function createOrcadRuntimeRpcServer( + runtime: OrcaRuntimeService, + userDataPath: string, + options: OrcadOptions, + bindHost: string +): OrcaRuntimeRpcServer { + return new OrcaRuntimeRpcServer({ + runtime, + userDataPath, + enableWebSocket: true, + // Pin operator exposure across restarts; pairing must not silently widen a loopback bind. + pinnedBindHost: bindHost, + serveNetworkSources: { + allowPublic: options.allowPublic === true, + refusePublic: options.refusePublic === true, + pairingAddress: options.pairingAddress ?? null + }, + webClientRoot: resolveOrcadWebClientRoot(options.webClientRoot), + ...orcadWsPortOptions(options.port) + }) +} diff --git a/src/main/orcad/testing/serve-lifecycle/orcad-serve-parity.integration.test.ts b/src/main/orcad/testing/serve-lifecycle/orcad-serve-parity.integration.test.ts index 7b936dde3eb..173abc0b2cb 100644 --- a/src/main/orcad/testing/serve-lifecycle/orcad-serve-parity.integration.test.ts +++ b/src/main/orcad/testing/serve-lifecycle/orcad-serve-parity.integration.test.ts @@ -1,19 +1,33 @@ -import { existsSync, mkdirSync, mkdtempSync, realpathSync, writeFileSync } from 'node:fs' +import { randomUUID } from 'node:crypto' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + writeFileSync +} from 'node:fs' import { tmpdir, userInfo } from 'node:os' import { join, resolve } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { runProcess, spawnProcess } from '@orca/process-host' -import { ORCAD_NODE_RUNTIME_MARKER_FILENAME } from '../../../../shared/orcad-artifacts' +import { + ORCAD_CLI_ENTRY_FILENAME, + ORCAD_NODE_RUNTIME_MARKER_FILENAME +} from '../../../../shared/orcad-artifacts' import { findBundledWebClientRoot, WEB_CLIENT_INDEX_FILENAME } from '../../../../shared/bundled-web-client-root' import { orcadServeArgs } from '../../../../shared/orcad-serve-argv' +import { orcadManagedStopRequestPath } from '../../host-lifecycle/managed-stop/orcad-managed-stop-request' +import { readWorktreeRemovalRecords } from '../../../workspaces/removal/worktree-removal-records' import { ORCAD_LOCK_FILE_NAME } from '../../instance-lock/orcad-instance-lock' import { resolveBundledOrcadRuntime } from '../../runtime-launch/orcad-bundled-runtime' import { skipForMissingInputs } from '../native-slots/orcad-node-slot-fixture' import { buildDaemonSessionClient } from '../daemon-processes/orcad-daemon-session-client-fixture' import type { ServeCleanupDiagnostics } from './orcad-serve-cleanup-diagnostics.test-fixture' +import { boundedText } from './orcad-serve-cleanup-redaction.test-fixture' import { killAndAwaitExit, killProfileDaemons, @@ -79,6 +93,7 @@ function serveEnv(userData: string): NodeJS.ProcessEnv { ), ORCA_BACKGROUND_LAUNCH: '1', ORCA_DISABLE_MACOS_LOGIN_SHELL: '1', + ORCA_VERSION: '0.0.0-serve-parity-test', ORCA_USER_DATA: userData } } @@ -90,6 +105,99 @@ function profile(): string { } describe.skipIf(skip)('orca serve on orcad', () => { + // Real server startup, CLI commands and graceful shutdown each cross process boundaries. + it('deletes a linked worktree through the real CLI with a durable host journal', async () => { + const userData = realpathSync.native(profile()) + const home = join(userData, 'isolated-home') + const repo = join(userData, 'repo') + const worktree = join(userData, 'feature') + for (const path of [home, repo]) { + mkdirSync(path, { recursive: true }) + } + const env = { + ...serveEnv(userData), + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: join(home, '.config'), + XDG_CACHE_HOME: join(home, '.cache'), + XDG_DATA_HOME: join(home, '.local', 'share'), + XDG_STATE_HOME: join(home, '.local', 'state'), + CLAUDE_CONFIG_DIR: join(home, '.claude'), + CODEX_HOME: join(home, '.codex'), + ORCA_E2E_USER_DATA_DIR: userData, + ORCA_E2E_HOME_DIR: home, + ORCA_USER_DATA_PATH: userData + } + const git = async (args: string[]) => { + const result = await runProcess({ program: 'git', args, cwd: repo, env, timeoutMs: 30_000 }) + expect(result.code, result.stderr).toBe(0) + return result.stdout + } + await git(['init', '-q']) + await git([ + '-c', + 'user.name=Removal Test', + '-c', + 'user.email=removal@example.invalid', + 'commit', + '--allow-empty', + '-qm', + 'seed' + ]) + await git(['worktree', 'add', '-q', worktree, '-b', 'feature']) + const cli = async (args: string[]) => { + reportServePhase(userData, 'beforeCli', { args }) + const result = await runProcess({ + program: runtime!, + args: [join(slotDir, ORCAD_CLI_ENTRY_FILENAME), ...args, '--json'], + cwd: repo, + env, + timeoutMs: 60_000 + }) + reportServePhase(userData, 'afterCli', { + args, + code: result.code, + failure: result.code === 0 ? undefined : boundedText(result.stdout + result.stderr) + }) + expect( + result.code, + JSON.stringify({ args, stdout: result.stdout, stderr: result.stderr }) + ).toBe(0) + const response = JSON.parse(result.stdout) + expect(response.ok).toBe(true) + return response.result + } + try { + await serveOnce(userData, ['--bind', '127.0.0.1', '--port', '0', '--json'], env) + await cli(['repo', 'add', '--path', repo]) + if (process.platform === 'win32') { + const catalog = await cli(['worktree', 'ps']) + reportServePhase(userData, 'registeredPaths', { + expectedWorktree: worktree, + gitWorktrees: await git(['worktree', 'list', '--porcelain']), + catalog: catalog.worktrees.map( + (row: { worktreeId: string; path: string; branch: string }) => ({ + id: row.worktreeId, + path: row.path, + branch: row.branch + }) + ) + }) + } + await cli(['worktree', 'rm', '--worktree', `path:${worktree}`, '--force', '--delete-branch']) + expect(existsSync(worktree)).toBe(false) + expect(await git(['worktree', 'list', '--porcelain'])).not.toContain( + `worktree ${worktree.replaceAll('\\', '/')}` + ) + expect(await git(['branch', '--list', 'feature'])).toBe('') + } finally { + await stopServeProfile(userData, null) + } + expect(await readWorktreeRemovalRecords(join(userData, 'profiles', 'local-default'))).toEqual( + [] + ) + }, 180_000) + it('refuses with exit 78 while the desktop app holds the profile', async () => { const userData = profile() writeFileSync( @@ -181,6 +289,33 @@ describe.skipIf(skip)('orca serve on orcad', () => { }) const running = new Map>() +const readinessByProfile = new Map() +const launchDiagnostics = new Map string }>() + +function reportServePhase( + userData: string, + phase: string, + details: Record = {} +): void { + if (process.platform !== 'win32' && process.env.ORCA_SERVE_CLEANUP_DIAGNOSTICS !== '1') { + return + } + const child = running.get(userData) + const launch = launchDiagnostics.get(userData) + console.error( + '[orcad-serve-stop]', + JSON.stringify({ + phase, + userData, + pid: child?.pid, + exitCode: child?.exitCode, + signalCode: child?.signalCode, + listenerVersionFromLaunchEnv: launch?.listenerVersion, + ...details, + stderrTail: boundedText(launch?.stderr() ?? '', 8_192) + }) + ) +} async function serveDaemonPid(userData: string): Promise { const { readiness, stderr } = await serveOnce(userData, [ @@ -198,23 +333,30 @@ async function serveDaemonPid(userData: string): Promise { } type ServeReadiness = { + runtimeId: string pairing?: { webClientUrl?: unknown } health?: { terminalDaemon?: { state?: string; pid: number } } } async function serveOnce( userData: string, - args: string[] + args: string[], + env = serveEnv(userData) ): Promise<{ readiness: ServeReadiness; stderr: string }> { const child = spawnProcess({ program: runtime!, args: [join(slotDir, 'orcad.js'), ...args], - env: serveEnv(userData), + env, timeoutMs: null }) running.set(userData, child) let stdout = '' let stderr = '' + launchDiagnostics.set(userData, { + listenerVersion: env.ORCA_VERSION ?? '0.0.0-orcad', + stderr: () => stderr + }) + reportServePhase(userData, 'spawned') child.stderr.on('data', (chunk: Buffer) => (stderr += chunk.toString('utf8'))) const line = await new Promise((resolveLine, reject) => { const timer = setTimeout(() => reject(new Error(`no readiness: ${stderr}`)), 120_000) @@ -228,20 +370,69 @@ async function serveOnce( }) }) const readiness: ServeReadiness = JSON.parse(line) + readinessByProfile.set(userData, readiness) + reportServePhase(userData, 'ready') return { readiness, stderr } } /** SIGTERM preserves daemon terminals on POSIX; Windows uses its stop-request file. */ async function stopServe(userData: string): Promise { const child = running.get(userData) - running.delete(userData) if (!child || child.exitCode !== null) { + running.delete(userData) + launchDiagnostics.delete(userData) return } - await recordProfileDaemons(userData) - const exited = new Promise((settle) => child.once('exit', settle)) - child.kill('SIGTERM') - await exited + let phase = 'beforeDaemonRecording' + reportServePhase(userData, phase) + const diagnosticTimer = setTimeout(() => reportServePhase(userData, `waiting:${phase}`), 10_000) + diagnosticTimer.unref() + try { + await recordProfileDaemons(userData) + phase = 'afterDaemonRecording' + reportServePhase(userData, phase) + const exited = new Promise((settle) => child.once('exit', settle)) + if (process.platform === 'win32') { + const lockPath = join(userData, ORCAD_LOCK_FILE_NAME) + const lock = JSON.parse(readFileSync(lockPath, 'utf8')) + const readiness = readinessByProfile.get(userData) + if (lock.pid !== child.pid || lock.startedAtMs === null || !readiness?.runtimeId) { + throw new Error('Cannot prove ownership of the serve process to stop') + } + const instance = { pid: lock.pid, startedAtMs: lock.startedAtMs, nonce: lock.nonce, lockPath } + const requestPath = orcadManagedStopRequestPath(instance) + phase = 'beforeStopPublication' + reportServePhase(userData, phase, { + lockVersion: lock.version, + startedAtMs: lock.startedAtMs, + requestPath + }) + writeFileSync( + requestPath, + JSON.stringify({ + schemaVersion: 1, + transactionId: randomUUID(), + version: lock.version, + runtimeId: readiness.runtimeId, + instance + }) + ) + } else { + child.kill('SIGTERM') + } + phase = 'awaitingExit' + reportServePhase(userData, phase) + await exited + reportServePhase(userData, 'exited') + running.delete(userData) + readinessByProfile.delete(userData) + launchDiagnostics.delete(userData) + } catch (error) { + reportServePhase(userData, `failed:${phase}`) + throw error + } finally { + clearTimeout(diagnosticTimer) + } } async function stopServeProfile(userData: string, shellPid: number | null): Promise { diff --git a/src/main/persistence/profile-shutdown-checkpoint.ts b/src/main/persistence/profile-shutdown-checkpoint.ts new file mode 100644 index 00000000000..3b43b0f6081 --- /dev/null +++ b/src/main/persistence/profile-shutdown-checkpoint.ts @@ -0,0 +1,15 @@ +import type { Store } from './loading-store/store' + +/** Save current state without closing the writer while removal still owns it. */ +export function beginProfileShutdownCheckpoint( + store: Pick | null | undefined, + paneGridFlush?: Promise | null +): Promise { + const checkpoint = Promise.resolve(paneGridFlush).then(async () => { + await store?.flushPendingOrThrowAsync({ drainToStableGeneration: false, fullCheckpoint: true }) + }) + void checkpoint.catch((error) => + console.error('[persistence] Failed to checkpoint before shutdown:', error) + ) + return checkpoint +} diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts index 5d303c13212..7f92380c70d 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts @@ -430,10 +430,12 @@ describe('OrcaRuntimeService', () => { expect(assertWorktreeCleanForRemoval).toHaveBeenCalledWith(registeredWorktree.path, false, { wslDistro: 'Ubuntu' }) - expect(removeWorktree).toHaveBeenCalledWith(repo.path, registeredWorktree.path, false, { - knownRemovedWorktree: registeredWorktree, - wslDistro: 'Ubuntu' - }) + expect(removeWorktree).toHaveBeenCalledWith( + repo.path, + registeredWorktree.path, + false, + expect.objectContaining({ knownRemovedWorktree: registeredWorktree, wslDistro: 'Ubuntu' }) + ) }) it('surfaces selected-runtime list failures during runtime worktree removal', async () => { diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts index dff2794d9b6..f3a0b35a716 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec.ts @@ -459,7 +459,6 @@ describe('OrcaRuntimeService', () => { 'Worktree is no longer registered with Git but its directory remains.' ) await expect(lstat(orphanPath)).resolves.toBeTruthy() - expect(removePathSpy).toHaveBeenCalledWith(orphanPath, {}) expect(removeWorktreeMeta).not.toHaveBeenCalled() expect(gitSpy).not.toHaveBeenCalledWith(['worktree', 'prune'], expect.anything()) } finally { diff --git a/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.test.ts b/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.test.ts index d684f5ddb61..39fb98dbfe9 100644 --- a/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.test.ts +++ b/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.test.ts @@ -105,6 +105,7 @@ type FinishOutcome = async function finishAfterRestart( options: { repoGone?: boolean + repoRows?: Repo[] branch?: string head?: string force?: boolean @@ -140,6 +141,11 @@ async function finishAfterRestart( const purged: string[] = [] const remember = vi.fn() const host = removalHost(options.repoGone, options.stopPtys) + if (options.repoRows) { + const rows = options.repoRows + host.store.getRepos = () => rows + host.store.getRepo = (id) => rows.find((row) => row.id === id) + } host.purge = ({ worktreeId }) => { purged.push(worktreeId) } @@ -250,3 +256,32 @@ describe('finishing an interrupted worktree removal after a restart', () => { }) installRealWorkerThreadChildProcesses() + +it.each(['remote-first', 'local-first', 'missing-local'])( + 'recovers only the local owner with a %s same-ID catalog', + async (order) => { + const remote: Repo = { + ...repo, + path: join(scratchDir, 'nonlocal-repo'), + connectionId: 'ssh-fixture' + } + const repoRows = + order === 'remote-first' + ? [remote, repo] + : order === 'local-first' + ? [repo, remote] + : [remote] + const { outcome, purged, records } = await finishAfterRestart({ repoRows }) + expect(outcome.status).toBe('removed') + expect(records).toEqual([]) + if (order === 'missing-local') { + expect(purged).toEqual([]) + expect(existsSync(worktreePath)).toBe(true) + expect(await isRegistered(worktreePath)).toBe(true) + } else { + expect(purged).toEqual([`repo-1::${worktreePath}`]) + expect(existsSync(worktreePath)).toBe(false) + expect(await isRegistered(worktreePath)).toBe(false) + } + } +) diff --git a/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.ts b/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.ts index f9178949ce2..03d5ef307c5 100644 --- a/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.ts +++ b/src/main/runtime/workspaces/removal/runtime-interrupted-local-worktree-removal.ts @@ -1,3 +1,7 @@ +import { + throwIfSignalAborted, + waitForPromiseWithSignal +} from '../../../../shared/abort-signal-reason' import type { RemoveWorktreeResult } from '../../../../shared/worktree/create-types' import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' import type { RuntimeStore } from '../../construction/runtime-store-contract' @@ -5,9 +9,12 @@ import { releaseStartupRemovalFence, type BackgroundWorktreeRemovalJob } from '../../../workspaces/removal/worktree-background-removal' -import { resolveWorktreeRemovalMetadata } from '../../../workspaces/removal/worktree-removal-repo-owner' +import { + resolveWorktreeRemovalMetadata, + resolveWorktreeRemovalRepoOwner +} from '../../../workspaces/removal/worktree-removal-repo-owner' import type { RuntimePreservedBranchCleanup } from '../../repositories/runtime-preserved-branch-cleanup' -import { listWorktreesStrict } from '../../../git/worktrees/catalog/worktree-listing' +import { listWorktreesSharedStrict as listWorktreesStrict } from '../../../git/worktrees/catalog/worktree-scan-cache' import { cleanupUnusedWorktreePushTargetRemote } from '../../../ipc/worktree-remote' import { authorizeWindowsCheckout, @@ -103,7 +110,7 @@ export function interruptedLocalWorktreeRemovalJob( }, publish: () => host.publish(record.repoId), cleanupPushTargetRemote: async () => { - const repo = host.store.getRepo(record.repoId) + const repo = localRemovalRepo(host.store, record.repoId) if (repo) { await cleanupUnusedWorktreePushTargetRemote( repo.path, @@ -133,7 +140,7 @@ async function finishInterruptedLocalWorktreeRemoval( args: InterruptedLocalWorktreeRemovalArgs ): Promise { const { record, store } = args - const repo = store.getRepo(record.repoId) + const repo = localRemovalRepo(store, record.repoId) if (!repo) { console.warn(`[worktrees] dropping removal of ${record.worktreePath}: its repo is gone`) return {} @@ -151,7 +158,11 @@ async function finishInterruptedLocalWorktreeRemoval( deleteBranch: record.deleteBranch, target: { id: record.worktreeId } } - const worktrees = await listWorktreesStrict(repo.path, localOptions) + const worktrees = await listWorktreesStrict(repo.path, { + ...localOptions, + signal: args.stopSignal + }) + throwIfSignalAborted(args.stopSignal) const registered = worktrees.some((worktree) => areWorktreePathsEqual(worktree.path, record.worktreePath) ) @@ -181,7 +192,15 @@ async function finishInterruptedLocalWorktreeRemoval( }, checkoutDeleteSignal: args.stopSignal } - if (!native && deletable && (await isCheckoutMissingGitLink(deletable.path, localOptions))) { + if ( + !native && + deletable && + (await waitForPromiseWithSignal( + isCheckoutMissingGitLink(deletable.path, localOptions), + args.stopSignal + )) + ) { + throwIfSignalAborted(args.stopSignal) // If Git deleted .git first, it cannot remove/validate the checkout; prune registration only and keep all files for the checks below. await removeStaleLocalWorktreeRegistration({ canonicalWorktreePath: deletable.path, @@ -205,14 +224,15 @@ async function finishInterruptedLocalWorktreeRemoval( true ) } + throwIfSignalAborted(args.stopSignal) const gate = await args.acquireWatcherRemoval(record.worktreePath) - if (args.stopPtys) { - try { - await args.stopPtys() - } catch (error) { - await gate.finish(false) - throw error - } + try { + throwIfSignalAborted(args.stopSignal) + await args.stopPtys?.() + throwIfSignalAborted(args.stopSignal) + } catch (error) { + await gate.finish(false) + throw error } if (deletable) { return finishRuntimeLocalWorktreeRemoval(finishArgs, deletable, gate, args.stopSignal) @@ -274,3 +294,8 @@ async function isCheckoutMissingGitLink( return getErrorCode(error) === 'ENOENT' } } + +function localRemovalRepo(store: RuntimeStore, repoId: string) { + const owner = resolveWorktreeRemovalRepoOwner(store, repoId, LOCAL_EXECUTION_HOST_ID) + return owner.kind === 'resolved' ? owner.repo : undefined +} diff --git a/src/main/runtime/workspaces/removal/runtime-registered-local-worktree-removal.ts b/src/main/runtime/workspaces/removal/runtime-registered-local-worktree-removal.ts index 8834b7664db..634a15c5c17 100644 --- a/src/main/runtime/workspaces/removal/runtime-registered-local-worktree-removal.ts +++ b/src/main/runtime/workspaces/removal/runtime-registered-local-worktree-removal.ts @@ -1,3 +1,4 @@ +import { worktreeRemovalLifetime } from '../../../workspaces/removal/worktree-removal-lifetime' import type { GitPushTarget, GitWorktreeInfo } from '../../../../shared/worktree/types' import { authorizeWindowsCheckout, @@ -18,7 +19,8 @@ import { getWorktreeSharedLinkPaths } from '../../../git/worktrees/content-paths import { runHook, getEffectiveHooks } from '../../../workspace-hooks/hooks' import { findExistingWorktreeSymlinkPaths } from '../../../git/worktrees/content-paths/worktree-symlink-detection' import { removeWorktreeLinkedPaths } from '../../../ipc/worktree-symlinks' -import { cleanupUnusedWorktreePushTargetRemote } from '../../../ipc/worktree-remote' +import { cleanupRemovedWorktreePushTarget } from './runtime-worktree-push-target-cleanup' +export { cleanupRemovedWorktreePushTarget } from './runtime-worktree-push-target-cleanup' import { runWorktreeChangeInvalidators } from '../../../ipc/worktrees/listing/worktree-change-invalidators' import { formatWorktreeRemovalError, @@ -29,6 +31,7 @@ import { cleanupLocalOrphanedWorktreeDirectory } from '../../../workspaces/remov import { recoverPartialLocalWorktreeRemoval } from '../../../workspaces/removal/local-worktree-removal-recovery' import { findRegisteredDeletableWorktree } from '../../../workspaces/removal/worktree-removal-safety' import { CLIENT_REMOVAL_HOME } from '../../../workspaces/removal/worktree-removal-home-guard' +import type { RuntimePreservedBranchCleanup } from '../../repositories/runtime-preserved-branch-cleanup' import type { RuntimeStore } from '../../construction/runtime-store-contract' import type { RuntimeWorktreeRemovalTarget } from '../catalog/runtime-worktree-selection' import { @@ -77,10 +80,7 @@ async function acceptRuntimeRegisteredLocalWorktreeRemoval(args: { /** Runs after every refusal and the teardown, before any checkout file is deleted. */ beforeCheckoutDelete?: (worktree: GitWorktreeInfo) => Promise closeWatchers: (path: string) => Promise - preserveBranchHead: ( - result: RemoveWorktreeResult | undefined, - fallbackHead: string | undefined - ) => RemoveWorktreeResult + preserveBranchHead: RuntimePreservedBranchCleanup['preserveHead'] finishRemoval: ( result: RemoveWorktreeResult | undefined, rememberBranch: boolean, @@ -176,50 +176,54 @@ async function acceptRuntimeRegisteredLocalWorktreeRemoval(args: { } } - const gate = await args.acquireWatcherRemoval(canonicalPath) - let accepted = false - try { - await args.stopPtys() - await args.beforeCheckoutDelete?.(refreshed) - if (!markerMissing && linkedPaths.length > 0) { - await removeWorktreeLinkedPaths(canonicalPath, linkedPaths) + const background = removesInBackground(canonicalPath, localOptions) + const prepare = async (): Promise => { + const gate = await args.acquireWatcherRemoval(canonicalPath) + let accepted = false + try { + await args.stopPtys() + await args.beforeCheckoutDelete?.(refreshed) + if (!markerMissing && linkedPaths.length > 0) { + await removeWorktreeLinkedPaths(canonicalPath, linkedPaths) + } + accepted = true + } finally { + if (!accepted) { + await gate.finish(false) + } } - accepted = true - } finally { - if (!accepted) { - await gate.finish(false) + const acceptedFields = { + ...(archiveHookOverride ? { archiveHookOverride } : {}), + ...(warning ? { warning } : {}) } + if (!background) { + const result = await finishRuntimeLocalWorktreeRemoval(args, refreshed, gate) + args.publish() + return { ...result, ...acceptedFields } + } + // After all refusals, detach slow Git deletion so request timeout cannot cancel it; clients follow the removing marker. + void startBackgroundWorktreeRemoval({ + removal: { + worktreeId: args.target.id, + repoId: repo.id, + repoPath: repo.path, + worktree: refreshed, + deleteBranch: args.deleteBranch, + force: args.force + }, + run: async (stopSignal) => { + const result = await finishRuntimeLocalWorktreeRemoval(args, refreshed, gate, stopSignal) + args.onRemoved() + return result + }, + publish: () => args.publish(), + admissionFailed: () => gate.finish(false), + cleanupPushTargetRemote: () => cleanupRemovedWorktreePushTarget(args) + }) + await waitForWorktreeRemovalAcceptance(args.target.id) + return { removing: true, ...acceptedFields } } - const acceptedFields = { - ...(archiveHookOverride ? { archiveHookOverride } : {}), - ...(warning ? { warning } : {}) - } - if (!removesInBackground(canonicalPath, localOptions)) { - const result = await finishRuntimeLocalWorktreeRemoval(args, refreshed, gate) - args.publish() - return { ...result, ...acceptedFields } - } - // After all refusals, detach slow Git deletion so request timeout cannot cancel it; clients follow the removing marker. - void startBackgroundWorktreeRemoval({ - removal: { - worktreeId: args.target.id, - repoId: repo.id, - repoPath: repo.path, - worktree: refreshed, - deleteBranch: args.deleteBranch, - force: args.force - }, - run: async (stopSignal) => { - const result = await finishRuntimeLocalWorktreeRemoval(args, refreshed, gate, stopSignal) - args.onRemoved() - return result - }, - publish: () => args.publish(), - admissionFailed: () => gate.finish(false), - cleanupPushTargetRemote: () => cleanupRemovedWorktreePushTarget(args) - }) - await waitForWorktreeRemovalAcceptance(args.target.id) - return { removing: true, ...acceptedFields } + return background ? worktreeRemovalLifetime.withPreparation(prepare) : prepare() } export type RuntimeLocalWorktreeRemovalFinishArgs = Pick< @@ -280,7 +284,8 @@ export async function finishRuntimeLocalWorktreeRemoval( repo.path, canonicalPath, localOptions, - args.closeWatchers + args.closeWatchers, + checkoutDeleteSignal ) await pruneGitWorktrees({ cwd: repo.path, ...localOptions }).catch(() => {}) await cleanupRemovedWorktreePushTarget(args) @@ -306,15 +311,3 @@ export async function finishRuntimeLocalWorktreeRemoval( await gate.finish(completed) } } - -export async function cleanupRemovedWorktreePushTarget( - args: RuntimeLocalWorktreeRemovalFinishArgs -): Promise { - await cleanupUnusedWorktreePushTargetRemote( - args.repo.path, - args.target.id, - args.removedPushTarget, - args.store, - args.localOptions - ) -} diff --git a/src/main/runtime/workspaces/removal/runtime-removal-recovery-scan-contract.test.ts b/src/main/runtime/workspaces/removal/runtime-removal-recovery-scan-contract.test.ts new file mode 100644 index 00000000000..f5e955e25dc --- /dev/null +++ b/src/main/runtime/workspaces/removal/runtime-removal-recovery-scan-contract.test.ts @@ -0,0 +1,285 @@ +import { GitAdmissionScheduler } from '../../../../shared/git/admission/git-admission-scheduler' +import { + acquireGitAdmission, + _resetGitAdmissionForTests +} from '../../../git/command-runner/admission/git-subprocess-admission' +import { worktreeRemovalLifetime } from '../../../workspaces/removal/worktree-removal-lifetime' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/repo-types' +import type { GitWorktreeInfo } from '../../../../shared/worktree/types' +import type { RuntimeStore } from '../../construction/runtime-store-contract' +import type { WorktreeRemovalRecord } from '../../../workspaces/removal/worktree-removal-records' + +const { scan, nativeGraph } = vi.hoisted(() => ({ scan: vi.fn(), nativeGraph: vi.fn() })) +vi.mock('../../../git/worktrees/catalog/worktree-list-reader', async (original) => ({ + ...(await original>()), + readTranslatedWorktreeGraph: nativeGraph +})) +vi.mock('../../../git/worktrees/catalog/worktree-listing', () => ({ + listWorktreesStrict: scan, + listWorktreeGraph: vi.fn(), + annotateSparseCheckoutStatus: vi.fn() +})) +vi.mock('../../../git/worktrees/removal/worktree-removal', () => ({ + finishUnregisteredWorktreeRemoval: async () => ({}), + deleteBranchOfUnregisteredWorktree: vi.fn(), + removeWorktree: vi.fn() +})) +vi.mock('../../../ipc/worktree-remote', () => ({ + cleanupUnusedWorktreePushTargetRemote: async () => {} +})) +vi.mock('./runtime-registered-local-worktree-removal', () => ({ + cleanupRemovedWorktreePushTarget: async () => {}, + finishRuntimeLocalWorktreeRemoval: vi.fn() +})) +vi.mock('../../../workspaces/execution-host/project-runtime-git-options', () => ({ + getLocalProjectWorktreeGitOptions: () => ({}) +})) +import { interruptedLocalWorktreeRemovalJob } from './runtime-interrupted-local-worktree-removal' +import { + _resetWorktreeScanCacheForTests, + bumpWorktreeScanGeneration +} from '../../../git/worktrees/catalog/worktree-scan-cache' +import { + _resetPendingWorktreeRemovalsForTests, + _settlePendingWorktreeRemovalsForTests, + resumeInterruptedWorktreeRemovals +} from '../../../workspaces/removal/worktree-background-removal' +import { prepareWorktreeRemovalsForStore } from '../../../workspaces/removal/worktree-removal-records-load' +import { + writeWorktreeRemovalRecords, + readWorktreeRemovalRecords +} from '../../../workspaces/removal/worktree-removal-records' + +const originalPlatform = process.platform +let directory = '' +afterEach(async () => { + _resetPendingWorktreeRemovalsForTests() + _resetWorktreeScanCacheForTests() + scan.mockReset() + nativeGraph.mockReset() + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + _resetGitAdmissionForTests() + if (directory) { + await rm(directory, { recursive: true, force: true }) + } + directory = '' +}) +function record(index: number, repoId: string): WorktreeRemovalRecord { + return { + worktreeId: `${repoId}::${index}`, + repoId, + repoPath: `/missing/${repoId}`, + worktreePath: `/missing/${repoId}/checkout-${index}`, + branch: 'feature', + head: 'abc', + force: false, + deleteBranch: false, + requestedAt: 1 + } +} +function host(repos: Repo[]) { + return { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: interrupted cleanup consumes only these repo and metadata reads; physical Git mutations are isolated below the scan seam. + store: { + getRepos: () => repos, + getRepo: (id: string) => repos.find((repo) => repo.id === id), + getWorktreeMeta: () => undefined + } as unknown as RuntimeStore, + acquireWatcherRemoval: vi.fn(async () => ({ finish: async () => {} })), + stopPtys: vi.fn(async () => {}), + closeWatchers: async () => {}, + preservedBranchCleanup: { preserveHead: () => ({}), remember: () => {} }, + purge: vi.fn(async (_record: WorktreeRemovalRecord) => {}), + onRemoved: vi.fn(), + publish: () => {} + } +} + +it.each([10, 100])( + 'shares the two owning repo inventories while completing %i saved removals', + async (population) => { + directory = await mkdtemp(join(tmpdir(), 'orca-recovery-scan-contract-')) + const repos = [0, 1].map((i) => ({ + id: `repo-${i}`, + path: `/missing/repo-${i}`, + displayName: 'repo', + badgeColor: '', + addedAt: 0 + })) + const records = Array.from({ length: population }, (_, index) => + record(index, `repo-${index % 2}`) + ) + await writeWorktreeRemovalRecords(directory, () => records) + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => repos + }) + let release!: () => void + const reading = new Promise((resolve) => { + release = resolve + }) + scan.mockImplementation(async () => { + await reading + return [] + }) + const runtime = host(repos) + resumeInterruptedWorktreeRemovals((saved) => interruptedLocalWorktreeRemovalJob(saved, runtime)) + // Each distinct repo needs one authoritative inventory; pending rows cannot require duplicate concurrent reads. + await vi.waitFor(() => expect(scan).toHaveBeenCalledTimes(2)) + release() + await _settlePendingWorktreeRemovalsForTests() + expect(runtime.purge.mock.calls.map(([saved]) => saved.worktreeId).sort()).toEqual( + records.map((saved) => saved.worktreeId).sort() + ) + expect(await readWorktreeRemovalRecords(directory)).toEqual([]) + } +) + +it('obtains a fresh inventory after a catalog generation changes and protects the replacement checkout', async () => { + const saved = record(0, 'repo-0') + const runtime = host([ + { id: 'repo-0', path: saved.repoPath, displayName: 'repo', badgeColor: '', addedAt: 0 } + ]) + let release!: () => void + const oldInventory = new Promise((resolve) => { + release = () => resolve([]) + }) + scan.mockReturnValueOnce(oldInventory).mockResolvedValueOnce([ + { + path: saved.worktreePath, + branch: 'replacement', + head: 'def', + isMainWorktree: false, + isBare: false + } + ]) + const signal = new AbortController().signal + const first = interruptedLocalWorktreeRemovalJob(saved, runtime).run(signal) + bumpWorktreeScanGeneration(saved.repoPath) + const replacement = interruptedLocalWorktreeRemovalJob(saved, runtime).run(signal) + await expect(replacement).rejects.toThrow('A different checkout is now at') + expect(scan).toHaveBeenCalledTimes(2) + expect(runtime.purge).not.toHaveBeenCalled() + release() + await first +}) + +it.each([10, 100])( + 'cancels queued recovery before spawning an inventory and preserves %i accepted records', + async (population) => { + directory = await mkdtemp(join(tmpdir(), 'orca-recovery-cancel-')) + _resetGitAdmissionForTests(new GitAdmissionScheduler({ generalCap: 1, generalHeadroom: 0 })) + const blocker = await acquireGitAdmission({ + args: ['status'], + cwd: '/held', + tier: 'background' + }) + const repos = [ + { id: 'repo-0', path: '/missing/repo-0', displayName: 'repo', badgeColor: '', addedAt: 0 } + ] + const records = Array.from({ length: population }, (_, index) => record(index, 'repo-0')) + const spawn = vi.fn() + scan.mockImplementation(async (cwd: string, options: { signal: AbortSignal }) => { + const grant = await acquireGitAdmission({ + args: ['worktree', 'list', '--porcelain'], + cwd, + signal: options.signal, + tier: 'background' + }) + try { + spawn() + return [] + } finally { + grant.release() + } + }) + try { + await writeWorktreeRemovalRecords(directory, () => records) + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => repos + }) + const runtime = host(repos) + resumeInterruptedWorktreeRemovals((saved) => + interruptedLocalWorktreeRemovalJob(saved, runtime) + ) + await vi.waitFor(() => expect(scan).toHaveBeenCalledOnce()) + await worktreeRemovalLifetime.close() + expect(spawn).not.toHaveBeenCalled() + expect(runtime.purge).not.toHaveBeenCalled() + expect( + (await readWorktreeRemovalRecords(directory)).map((saved) => saved.worktreeId).sort() + ).toEqual(records.map((saved) => saved.worktreeId).sort()) + } finally { + blocker.release() + } + } +) + +it.each([10, 100])( + 'cancels the shared native authorization after outer inventory for %i saved removals', + async (population) => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + directory = await mkdtemp(join(tmpdir(), 'orca-native-recovery-cancel-')) + _resetGitAdmissionForTests(new GitAdmissionScheduler({ generalCap: 1, generalHeadroom: 0 })) + const blocker = await acquireGitAdmission({ + args: ['status'], + cwd: '/held', + tier: 'background' + }) + const records = Array.from({ length: population }, (_, index) => record(index, 'repo-0')) + const repos = [ + { id: 'repo-0', path: '/missing/repo-0', displayName: 'repo', badgeColor: '', addedAt: 0 } + ] + scan.mockResolvedValue( + records.map((saved) => ({ + path: saved.worktreePath, + branch: 'refs/heads/feature', + head: 'abc', + isBare: false, + isMainWorktree: false + })) + ) + const spawned = vi.fn() + nativeGraph.mockImplementation(async (cwd: string, options: { signal: AbortSignal }) => { + const grant = await acquireGitAdmission({ + args: ['worktree', 'list', '--porcelain'], + cwd, + signal: options.signal, + tier: 'background' + }) + try { + spawned() + return [] + } finally { + grant.release() + } + }) + try { + await writeWorktreeRemovalRecords(directory, () => records) + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => repos + }) + const runtime = host(repos) + resumeInterruptedWorktreeRemovals((saved) => + interruptedLocalWorktreeRemovalJob(saved, runtime) + ) + await vi.waitFor(() => expect(nativeGraph).toHaveBeenCalledOnce()) + await worktreeRemovalLifetime.close() + expect(spawned).not.toHaveBeenCalled() + expect(runtime.acquireWatcherRemoval).not.toHaveBeenCalled() + expect(runtime.stopPtys).not.toHaveBeenCalled() + expect(runtime.purge).not.toHaveBeenCalled() + expect( + (await readWorktreeRemovalRecords(directory)).map((saved) => saved.worktreeId).sort() + ).toEqual(records.map((saved) => saved.worktreeId).sort()) + } finally { + blocker.release() + } + } +) diff --git a/src/main/runtime/workspaces/removal/runtime-worktree-push-target-cleanup.ts b/src/main/runtime/workspaces/removal/runtime-worktree-push-target-cleanup.ts new file mode 100644 index 00000000000..ba7164195cf --- /dev/null +++ b/src/main/runtime/workspaces/removal/runtime-worktree-push-target-cleanup.ts @@ -0,0 +1,9 @@ +import { cleanupUnusedWorktreePushTargetRemote } from '../../../ipc/worktree-remote' +import type { RuntimeLocalWorktreeRemovalFinishArgs } from './runtime-registered-local-worktree-removal' + +export async function cleanupRemovedWorktreePushTarget( + args: RuntimeLocalWorktreeRemovalFinishArgs +): Promise { + const { repo, target, removedPushTarget: pushTarget, store, localOptions: options } = args + await cleanupUnusedWorktreePushTargetRemote(repo.path, target.id, pushTarget, store, options) +} diff --git a/src/main/startup/main-process-ready-persistence-cleanup.test.ts b/src/main/startup/main-process-ready-persistence-cleanup.test.ts index 4151a587e4d..1639ff60b4b 100644 --- a/src/main/startup/main-process-ready-persistence-cleanup.test.ts +++ b/src/main/startup/main-process-ready-persistence-cleanup.test.ts @@ -1,9 +1,31 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { Store } from '../persistence/loading-store/store' +import { + closeTestStores, + createSqliteTestStore, + readPersistedStateJson +} from '../persistence/persistence-test-harness' +import { hostWorktreeTarget } from '../../shared/worktree/host-qualified-identity' +import { + worktreeRemovalLifetime, + _resetWorktreeRemovalLifetimeForTests +} from '../workspaces/removal/worktree-removal-lifetime' +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { holdRemovalMetadata } from '../workspaces/removal/worktree-removal-lifetime.test-fixtures' +import { readWorktreeRemovalRecords } from '../workspaces/removal/worktree-removal-records' +import { _resetPendingWorktreeRemovalsForTests } from '../workspaces/removal/worktree-background-removal' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ServeDesktopActivationGate } from './serve-launch/serve-desktop-activation' const { state, foundation, runtime, i18n, launch } = vi.hoisted(() => ({ state: { - store: { freezeWritesAsync: vi.fn(async () => {}) }, + profileShutdownCheckpoint: null as Promise | null, + store: { + flushPendingOrThrowAsync: vi.fn(async () => {}), + flushFinalOrThrowAsync: vi.fn(async () => {}), + freezeWritesAsync: vi.fn(async () => {}) + }, profileStateAdmission: initialAdmission(), desktopActivationGate: initialActivationGate(), mainProcessI18nReady: Promise.resolve() @@ -39,10 +61,22 @@ const options = { handleMacAppActivation: () => {} } +const stubStore = state.store + +afterEach(async () => { + await closeTestStores() + vi.restoreAllMocks() + vi.useRealTimers() + _resetPendingWorktreeRemovalsForTests() +}) + beforeEach(() => { + state.store = stubStore + _resetWorktreeRemovalLifetimeForTests() vi.clearAllMocks() state.profileStateAdmission = { release: vi.fn() } state.desktopActivationGate = null + state.profileShutdownCheckpoint = null }) describe('startup persistence lifetime', () => { @@ -82,3 +116,119 @@ describe('startup persistence lifetime', () => { } }) }) + +it('freezes the profile after a drained removal fails without releasing its ownership', async () => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + await expect( + worktreeRemovalLifetime.prepare( + async () => { + throw new Error('load failed') + }, + () => {}, + async () => {} + ) + ).rejects.toThrow('load failed') + const failure = new Error('runtime startup failed') + runtime.mockRejectedValueOnce(failure) + await expect(initializeMainProcessReady(options)).rejects.toBe(failure) + expect(state.store.flushFinalOrThrowAsync).toHaveBeenCalledOnce() + expect(state.store.freezeWritesAsync).toHaveBeenCalledOnce() + expect(state.profileStateAdmission?.release).not.toHaveBeenCalled() + } finally { + log.mockRestore() + } +}) + +it('joins successful removal metadata and its journal before failed startup freezes or releases the profile', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-startup-removal-')) + try { + const pending = await holdRemovalMetadata(directory) + const failure = new Error('runtime startup failed') + runtime.mockRejectedValueOnce(failure) + const rejected = expect(initializeMainProcessReady(options)).rejects.toBe(failure) + await vi.waitFor(() => expect(worktreeRemovalLifetime.closing).toBe(true)) + expect(state.store.freezeWritesAsync).not.toHaveBeenCalled() + expect(state.profileStateAdmission?.release).not.toHaveBeenCalled() + expect(await readWorktreeRemovalRecords(directory)).toHaveLength(1) + pending.release() + await rejected + await pending.result + expect(await readFile(join(directory, 'metadata-finished'), 'utf8')).toBe('removed') + expect(await readWorktreeRemovalRecords(directory)).toEqual([]) + expect(state.store.freezeWritesAsync).toHaveBeenCalledOnce() + expect(state.profileStateAdmission).toBeUndefined() + } finally { + vi.restoreAllMocks() + _resetPendingWorktreeRemovalsForTests() + await rm(directory, { recursive: true, force: true }) + } +}) + +it('persists successful removal metadata after the early startup checkpoint before releasing its profile', async () => { + vi.useFakeTimers() + const directory = await mkdtemp(join(tmpdir(), 'orca-startup-final-save-')) + const dataFile = join(directory, 'orca-data.json') + const profile = createSqliteTestStore(Store, { dataFile }) + const target = hostWorktreeTarget({ id: 'repo::feature', hostId: 'local' }) + profile.setWorktreeMeta(target, { displayName: 'Pending workspace' }) + await profile.flushPendingOrThrowAsync({ fullCheckpoint: true }) + const checkpoint = Promise.withResolvers() + state.store = { + flushPendingOrThrowAsync: vi.fn(async () => { + await profile.flushPendingOrThrowAsync({ fullCheckpoint: true }) + checkpoint.resolve() + }), + flushFinalOrThrowAsync: vi.fn(() => profile.flushFinalOrThrowAsync()), + freezeWritesAsync: vi.fn(() => profile.freezeWritesAsync()) + } + const admission = state.profileStateAdmission + const pending = await holdRemovalMetadata(directory, () => profile.removeWorktreeMeta(target)) + const failure = new Error('runtime startup failed') + runtime.mockRejectedValueOnce(failure) + const rejected = expect(initializeMainProcessReady(options)).rejects.toBe(failure) + try { + await checkpoint.promise + expect( + JSON.parse(readPersistedStateJson(dataFile)).worktreeMeta['repo::feature'] + ).toMatchObject({ displayName: 'Pending workspace' }) + expect(state.store.flushFinalOrThrowAsync).not.toHaveBeenCalled() + expect(admission?.release).not.toHaveBeenCalled() + pending.release() + await rejected + await pending.result + expect( + JSON.parse(readPersistedStateJson(dataFile)).worktreeMeta['repo::feature'] + ).toBeUndefined() + expect(await readWorktreeRemovalRecords(directory)).toEqual([]) + expect(admission?.release).toHaveBeenCalledOnce() + } finally { + pending.release() + await closeTestStores() + await rm(directory, { recursive: true, force: true }) + } +}) + +it.each(['final-save', 'early-checkpoint'] as const)( + 'still freezes after a %s failure and preserves the startup error', + async (phase) => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + const failure = new Error('runtime startup failed') + runtime.mockRejectedValueOnce(failure) + if (phase === 'final-save') { + state.store.flushFinalOrThrowAsync.mockRejectedValueOnce(new Error('save failed')) + } else { + state.store.flushPendingOrThrowAsync.mockRejectedValueOnce(new Error('early save failed')) + } + const admission = state.profileStateAdmission + await expect(initializeMainProcessReady(options)).rejects.toBe(failure) + expect(state.store.flushFinalOrThrowAsync).toHaveBeenCalledOnce() + expect(state.store.freezeWritesAsync).toHaveBeenCalledOnce() + if (phase === 'final-save') { + expect(admission?.release).not.toHaveBeenCalled() + } else { + expect(admission?.release).toHaveBeenCalledOnce() + } + log.mockRestore() + } +) diff --git a/src/main/startup/main-process-ready-runtime.ts b/src/main/startup/main-process-ready-runtime.ts index 591411a8cb3..ea1ae881e56 100644 --- a/src/main/startup/main-process-ready-runtime.ts +++ b/src/main/startup/main-process-ready-runtime.ts @@ -37,7 +37,8 @@ import { collectWorktreeTrashSweepRoots, sweepStaleWorktreeTrash } from '../workspaces/removal/worktree-trash' -import { loadWorktreeRemovalRecordsForStore } from './workspace-startup/worktree-removal-records-load' +import { prepareWorktreeRemovalsForStore } from '../workspaces/removal/worktree-removal-records-load' +import { worktreeRemovalLifetime } from '../workspaces/removal/worktree-removal-lifetime' import { runAfterFirstWindowShown } from './main-window/first-window-deferral' import { startSpeechModelWarmup } from '../speech/speech-model-warmup' import { logStartupMilestone } from './diagnostics/startup-diagnostics' @@ -62,7 +63,7 @@ export async function initializeReadyRuntimeServices(): Promise { // Renderers fork from zygotes that exist already, so this must land before the first window. await refuseRealtimeSchedulingUnderZeroRtTime() // Load interrupted deletion records before listings so first paint shows Deleting. - await loadWorktreeRemovalRecordsForStore(store) + await prepareWorktreeRemovalsForStore(store) initializeMainProcessObservers() initializeMainProcessAccountServices() const runtime = initializeMainProcessRuntime() @@ -110,7 +111,7 @@ export async function initializeReadyRuntimeServices(): Promise { ).catch((error) => { console.warn('[worktrees] Failed to sweep leftover worktree directories:', error) }) - runtime.finishInterruptedWorktreeRemovals() + worktreeRemovalLifetime.resume(() => runtime.finishInterruptedWorktreeRemovals()) }, WORKTREE_TRASH_SWEEP_FALLBACK_MS) // Defer noncritical plugin-file refresh past first paint. runAfterFirstWindowShown(() => { diff --git a/src/main/startup/main-process-ready.ts b/src/main/startup/main-process-ready.ts index 74c5043cd86..52dd0c172cd 100644 --- a/src/main/startup/main-process-ready.ts +++ b/src/main/startup/main-process-ready.ts @@ -1,3 +1,5 @@ +import { beginProfileShutdownCheckpoint } from '../persistence/profile-shutdown-checkpoint' +import { worktreeRemovalLifetime } from '../workspaces/removal/worktree-removal-lifetime' import { initializeMainProcessI18nAndMenu } from './main-window/main-process-i18n-menu' import { mainProcessState as state } from './main-process-state' import { initializeReadyFoundation } from './main-process-ready-foundation' @@ -39,9 +41,33 @@ export async function initializeMainProcessReady( // Startup now exits after failure; reopening would use a closing profile writer. state.desktopActivationGate = null try { - await state.store?.freezeWritesAsync() - state.profileStateAdmission?.release() - state.profileStateAdmission = undefined + state.profileShutdownCheckpoint ??= beginProfileShutdownCheckpoint( + state.store, + state.runtime?.flushPaneGrids() + ) + const removalStopping = worktreeRemovalLifetime.close().then( + () => true, + (closeError) => { + console.error( + '[worktrees] Failed to stop workspace deletion after startup failure:', + closeError + ) + return false + } + ) + const [removalStopped] = await Promise.all([ + removalStopping, + state.profileShutdownCheckpoint.catch(() => {}) + ]) + try { + await state.store?.flushFinalOrThrowAsync() + } finally { + await state.store?.freezeWritesAsync() + } + if (removalStopped) { + state.profileStateAdmission?.release() + state.profileStateAdmission = undefined + } } catch (closeError) { console.error( '[persistence] Failed to close profile persistence after startup failure:', diff --git a/src/main/startup/main-process-state.ts b/src/main/startup/main-process-state.ts index 864688b8ace..1050302384d 100644 --- a/src/main/startup/main-process-state.ts +++ b/src/main/startup/main-process-state.ts @@ -86,6 +86,7 @@ export const mainProcessState = { /** User quit bypasses running-process confirmation during close. */ isQuitting: false, store: null as Store | null, + profileShutdownCheckpoint: null as Promise | null, profileStateStartup: createInitialProfileStateStartup(), profileStateAdmission: createInitialProfileStateAdmission(), stats: null as StatsCollector | null, diff --git a/src/main/startup/quit/main-process-quit-pending-writer.test.ts b/src/main/startup/quit/main-process-quit-pending-writer.test.ts index 4a565b9e709..1e69dbc82f8 100644 --- a/src/main/startup/quit/main-process-quit-pending-writer.test.ts +++ b/src/main/startup/quit/main-process-quit-pending-writer.test.ts @@ -1,5 +1,13 @@ import { EventEmitter } from 'node:events' +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { afterEach, expect, it, vi } from 'vitest' +import { + closeTestStores, + createSqliteTestStore, + readPersistedStateJson +} from '../../persistence/persistence-test-harness' import { WILL_QUIT_TEARDOWN_DEADLINE_MS } from '../../../shared/quit-teardown-deadline' const settled = () => vi.fn(async () => {}) @@ -8,17 +16,19 @@ const stopAgentHookServer = idle() const stopStructuredAgentSessionRuntime = settled() const beginSshShutdown = settled() const killAllPty = idle() +const disconnectDaemon = settled() +const closeAllWatchers = settled() // Every teardown member settles except the profile store, so only the writer can hold quit. const dependencies: Record> = { - '../../ipc/filesystem/watchers/filesystem-watcher-shutdown': { closeAllWatchers: settled() }, + '../../ipc/filesystem/watchers/filesystem-watcher-shutdown': { closeAllWatchers }, '../../ipc/worktrees/creation/worktree-base-directory-watcher': { disposeWorktreeBaseDirectoryWatchers: settled() }, '../../ipc/folder-repo-git-upgrade': { stopFolderRepoGitUpgradeWatch: idle() }, '../../ipc/pty/kill-all': { killAllPty }, '../../daemon/providers/lifecycle/daemon-provider-state': { - disconnectDaemon: settled(), + disconnectDaemon, shutdownDaemon: settled() }, '../../ipc/ssh/ssh-shutdown-drain': { beginSshShutdown }, @@ -63,9 +73,6 @@ const dependencies: Record> = { '../../git/refs/maintenance/local-repo-ref-maintenance': { awaitPackedRefsLockRelease: settled() }, - '../../workspaces/removal/worktree-background-removal': { - stopBackgroundWorktreeRemovals: idle() - }, '../../dock/unread-badge': { setUnreadDockBadgeCount: idle() }, '../../tray/system-tray': { destroySystemTray: idle() }, '../../telemetry/client': { shutdownTelemetry: settled() }, @@ -94,8 +101,11 @@ afterEach(() => { async function startQuit( flushFinalOrThrowAsync: () => Promise, - services: Record = {} + services: Record = {}, + beforeQuit?: () => Promise ) { + disconnectDaemon.mockClear() + closeAllWatchers.mockClear() beginSshShutdown.mockClear() killAllPty.mockClear() vi.resetModules() @@ -103,6 +113,7 @@ async function startQuit( const app = Object.assign(new EventEmitter(), { quit: vi.fn() }) const admission = { release: vi.fn() } const store = { + flushPendingOrThrowAsync: settled(), flushFinalOrThrowAsync: vi.fn(flushFinalOrThrowAsync), freezeWritesAsync: settled() } @@ -124,6 +135,7 @@ async function startQuit( const { quitTeardownStartGate } = await import('./quit-teardown-start-gate') quitTeardownStartGate.resetForTests() installMainProcessQuitHandlers() + await beforeQuit?.() app.emit('before-quit', { defaultPrevented: false }) app.emit('will-quit', { preventDefault: vi.fn() }) return { app, admission, store, state } @@ -143,3 +155,143 @@ it('quits at the teardown deadline without releasing admission while the writer expect(admission.release).not.toHaveBeenCalled() expect(state.profileStateAdmission).toBe(admission) }) + +it('keeps profile, watcher and daemon ownership until successful removal metadata and journal settle', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-quit-removal-')) + let release!: () => void + let readRecords!: () => Promise + let reset!: () => void + const { store, admission } = await startQuit( + async () => {}, + { watcherShutdownDone: false }, + async () => { + const { worktreeRemovalLifetime } = + await import('../../workspaces/removal/worktree-removal-lifetime') + const { holdRemovalMetadata } = + await import('../../workspaces/removal/worktree-removal-lifetime.test-fixtures') + release = (await holdRemovalMetadata(directory)).release + const records = await import('../../workspaces/removal/worktree-removal-records') + readRecords = () => records.readWorktreeRemovalRecords(directory) + reset = (await import('../../workspaces/removal/worktree-background-removal')) + ._resetPendingWorktreeRemovalsForTests + expect(worktreeRemovalLifetime.closing).toBe(false) + } + ) + await vi.advanceTimersByTimeAsync(0) + expect(store.flushFinalOrThrowAsync).not.toHaveBeenCalled() + expect(admission.release).not.toHaveBeenCalled() + expect(disconnectDaemon).not.toHaveBeenCalled() + expect(closeAllWatchers).not.toHaveBeenCalled() + expect(await readRecords()).toHaveLength(1) + release() + await vi.waitFor(() => expect(admission.release).toHaveBeenCalledOnce()) + expect(await readFile(join(directory, 'metadata-finished'), 'utf8')).toBe('removed') + expect(await readRecords()).toEqual([]) + reset() + await rm(directory, { recursive: true, force: true }) + expect(store.freezeWritesAsync).toHaveBeenCalledOnce() + expect(admission.release).toHaveBeenCalledOnce() + expect(disconnectDaemon).toHaveBeenCalledOnce() + expect(closeAllWatchers).toHaveBeenCalledOnce() +}) + +it('finishes saving after a quiescent removal reports failure, retaining the failed ownership fence', async () => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { store, admission } = await startQuit( + async () => {}, + { watcherShutdownDone: false }, + async () => { + const { worktreeRemovalLifetime } = + await import('../../workspaces/removal/worktree-removal-lifetime') + await expect( + worktreeRemovalLifetime.prepare( + async () => { + throw new Error('load failed') + }, + () => {}, + async () => {} + ) + ).rejects.toThrow('load failed') + } + ) + await vi.advanceTimersByTimeAsync(0) + expect(store.flushFinalOrThrowAsync).toHaveBeenCalledOnce() + expect(store.freezeWritesAsync).toHaveBeenCalledOnce() + expect(admission.release).not.toHaveBeenCalled() + expect(disconnectDaemon).toHaveBeenCalledOnce() + expect(closeAllWatchers).toHaveBeenCalledOnce() + expect(log).toHaveBeenCalledWith( + '[worktrees] Failed to finish stopping workspace deletion:', + expect.any(Error) + ) +}) + +it('checkpoints pane state during a held synchronous deletion, then saves later metadata before releasing ownership', async () => { + const directory = await mkdtemp(join(tmpdir(), 'orca-quit-checkpoint-')) + const { Store } = await import('../../persistence/loading-store/store') + const dataFile = join(directory, 'orca-data.json') + const profile = createSqliteTestStore(Store, { dataFile }) + const savedTab = () => JSON.parse(readPersistedStateJson(dataFile)).workspaceSession.activeTabId + await profile.flushPendingOrThrowAsync({ fullCheckpoint: true }) + const checkpointSaved = Promise.withResolvers() + const savePending = profile.flushPendingOrThrowAsync.bind(profile) + vi.spyOn(profile, 'flushPendingOrThrowAsync').mockImplementation(async (...args) => { + try { + await savePending(...args) + checkpointSaved.resolve() + } catch (error) { + checkpointSaved.reject(error) + throw error + } + }) + const final = vi.spyOn(profile, 'flushFinalOrThrowAsync') + const freeze = vi.spyOn(profile, 'freezeWritesAsync') + let release!: () => void + let captureGrid!: () => void + const grid = new Promise((resolve) => { + captureGrid = () => { + profile.getWorkspaceSession().activeTabId = 'pane-grid-before-stop' + resolve() + } + }) + const runtime = { + flushPaneGrids: () => grid, + getOffscreenBrowserBackend: () => null, + getAgentBrowserBridge: () => null, + getEmulatorBridge: () => null, + disposeSkillUploadSessions: async () => {}, + getRuntimeId: () => undefined, + closeNotificationDismissals: async () => {} + } + try { + const { admission, app } = await startQuit( + async () => {}, + { store: profile, runtime, watcherShutdownDone: false }, + async () => { + const { worktreeRemovalLifetime } = + await import('../../workspaces/removal/worktree-removal-lifetime') + release = worktreeRemovalLifetime.acquirePreparation() + } + ) + captureGrid() + await grid + await checkpointSaved.promise + expect(savedTab()).toBe('pane-grid-before-stop') + await vi.advanceTimersByTimeAsync(WILL_QUIT_TEARDOWN_DEADLINE_MS) + expect(app.quit).toHaveBeenCalledOnce() + expect(final).not.toHaveBeenCalled() + expect(freeze).not.toHaveBeenCalled() + expect(admission.release).not.toHaveBeenCalled() + expect(closeAllWatchers).not.toHaveBeenCalled() + expect(disconnectDaemon).not.toHaveBeenCalled() + profile.getWorkspaceSession().activeTabId = 'metadata-after-delete' + release() + await vi.waitFor(() => expect(admission.release).toHaveBeenCalledOnce()) + expect(savedTab()).toBe('metadata-after-delete') + expect(freeze).toHaveBeenCalledOnce() + } finally { + release?.() + await closeTestStores() + await rm(directory, { recursive: true, force: true }) + } +}) diff --git a/src/main/startup/quit/main-process-quit.ts b/src/main/startup/quit/main-process-quit.ts index e080011e4bc..68ea61e20a9 100644 --- a/src/main/startup/quit/main-process-quit.ts +++ b/src/main/startup/quit/main-process-quit.ts @@ -1,3 +1,4 @@ +import { beginProfileShutdownCheckpoint } from '../../persistence/profile-shutdown-checkpoint' import { killWslChannelsInCloseGrace } from '../../wsl/execution/wsl-command-channel' import { markHostShuttingDown } from './host-shutdown-state' import { app, type Event } from 'electron' @@ -23,7 +24,7 @@ import { browserManager } from '../../browser/guest-lifecycle/manager/browser-ma import { stopCodexStateDbBackfillRecoveries } from '../../codex/history/index/codex-state-db-backfill-recovery' import { stopCodexAccountSessionBridges } from '../../codex/history/bridges/codex-account-session-bridge' import { awaitPackedRefsLockRelease } from '../../git/refs/maintenance/local-repo-ref-maintenance' -import { stopBackgroundWorktreeRemovals } from '../../workspaces/removal/worktree-background-removal' +import { worktreeRemovalLifetime } from '../../workspaces/removal/worktree-removal-lifetime' import { settleTeardownWithinDeadline, settleWithinMs } from './quit-teardown-deadline' import { quitTeardownStartGate } from './quit-teardown-start-gate' import { setUnreadDockBadgeCount } from '../../dock/unread-badge' @@ -34,7 +35,7 @@ import { isQuittingForUpdate } from '../../updater/updater-service' import { recordUpdaterLifecycle } from '../../updater/updater-lifecycle-diagnostics' import { stopTccPromptNotice } from '../../macos-permissions/macos-tcc-prompt-notice' import { cancelHistoryGc } from '../../pty/history/terminal-history-gc' -import { shouldQuitWhenAllWindowsClosed } from './window-all-closed-quit-policy' +import { installWindowAllClosedHandler } from './window-all-closed-quit-handler' import { desktopConnectorCloud } from '../cloud-startup/main-process-connector-startup' import { mainProcessState as state } from '../main-process-state' import { isDevParentShutdownRequested } from '../configure-process' @@ -117,6 +118,15 @@ function installWillQuitHandler(): void { const paneGridFlush = state.runtime?.flushPaneGrids() ?? null // A renderer can veto before-quit; push must survive until quit is committed. markHostShuttingDown() + state.profileShutdownCheckpoint ??= beginProfileShutdownCheckpoint(state.store, paneGridFlush) + const removalShutdown = worktreeRemovalLifetime.close() + const removalQuiescence = removalShutdown.then( + () => true, + (error) => { + console.error('[worktrees] Failed to finish stopping workspace deletion:', error) + return false + } + ) state.desktopPushService?.stop() state.unsubscribeSystemResumeBroadcast?.() state.unsubscribeSystemResumeBroadcast = null @@ -169,8 +179,6 @@ function installWillQuitHandler(): void { REF_MAINTENANCE_QUIT_DEADLINE_MS ).then(() => {}) state.uninstallRepoMaintenanceIdleGate = null - // Deletes may take minutes; stop now and let durable records resume them next launch. - stopBackgroundWorktreeRemovals() // Chat teardown reads background work afterward; clearing status loses subagent/command-only resume offers. agentHookServer.stop({ keepStatus: true }) // Windows bare hook paths lack POSIX ORCA_PANE_KEY guards, so remove Grok hooks on quit; bound cleanup because GROK_HOME can be a stalled network mount. @@ -214,7 +222,7 @@ function installWillQuitHandler(): void { const emulatorShutdown = state.runtime?.getEmulatorBridge()?.destroyAllSessions() ?? Promise.resolve() const finalStore = state.store - const beginSshShutdownAndFinalSave = () => { + const beginSshShutdownAndFinalSave = (removalStopped: boolean) => { // Mark SSH leases detached synchronously immediately before final flush; no await may separate mutation from save. const sshShutdown = beginSshShutdown() killAllPty() @@ -226,18 +234,22 @@ function installWillQuitHandler(): void { try { await finalStore.flushFinalOrThrowAsync() await finalStore.freezeWritesAsync() - state.profileStateAdmission?.release() - state.profileStateAdmission = undefined + if (removalStopped) { + state.profileStateAdmission?.release() + state.profileStateAdmission = undefined + } } catch (error) { console.error('[persistence] Failed to finalize profile state:', error) } })() return { sshShutdown, watcherShutdown, storeFlush } } - // Flush pending pane grids first (bounded at 1s); without one, begin final save synchronously. - const finalSaves = paneGridFlush - ? paneGridFlush.then(beginSshShutdownAndFinalSave) - : beginSshShutdownAndFinalSave() + // Removal writers and pending pane grids must settle before the profile freezes. + const finalSaves = Promise.all([ + paneGridFlush, + removalQuiescence, + state.profileShutdownCheckpoint.catch(() => {}) + ]).then(([, stopped]) => beginSshShutdownAndFinalSave(stopped)) const sshShutdown = Promise.resolve(finalSaves).then((saves) => saves.sshShutdown) const storeFlush = Promise.resolve(finalSaves).then((saves) => saves.storeFlush) const watcherShutdown = Promise.resolve(finalSaves).then((saves) => saves.watcherShutdown) @@ -269,9 +281,13 @@ function installWillQuitHandler(): void { rpcStopAndClear ]).then(() => state.runtime?.closeNotificationDismissals()) // Normal quits preserve the detached daemon for warm reattach; dev-parent death leaves its temporary profile ownerless. - const daemonTeardown = isDevParentShutdownRequested() ? shutdownDaemon() : disconnectDaemon() + const daemonTeardown = removalQuiescence.then(() => + isDevParentShutdownRequested() ? shutdownDaemon() : disconnectDaemon() + ) // Bound unsettled transports and final writes so quit cannot hang (#9447). Failures still exit; atomic swaps preserve old files if the final debounce is lost. settleTeardownWithinDeadline([ + { name: 'worktree-removals', promise: removalShutdown }, + { name: 'state-checkpoint', promise: state.profileShutdownCheckpoint }, { name: 'daemon', promise: daemonTeardown }, { name: 'browser', promise: browserShutdown }, { name: 'runtime-rpc', promise: rpcStopAndClear }, @@ -308,22 +324,6 @@ function installWillQuitHandler(): void { }) } -function installWindowAllClosedHandler(): void { - app.on('window-all-closed', () => { - // Keep serve RPC alive after disposable windows close; re-trigger macOS quit after renderer buffer-capture deferral. - if ( - shouldQuitWhenAllWindowsClosed({ - platform: process.platform, - isQuitting: state.isQuitting, - isServeMode: state.isServeMode, - hasOpenedMainWindow: state.hasOpenedMainWindow - }) - ) { - app.quit() - } - }) -} - export function installMainProcessQuitHandlers(): void { // app.exit skips Electron quit events; stop the log child on forced exits too. process.once('exit', stopTccPromptNotice) diff --git a/src/main/startup/quit/window-all-closed-quit-handler.ts b/src/main/startup/quit/window-all-closed-quit-handler.ts new file mode 100644 index 00000000000..19678b485b4 --- /dev/null +++ b/src/main/startup/quit/window-all-closed-quit-handler.ts @@ -0,0 +1,19 @@ +import { app } from 'electron' +import { mainProcessState as state } from '../main-process-state' +import { shouldQuitWhenAllWindowsClosed } from './window-all-closed-quit-policy' + +export function installWindowAllClosedHandler(): void { + app.on('window-all-closed', () => { + // Keep serve RPC alive after disposable windows close; re-trigger macOS quit after renderer buffer-capture deferral. + if ( + shouldQuitWhenAllWindowsClosed({ + platform: process.platform, + isQuitting: state.isQuitting, + isServeMode: state.isServeMode, + hasOpenedMainWindow: state.hasOpenedMainWindow + }) + ) { + app.quit() + } + }) +} diff --git a/src/main/startup/workspace-startup/worktree-removal-records-load.ts b/src/main/startup/workspace-startup/worktree-removal-records-load.ts index 453a274ef33..35c513432da 100644 --- a/src/main/startup/workspace-startup/worktree-removal-records-load.ts +++ b/src/main/startup/workspace-startup/worktree-removal-records-load.ts @@ -1,17 +1 @@ -import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' -import type { Repo } from '../../../shared/repo-types' -import { loadWorktreeRemovalRecords } from '../../workspaces/removal/worktree-background-removal' - -/** Retain failed deletions only while this host has the local repo copy; an SSH copy under the same ID cannot show its deletion row. */ -export function loadWorktreeRemovalRecordsForStore(store: { - getProfileStorageDirectory: () => string - getRepos: () => readonly Pick[] -}): Promise { - return loadWorktreeRemovalRecords(store.getProfileStorageDirectory(), (repoId) => - store - .getRepos() - .some( - (repo) => repo.id === repoId && getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID - ) - ) -} +export { loadWorktreeRemovalRecordsForStore } from '../../workspaces/removal/worktree-removal-records-load' diff --git a/src/main/workspaces/removal/local-orphaned-worktree-cleanup.test.ts b/src/main/workspaces/removal/local-orphaned-worktree-cleanup.test.ts new file mode 100644 index 00000000000..5979c40b263 --- /dev/null +++ b/src/main/workspaces/removal/local-orphaned-worktree-cleanup.test.ts @@ -0,0 +1,62 @@ +import { beforeEach, expect, it, vi } from 'vitest' +const { prove, missing, remove } = vi.hoisted(() => ({ + prove: vi.fn(), + missing: vi.fn(), + remove: vi.fn() +})) +vi.mock('./local-worktree-filesystem', async (importOriginal) => ({ + ...(await importOriginal>()), + removeLocalWorktreePath: remove +})) +vi.mock('./worktree-removal-safety', async (importOriginal) => ({ + ...(await importOriginal>()), + canSafelyRemoveOrphanedWorktreeDirectory: prove, + isWorktreePathMissing: missing +})) +import { cleanupLocalOrphanedWorktreeDirectory } from './local-orphaned-worktree-cleanup' +beforeEach(() => { + prove.mockReset().mockResolvedValue(true) + missing.mockReset().mockResolvedValue(true) + remove.mockReset().mockResolvedValue(undefined) +}) +it.each(['proof', 'watcher', 'delete', 'success'] as const)( + 'does not convert cancelled orphan cleanup into absent-path success at %s', + async (phase) => { + const controller = new AbortController() + if (phase === 'proof') { + prove.mockImplementationOnce(async () => { + controller.abort() + return true + }) + } + const watcher = vi.fn(async () => { + if (phase === 'watcher') { + controller.abort() + } + }) + remove.mockImplementationOnce(async () => { + controller.abort() + if (phase === 'delete') { + throw controller.signal.reason + } + }) + const pending = cleanupLocalOrphanedWorktreeDirectory( + '/repo', + '/repo/feature', + {}, + watcher, + controller.signal + ) + if (phase === 'success') { + await expect(pending).resolves.toBeUndefined() + expect(missing).toHaveBeenCalledOnce() + } else { + const error = await pending.catch((error: unknown) => error) + expect(error).toBe(controller.signal.reason) + expect(missing).not.toHaveBeenCalled() + } + if (phase === 'proof' || phase === 'watcher') { + expect(remove).not.toHaveBeenCalled() + } + } +) diff --git a/src/main/workspaces/removal/local-orphaned-worktree-cleanup.ts b/src/main/workspaces/removal/local-orphaned-worktree-cleanup.ts index 90e3cec0a2a..aacc76e4279 100644 --- a/src/main/workspaces/removal/local-orphaned-worktree-cleanup.ts +++ b/src/main/workspaces/removal/local-orphaned-worktree-cleanup.ts @@ -10,25 +10,31 @@ import { ORPHANED_WORKTREE_DIRECTORY_MESSAGE } from './worktree-removal-safety' import { CLIENT_REMOVAL_HOME } from './worktree-removal-home-guard' +import { throwIfSignalAborted } from '../../../shared/abort-signal-reason' export async function cleanupLocalOrphanedWorktreeDirectory( repoPath: string, path: string, options: LocalWorktreeFilesystemOptions, - closeWatchers: (path: string) => Promise + closeWatchers: (path: string) => Promise, + signal?: AbortSignal ): Promise { + throwIfSignalAborted(signal) const access = getLocalWorktreePathAccess(options) const runtimePath = toLocalWorktreeRuntimePath(path, options) - if ( - await canSafelyRemoveOrphanedWorktreeDirectory( - runtimePath, - toLocalWorktreeRuntimePath(repoPath, options), - CLIENT_REMOVAL_HOME, - access - ) - ) { + const safe = await canSafelyRemoveOrphanedWorktreeDirectory( + runtimePath, + toLocalWorktreeRuntimePath(repoPath, options), + CLIENT_REMOVAL_HOME, + access + ) + throwIfSignalAborted(signal) + if (safe) { await closeWatchers(path) - await removeLocalWorktreePath(path, options).catch(() => {}) + throwIfSignalAborted(signal) + await removeLocalWorktreePath(path, options, signal).catch(() => { + throwIfSignalAborted(signal) + }) } else { console.warn(`[worktrees] Refusing recursive cleanup for unproven worktree directory: ${path}`) } diff --git a/src/main/workspaces/removal/local-worktree-filesystem.ts b/src/main/workspaces/removal/local-worktree-filesystem.ts index d842638cb41..1eecdc7249d 100644 --- a/src/main/workspaces/removal/local-worktree-filesystem.ts +++ b/src/main/workspaces/removal/local-worktree-filesystem.ts @@ -1,4 +1,5 @@ import { posix } from 'node:path' +import { throwIfSignalAborted } from '../../../shared/abort-signal-reason' import { runProcess } from '@orca/process-host' import { lstat, stat, readFile, realpath } from 'node:fs/promises' import { buildWslExecArgs } from '../../../shared/wsl-login-shell-command' @@ -121,6 +122,7 @@ export async function removeLocalWorktreePath( ): Promise { const distro = options.wslDistro?.trim() if (!shouldUseWslFilesystem(options) || !distro) { + throwIfSignalAborted(signal) await removeHostTree(targetPath, { signal }) return } diff --git a/src/main/workspaces/removal/local-worktree-removal-recovery.test.ts b/src/main/workspaces/removal/local-worktree-removal-recovery.test.ts index ee90666ddcb..7a7e3de7b68 100644 --- a/src/main/workspaces/removal/local-worktree-removal-recovery.test.ts +++ b/src/main/workspaces/removal/local-worktree-removal-recovery.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { NESTED_WORKTREE_REMOVAL_PREFIX } from '../../../shared/worktree/removal/nested-removal' import type * as LocalWorktreeFilesystem from './local-worktree-filesystem' const { gitExecFileAsyncMock, listWorktreesStrictMock, removeLocalWorktreePathMock, hostRmMock } = @@ -79,6 +80,114 @@ describe('recoverPartialLocalWorktreeRemoval', () => { signal: controller.signal }) ).rejects.toThrow(/aborted/i) + expect(removeLocalWorktreePathMock).toHaveBeenCalledWith( + 'C:/repo/worktree/feature', + {}, + controller.signal + ) + expect(closeWatcher).toHaveBeenCalledOnce() + expect(hostRmMock).toHaveBeenCalledOnce() + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + }) + + it.each(['inventory', 'watcher', 'delete', 'success'] as const)( + 'keeps partial-removal cancellation distinct from success at %s', + async (phase) => { + const controller = new AbortController() + if (phase === 'inventory') { + listWorktreesStrictMock.mockImplementationOnce(async () => { + controller.abort() + return [] + }) + } + const closeWatcher = vi.fn(async () => { + if (phase === 'watcher') { + controller.abort() + } + }) + removeLocalWorktreePathMock.mockImplementationOnce(async () => { + controller.abort() + if (phase === 'delete') { + throw controller.signal.reason + } + }) + const pending = recoverPartialLocalWorktreeRemoval({ + error: new Error('Git deregistered but left files'), + force: true, + canonicalWorktreePath: '/repo/feature', + repoPath: '/repo', + localWorktreeGitOptions: {}, + registeredWorktree: { branch: 'feature', head: 'abc' }, + deleteBranch: true, + closeWatcher, + signal: controller.signal + }) + if (phase === 'success') { + await expect(pending).resolves.toBeDefined() + } else { + const error = await pending.catch((error: unknown) => error) + expect(error).toBe(controller.signal.reason) + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + } + if (phase === 'inventory' || phase === 'watcher') { + expect(removeLocalWorktreePathMock).not.toHaveBeenCalled() + } + } + ) + + it('does not recursively delete while watcher teardown remains unconfirmed', async () => { + await withPlatform('win32', async () => { + const watcherError = new Error('file watcher process did not exit after termination deadline') + + await expect( + recoverPartialLocalWorktreeRemoval({ + error: new Error('git worktree remove failed'), + force: true, + canonicalWorktreePath: 'C:/repo/worktree/feature', + repoPath: 'C:/repo', + localWorktreeGitOptions: {}, + registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, + deleteBranch: true, + closeWatcher: vi.fn().mockRejectedValue(watcherError) + }) + ).rejects.toBe(watcherError) + + expect(removeLocalWorktreePathMock).not.toHaveBeenCalled() + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + }) + + it('does not delete a checkout that now contains a registered worktree', async () => { + await withPlatform('darwin', async () => { + listWorktreesStrictMock.mockResolvedValue([ + { + path: '/Users/dev/orca/workspaces/feature/nested', + head: 'def456', + branch: 'refs/heads/nested', + isBare: false, + isMainWorktree: false + } + ]) + const closeWatcher = vi.fn().mockResolvedValue(undefined) + + await expect( + recoverPartialLocalWorktreeRemoval({ + error: Object.assign(new Error('git worktree remove failed'), { + stderr: + "error: failed to delete '/Users/dev/orca/workspaces/feature': Directory not empty" + }), + force: true, + canonicalWorktreePath: '/Users/dev/orca/workspaces/feature', + repoPath: '/Users/dev/repo', + localWorktreeGitOptions: {}, + registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, + deleteBranch: true, + closeWatcher + }) + ).rejects.toThrow(NESTED_WORKTREE_REMOVAL_PREFIX) + expect(closeWatcher).not.toHaveBeenCalled() + expect(removeLocalWorktreePathMock).not.toHaveBeenCalled() }) }) diff --git a/src/main/workspaces/removal/local-worktree-removal-recovery.ts b/src/main/workspaces/removal/local-worktree-removal-recovery.ts index 3ede1ed9421..9a3aa9a8898 100644 --- a/src/main/workspaces/removal/local-worktree-removal-recovery.ts +++ b/src/main/workspaces/removal/local-worktree-removal-recovery.ts @@ -12,6 +12,7 @@ import { import { deleteBranchOfUnregisteredWorktree } from '../../git/worktrees/removal/worktree-removal' import { removeLocalWorktreePath } from './local-worktree-filesystem' import { assertWorktreeDoesNotContainRegisteredWorktree } from './worktree-removal-safety' +import { throwIfSignalAborted } from '../../../shared/abort-signal-reason' type LocalPartialRemovalRecoveryArgs = { error: unknown @@ -107,15 +108,19 @@ export async function removeStaleLocalWorktreeRegistration( export async function recoverPartialLocalWorktreeRemoval( args: LocalPartialRemovalRecoveryArgs ): Promise { + throwIfSignalAborted(args.signal) // Reject Git locks before recovery can recursively delete files. assertWorktreeUnlockedForRemoval(args.registeredWorktree) - if (!(await isRecoverablePartialRemovalFailure(args))) { + const recoverable = await isRecoverablePartialRemovalFailure(args) + throwIfSignalAborted(args.signal) + if (!recoverable) { return undefined } // Release watcher handles before recursively finishing Git’s partial removal; abort if closing fails. await args.closeWatcher(args.canonicalWorktreePath) + throwIfSignalAborted(args.signal) try { await removeLocalWorktreePath( args.canonicalWorktreePath, @@ -123,6 +128,7 @@ export async function recoverPartialLocalWorktreeRemoval( args.signal ) } catch (error) { + throwIfSignalAborted(args.signal) throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, args.force)) } const { branch, head } = args.registeredWorktree diff --git a/src/main/workspaces/removal/worktree-background-removal.test.ts b/src/main/workspaces/removal/worktree-background-removal.test.ts index 51ed36ee159..c39cbe0f0ce 100644 --- a/src/main/workspaces/removal/worktree-background-removal.test.ts +++ b/src/main/workspaces/removal/worktree-background-removal.test.ts @@ -40,6 +40,7 @@ function deferred(): { promise: Promise; resolve: (value: T) => void } { describe('background worktree removal', () => { let directory = '' beforeEach(async () => { + _resetPendingWorktreeRemovalsForTests() directory = await mkdtemp(join(tmpdir(), 'orca-background-removal-')) await loadWorktreeRemovalRecords(directory) }) diff --git a/src/main/workspaces/removal/worktree-background-removal.ts b/src/main/workspaces/removal/worktree-background-removal.ts index c8da3913a78..89e76af5d80 100644 --- a/src/main/workspaces/removal/worktree-background-removal.ts +++ b/src/main/workspaces/removal/worktree-background-removal.ts @@ -21,6 +21,7 @@ import { readWorktreeRemovalRecords, type WorktreeRemovalRecord } from './worktr import { failureLeftByGit } from './worktree-removal-failure' import { differentCheckoutAtPathError, isCheckoutRegistered } from './worktree-removal-leftover' import { + closeWorktreeRemovalRecords, failedWorktreeRemovals, finishedWorktreeRemovals, pendingWorktreeRemovals, @@ -29,6 +30,10 @@ import { recordWorktreeRemoval, setWorktreeRemovalRecordsDirectory } from './worktree-removal-table' +import { + worktreeRemovalLifetime, + _resetWorktreeRemovalLifetimeForTests +} from './worktree-removal-lifetime' export type BackgroundWorktreeRemovalJob = { /** Abort only the checkout delete on orderly quit. */ @@ -42,7 +47,6 @@ export type BackgroundWorktreeRemovalJob = { } const jobsByWorktreeId = new Map>() -const stopControllers = new Set() // Loaded removals' terminal/watcher fences, held until the resumed job takes its own gate. const startupFencesByWorktreeId = new Map() @@ -144,27 +148,35 @@ export function retryFailedWorktreeRemoval( return waitForPendingWorktreeRemoval(worktreeId, hostId, deleteBranch) } assertAcceptedBranchChoice(failed, deleteBranch) - const { failure: _failure, ...original } = failed - const record = { ...original, force: force ?? original.force } - const settlement = addPendingRemoval(record) - const job = jobFor(record) - const leftoverOnly: BackgroundWorktreeRemovalJob = { - ...job, - run: async (stopSignal) => { - // A newly registered checkout at this path needs a fresh delete, not the leftover's recorded choices. - if (await isCheckoutRegistered(record)) { - throw differentCheckoutAtPathError(record.worktreePath) + const release = worktreeRemovalLifetime.acquirePreparation() + try { + const { failure: _failure, ...original } = failed + const record = { ...original, force: force ?? original.force } + const settlement = addPendingRemoval(record) + const job = jobFor(record) + const leftoverOnly: BackgroundWorktreeRemovalJob = { + ...job, + run: async (stopSignal) => { + // A newly registered checkout at this path needs a fresh delete, not the leftover's recorded choices. + if (await isCheckoutRegistered(record, { signal: stopSignal })) { + throw differentCheckoutAtPathError(record.worktreePath) + } + return job.run(stopSignal) } - return job.run(stopSignal) } + runBackgroundWorktreeRemoval(record, leftoverOnly, recordWorktreeRemoval(record)) + return settlement.result + } finally { + release() } - runBackgroundWorktreeRemoval(record, leftoverOnly, recordWorktreeRemoval(record)) - return settlement.result } export function resumeInterruptedWorktreeRemovals( jobFor: (record: WorktreeRemovalRecord) => BackgroundWorktreeRemovalJob ): void { + if (worktreeRemovalLifetime.closing) { + return + } for (const record of pendingWorktreeRemovals.values()) { if (!jobsByWorktreeId.has(record.worktreeId)) { runBackgroundWorktreeRemoval(record, jobFor(record), Promise.resolve()) @@ -174,9 +186,7 @@ export function resumeInterruptedWorktreeRemovals( /** Abort checkout deletes on quit without waiting for Git to exit. */ export function stopBackgroundWorktreeRemovals(): void { - for (const controller of stopControllers) { - controller.abort() - } + worktreeRemovalLifetime.abortCleanup() } function runBackgroundWorktreeRemoval( @@ -184,16 +194,14 @@ function runBackgroundWorktreeRemoval( job: BackgroundWorktreeRemovalJob, recorded: Promise ): void { - const controller = new AbortController() - stopControllers.add(controller) + const signal = worktreeRemovalLifetime.signal const settled: Promise = settleBackgroundWorktreeRemoval( record, settlementsByWorktreeId.get(record.worktreeId), job, recorded, - controller.signal + signal ).finally(() => { - stopControllers.delete(controller) if (jobsByWorktreeId.get(record.worktreeId) === settled) { jobsByWorktreeId.delete(record.worktreeId) } @@ -291,12 +299,21 @@ export async function _settlePendingWorktreeRemovalsForTests(): Promise { } } +export async function drainBackgroundWorktreeRemovals(): Promise { + const results = await Promise.allSettled(jobsByWorktreeId.values()) + await closeWorktreeRemovalRecords() + const errors = results.flatMap((result) => (result.status === 'rejected' ? [result.reason] : [])) + if (errors.length > 0) { + throw new AggregateError(errors, 'Could not finish stopping workspace deletion.') + } +} + export function _resetPendingWorktreeRemovalsForTests(): void { + _resetWorktreeRemovalLifetimeForTests() pendingWorktreeRemovals.clear() failedWorktreeRemovals.clear() jobsByWorktreeId.clear() settlementsByWorktreeId.clear() - stopControllers.clear() for (const fence of startupFencesByWorktreeId.values()) { fence.release() } diff --git a/src/main/workspaces/removal/worktree-removal-leftover.ts b/src/main/workspaces/removal/worktree-removal-leftover.ts index 5d735a5cd72..35c144eb8f0 100644 --- a/src/main/workspaces/removal/worktree-removal-leftover.ts +++ b/src/main/workspaces/removal/worktree-removal-leftover.ts @@ -5,7 +5,8 @@ import { lstat } from 'node:fs/promises' import { join } from 'node:path' import type { RemoveWorktreeResult } from '../../../shared/worktree/create-types' import type { GitWorktreeInfo } from '../../../shared/worktree/types' -import { listWorktreesStrict } from '../../git/worktrees/catalog/worktree-listing' +import { listWorktreesSharedStrict as listWorktreesStrict } from '../../git/worktrees/catalog/worktree-scan-cache' +import type { GitWorktreeExecOptions } from '../../git/worktrees/worktree-operation-options' import { getErrorCode, normalizeLocalBranchRef @@ -39,20 +40,26 @@ export function differentCheckoutAtPathError(worktreePath: string): Error { ) } -export async function registeredRemovalCheckout(record: { - repoPath: string - worktreePath: string -}): Promise { - return (await listWorktreesStrict(record.repoPath)).find((worktree) => +export async function registeredRemovalCheckout( + record: { + repoPath: string + worktreePath: string + }, + options: GitWorktreeExecOptions = {} +): Promise { + return (await listWorktreesStrict(record.repoPath, options)).find((worktree) => areWorktreePathsEqual(worktree.path, record.worktreePath) ) } -export async function isCheckoutRegistered(record: { - repoPath: string - worktreePath: string -}): Promise { - return !!(await registeredRemovalCheckout(record)) +export async function isCheckoutRegistered( + record: { + repoPath: string + worktreePath: string + }, + options: GitWorktreeExecOptions = {} +): Promise { + return !!(await registeredRemovalCheckout(record, options)) } export function isRecordedCheckout( diff --git a/src/main/workspaces/removal/worktree-removal-lifetime.test-fixtures.ts b/src/main/workspaces/removal/worktree-removal-lifetime.test-fixtures.ts new file mode 100644 index 00000000000..4b4d4b81985 --- /dev/null +++ b/src/main/workspaces/removal/worktree-removal-lifetime.test-fixtures.ts @@ -0,0 +1,80 @@ +import { join } from 'node:path' +import { writeFile } from 'node:fs/promises' +import { vi } from 'vitest' +import * as gitRemoval from '../../git/worktrees/removal/worktree-removal' +import * as pushTarget from '../../ipc/worktree-remote' +import { finishRuntimeLocalWorktreeRemoval } from '../../runtime/workspaces/removal/runtime-registered-local-worktree-removal' +import type { RuntimeStore } from '../../runtime/construction/runtime-store-contract' +import { prepareWorktreeRemovalsForStore } from './worktree-removal-records-load' +import { startBackgroundWorktreeRemoval } from './worktree-background-removal' + +/** Hold the real successful job after Git finishes, inside its host metadata callback. */ +export async function holdRemovalMetadata( + directory: string, + finishMetadata: () => void = () => {} +) { + await prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => [{ id: 'repo' }] + }) + vi.spyOn(gitRemoval, 'removeWorktree').mockResolvedValue({}) + vi.spyOn(pushTarget, 'cleanupUnusedWorktreePushTargetRemote').mockResolvedValue() + let entered!: () => void + const metadata = new Promise((resolve) => { + entered = resolve + }) + let release!: () => void + const held = new Promise((resolve) => { + release = resolve + }) + const worktree = { + path: join(directory, 'checkout'), + branch: 'refs/heads/feature', + head: 'abc', + isMainWorktree: false, + isBare: false + } + const result = startBackgroundWorktreeRemoval({ + removal: { + worktreeId: 'repo::feature', + repoId: 'repo', + repoPath: join(directory, 'repo'), + worktree, + force: false, + deleteBranch: false + }, + publish: () => {}, + run: (signal) => + finishRuntimeLocalWorktreeRemoval( + { + repo: { + id: 'repo', + path: join(directory, 'repo'), + displayName: 'repo', + badgeColor: '', + addedAt: 0 + }, + target: { id: 'repo::feature' }, + removedPushTarget: undefined, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mocked push-target seam does not read the store; metadata completion below is real awaited I/O. + store: {} as RuntimeStore, + localOptions: {}, + force: false, + deleteBranch: false, + closeWatchers: async () => {}, + preserveBranchHead: (value) => value ?? {}, + finishRemoval: async () => { + entered() + await held + await writeFile(join(directory, 'metadata-finished'), 'removed') + finishMetadata() + } + }, + worktree, + { finish: async () => {} }, + signal + ) + }) + await metadata + return { release, result } +} diff --git a/src/main/workspaces/removal/worktree-removal-lifetime.test.ts b/src/main/workspaces/removal/worktree-removal-lifetime.test.ts new file mode 100644 index 00000000000..13477362053 --- /dev/null +++ b/src/main/workspaces/removal/worktree-removal-lifetime.test.ts @@ -0,0 +1,208 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { + _resetPendingWorktreeRemovalsForTests, + _settlePendingWorktreeRemovalsForTests, + startBackgroundWorktreeRemoval, + resumeInterruptedWorktreeRemovals, + waitForWorktreeRemovalAcceptance +} from './worktree-background-removal' +import { prepareWorktreeRemovalsForStore } from './worktree-removal-records-load' +import { worktreeRemovalLifetime } from './worktree-removal-lifetime' +import { + readWorktreeRemovalRecords, + worktreeRemovalRecordsFile, + writeWorktreeRemovalRecords +} from './worktree-removal-records' +import { + assertNoPendingWorktreeRemovalConflict, + persistWorktreeRemovalRecords +} from './worktree-removal-table' +import { withSidecarSnapshotQueue } from '../../filesystem/sidecar-snapshot-file' +import * as failureInspection from './worktree-removal-failure' + +function deferred() { + let resolve!: () => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} +const removal = { + worktreeId: 'repo::feature', + repoId: 'repo', + repoPath: '/unused/repo', + worktree: { path: '/unused/feature', branch: 'feature', head: 'abc' }, + deleteBranch: false, + force: true +} +let directory = '' +const prepare = () => + prepareWorktreeRemovalsForStore({ + getProfileStorageDirectory: () => directory, + getRepos: () => [{ id: 'repo' }] + }) +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-removal-lifetime-')) +}) +afterEach(async () => { + vi.restoreAllMocks() + _resetPendingWorktreeRemovalsForTests() + await rm(directory, { recursive: true, force: true }) +}) + +it('lets owned teardown admit during close without running cleanup, and recovers its saved choices', async () => { + await prepare() + const release = worktreeRemovalLifetime.acquirePreparation() + const closing = worktreeRemovalLifetime.close() + expect(worktreeRemovalLifetime.close()).toBe(closing) + expect(() => worktreeRemovalLifetime.acquirePreparation()).toThrow('Orca is stopping') + const run = vi.fn(async () => ({})) + void startBackgroundWorktreeRemoval({ removal, run, publish: () => {} }) + release() + await waitForWorktreeRemovalAcceptance(removal.worktreeId) + await closing + expect(run).not.toHaveBeenCalled() + expect(await readWorktreeRemovalRecords(directory)).toEqual([ + expect.objectContaining({ + worktreeId: 'repo::feature', + force: true, + deleteBranch: false + }) + ]) + _resetPendingWorktreeRemovalsForTests() + await prepare() + resumeInterruptedWorktreeRemovals(() => ({ run, publish: () => {} })) + await _settlePendingWorktreeRemovalsForTests() + expect(run).toHaveBeenCalledOnce() + expect(await readWorktreeRemovalRecords(directory)).toEqual([]) +}) + +it('keeps failure writes live through inspection and parks that failure across restart', async () => { + await prepare() + const inspecting = deferred() + const inspected = deferred() + vi.spyOn(failureInspection, 'failureLeftByGit').mockImplementation(async () => { + inspecting.resolve() + await inspected.promise + return { message: 'Files are in use. Retry deletion.', failedAt: 123 } + }) + const result = startBackgroundWorktreeRemoval({ + removal, + run: async () => { + throw new Error('files in use') + }, + publish: () => {} + }) + const refused = expect(result).rejects.toThrow('files in use') + await inspecting.promise + const closing = worktreeRemovalLifetime.close() + inspected.resolve() + await closing + await refused + _resetPendingWorktreeRemovalsForTests() + await prepare() + const run = vi.fn(async () => ({})) + resumeInterruptedWorktreeRemovals(() => ({ run, publish: () => {} })) + expect(run).not.toHaveBeenCalled() + expect(() => + assertNoPendingWorktreeRemovalConflict('/unused/repo', { branch: 'feature' }) + ).toThrow('Cleanup failed') + expect(await readWorktreeRemovalRecords(directory)).toEqual([ + expect.objectContaining({ + failure: { message: 'Files are in use. Retry deletion.', failedAt: 123 } + }) + ]) +}) + +it('does not start deferred loaded recovery after close, but retains it for the next host', async () => { + const { worktree, ...fields } = removal + await writeWorktreeRemovalRecords(directory, () => [ + { ...fields, worktreePath: worktree.path, branch: 'feature', head: 'abc', requestedAt: 1 } + ]) + await prepare() + const queuedCallback = () => + worktreeRemovalLifetime.resume(() => + resumeInterruptedWorktreeRemovals(() => ({ + run: async () => { + throw new Error('late recovery ran') + }, + publish: () => {} + })) + ) + await worktreeRemovalLifetime.close() + queuedCallback() + expect(await readWorktreeRemovalRecords(directory)).toHaveLength(1) + _resetPendingWorktreeRemovalsForTests() + await prepare() + resumeInterruptedWorktreeRemovals(() => ({ run: async () => ({}), publish: () => {} })) + await _settlePendingWorktreeRemovalsForTests() + expect(await readWorktreeRemovalRecords(directory)).toEqual([]) +}) + +it('joins listing writes outside jobs and seals submissions before a successor writes', async () => { + await prepare() + const writer = deferred() + const entered = deferred() + void withSidecarSnapshotQueue(worktreeRemovalRecordsFile(directory), async () => { + entered.resolve() + await writer.promise + }) + await entered.promise + void persistWorktreeRemovalRecords() + let released = false + const closing = worktreeRemovalLifetime.close().then(() => { + released = true + }) + await Promise.resolve() + expect(released).toBe(false) + writer.resolve() + await closing + await writeWorktreeRemovalRecords(directory, () => [ + { + worktreeId: 'successor', + repoId: 'repo', + repoPath: '/next', + worktreePath: '/next/feature', + branch: 'next', + head: 'def', + force: false, + deleteBranch: false, + requestedAt: 2 + } + ]) + await persistWorktreeRemovalRecords() + expect(await readWorktreeRemovalRecords(directory)).toEqual([ + expect.objectContaining({ worktreeId: 'successor' }) + ]) +}) + +it('joins preparation that is still loading before allowing a failed startup to unwind', async () => { + const loading = deferred() + const draining = deferred() + const ready = worktreeRemovalLifetime.prepare( + () => loading.promise, + () => {}, + () => draining.promise + ) + const startupFailure = expect(ready).rejects.toThrow('Orca stopped while preparing') + const closing = worktreeRemovalLifetime.close() + const stopFailure = expect(closing).rejects.toThrow('Orca stopped while preparing') + loading.resolve() + await startupFailure + let released = false + void closing.then( + () => { + released = true + }, + () => { + released = true + } + ) + await Promise.resolve() + expect(released).toBe(false) + draining.resolve() + await stopFailure +}) diff --git a/src/main/workspaces/removal/worktree-removal-lifetime.ts b/src/main/workspaces/removal/worktree-removal-lifetime.ts new file mode 100644 index 00000000000..65884e7156d --- /dev/null +++ b/src/main/workspaces/removal/worktree-removal-lifetime.ts @@ -0,0 +1,83 @@ +/** Own destructive preparation through its handoff to a journaled job, before host resources close. */ +class WorktreeRemovalLifetime { + private readonly cleanup = new AbortController() + get signal(): AbortSignal { + return this.cleanup.signal + } + abortCleanup(): void { + this.cleanup.abort() + } + private ready: Promise = Promise.resolve() + private readonly preparations = new Set>() + private stop = (): void => {} + private drain = async (): Promise => {} + private completion?: Promise + closing = false + + prepare(load: () => Promise, stop: () => void, drain: () => Promise): Promise { + if (this.closing) { + throw new Error('Orca is stopping. Reconnect or restart Orca, then retry deletion.') + } + this.stop = stop + this.drain = drain + this.ready = load().then(() => { + if (this.closing) { + throw new Error('Orca stopped while preparing workspace deletion. Restart Orca and retry.') + } + }) + return this.ready + } + + acquirePreparation(): () => void { + if (this.closing) { + throw new Error('Orca is stopping. Reconnect or restart Orca, then retry deletion.') + } + let release!: () => void + const preparation = new Promise((resolve) => { + release = resolve + }) + this.preparations.add(preparation) + return () => { + this.preparations.delete(preparation) + release() + } + } + + async withPreparation(prepare: () => Promise): Promise { + const release = this.acquirePreparation() + try { + return await prepare() + } finally { + release() + } + } + + resume(resume: () => void): void { + if (!this.closing) { + resume() + } + } + + close(): Promise { + if (!this.completion) { + this.closing = true + this.abortCleanup() + this.stop() + this.completion = (async () => { + const loaded = await Promise.allSettled([this.ready]) + await Promise.all(this.preparations) + await this.drain() + if (loaded[0].status === 'rejected') { + throw loaded[0].reason + } + })() + } + return this.completion + } +} + +export let worktreeRemovalLifetime = new WorktreeRemovalLifetime() + +export function _resetWorktreeRemovalLifetimeForTests(): void { + worktreeRemovalLifetime = new WorktreeRemovalLifetime() +} diff --git a/src/main/workspaces/removal/worktree-removal-records-load.ts b/src/main/workspaces/removal/worktree-removal-records-load.ts new file mode 100644 index 00000000000..93be358fad0 --- /dev/null +++ b/src/main/workspaces/removal/worktree-removal-records-load.ts @@ -0,0 +1,32 @@ +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' +import type { Repo } from '../../../shared/repo-types' +import { + loadWorktreeRemovalRecords, + stopBackgroundWorktreeRemovals, + drainBackgroundWorktreeRemovals +} from './worktree-background-removal' +import { worktreeRemovalLifetime } from './worktree-removal-lifetime' + +/** Retain failed deletions only while this host has the local repo copy; an SSH copy under the same ID cannot show its deletion row. */ +export function loadWorktreeRemovalRecordsForStore(store: { + getProfileStorageDirectory: () => string + getRepos: () => readonly Pick[] +}): Promise { + return loadWorktreeRemovalRecords(store.getProfileStorageDirectory(), (repoId) => + store + .getRepos() + .some( + (repo) => repo.id === repoId && getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID + ) + ) +} + +export function prepareWorktreeRemovalsForStore( + store: Parameters[0] +): Promise { + return worktreeRemovalLifetime.prepare( + () => loadWorktreeRemovalRecordsForStore(store), + stopBackgroundWorktreeRemovals, + drainBackgroundWorktreeRemovals + ) +} diff --git a/src/main/workspaces/removal/worktree-removal-table.ts b/src/main/workspaces/removal/worktree-removal-table.ts index 363fab881aa..87a5b82dc73 100644 --- a/src/main/workspaces/removal/worktree-removal-table.ts +++ b/src/main/workspaces/removal/worktree-removal-table.ts @@ -2,7 +2,12 @@ import { WorktreeRemovalReservations } from './worktree-removal-reservations' import { lstat } from 'node:fs/promises' import { getErrorCode } from '../../git/worktrees/worktree-operation-options' import { areWorktreePathsEqual } from '../../git/worktrees/catalog/worktree-path-comparison' -import { writeWorktreeRemovalRecords, type WorktreeRemovalRecord } from './worktree-removal-records' +import { + writeWorktreeRemovalRecords, + worktreeRemovalRecordsFile, + type WorktreeRemovalRecord +} from './worktree-removal-records' +import { withSidecarSnapshotQueue } from '../../filesystem/sidecar-snapshot-file' export const pendingWorktreeRemovals = new WorktreeRemovalReservations() export const recordedWorktreeRemovals = new WeakSet() @@ -16,6 +21,14 @@ export function setWorktreeRemovalRecordsDirectory(directory: string | null): vo recordsDirectory = directory } +export function closeWorktreeRemovalRecords(): Promise { + const directory = recordsDirectory + recordsDirectory = null + return directory + ? withSidecarSnapshotQueue(worktreeRemovalRecordsFile(directory), async () => {}) + : Promise.resolve() +} + export function persistWorktreeRemovalRecords(): Promise { if (!recordsDirectory) { return Promise.resolve()