From 7c0ce3ef66fdda18e544704201908d114859778f Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 19 Jan 2026 23:50:35 -0500 Subject: [PATCH] fix: handle non-existent workspace repo in checkpoint cleanup - Check if workspace repo directory exists before git operations in deleteTask() - Reduce log verbosity - only log actual errors, not expected cases - Add test case for non-existent directory handling --- src/core/webview/ClineProvider.ts | 9 +++---- .../checkpoints/ShadowCheckpointService.ts | 15 ++++++----- .../__tests__/ShadowCheckpointService.spec.ts | 25 +++++++++++++++++++ 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index bbe8d388a7..1567d4864a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1802,8 +1802,8 @@ export class ClineProvider try { await ShadowCheckpointService.deleteTask({ taskId, globalStorageDir, workspaceDir }) } catch (error) { - console.error( - `[deleteTaskWithId${taskId}] failed to delete associated shadow repository or branch: ${error instanceof Error ? error.message : String(error)}`, + this.log( + `[deleteTaskWithId] failed to delete shadow repository for task ${taskId}: ${error instanceof Error ? error.message : String(error)}`, ) } @@ -1811,10 +1811,9 @@ export class ClineProvider try { const dirPath = await getTaskDirectoryPath(globalStoragePath, taskId) await fs.rm(dirPath, { recursive: true, force: true }) - console.log(`[deleteTaskWithId${taskId}] removed task directory`) } catch (error) { - console.error( - `[deleteTaskWithId${taskId}] failed to remove task directory: ${error instanceof Error ? error.message : String(error)}`, + this.log( + `[deleteTaskWithId] failed to remove task directory for task ${taskId}: ${error instanceof Error ? error.message : String(error)}`, ) } } diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index fee08b2fa4..417ba9d887 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -449,22 +449,21 @@ export abstract class ShadowCheckpointService extends EventEmitter { workspaceDir: string }) { const workspaceRepoDir = this.workspaceRepoDir({ globalStorageDir, workspaceDir }) + + // Check if the workspace repo directory exists before attempting git operations + if (!(await fileExistsAtPath(workspaceRepoDir))) { + return + } + const branchName = `roo-${taskId}` const git = createSanitizedGit(workspaceRepoDir) - const success = await this.deleteBranch(git, branchName) - - if (success) { - console.log(`[${this.name}#deleteTask.${taskId}] deleted branch ${branchName}`) - } else { - console.error(`[${this.name}#deleteTask.${taskId}] failed to delete branch ${branchName}`) - } + await this.deleteBranch(git, branchName) } public static async deleteBranch(git: SimpleGit, branchName: string) { const branches = await git.branchLocal() if (!branches.all.includes(branchName)) { - console.error(`[${this.constructor.name}#deleteBranch] branch ${branchName} does not exist`) return false } diff --git a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts index 07efa4ac2c..7c531b6d3b 100644 --- a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts +++ b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts @@ -928,5 +928,30 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( } }) }) + + describe(`${klass.name}#deleteTask`, () => { + it("handles non-existent workspace repo directory gracefully", async () => { + const nonExistentWorkspaceDir = path.join(tmpDir, `non-existent-workspace-${Date.now()}`) + const nonExistentGlobalStorageDir = path.join(tmpDir, `non-existent-storage-${Date.now()}`) + const taskIdToDelete = "non-existent-task" + + // Verify the workspace repo directory doesn't exist + const workspaceRepoDir = path.join( + nonExistentGlobalStorageDir, + "checkpoints", + klass.hashWorkspaceDir(nonExistentWorkspaceDir), + ) + expect(await fileExistsAtPath(workspaceRepoDir)).toBe(false) + + // Should not throw when the directory doesn't exist + await expect( + klass.deleteTask({ + taskId: taskIdToDelete, + globalStorageDir: nonExistentGlobalStorageDir, + workspaceDir: nonExistentWorkspaceDir, + }), + ).resolves.not.toThrow() + }) + }) }, )