Skip to content

Commit 784e3ad

Browse files
authored
fix(archive): don't let a broken worktree block archiving (#3049)
1 parent 85deab0 commit 784e3ad

2 files changed

Lines changed: 117 additions & 24 deletions

File tree

packages/workspace-server/src/services/archive/archive.integration.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,67 @@ describe("ArchiveService integration", () => {
395395
expect(archived.branchName).toBeNull();
396396
expect(ctx.archiveRepo.findAll()).toHaveLength(1);
397397
}));
398+
399+
it("archive succeeds when worktree has an in-progress merge conflict", () =>
400+
withTestContext({}, async (ctx) => {
401+
const { worktreePath } = await ctx.setupWorktree("detached");
402+
403+
const wt = (cmd: string) =>
404+
execSync(`git ${cmd}`, {
405+
cwd: worktreePath,
406+
encoding: "utf8",
407+
stdio: "pipe",
408+
});
409+
410+
await fs.writeFile(path.join(worktreePath, "c.txt"), "base\n");
411+
wt("add c.txt");
412+
wt("commit -m base");
413+
wt("tag base_commit");
414+
415+
await fs.writeFile(path.join(worktreePath, "c.txt"), "AAA\n");
416+
wt("add c.txt");
417+
wt("commit -m a");
418+
wt("tag commit_a");
419+
420+
wt("reset --hard base_commit");
421+
await fs.writeFile(path.join(worktreePath, "c.txt"), "BBB\n");
422+
wt("add c.txt");
423+
wt("commit -m b");
424+
425+
wt("merge commit_a || true");
426+
427+
const archived = await ctx.service.archiveTask(ctx.archiveInput());
428+
429+
expect(archived.checkpointId).toBeNull();
430+
expect(await pathExists(worktreePath)).toBe(false);
431+
expect(ctx.archiveRepo.findAll()).toHaveLength(1);
432+
}));
433+
434+
it("archive succeeds and drops the checkpoint when worktree removal fails", () =>
435+
withTestContext({}, async (ctx) => {
436+
await ctx.setupWorktree("detached");
437+
438+
// Simulate git refusing to remove the worktree (e.g. a stale lock).
439+
// Capture succeeded, so the checkpoint ref exists — but since the
440+
// worktree stays registered, keeping the restore point would make a
441+
// later unarchive fail to re-add it. The archive must still be recorded
442+
// and its checkpoint dropped.
443+
const deleteSpy = vi
444+
.spyOn(WorktreeManager.prototype, "deleteWorktree")
445+
.mockRejectedValue(new Error("worktree is locked"));
446+
447+
try {
448+
const archived = await ctx.service.archiveTask(ctx.archiveInput());
449+
450+
expect(deleteSpy).toHaveBeenCalled();
451+
expect(archived.checkpointId).toBeNull();
452+
const records = ctx.archiveRepo.findAll();
453+
expect(records).toHaveLength(1);
454+
expect(records[0].checkpointId).toBeNull();
455+
} finally {
456+
deleteSpy.mockRestore();
457+
}
458+
}));
398459
});
399460

400461
describe("local/cloud mode", () => {

packages/workspace-server/src/services/archive/archive.ts

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -246,24 +246,30 @@ export class ArchiveService {
246246
archivedTask.branchName = actualBranch;
247247
}
248248

249-
await step(
250-
async () => {
251-
if (!archivedTask.checkpointId) {
252-
throw new Error("checkpointId must be set for worktree mode");
253-
}
254-
await this.captureWorktreeCheckpoint(
255-
folderPath,
256-
worktreePath,
257-
archivedTask.checkpointId,
258-
);
259-
},
260-
async () => {
261-
if (archivedTask.checkpointId) {
249+
const checkpointId = archivedTask.checkpointId;
250+
try {
251+
if (!checkpointId) {
252+
throw new Error("checkpointId must be set for worktree mode");
253+
}
254+
await step(
255+
() =>
256+
this.captureWorktreeCheckpoint(
257+
folderPath,
258+
worktreePath,
259+
checkpointId,
260+
),
261+
async () => {
262262
const git = createGitClient(folderPath);
263-
await deleteCheckpoint(git, archivedTask.checkpointId);
264-
}
265-
},
266-
);
263+
await deleteCheckpoint(git, checkpointId);
264+
},
265+
);
266+
} catch (error) {
267+
this.log.warn(
268+
`Failed to capture checkpoint for ${worktreePath}; archiving without a restore point`,
269+
{ error },
270+
);
271+
archivedTask.checkpointId = null;
272+
}
267273
}
268274

269275
await step(
@@ -277,13 +283,39 @@ export class ArchiveService {
277283

278284
await step(
279285
async () => {
280-
const manager = new WorktreeManager({
281-
mainRepoPath: folderPath,
282-
worktreeBasePath: this.workspaceSettings.getWorktreeLocation(),
283-
});
284-
await manager.deleteWorktree(worktreePath);
285-
const parentDir = path.dirname(worktreePath);
286-
await forceRemove(parentDir);
286+
try {
287+
const manager = new WorktreeManager({
288+
mainRepoPath: folderPath,
289+
worktreeBasePath: this.workspaceSettings.getWorktreeLocation(),
290+
});
291+
await manager.deleteWorktree(worktreePath);
292+
const parentDir = path.dirname(worktreePath);
293+
await forceRemove(parentDir);
294+
} catch (error) {
295+
this.log.warn(
296+
`Failed to remove worktree at ${worktreePath}; archiving anyway (on-disk worktree may need manual cleanup)`,
297+
{ error },
298+
);
299+
// The worktree is still registered under its original name, so a
300+
// later unarchive can't re-add it from the checkpoint (git rejects
301+
// the duplicate name/path), leaving the task un-restorable. Drop
302+
// the restore point — and its now-orphaned checkpoint ref — so the
303+
// archive record stays internally consistent, matching how a
304+
// failed capture above already sets checkpointId to null.
305+
const orphanedCheckpointId = archivedTask.checkpointId;
306+
if (orphanedCheckpointId) {
307+
archivedTask.checkpointId = null;
308+
try {
309+
const git = createGitClient(folderPath);
310+
await deleteCheckpoint(git, orphanedCheckpointId);
311+
} catch (cleanupError) {
312+
this.log.warn(
313+
`Failed to delete orphaned checkpoint ${orphanedCheckpointId}`,
314+
{ error: cleanupError },
315+
);
316+
}
317+
}
318+
}
287319
},
288320
async () => {},
289321
);

0 commit comments

Comments
 (0)