fix: archive worktrees even when teardown fails

* Archive workspace record even if teardown fails

* Fix teardown test for cross-platform commands
This commit is contained in:
Matt Cowger
2026-06-01 15:34:51 +08:00
committed by GitHub
parent ae5dfc0b3c
commit b8ccd543c4
2 changed files with 73 additions and 22 deletions
@@ -5,7 +5,11 @@ import type { AgentStorage, StoredAgentRecord } from "./agent/agent-storage.js";
import type { WorkspaceGitService } from "./workspace-git-service.js"; import type { WorkspaceGitService } from "./workspace-git-service.js";
import { normalizeWorkspaceId as normalizePersistedWorkspaceId } from "./workspace-registry-model.js"; import { normalizeWorkspaceId as normalizePersistedWorkspaceId } from "./workspace-registry-model.js";
import type { GitHubService } from "../services/github-service.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"; import type { TerminalManager } from "../terminal/terminal-manager.js";
export interface ArchivePaseoWorktreeDependencies { export interface ArchivePaseoWorktreeDependencies {
@@ -104,14 +108,27 @@ export async function archivePaseoWorktree(
} }
} }
await deletePaseoWorktree({ let teardownError: WorktreeTeardownError | null = null;
cwd: options.repoRoot, try {
worktreePath: targetPath, await deletePaseoWorktree({
worktreesRoot: options.worktreesRoot, cwd: options.repoRoot,
paseoHome: dependencies.paseoHome, 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 { try {
await dependencies.workspaceGitService.getSnapshot(options.repoRoot, { await dependencies.workspaceGitService.getSnapshot(options.repoRoot, {
force: true, force: true,
@@ -136,11 +153,17 @@ export async function archivePaseoWorktree(
} catch (error) { } catch (error) {
dependencies.sessionLogger?.warn( dependencies.sessionLogger?.warn(
{ err: error, workspaceId }, { 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 { } finally {
dependencies.clearWorkspaceArchiving(affectedWorkspaceIdList); dependencies.clearWorkspaceArchiving(affectedWorkspaceIdList);
await dependencies.emitWorkspaceUpdatesForWorkspaceIds(affectedWorkspaceIdList); await dependencies.emitWorkspaceUpdatesForWorkspaceIds(affectedWorkspaceIdList);
@@ -1935,8 +1935,20 @@ describe("archivePaseoWorktree", () => {
}); });
}); });
test("clears archiving state and leaves workspace records active when worktree delete fails", async () => { test("archives the workspace record even when the teardown script fails", async () => {
const { tempDir, repoDir } = createGitRepo(); 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); cleanupPaths.push(tempDir);
const paseoHome = path.join(tempDir, ".paseo"); const paseoHome = path.join(tempDir, ".paseo");
@@ -1949,12 +1961,21 @@ describe("archivePaseoWorktree", () => {
paseoHome, paseoHome,
}); });
const archivingByWorkspaceId = new Map<string, string>(); const archivingByWorkspaceId = new Map<string, string>();
const emittedUpdates: Array<{ const archivedWorkspaceIds = new Set<string>();
kind: "upsert"; const emittedUpdates: Array<
workspaceId: string; | {
archivingAt: string | null; kind: "upsert";
}> = []; workspaceId: string;
const archiveWorkspaceRecord = vi.fn(async () => {}); archivingAt: string | null;
}
| {
kind: "remove";
workspaceId: string;
}
> = [];
const archiveWorkspaceRecord = vi.fn(async (workspaceId: string) => {
archivedWorkspaceIds.add(workspaceId);
});
await expect( await expect(
archivePaseoWorktree( archivePaseoWorktree(
@@ -1973,6 +1994,13 @@ describe("archivePaseoWorktree", () => {
archiveWorkspaceRecord, archiveWorkspaceRecord,
emitWorkspaceUpdatesForWorkspaceIds: vi.fn(async (workspaceIds: Iterable<string>) => { emitWorkspaceUpdatesForWorkspaceIds: vi.fn(async (workspaceIds: Iterable<string>) => {
for (const workspaceId of workspaceIds) { for (const workspaceId of workspaceIds) {
if (archivedWorkspaceIds.has(workspaceId)) {
emittedUpdates.push({
kind: "remove",
workspaceId,
});
continue;
}
emittedUpdates.push({ emittedUpdates.push({
kind: "upsert", kind: "upsert",
workspaceId, workspaceId,
@@ -1996,23 +2024,23 @@ describe("archivePaseoWorktree", () => {
}, },
{ {
targetPath: created.worktreePath, targetPath: created.worktreePath,
repoRoot: null, repoRoot: repoDir,
requestId: "req-archive-delete-fails", 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(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({ expect(emittedUpdates[0]).toEqual({
kind: "upsert", kind: "upsert",
workspaceId: created.worktreePath, workspaceId: created.worktreePath,
archivingAt: expect.any(String), archivingAt: expect.any(String),
}); });
expect(emittedUpdates.at(-1)).toEqual({ expect(emittedUpdates.at(-1)).toEqual({
kind: "upsert", kind: "remove",
workspaceId: created.worktreePath, workspaceId: created.worktreePath,
archivingAt: null,
}); });
}); });