From b8ccd543c4ab5bae437254d7d07810c9a46f72be Mon Sep 17 00:00:00 2001 From: Matt Cowger Date: Mon, 1 Jun 2026 00:34:51 -0700 Subject: [PATCH] fix: archive worktrees even when teardown fails * Archive workspace record even if teardown fails * Fix teardown test for cross-platform commands --- .../server/paseo-worktree-archive-service.ts | 41 ++++++++++---- .../src/server/worktree-session.test.ts | 54 ++++++++++++++----- 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/packages/server/src/server/paseo-worktree-archive-service.ts b/packages/server/src/server/paseo-worktree-archive-service.ts index b3d4daf55..7f271e0e0 100644 --- a/packages/server/src/server/paseo-worktree-archive-service.ts +++ b/packages/server/src/server/paseo-worktree-archive-service.ts @@ -5,7 +5,11 @@ import type { AgentStorage, StoredAgentRecord } from "./agent/agent-storage.js"; import type { WorkspaceGitService } from "./workspace-git-service.js"; import { normalizeWorkspaceId as normalizePersistedWorkspaceId } from "./workspace-registry-model.js"; import type { GitHubService } from "../services/github-service.js"; -import { deletePaseoWorktree, resolvePaseoWorktreeRootForCwd } from "../utils/worktree.js"; +import { + deletePaseoWorktree, + resolvePaseoWorktreeRootForCwd, + WorktreeTeardownError, +} from "../utils/worktree.js"; import type { TerminalManager } from "../terminal/terminal-manager.js"; export interface ArchivePaseoWorktreeDependencies { @@ -104,14 +108,27 @@ export async function archivePaseoWorktree( } } - await deletePaseoWorktree({ - cwd: options.repoRoot, - worktreePath: targetPath, - worktreesRoot: options.worktreesRoot, - paseoHome: dependencies.paseoHome, - }); + let teardownError: WorktreeTeardownError | null = null; + try { + await deletePaseoWorktree({ + cwd: options.repoRoot, + worktreePath: targetPath, + worktreesRoot: options.worktreesRoot, + paseoHome: dependencies.paseoHome, + }); + } catch (error) { + if (error instanceof WorktreeTeardownError) { + teardownError = error; + dependencies.sessionLogger?.warn( + { err: error, targetPath }, + "Worktree teardown failed during archive; archiving workspace record anyway", + ); + } else { + throw error; + } + } - if (options.repoRoot) { + if (!teardownError && options.repoRoot) { try { await dependencies.workspaceGitService.getSnapshot(options.repoRoot, { force: true, @@ -136,11 +153,17 @@ export async function archivePaseoWorktree( } catch (error) { dependencies.sessionLogger?.warn( { err: error, workspaceId }, - "Failed to archive workspace record; worktree FS already removed", + teardownError + ? "Failed to archive workspace record after teardown failed" + : "Failed to archive workspace record; worktree FS already removed", ); } }), ); + + if (teardownError) { + throw teardownError; + } } finally { dependencies.clearWorkspaceArchiving(affectedWorkspaceIdList); await dependencies.emitWorkspaceUpdatesForWorkspaceIds(affectedWorkspaceIdList); diff --git a/packages/server/src/server/worktree-session.test.ts b/packages/server/src/server/worktree-session.test.ts index 3a099d45c..04f7c2592 100644 --- a/packages/server/src/server/worktree-session.test.ts +++ b/packages/server/src/server/worktree-session.test.ts @@ -1935,8 +1935,20 @@ describe("archivePaseoWorktree", () => { }); }); - test("clears archiving state and leaves workspace records active when worktree delete fails", async () => { - const { tempDir, repoDir } = createGitRepo(); + test("archives the workspace record even when the teardown script fails", async () => { + const teardownLogPath = isPlatform("win32") + ? 'Set-Content -Path (Join-Path $env:PASEO_SOURCE_CHECKOUT_PATH "teardown-start.log") -Value "started"' + : 'echo "started" > "$PASEO_SOURCE_CHECKOUT_PATH/teardown-start.log"'; + const failingTeardownCommand = isPlatform("win32") + ? 'Write-Error "boom"; exit 9' + : "echo boom 1>&2; exit 9"; + const { tempDir, repoDir } = createGitRepo({ + paseoConfig: { + worktree: { + teardown: [teardownLogPath, failingTeardownCommand], + }, + }, + }); cleanupPaths.push(tempDir); const paseoHome = path.join(tempDir, ".paseo"); @@ -1949,12 +1961,21 @@ describe("archivePaseoWorktree", () => { paseoHome, }); const archivingByWorkspaceId = new Map(); - const emittedUpdates: Array<{ - kind: "upsert"; - workspaceId: string; - archivingAt: string | null; - }> = []; - const archiveWorkspaceRecord = vi.fn(async () => {}); + const archivedWorkspaceIds = new Set(); + const emittedUpdates: Array< + | { + kind: "upsert"; + workspaceId: string; + archivingAt: string | null; + } + | { + kind: "remove"; + workspaceId: string; + } + > = []; + const archiveWorkspaceRecord = vi.fn(async (workspaceId: string) => { + archivedWorkspaceIds.add(workspaceId); + }); await expect( archivePaseoWorktree( @@ -1973,6 +1994,13 @@ describe("archivePaseoWorktree", () => { archiveWorkspaceRecord, emitWorkspaceUpdatesForWorkspaceIds: vi.fn(async (workspaceIds: Iterable) => { for (const workspaceId of workspaceIds) { + if (archivedWorkspaceIds.has(workspaceId)) { + emittedUpdates.push({ + kind: "remove", + workspaceId, + }); + continue; + } emittedUpdates.push({ kind: "upsert", workspaceId, @@ -1996,23 +2024,23 @@ describe("archivePaseoWorktree", () => { }, { targetPath: created.worktreePath, - repoRoot: null, + repoRoot: repoDir, requestId: "req-archive-delete-fails", }, ), - ).rejects.toThrow("cwd or worktreesRoot is required to delete a Paseo worktree"); + ).rejects.toThrow("Worktree teardown command failed"); expect(existsSync(created.worktreePath)).toBe(true); - expect(archiveWorkspaceRecord).not.toHaveBeenCalled(); + expect(existsSync(path.join(repoDir, "teardown-start.log"))).toBe(true); + expect(archiveWorkspaceRecord).toHaveBeenCalledWith(created.worktreePath); expect(emittedUpdates[0]).toEqual({ kind: "upsert", workspaceId: created.worktreePath, archivingAt: expect.any(String), }); expect(emittedUpdates.at(-1)).toEqual({ - kind: "upsert", + kind: "remove", workspaceId: created.worktreePath, - archivingAt: null, }); });