From ca54d1e2fe214654adf62f0b8ec61772ee310349 Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 10 Feb 2025 23:51:51 -0800 Subject: [PATCH 1/5] Stashless checkpoints --- src/services/checkpoints/CheckpointService.ts | 130 +++++------------- 1 file changed, 31 insertions(+), 99 deletions(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 672edfd0cc3..438b084ad70 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -51,6 +51,8 @@ export type CheckpointServiceOptions = { export class CheckpointService { private static readonly USER_NAME = "Roo Code" private static readonly USER_EMAIL = "support@roocode.com" + private static readonly CHECKPOINT_BRANCH = "roo-code-checkpoints" + private static readonly STASH_BRANCH = "roo-code-stash" private _currentCheckpoint?: string @@ -72,39 +74,6 @@ export class CheckpointService { private readonly log: (message: string) => void, ) {} - private async pushStash() { - const status = await this.git.status() - - if (status.files.length > 0) { - await this.git.stash(["-u"]) // Includes tracked and untracked files. - return true - } - - return false - } - - private async applyStash() { - const stashList = await this.git.stashList() - - if (stashList.all.length > 0) { - await this.git.stash(["apply"]) // Applies the most recent stash only. - return true - } - - return false - } - - private async popStash() { - const stashList = await this.git.stashList() - - if (stashList.all.length > 0) { - await this.git.stash(["pop", "--index"]) // Pops the most recent stash only. - return true - } - - return false - } - private async ensureBranch(expectedBranch: string) { const branch = await this.git.revparse(["--abbrev-ref", "HEAD"]) @@ -156,84 +125,47 @@ export class CheckpointService { public async saveCheckpoint(message: string) { await this.ensureBranch(this.mainBranch) - // Attempt to stash pending changes (including untracked files). - const pendingChanges = await this.pushStash() - - // Get the latest commit on the hidden branch before we reset it. - const latestHash = await this.git.revparse([this.hiddenBranch]) - - // Check if there is any diff relative to the latest commit. - if (!pendingChanges) { - const diff = await this.git.diff([latestHash]) - - if (!diff) { - this.log(`[saveCheckpoint] No changes detected, giving up`) - return undefined - } - } - - await this.git.checkout(this.hiddenBranch) - - const reset = async () => { - await this.git.reset(["HEAD", "."]) - await this.git.clean([CleanOptions.FORCE, CleanOptions.RECURSIVE]) - await this.git.reset(["--hard", latestHash]) - await this.git.checkout(this.mainBranch) - await this.popStash() - } + // Create temporary branch with all current changes + const tempBranch = `${CheckpointService.STASH_BRANCH}-${Date.now()}` + await this.git.checkout(["-b", tempBranch]) try { - // Reset hidden branch to match main and apply the pending changes. - await this.git.reset(["--hard", this.mainBranch]) - - if (pendingChanges) { - await this.applyStash() - } - - // Using "-A" ensures that deletions are staged as well. + // Stage and commit all changes to temporary branch await this.git.add(["-A"]) - const diff = await this.git.diff([latestHash]) - - if (!diff) { - this.log(`[saveCheckpoint] No changes detected, resetting and giving up`) - await reset() - return undefined - } - - // Otherwise, commit the changes. - const status = await this.git.status() - this.log(`[saveCheckpoint] Changes detected, committing ${JSON.stringify(status)}`) - - // Allow empty commits in order to correctly handle deletion of - // untracked files (see unit tests for an example of this). - // Additionally, skip pre-commit hooks so that they don't slow - // things down or tamper with the contents of the commit. - const commit = await this.git.commit(message, undefined, { + await this.git.commit(message, undefined, { "--allow-empty": null, "--no-verify": null, }) - await this.git.checkout(this.mainBranch) + // Get the latest commit on the hidden branch before we reset it + const latestHash = await this.git.revparse([this.hiddenBranch]) + await this.git.checkout(this.hiddenBranch) - if (pendingChanges) { - await this.popStash() - } + try { + // Reset hidden branch to match main and apply the changes + await this.git.reset(["--hard", this.mainBranch]) - this.currentCheckpoint = commit.commit + // Cherry-pick the temporary commit + const commit = await this.git.raw(["cherry-pick", tempBranch]) - return commit - } catch (err) { - this.log(`[saveCheckpoint] Failed to save checkpoint: ${err instanceof Error ? err.message : String(err)}`) + // Return to main branch and cleanup + await this.git.checkout(this.mainBranch) + await this.git.branch(["-D", tempBranch]) - // If we're not on the main branch then we need to trigger a reset - // to return to the main branch and restore it's previous state. - const currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"]) - - if (currentBranch.trim() !== this.mainBranch) { - await reset() + this.currentCheckpoint = commit + return { commit } + } catch (err) { + // If something went wrong after switching to hidden branch + await this.git.reset(["--hard", latestHash]) + await this.git.checkout(["-f", this.mainBranch]) + await this.git.branch(["-D", tempBranch]) + throw err } - - throw err + } catch (err) { + // If something went wrong before switching to hidden branch + await this.git.checkout(["-f", this.mainBranch]) + await this.git.branch(["-D", tempBranch]) + throw new Error(`Failed to save checkpoint: ${err instanceof Error ? err.message : String(err)}`) } } From 9d8360663eef9d15931eed59464e99957d5d8572 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 03:16:49 -0800 Subject: [PATCH 2/5] New checkpoints algorithm --- src/services/checkpoints/CheckpointService.ts | 220 +++++++++++++++--- .../__tests__/CheckpointService.test.ts | 5 +- 2 files changed, 186 insertions(+), 39 deletions(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 438b084ad70..3d8026923ce 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -11,6 +11,12 @@ export type CheckpointServiceOptions = { log?: (message: string) => void } +export type StagedFile = { + path: string + isStaged: boolean + isPartiallyStaged: boolean +} + /** * The CheckpointService provides a mechanism for storing a snapshot of the * current VSCode workspace each time a Roo Code tool is executed. It uses Git @@ -23,11 +29,11 @@ export type CheckpointServiceOptions = { * - A hidden branch for storing checkpoints. * * Saving a checkpoint: - * - Current changes are stashed (including untracked files). + * - A temporary branch is created to store the current state. + * - All changes (including untracked files) are staged and committed on the temp branch. * - The hidden branch is reset to match main. - * - Stashed changes are applied and committed as a checkpoint on the hidden - * branch. - * - We return to the main branch with the original state restored. + * - The temporary branch commit is cherry-picked onto the hidden branch. + * - The workspace is restored to its original state and the temp branch is deleted. * * Restoring a checkpoint: * - The workspace is restored to the state of the specified checkpoint using @@ -37,6 +43,7 @@ export type CheckpointServiceOptions = { * - Non-destructive version control (main branch remains untouched). * - Preservation of the full history of checkpoints. * - Safe restoration to any previous checkpoint. + * - Atomic checkpoint operations with proper error recovery. * * NOTES * @@ -122,51 +129,188 @@ export class CheckpointService { return result } + private async restoreMain({ + branch, + stashSha, + force = false, + }: { + branch: string + stashSha: string + force?: boolean + }) { + if (force) { + await this.git.checkout(["-f", this.mainBranch]) + } else { + await this.git.checkout(this.mainBranch) + } + + if (stashSha) { + console.log(`[restoreMain] applying stash ${stashSha}`) + await this.git.raw(["stash", "apply", "--index", stashSha]) + } + + console.log(`[restoreMain] restoring from ${branch}`) + await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."]) + } + public async saveCheckpoint(message: string) { await this.ensureBranch(this.mainBranch) - // Create temporary branch with all current changes - const tempBranch = `${CheckpointService.STASH_BRANCH}-${Date.now()}` - await this.git.checkout(["-b", tempBranch]) - + const status = await this.git.status() + console.log(`[saveCheckpoint] status: ${JSON.stringify(status)}`) + const stashSha = (await this.git.raw(["stash", "create"])).trim() + console.log(`[saveCheckpoint] stashSha: ${stashSha}`) + const latestHash = await this.git.revparse([this.hiddenBranch]) + + /** + * PHASE: Create stash + * Mutations: + * - Create branch + * - Change branch + */ + const stashBranch = `${CheckpointService.STASH_BRANCH}-${Date.now()}` + await this.git.checkout(["-b", stashBranch]) + this.log(`[saveCheckpoint] created and checked out ${stashBranch}`) + + /** + * Phase: Stage stash + * Mutations: None + * Recovery: + * - UNDO: Create branch + * - UNDO: Change branch + */ try { - // Stage and commit all changes to temporary branch await this.git.add(["-A"]) - await this.git.commit(message, undefined, { - "--allow-empty": null, - "--no-verify": null, - }) + const status = await this.git.status() + this.log(`[saveCheckpoint] status: ${JSON.stringify(status)}`) + } catch (err) { + await this.git.checkout(["-f", this.mainBranch]) + await this.git.branch(["-D", stashBranch]).catch(() => {}) - // Get the latest commit on the hidden branch before we reset it - const latestHash = await this.git.revparse([this.hiddenBranch]) - await this.git.checkout(this.hiddenBranch) + throw new Error( + `[saveCheckpoint] Failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`, + ) + } - try { - // Reset hidden branch to match main and apply the changes - await this.git.reset(["--hard", this.mainBranch]) + /** + * Phase: Commit stash + * Mutations: + * - Commit stash + * - Change branch + * Recovery: + * - UNDO: Create branch + * - UNDO: Change branch + */ + try { + // TODO: Add a test to see if empty commits break this. + const tempCommit = await this.git.commit(message, undefined, { "--no-verify": null }) + this.log(`[saveCheckpoint] tempCommit: ${message} -> ${JSON.stringify(tempCommit)}`) + } catch (err) { + await this.git.checkout(["-f", this.mainBranch]) + await this.git.branch(["-D", stashBranch]).catch(() => {}) - // Cherry-pick the temporary commit - const commit = await this.git.raw(["cherry-pick", tempBranch]) + throw new Error( + `[saveCheckpoint] Failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`, + ) + } - // Return to main branch and cleanup - await this.git.checkout(this.mainBranch) - await this.git.branch(["-D", tempBranch]) + /** + * PHASE: Diff + * Mutations: + * - Checkout hidden branch + * Recovery: + * - UNDO: Create branch + * - UNDO: Change branch + * - UNDO: Commit stash + */ + let diff - this.currentCheckpoint = commit - return { commit } + try { + diff = await this.git.diff([latestHash, stashBranch]) + } catch (err) { + await this.restoreMain({ branch: stashBranch, stashSha, force: true }) + await this.git.branch(["-D", stashBranch]).catch(() => {}) + + throw new Error( + `[saveCheckpoint] Failed in diff phase: ${err instanceof Error ? err.message : String(err)}`, + ) + } + + if (!diff) { + this.log("[saveCheckpoint] no diff") + await this.restoreMain({ branch: stashBranch, stashSha }) + await this.git.branch(["-D", stashBranch]) + return undefined + } + + /** + * PHASE: Reset + * Mutations: + * - Reset hidden branch + * Recovery: + * - UNDO: Create branch + * - UNDO: Change branch + * - UNDO: Commit stash + */ + try { + await this.git.checkout(this.hiddenBranch) + this.log(`[saveCheckpoint] checked out ${this.hiddenBranch}`) + await this.git.reset(["--hard", this.mainBranch]) + this.log(`[saveCheckpoint] reset ${this.hiddenBranch}`) + } catch (err) { + await this.restoreMain({ branch: stashBranch, stashSha, force: true }) + await this.git.branch(["-D", stashBranch]).catch(() => {}) + + throw new Error( + `[saveCheckpoint] Failed in reset phase: ${err instanceof Error ? err.message : String(err)}`, + ) + } + + /** + * PHASE: Cherry pick + * Mutations: + * - Hidden commit (NOTE: reset on hidden branch no longer needed in + * success scenario.) + * Recovery: + * - UNDO: Create branch + * - UNDO: Change branch + * - UNDO: Commit stash + * - UNDO: Reset hidden branch + */ + let commit = "" + + try { + try { + await this.git.raw(["cherry-pick", stashBranch]) } catch (err) { - // If something went wrong after switching to hidden branch - await this.git.reset(["--hard", latestHash]) - await this.git.checkout(["-f", this.mainBranch]) - await this.git.branch(["-D", tempBranch]) - throw err + // Check if we're in the middle of a cherry-pick. + // If the cherry-pick resulted in an empty commit (e.g., only + // deletions) then complete it with --allow-empty. + // Otherwise, rethrow the error. + if (existsSync(path.join(this.baseDir, ".git/CHERRY_PICK_HEAD"))) { + await this.git.raw(["commit", "--allow-empty", "--no-edit"]) + } else { + throw err + } } + + commit = await this.git.revparse(["HEAD"]) + this.currentCheckpoint = commit + this.log(`[saveCheckpoint] cherry-pick commit = ${commit}`) } catch (err) { - // If something went wrong before switching to hidden branch - await this.git.checkout(["-f", this.mainBranch]) - await this.git.branch(["-D", tempBranch]) - throw new Error(`Failed to save checkpoint: ${err instanceof Error ? err.message : String(err)}`) + await this.git.reset(["--hard", latestHash]).catch(() => {}) + await this.restoreMain({ branch: stashBranch, stashSha, force: true }) + await this.git.branch(["-D", stashBranch]).catch(() => {}) + + throw new Error( + `[saveCheckpoint] Failed in cherry pick phase: ${err instanceof Error ? err.message : String(err)}`, + ) } + + await this.restoreMain({ branch: stashBranch, stashSha }) + await this.git.branch(["-D", stashBranch]) + + return { commit } } public async restoreCheckpoint(commitHash: string) { @@ -264,12 +408,12 @@ export class CheckpointService { const currentBranch = await git.revparse(["--abbrev-ref", "HEAD"]) const currentSha = await git.revparse(["HEAD"]) - const hiddenBranch = `roo-code-checkpoints-${taskId}` + const hiddenBranch = `${CheckpointService.CHECKPOINT_BRANCH}-${taskId}` const branchSummary = await git.branch() if (!branchSummary.all.includes(hiddenBranch)) { - await git.checkoutBranch(hiddenBranch, currentBranch) // git checkout -b - await git.checkout(currentBranch) // git checkout + await git.checkoutBranch(hiddenBranch, currentBranch) + await git.checkout(currentBranch) } return { currentBranch, currentSha, hiddenBranch } diff --git a/src/services/checkpoints/__tests__/CheckpointService.test.ts b/src/services/checkpoints/__tests__/CheckpointService.test.ts index caa0952fa1d..3dea93be827 100644 --- a/src/services/checkpoints/__tests__/CheckpointService.test.ts +++ b/src/services/checkpoints/__tests__/CheckpointService.test.ts @@ -68,7 +68,7 @@ describe("CheckpointService", () => { git = repo.git testFile = repo.testFile - service = await CheckpointService.create({ taskId, git, baseDir, log: () => {} }) + service = await CheckpointService.create({ taskId, git, baseDir }) }) afterEach(async () => { @@ -295,8 +295,11 @@ describe("CheckpointService", () => { await fs.writeFile(testFile, "I am tracked!") const untrackedFile = path.join(service.baseDir, "new.txt") await fs.writeFile(untrackedFile, "I am untracked!") + const commit1 = await service.saveCheckpoint("First checkpoint") expect(commit1?.commit).toBeTruthy() + expect(await fs.readFile(testFile, "utf-8")).toBe("I am tracked!") + expect(await fs.readFile(untrackedFile, "utf-8")).toBe("I am untracked!") await fs.unlink(testFile) await fs.unlink(untrackedFile) From 971d29b1fb89c190cf300e6c9abbb6a7f6a07da7 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 03:23:22 -0800 Subject: [PATCH 3/5] Revert debugging --- src/services/checkpoints/__tests__/CheckpointService.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/services/checkpoints/__tests__/CheckpointService.test.ts b/src/services/checkpoints/__tests__/CheckpointService.test.ts index 3dea93be827..caa0952fa1d 100644 --- a/src/services/checkpoints/__tests__/CheckpointService.test.ts +++ b/src/services/checkpoints/__tests__/CheckpointService.test.ts @@ -68,7 +68,7 @@ describe("CheckpointService", () => { git = repo.git testFile = repo.testFile - service = await CheckpointService.create({ taskId, git, baseDir }) + service = await CheckpointService.create({ taskId, git, baseDir, log: () => {} }) }) afterEach(async () => { @@ -295,11 +295,8 @@ describe("CheckpointService", () => { await fs.writeFile(testFile, "I am tracked!") const untrackedFile = path.join(service.baseDir, "new.txt") await fs.writeFile(untrackedFile, "I am untracked!") - const commit1 = await service.saveCheckpoint("First checkpoint") expect(commit1?.commit).toBeTruthy() - expect(await fs.readFile(testFile, "utf-8")).toBe("I am tracked!") - expect(await fs.readFile(untrackedFile, "utf-8")).toBe("I am untracked!") await fs.unlink(testFile) await fs.unlink(untrackedFile) From a985494b9597fc04210aa3c3db6798fea0de1f73 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 03:23:59 -0800 Subject: [PATCH 4/5] Remove unused type --- src/services/checkpoints/CheckpointService.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 3d8026923ce..47d785d7526 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -11,12 +11,6 @@ export type CheckpointServiceOptions = { log?: (message: string) => void } -export type StagedFile = { - path: string - isStaged: boolean - isPartiallyStaged: boolean -} - /** * The CheckpointService provides a mechanism for storing a snapshot of the * current VSCode workspace each time a Roo Code tool is executed. It uses Git From 6dbb59642fa7b67c128c724c1f2c80171257f42a Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 03:25:33 -0800 Subject: [PATCH 5/5] Revert debugging --- src/services/checkpoints/CheckpointService.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 47d785d7526..871179d500f 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -139,22 +139,19 @@ export class CheckpointService { } if (stashSha) { - console.log(`[restoreMain] applying stash ${stashSha}`) + this.log(`[restoreMain] applying stash ${stashSha}`) await this.git.raw(["stash", "apply", "--index", stashSha]) } - console.log(`[restoreMain] restoring from ${branch}`) + this.log(`[restoreMain] restoring from ${branch}`) await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."]) } public async saveCheckpoint(message: string) { await this.ensureBranch(this.mainBranch) - const status = await this.git.status() - console.log(`[saveCheckpoint] status: ${JSON.stringify(status)}`) const stashSha = (await this.git.raw(["stash", "create"])).trim() - console.log(`[saveCheckpoint] stashSha: ${stashSha}`) - const latestHash = await this.git.revparse([this.hiddenBranch]) + const latestSha = await this.git.revparse([this.hiddenBranch]) /** * PHASE: Create stash @@ -220,7 +217,7 @@ export class CheckpointService { let diff try { - diff = await this.git.diff([latestHash, stashBranch]) + diff = await this.git.diff([latestSha, stashBranch]) } catch (err) { await this.restoreMain({ branch: stashBranch, stashSha, force: true }) await this.git.branch(["-D", stashBranch]).catch(() => {}) @@ -292,7 +289,7 @@ export class CheckpointService { this.currentCheckpoint = commit this.log(`[saveCheckpoint] cherry-pick commit = ${commit}`) } catch (err) { - await this.git.reset(["--hard", latestHash]).catch(() => {}) + await this.git.reset(["--hard", latestSha]).catch(() => {}) await this.restoreMain({ branch: stashBranch, stashSha, force: true }) await this.git.branch(["-D", stashBranch]).catch(() => {})