From d6585f7c3a99c60937c9406a323c7900b431884f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Apr 2026 15:50:48 +0700 Subject: [PATCH] fix(server): harden worktree archive with parallel teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fan out agent close + terminal kill concurrently with Promise.allSettled so one failure never leaves the worktree half-dead. Await real PTY exit with SIGHUP→SIGKILL escalation before rm to avoid EBUSY races. Tolerate missing repoRoot and retry directory removal with backoff. Purge stale client workspace layout/tab state after successful archive. --- .../src/stores/checkout-git-actions-store.ts | 19 ++ .../app/src/stores/workspace-layout-store.ts | 32 +++ .../app/src/stores/workspace-tabs-store.ts | 26 +++ .../src/server/worktree-session.test.ts | 197 ++++++++++++++++++ .../server/src/server/worktree-session.ts | 126 +++++++---- .../server/src/terminal/terminal-manager.ts | 19 ++ packages/server/src/terminal/terminal.ts | 62 ++++++ packages/server/src/utils/worktree.test.ts | 25 +++ packages/server/src/utils/worktree.ts | 96 +++++++-- 9 files changed, 537 insertions(+), 65 deletions(-) diff --git a/packages/app/src/stores/checkout-git-actions-store.ts b/packages/app/src/stores/checkout-git-actions-store.ts index c5c5f1177..c49ce2fd1 100644 --- a/packages/app/src/stores/checkout-git-actions-store.ts +++ b/packages/app/src/stores/checkout-git-actions-store.ts @@ -1,6 +1,11 @@ import { create } from "zustand"; import { useSessionStore } from "@/stores/session-store"; import { queryClient } from "@/query/query-client"; +import { + buildWorkspaceTabPersistenceKey, + useWorkspaceLayoutStore, +} from "@/stores/workspace-layout-store"; +import { useWorkspaceTabsStore } from "@/stores/workspace-tabs-store"; const SUCCESS_DISPLAY_MS = 1000; @@ -123,6 +128,19 @@ function removeWorktreeFromCachedLists(input: { serverId: string; worktreePath: ); } +function purgeArchivedWorkspaceState(input: { serverId: string; worktreePath: string }): void { + const serverId = input.serverId.trim(); + const workspaceId = input.worktreePath.trim(); + if (!serverId || !workspaceId) { + return; + } + const workspaceKey = buildWorkspaceTabPersistenceKey({ serverId, workspaceId }); + if (workspaceKey) { + useWorkspaceLayoutStore.getState().purgeWorkspace(workspaceKey); + } + useWorkspaceTabsStore.getState().purgeWorkspace({ serverId, workspaceId }); +} + const successTimers = new Map>(); const inFlight = new Map>(); @@ -320,6 +338,7 @@ export const useCheckoutGitActionsStore = create() } removeWorktreeFromCachedLists({ serverId, worktreePath }); invalidateWorktreeList(); + purgeArchivedWorkspaceState({ serverId, worktreePath }); }, }); }, diff --git a/packages/app/src/stores/workspace-layout-store.ts b/packages/app/src/stores/workspace-layout-store.ts index bd633b9e4..6bcadef14 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -99,6 +99,7 @@ interface WorkspaceLayoutStore { unpinAgent: (workspaceKey: string, agentId: string) => void; hideAgent: (workspaceKey: string, agentId: string) => void; unhideAgent: (workspaceKey: string, agentId: string) => void; + purgeWorkspace: (workspaceKey: string) => void; } const MAX_TREE_DEPTH = 4; @@ -669,6 +670,37 @@ export const useWorkspaceLayoutStore = create()( }; }); }, + purgeWorkspace: (workspaceKey) => { + const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); + if (!normalizedWorkspaceKey) { + return; + } + + set((state) => { + const hasAny = + normalizedWorkspaceKey in state.layoutByWorkspace || + normalizedWorkspaceKey in state.splitSizesByWorkspace || + normalizedWorkspaceKey in state.pinnedAgentIdsByWorkspace || + normalizedWorkspaceKey in state.hiddenAgentIdsByWorkspace; + if (!hasAny) { + return state; + } + const { [normalizedWorkspaceKey]: _layout, ...layoutByWorkspace } = + state.layoutByWorkspace; + const { [normalizedWorkspaceKey]: _splits, ...splitSizesByWorkspace } = + state.splitSizesByWorkspace; + const { [normalizedWorkspaceKey]: _pinned, ...pinnedAgentIdsByWorkspace } = + state.pinnedAgentIdsByWorkspace; + const { [normalizedWorkspaceKey]: _hidden, ...hiddenAgentIdsByWorkspace } = + state.hiddenAgentIdsByWorkspace; + return { + layoutByWorkspace, + splitSizesByWorkspace, + pinnedAgentIdsByWorkspace, + hiddenAgentIdsByWorkspace, + }; + }); + }, }), { name: "workspace-layout-state", diff --git a/packages/app/src/stores/workspace-tabs-store.ts b/packages/app/src/stores/workspace-tabs-store.ts index 96582500b..4cef62be7 100644 --- a/packages/app/src/stores/workspace-tabs-store.ts +++ b/packages/app/src/stores/workspace-tabs-store.ts @@ -93,6 +93,7 @@ type WorkspaceTabsState = { }) => string | null; reorderTabs: (input: { serverId: string; workspaceId: string; tabIds: string[] }) => void; getWorkspaceTabs: (input: { serverId: string; workspaceId: string }) => WorkspaceTab[]; + purgeWorkspace: (input: { serverId: string; workspaceId: string }) => void; }; export const useWorkspaceTabsStore = create()( @@ -326,6 +327,31 @@ export const useWorkspaceTabsStore = create()( } return get().uiTabsByWorkspace[key] ?? []; }, + purgeWorkspace: ({ serverId, workspaceId }) => { + const key = buildWorkspaceTabPersistenceKey({ serverId, workspaceId }); + if (!key) { + return; + } + set((state) => { + if ( + !(key in state.uiTabsByWorkspace) && + !(key in state.tabOrderByWorkspace) && + !(key in state.focusedTabIdByWorkspace) + ) { + return state; + } + const { [key]: _tabs, ...remainingUiTabsByWorkspace } = state.uiTabsByWorkspace; + const { [key]: _order, ...remainingTabOrderByWorkspace } = state.tabOrderByWorkspace; + const { [key]: _focused, ...remainingFocusedTabIdByWorkspace } = + state.focusedTabIdByWorkspace; + return { + ...state, + uiTabsByWorkspace: remainingUiTabsByWorkspace, + tabOrderByWorkspace: remainingTabOrderByWorkspace, + focusedTabIdByWorkspace: remainingFocusedTabIdByWorkspace, + }; + }); + }, }), { name: "workspace-tabs-state", diff --git a/packages/server/src/server/worktree-session.test.ts b/packages/server/src/server/worktree-session.test.ts index e9aa9f1ff..288506ce1 100644 --- a/packages/server/src/server/worktree-session.test.ts +++ b/packages/server/src/server/worktree-session.test.ts @@ -15,7 +15,9 @@ import type { SessionOutboundMessage } from "./messages.js"; import { ScriptRouteStore } from "./script-proxy.js"; import * as worktreeBootstrap from "./worktree-bootstrap.js"; import { + archivePaseoWorktree, buildAgentSessionConfig, + handlePaseoWorktreeArchiveRequest, runWorktreeSetupInBackground, handleCreatePaseoWorktreeRequest, handleWorkspaceSetupStatusRequest, @@ -1010,3 +1012,198 @@ describe("handleCreatePaseoWorktreeRequest", () => { } }); }); + +describe("archivePaseoWorktree", () => { + const cleanupPaths: string[] = []; + + afterEach(() => { + for (const target of cleanupPaths.splice(0)) { + rmSync(target, { recursive: true, force: true }); + } + }); + + function createIsPathWithinRoot() { + return (rootPath: string, candidatePath: string) => { + const normalizedRoot = path.resolve(rootPath); + const normalizedCandidate = path.resolve(candidatePath); + return ( + normalizedCandidate === normalizedRoot || + normalizedCandidate.startsWith(normalizedRoot + path.sep) + ); + }; + } + + test("runs agent close and terminal teardown concurrently and removes the worktree", async () => { + const { tempDir, repoDir } = createGitRepo(); + cleanupPaths.push(tempDir); + + const paseoHome = path.join(tempDir, ".paseo"); + const created = await createWorktree({ + branchName: "archive-parallel", + cwd: repoDir, + baseBranch: "main", + worktreeSlug: "archive-parallel", + runSetup: false, + paseoHome, + }); + + const teardownStartTimes: Record = {}; + const teardownEndTimes: Record = {}; + const closeAgentSpy = vi.fn(async (agentId: string) => { + teardownStartTimes[agentId] = Date.now(); + await new Promise((resolve) => setTimeout(resolve, 100)); + teardownEndTimes[agentId] = Date.now(); + }); + const killTerminalsUnderPath = vi.fn(async () => { + teardownStartTimes.__terminals = Date.now(); + await new Promise((resolve) => setTimeout(resolve, 100)); + teardownEndTimes.__terminals = Date.now(); + }); + + const emitted: SessionOutboundMessage[] = []; + const removedAgents = await archivePaseoWorktree( + { + paseoHome, + agentManager: { + listAgents: () => [ + { id: "agent-1", cwd: created.worktreePath } as any, + { id: "agent-2", cwd: created.worktreePath } as any, + ], + closeAgent: closeAgentSpy, + }, + agentStorage: { + list: async () => [], + remove: vi.fn(async () => {}), + } as any, + archiveWorkspaceRecord: vi.fn(async () => {}), + emit: (msg) => emitted.push(msg), + emitWorkspaceUpdatesForCwds: vi.fn(async () => {}), + isPathWithinRoot: createIsPathWithinRoot(), + killTerminalsUnderPath, + sessionLogger: createLogger(), + }, + { + targetPath: created.worktreePath, + repoRoot: repoDir, + requestId: "req-archive-parallel", + }, + ); + + expect(removedAgents).toEqual(expect.arrayContaining(["agent-1", "agent-2"])); + expect(existsSync(created.worktreePath)).toBe(false); + expect(closeAgentSpy).toHaveBeenCalledTimes(2); + expect(killTerminalsUnderPath).toHaveBeenCalledWith(created.worktreePath); + + // All teardown work must overlap — sequential would take ~300ms, parallel ~100ms. + const starts = Object.values(teardownStartTimes); + const ends = Object.values(teardownEndTimes); + const maxEnd = Math.max(...ends); + const minStart = Math.min(...starts); + expect(maxEnd - minStart).toBeLessThan(220); + }); + + test("proceeds to FS delete even when terminal teardown rejects", async () => { + const { tempDir, repoDir } = createGitRepo(); + cleanupPaths.push(tempDir); + + const paseoHome = path.join(tempDir, ".paseo"); + const created = await createWorktree({ + branchName: "archive-terminal-throws", + cwd: repoDir, + baseBranch: "main", + worktreeSlug: "archive-terminal-throws", + runSetup: false, + paseoHome, + }); + + const killTerminalsUnderPath = vi.fn(async () => { + throw new Error("simulated terminal teardown failure"); + }); + + await archivePaseoWorktree( + { + paseoHome, + agentManager: { + listAgents: () => [], + closeAgent: vi.fn(async () => {}), + }, + agentStorage: { + list: async () => [], + remove: vi.fn(async () => {}), + } as any, + archiveWorkspaceRecord: vi.fn(async () => {}), + emit: vi.fn(), + emitWorkspaceUpdatesForCwds: vi.fn(async () => {}), + isPathWithinRoot: createIsPathWithinRoot(), + killTerminalsUnderPath, + sessionLogger: createLogger(), + }, + { + targetPath: created.worktreePath, + repoRoot: repoDir, + requestId: "req-archive-terminal-throws", + }, + ); + + expect(killTerminalsUnderPath).toHaveBeenCalledTimes(1); + expect(existsSync(created.worktreePath)).toBe(false); + }); + + test("succeeds when git has forgotten about the worktree (no repoRoot)", async () => { + const { tempDir, repoDir } = createGitRepo(); + cleanupPaths.push(tempDir); + + const paseoHome = path.join(tempDir, ".paseo"); + const created = await createWorktree({ + branchName: "archive-orphan", + cwd: repoDir, + baseBranch: "main", + worktreeSlug: "archive-orphan", + runSetup: false, + paseoHome, + }); + + // Simulate a prior failed archive that stripped git's admin dir. + rmSync(path.join(repoDir, ".git", "worktrees", "archive-orphan"), { + recursive: true, + force: true, + }); + expect(existsSync(created.worktreePath)).toBe(true); + + const emitted: SessionOutboundMessage[] = []; + await handlePaseoWorktreeArchiveRequest( + { + paseoHome, + agentManager: { + listAgents: () => [], + closeAgent: vi.fn(async () => {}), + }, + agentStorage: { + list: async () => [], + remove: vi.fn(async () => {}), + } as any, + archiveWorkspaceRecord: vi.fn(async () => {}), + emit: (msg) => emitted.push(msg), + emitWorkspaceUpdatesForCwds: vi.fn(async () => {}), + isPathWithinRoot: createIsPathWithinRoot(), + killTerminalsUnderPath: vi.fn(async () => {}), + sessionLogger: createLogger(), + }, + { + type: "paseo_worktree_archive_request", + requestId: "req-archive-orphan", + worktreePath: created.worktreePath, + }, + ); + + const response = emitted.find( + ( + message, + ): message is Extract => + message.type === "paseo_worktree_archive_response", + ); + expect(response?.payload.success).toBe(true); + expect(response?.payload.error).toBeNull(); + expect(existsSync(created.worktreePath)).toBe(false); + }); +}); diff --git a/packages/server/src/server/worktree-session.ts b/packages/server/src/server/worktree-session.ts index fd21bff89..e8f127f40 100644 --- a/packages/server/src/server/worktree-session.ts +++ b/packages/server/src/server/worktree-session.ts @@ -96,6 +96,7 @@ type ArchivePaseoWorktreeDependencies = { emitWorkspaceUpdatesForCwds: (cwds: Iterable) => Promise; isPathWithinRoot: (rootPath: string, candidatePath: string) => boolean; killTerminalsUnderPath: (rootPath: string) => Promise; + sessionLogger?: Logger; }; type RegisterPendingWorktreeWorkspaceDependencies = { @@ -169,6 +170,7 @@ type HandleCreatePaseoWorktreeRequestDependencies = { type KillTerminalsUnderPathDependencies = { isPathWithinRoot: (rootPath: string, candidatePath: string) => boolean; killTrackedTerminal: (terminalId: string, options?: { emitExit: boolean }) => void; + detachTerminalStream?: (terminalId: string, options: { emitExit: boolean }) => void; sessionLogger: Logger; terminalManager: TerminalManager | null; }; @@ -487,7 +489,8 @@ export async function archivePaseoWorktree( dependencies: ArchivePaseoWorktreeDependencies, options: { targetPath: string; - repoRoot: string; + repoRoot: string | null; + worktreesRoot?: string; requestId: string; }, ): Promise { @@ -502,53 +505,73 @@ export async function archivePaseoWorktree( const removedAgents = new Set(); const affectedWorkspaceCwds = new Set([targetPath]); const affectedWorkspaceIds = new Set([normalizePersistedWorkspaceId(targetPath)]); - const agents = dependencies.agentManager.listAgents(); - for (const agent of agents) { - if (!dependencies.isPathWithinRoot(targetPath, agent.cwd)) { - continue; - } + const liveAgents = dependencies.agentManager + .listAgents() + .filter((agent) => dependencies.isPathWithinRoot(targetPath, agent.cwd)); + for (const agent of liveAgents) { removedAgents.add(agent.id); affectedWorkspaceCwds.add(agent.cwd); affectedWorkspaceIds.add(normalizePersistedWorkspaceId(agent.cwd)); - try { - await dependencies.agentManager.closeAgent(agent.id); - } catch { - // ignore cleanup errors - } - try { - await dependencies.agentStorage.remove(agent.id); - } catch { - // ignore cleanup errors - } } - const registryRecords = await dependencies.agentStorage.list(); - for (const record of registryRecords) { - if (!dependencies.isPathWithinRoot(targetPath, record.cwd)) { - continue; - } - + const storedRecords = await dependencies.agentStorage.list().catch(() => []); + const matchingStoredRecords = storedRecords.filter((record) => + dependencies.isPathWithinRoot(targetPath, record.cwd), + ); + for (const record of matchingStoredRecords) { removedAgents.add(record.id); affectedWorkspaceCwds.add(record.cwd); affectedWorkspaceIds.add(normalizePersistedWorkspaceId(record.cwd)); - try { - await dependencies.agentStorage.remove(record.id); - } catch { - // ignore cleanup errors + } + + const agentIdsToRemoveFromStorage = new Set([ + ...liveAgents.map((agent) => agent.id), + ...matchingStoredRecords.map((record) => record.id), + ]); + + // Fan out agent close + terminal teardown concurrently. We never let a + // per-item failure abort the archive — the FS delete must still run so the + // worktree doesn't get stuck half-dead. + const teardownResults = await Promise.allSettled([ + ...liveAgents.map((agent) => dependencies.agentManager.closeAgent(agent.id)), + dependencies.killTerminalsUnderPath(targetPath), + ]); + + for (const result of teardownResults) { + if (result.status === "rejected") { + dependencies.sessionLogger?.warn( + { err: result.reason, targetPath }, + "Worktree teardown step failed during archive; continuing", + ); } } - await dependencies.killTerminalsUnderPath(targetPath); + // Agent storage removal runs after closeAgent so file handles on the agent + // state file are released; still allSettled so a single bad record can't + // derail the rest. + await Promise.allSettled( + Array.from(agentIdsToRemoveFromStorage).map((agentId) => + dependencies.agentStorage.remove(agentId), + ), + ); await deletePaseoWorktree({ cwd: options.repoRoot, worktreePath: targetPath, + worktreesRoot: options.worktreesRoot, paseoHome: dependencies.paseoHome, }); for (const workspaceId of affectedWorkspaceIds) { - await dependencies.archiveWorkspaceRecord(workspaceId); + try { + await dependencies.archiveWorkspaceRecord(workspaceId); + } catch (error) { + dependencies.sessionLogger?.warn( + { err: error, workspaceId }, + "Failed to archive workspace record; worktree FS already removed", + ); + } } for (const agentId of removedAgents) { @@ -612,14 +635,15 @@ export async function handlePaseoWorktreeArchiveRequest( return; } + // repoRoot is best-effort: if git has forgotten about the worktree we + // still proceed using the path-derived worktreesRoot, since the ownership + // check already proved the path lives under $PASEO_HOME/worktrees. repoRoot = ownership.repoRoot ?? repoRoot ?? null; - if (!repoRoot) { - throw new Error("Unable to resolve repo root for worktree"); - } const removedAgents = await archivePaseoWorktree(dependencies, { targetPath, repoRoot, + worktreesRoot: ownership.worktreeRoot, requestId, }); @@ -953,34 +977,48 @@ export async function killTerminalsUnderPath( dependencies: KillTerminalsUnderPathDependencies, rootPath: string, ): Promise { - if (!dependencies.terminalManager) { + const terminalManager = dependencies.terminalManager; + if (!terminalManager) { return; } - const cleanupErrors: Array<{ cwd: string; message: string }> = []; - const terminalDirectories = [...dependencies.terminalManager.listDirectories()]; + const terminalIds: string[] = []; + const terminalDirectories = [...terminalManager.listDirectories()]; for (const terminalCwd of terminalDirectories) { if (!dependencies.isPathWithinRoot(rootPath, terminalCwd)) { continue; } - try { - const terminals = await dependencies.terminalManager.getTerminals(terminalCwd); - for (const terminal of [...terminals]) { - dependencies.killTrackedTerminal(terminal.id, { emitExit: true }); + const terminals = await terminalManager.getTerminals(terminalCwd); + for (const terminal of terminals) { + terminalIds.push(terminal.id); } } catch (error) { - const message = error instanceof Error ? error.message : String(error); - cleanupErrors.push({ cwd: terminalCwd, message }); dependencies.sessionLogger.warn( { err: error, cwd: terminalCwd }, - "Failed to clean up worktree terminals during archive", + "Failed to enumerate worktree terminals during archive", ); } } - if (cleanupErrors.length > 0) { - const details = cleanupErrors.map((entry) => `${entry.cwd}: ${entry.message}`).join("; "); - throw new Error(`Failed to clean up worktree terminals during archive (${details})`); + if (terminalIds.length === 0) { + return; } + + await Promise.allSettled( + terminalIds.map(async (terminalId) => { + try { + dependencies.detachTerminalStream?.(terminalId, { emitExit: true }); + await terminalManager.killTerminalAndWait(terminalId, { + gracefulTimeoutMs: 2000, + forceTimeoutMs: 1500, + }); + } catch (error) { + dependencies.sessionLogger.warn( + { err: error, terminalId }, + "Terminal kill escalation failed during archive; proceeding anyway", + ); + } + }), + ); } diff --git a/packages/server/src/terminal/terminal-manager.ts b/packages/server/src/terminal/terminal-manager.ts index 6dc49fe11..6916bacbd 100644 --- a/packages/server/src/terminal/terminal-manager.ts +++ b/packages/server/src/terminal/terminal-manager.ts @@ -28,6 +28,10 @@ export interface TerminalManager { registerCwdEnv(options: { cwd: string; env: Record }): void; getTerminal(id: string): TerminalSession | undefined; killTerminal(id: string): void; + killTerminalAndWait( + id: string, + options?: { gracefulTimeoutMs?: number; forceTimeoutMs?: number }, + ): Promise; listDirectories(): string[]; killAll(): void; subscribeTerminalsChanged(listener: TerminalsChangedListener): () => void; @@ -200,6 +204,21 @@ export function createTerminalManager(): TerminalManager { removeSessionById(id, { kill: true }); }, + async killTerminalAndWait( + id: string, + options?: { gracefulTimeoutMs?: number; forceTimeoutMs?: number }, + ): Promise { + const session = terminalsById.get(id); + if (!session) { + return; + } + try { + await session.killAndWait(options); + } finally { + removeSessionById(id, { kill: false }); + } + }, + listDirectories(): string[] { return Array.from(terminalsByCwd.keys()); }, diff --git a/packages/server/src/terminal/terminal.ts b/packages/server/src/terminal/terminal.ts index 7b0141a76..039459c1c 100644 --- a/packages/server/src/terminal/terminal.ts +++ b/packages/server/src/terminal/terminal.ts @@ -44,6 +44,7 @@ export interface TerminalSession { getTitle(): string | undefined; getExitInfo(): TerminalExitInfo | null; kill(): void; + killAndWait(options?: { gracefulTimeoutMs?: number; forceTimeoutMs?: number }): Promise; } export interface CreateTerminalOptions { @@ -527,6 +528,8 @@ export async function createTerminal(options: CreateTerminalOptions): Promise void>(); let exitInfo: TerminalExitInfo | null = null; let recentOutputText = ""; let title: string | undefined; @@ -670,6 +673,15 @@ export async function createTerminal(options: CreateTerminalOptions): Promise { killed = true; + processExited = true; + for (const waiter of Array.from(processExitWaiters)) { + try { + waiter(); + } catch { + // no-op + } + } + processExitWaiters.clear(); emitExit( buildExitInfo({ exitCode: event.exitCode, @@ -783,6 +795,55 @@ export async function createTerminal(options: CreateTerminalOptions): Promise { + if (processExited) { + return Promise.resolve(true); + } + return new Promise((resolve) => { + const waiter = (): void => { + clearTimeout(timer); + resolve(true); + }; + const timer = setTimeout(() => { + processExitWaiters.delete(waiter); + resolve(false); + }, timeoutMs); + processExitWaiters.add(waiter); + }); + } + + async function killAndWait(options?: { + gracefulTimeoutMs?: number; + forceTimeoutMs?: number; + }): Promise { + const gracefulTimeoutMs = options?.gracefulTimeoutMs ?? 2000; + const forceTimeoutMs = options?.forceTimeoutMs ?? 1000; + + if (processExited) { + kill(); + return; + } + + try { + ptyProcess.kill(); + } catch { + // process may already be gone + } + + const exitedGracefully = await waitForProcessExit(gracefulTimeoutMs); + if (!exitedGracefully) { + try { + ptyProcess.kill("SIGKILL"); + } catch { + // process may already be gone + } + await waitForProcessExit(forceTimeoutMs); + } + + // Finalize bookkeeping (idempotent if ptyProcess.onExit already fired). + kill(); + } + // Small delay to let shell initialize await new Promise((resolve) => setTimeout(resolve, 50)); @@ -799,5 +860,6 @@ export async function createTerminal(options: CreateTerminalOptions): Promise { deletePaseoWorktree({ cwd: repoDir, worktreePath: created.worktreePath, paseoHome }), ).resolves.toBeUndefined(); }); + + it("deletes a worktree when the parent repo root is not available", async () => { + const created = await createWorktree({ + branchName: "no-cwd-branch", + cwd: repoDir, + baseBranch: "main", + worktreeSlug: "no-cwd", + paseoHome, + }); + + const ownership = await isPaseoOwnedWorktreeCwd(created.worktreePath, { paseoHome }); + expect(ownership.allowed).toBe(true); + expect(ownership.worktreeRoot).toBeTruthy(); + + // Simulate the handler path when git has forgotten about the worktree: + // caller forwards the path-derived worktreesRoot from the ownership check. + await deletePaseoWorktree({ + cwd: null, + worktreePath: created.worktreePath, + worktreesRoot: ownership.worktreeRoot, + paseoHome, + }); + + expect(existsSync(created.worktreePath)).toBe(false); + }); }); describe("slugify", () => { diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 095b66925..5dc1b44a8 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -1,6 +1,7 @@ import { execFile, spawn } from "child_process"; import { promisify } from "util"; import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, statSync } from "fs"; +import { rm, stat } from "fs/promises"; import { join, basename, dirname, resolve, sep } from "path"; import net from "node:net"; import { createHash } from "node:crypto"; @@ -1027,20 +1028,33 @@ export async function deletePaseoWorktree({ cwd, worktreePath, worktreeSlug, + worktreesRoot, paseoHome, }: { - cwd: string; + cwd: string | null; worktreePath?: string; worktreeSlug?: string; + worktreesRoot?: string; paseoHome?: string; }): Promise { if (!worktreePath && !worktreeSlug) { throw new Error("worktreePath or worktreeSlug is required"); } - const worktreesRoot = await getPaseoWorktreesRoot(cwd, paseoHome); - const resolvedRoot = normalizePathForOwnership(worktreesRoot) + sep; - const requestedPath = worktreePath ?? join(worktreesRoot, worktreeSlug!); + // Resolve the worktrees-root. With a repo cwd we hash it the normal way; if + // git has forgotten about the worktree we expect the caller to hand us the + // path-derived worktreesRoot from the ownership check. + let resolvedWorktreesRoot: string; + if (worktreesRoot) { + resolvedWorktreesRoot = worktreesRoot; + } else if (cwd) { + resolvedWorktreesRoot = await getPaseoWorktreesRoot(cwd, paseoHome); + } else { + throw new Error("cwd or worktreesRoot is required to delete a Paseo worktree"); + } + + const resolvedRoot = normalizePathForOwnership(resolvedWorktreesRoot) + sep; + const requestedPath = worktreePath ?? join(resolvedWorktreesRoot, worktreeSlug!); const resolvedRequested = normalizePathForOwnership(requestedPath); const resolvedWorktree = (await resolvePaseoWorktreeRootForCwd(requestedPath, { paseoHome }))?.worktreePath ?? @@ -1050,32 +1064,72 @@ export async function deletePaseoWorktree({ throw new Error("Refusing to delete non-Paseo worktree"); } - if (existsSync(resolvedWorktree)) { + if (await pathExists(resolvedWorktree)) { await runWorktreeTeardownCommands({ worktreePath: resolvedWorktree, }); } - try { - await runGitCommand(["worktree", "remove", resolvedWorktree, "--force"], { - cwd, - timeout: 120_000, - }); - } catch { - // `git worktree remove` fails if the admin dir is already gone (e.g. a prior - // archive attempt removed it before the working tree could be fully cleaned - // up). Fall through to the rmSync fallback below so the retry is idempotent. + if (cwd) { + try { + await runGitCommand(["worktree", "remove", resolvedWorktree, "--force"], { + cwd, + timeout: 120_000, + }); + } catch { + // `git worktree remove` fails if the admin dir is already gone (e.g. a + // prior archive attempt removed it before the working tree could be + // fully cleaned up), or if the repo root has moved. Fall through to the + // rm retry loop below so the operation stays idempotent. + } } - if (existsSync(resolvedWorktree)) { - rmSync(resolvedWorktree, { recursive: true, force: true }); + await removeDirectoryWithRetries(resolvedWorktree); + + if (cwd) { + try { + await runGitCommand(["worktree", "prune"], { cwd, timeout: 30_000 }); + } catch { + // not critical; git will prune lazily + } + } +} + +async function pathExists(path: string): Promise { + try { + await stat(path); + return true; + } catch { + return false; + } +} + +async function removeDirectoryWithRetries(path: string): Promise { + if (!(await pathExists(path))) { + return; } - // Clean up any stale admin entries git may still have cached for this path. - try { - await runGitCommand(["worktree", "prune"], { cwd, timeout: 30_000 }); - } catch { - // not critical; git will prune lazily + const delaysMs = [0, 100, 300, 700, 1500]; + let lastError: unknown = null; + for (const delay of delaysMs) { + if (delay > 0) { + await new Promise((resolve) => setTimeout(resolve, delay)); + } + try { + await rm(path, { recursive: true, force: true }); + if (!(await pathExists(path))) { + return; + } + lastError = new Error(`Directory still present after rm: ${path}`); + } catch (error) { + lastError = error; + } + } + + if (await pathExists(path)) { + throw lastError instanceof Error + ? lastError + : new Error(`Failed to remove worktree directory: ${path}`); } }