From 68ea2e85885b14b7c98b65b4033c039af93c6a88 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 10:14:36 -0800 Subject: [PATCH 1/6] Checkpoints logging tweaks --- src/core/Cline.ts | 21 +--- src/services/checkpoints/CheckpointService.ts | 97 +++++++++++++------ 2 files changed, 69 insertions(+), 49 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 3b221eb7845..7d7bedcabd1 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -3288,12 +3288,7 @@ export class Cline { ]), ) } catch (err) { - this.providerRef - .deref() - ?.log( - `[checkpointDiff] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`, - ) - + this.providerRef.deref()?.log("[checkpointDiff] disabling checkpoints for this task") this.checkpointsEnabled = false } } @@ -3315,12 +3310,7 @@ export class Cline { await this.say("checkpoint_saved", commit.commit) } } catch (err) { - this.providerRef - .deref() - ?.log( - `[checkpointSave] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`, - ) - + this.providerRef.deref()?.log("[checkpointSave] disabling checkpoints for this task") this.checkpointsEnabled = false } } @@ -3390,12 +3380,7 @@ export class Cline { // Cline instance. this.providerRef.deref()?.cancelTask() } catch (err) { - this.providerRef - .deref() - ?.log( - `[restoreCheckpoint] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`, - ) - + this.providerRef.deref()?.log("[checkpointRestore] disabling checkpoints for this task") this.checkpointsEnabled = false } } diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 53f0647a67c..c2f57a35aba 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -132,19 +132,59 @@ export class CheckpointService { stashSha: string force?: boolean }) { - if (force) { - await this.git.checkout(["-f", this.mainBranch]) - } else { - await this.git.checkout(this.mainBranch) + let currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"]) + + if (currentBranch !== this.mainBranch) { + if (force) { + try { + await this.git.checkout(["-f", this.mainBranch]) + } catch (err) { + this.log( + `[restoreMain] failed to force checkout ${this.mainBranch}: ${err instanceof Error ? err.message : String(err)}`, + ) + } + } else { + try { + await this.git.checkout(this.mainBranch) + } catch (err) { + this.log( + `[restoreMain] failed to checkout ${this.mainBranch}: ${err instanceof Error ? err.message : String(err)}`, + ) + + // Escalate to a forced checkout if we can't checkout the + // main branch under nor + currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"]) + + if (currentBranch !== this.mainBranch) { + await this.git.checkout(["-f", this.mainBranch]).catch(() => {}) + } + } + } + } + + currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"]) + + if (currentBranch !== this.mainBranch) { + throw new Error(`Unable to restore ${this.mainBranch}`) } if (stashSha) { this.log(`[restoreMain] applying stash ${stashSha}`) - await this.git.raw(["stash", "apply", "--index", stashSha]) + + try { + await this.git.raw(["stash", "apply", "--index", stashSha]) + } catch (err) { + this.log(`[restoreMain] Failed to apply stash: ${err instanceof Error ? err.message : String(err)}`) + } } - this.log(`[restoreMain] restoring from ${branch}`) - await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."]) + this.log(`[restoreMain] restoring from ${branch} branch`) + + try { + await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."]) + } catch (err) { + this.log(`[restoreMain] Failed to restore branch: ${err instanceof Error ? err.message : String(err)}`) + } } public async saveCheckpoint(message: string) { @@ -172,15 +212,13 @@ export class CheckpointService { */ try { await this.git.add(["-A"]) - const status = await this.git.status() - this.log(`[saveCheckpoint] status: ${JSON.stringify(status)}`) } catch (err) { + this.log( + `[saveCheckpoint] failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`, + ) await this.git.checkout(["-f", this.mainBranch]) await this.git.branch(["-D", stashBranch]).catch(() => {}) - - throw new Error( - `[saveCheckpoint] Failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`, - ) + throw err } /** @@ -194,15 +232,15 @@ export class CheckpointService { */ 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)}`) + const stashCommit = await this.git.commit(message, undefined, { "--no-verify": null }) + this.log(`[saveCheckpoint] stashCommit: ${message} -> ${JSON.stringify(stashCommit)}`) } catch (err) { + this.log( + `[saveCheckpoint] failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`, + ) await this.git.checkout(["-f", this.mainBranch]) await this.git.branch(["-D", stashBranch]).catch(() => {}) - - throw new Error( - `[saveCheckpoint] Failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`, - ) + throw err } /** @@ -219,12 +257,10 @@ export class CheckpointService { try { diff = await this.git.diff([latestSha, stashBranch]) } catch (err) { + this.log(`[saveCheckpoint] failed in diff phase: ${err instanceof Error ? err.message : String(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)}`, - ) + throw err } if (!diff) { @@ -249,12 +285,10 @@ export class CheckpointService { await this.git.reset(["--hard", this.mainBranch]) this.log(`[saveCheckpoint] reset ${this.hiddenBranch}`) } catch (err) { + this.log(`[saveCheckpoint] failed in reset phase: ${err instanceof Error ? err.message : String(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)}`, - ) + throw err } /** @@ -289,17 +323,18 @@ export class CheckpointService { this.currentCheckpoint = commit this.log(`[saveCheckpoint] cherry-pick commit = ${commit}`) } catch (err) { + this.log( + `[saveCheckpoint] failed in cherry pick phase: ${err instanceof Error ? err.message : String(err)}`, + ) await this.git.reset(["--hard", latestSha]).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)}`, - ) + throw err } await this.restoreMain({ branch: stashBranch, stashSha }) await this.git.branch(["-D", stashBranch]) + this.log(`[saveCheckpoint] saved checkpoint ${commit}`) return { commit } } From 6b351876e7c58d0b18af627a9944e473ea448adb Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 10:16:51 -0800 Subject: [PATCH 2/6] Add duration logging --- src/services/checkpoints/CheckpointService.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index c2f57a35aba..36bdcb1fb34 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -188,6 +188,8 @@ export class CheckpointService { } public async saveCheckpoint(message: string) { + const startTime = Date.now() + await this.ensureBranch(this.mainBranch) const stashSha = (await this.git.raw(["stash", "create"])).trim() @@ -334,7 +336,11 @@ export class CheckpointService { await this.restoreMain({ branch: stashBranch, stashSha }) await this.git.branch(["-D", stashBranch]) - this.log(`[saveCheckpoint] saved checkpoint ${commit}`) + + // We've gotten reports that checkpoints can be slow in some cases, so + // we'll log the duration of the checkpoint save. + const duration = Date.now() - startTime + this.log(`[saveCheckpoint] saved checkpoint ${commit} in ${duration}ms`) return { commit } } From 37c2a1760b9b300bbda72b1436e47f732c15bb3f Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 10:24:51 -0800 Subject: [PATCH 3/6] Check for empty stash commits --- src/services/checkpoints/CheckpointService.ts | 16 ++++++++++++---- .../__tests__/CheckpointService.test.ts | 7 +++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 36bdcb1fb34..94bd7b65371 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -218,7 +218,7 @@ export class CheckpointService { this.log( `[saveCheckpoint] failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`, ) - await this.git.checkout(["-f", this.mainBranch]) + await this.restoreMain({ branch: stashBranch, stashSha, force: true }) await this.git.branch(["-D", stashBranch]).catch(() => {}) throw err } @@ -232,19 +232,27 @@ export class CheckpointService { * - UNDO: Create branch * - UNDO: Change branch */ + let stashCommit + try { - // TODO: Add a test to see if empty commits break this. - const stashCommit = await this.git.commit(message, undefined, { "--no-verify": null }) + stashCommit = await this.git.commit(message, undefined, { "--no-verify": null }) this.log(`[saveCheckpoint] stashCommit: ${message} -> ${JSON.stringify(stashCommit)}`) } catch (err) { this.log( `[saveCheckpoint] failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`, ) - await this.git.checkout(["-f", this.mainBranch]) + await this.restoreMain({ branch: stashBranch, stashSha, force: true }) await this.git.branch(["-D", stashBranch]).catch(() => {}) throw err } + if (!stashCommit) { + this.log("[saveCheckpoint] no stash commit") + await this.restoreMain({ branch: stashBranch, stashSha }) + await this.git.branch(["-D", stashBranch]) + return undefined + } + /** * PHASE: Diff * Mutations: diff --git a/src/services/checkpoints/__tests__/CheckpointService.test.ts b/src/services/checkpoints/__tests__/CheckpointService.test.ts index 9165da332af..c19ef0830f1 100644 --- a/src/services/checkpoints/__tests__/CheckpointService.test.ts +++ b/src/services/checkpoints/__tests__/CheckpointService.test.ts @@ -210,9 +210,12 @@ describe("CheckpointService", () => { }) it("does not create a checkpoint if there are no pending changes", async () => { + const commit0 = await service.saveCheckpoint("Zeroth checkpoint") + expect(commit0?.commit).toBeFalsy() + await fs.writeFile(testFile, "Ahoy, world!") - const commit = await service.saveCheckpoint("First checkpoint") - expect(commit?.commit).toBeTruthy() + const commit1 = await service.saveCheckpoint("First checkpoint") + expect(commit1?.commit).toBeTruthy() const commit2 = await service.saveCheckpoint("Second checkpoint") expect(commit2?.commit).toBeFalsy() From d8960788b44a6d2fcd66c24a148a68821240d9b8 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 11:10:47 -0800 Subject: [PATCH 4/6] Store initial checkpoint --- src/core/Cline.ts | 27 ++++++++++++++++--- src/shared/ExtensionMessage.ts | 1 + webview-ui/src/components/chat/ChatRow.tsx | 1 + .../chat/checkpoints/CheckpointMenu.tsx | 16 ++++++----- .../chat/checkpoints/CheckpointSaved.tsx | 21 ++++++++++++--- .../src/components/chat/checkpoints/schema.ts | 9 +++++++ 6 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 webview-ui/src/components/chat/checkpoints/schema.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 7d7bedcabd1..bacade3f741 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -370,7 +370,13 @@ export class Cline { this.askResponseImages = images } - async say(type: ClineSay, text?: string, images?: string[], partial?: boolean): Promise { + async say( + type: ClineSay, + text?: string, + images?: string[], + partial?: boolean, + checkpoint?: Record, + ): Promise { if (this.abort) { throw new Error("Roo Code instance aborted") } @@ -423,7 +429,7 @@ export class Cline { // this is a new non-partial message, so add it like normal const sayTs = Date.now() this.lastMessageTs = sayTs - await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images }) + await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, checkpoint }) await this.providerRef.deref()?.postStateToWebview() } } @@ -2747,6 +2753,13 @@ export class Cline { // get previous api req's index to check token usage and determine if we need to truncate conversation history const previousApiReqIndex = findLastIndex(this.clineMessages, (m) => m.say === "api_req_started") + // Save checkpoint if this is the first API request. + const isFirstRequest = this.clineMessages.filter((m) => m.say === "api_req_started").length === 0 + + if (isFirstRequest) { + await this.checkpointSave() + } + // getting verbose details is an expensive operation, it uses globby to top-down build file structure of project which for large projects can take a few seconds // for the best UX we show a placeholder api_req_started message with a loading spinner as this happens await this.say( @@ -3299,6 +3312,7 @@ export class Cline { } try { + const isFirst = !this.checkpointService const service = await this.getCheckpointService() const commit = await service.saveCheckpoint(`Task: ${this.taskId}, Time: ${Date.now()}`) @@ -3307,7 +3321,14 @@ export class Cline { .deref() ?.postMessageToWebview({ type: "currentCheckpointUpdated", text: service.currentCheckpoint }) - await this.say("checkpoint_saved", commit.commit) + // Checkpoint metadata required by the UI. + const checkpoint = { + isFirst, + from: service.baseCommitHash, + to: commit.commit, + } + + await this.say("checkpoint_saved", commit.commit, undefined, undefined, checkpoint) } } catch (err) { this.providerRef.deref()?.log("[checkpointSave] disabling checkpoints for this task") diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 33a5767f326..29bc0ec3b62 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -139,6 +139,7 @@ export interface ClineMessage { partial?: boolean reasoning?: string conversationHistoryIndex?: number + checkpoint?: Record } export type ClineAsk = diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index c18fa848892..bf359fe2cd9 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -761,6 +761,7 @@ export const ChatRowContent = ({ ) diff --git a/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx b/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx index 5eba795b736..b3e88013a81 100644 --- a/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx +++ b/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx @@ -1,17 +1,19 @@ import { useState, useEffect, useCallback } from "react" import { CheckIcon, Cross2Icon } from "@radix-ui/react-icons" -import { vscode } from "../../../utils/vscode" - import { Button, Popover, PopoverContent, PopoverTrigger } from "@/components/ui" +import { vscode } from "../../../utils/vscode" +import { Checkpoint } from "./schema" + type CheckpointMenuProps = { ts: number commitHash: string + checkpoint?: Checkpoint currentCheckpointHash?: string } -export const CheckpointMenu = ({ ts, commitHash, currentCheckpointHash }: CheckpointMenuProps) => { +export const CheckpointMenu = ({ ts, commitHash, checkpoint, currentCheckpointHash }: CheckpointMenuProps) => { const [portalContainer, setPortalContainer] = useState() const [isOpen, setIsOpen] = useState(false) const [isConfirming, setIsConfirming] = useState(false) @@ -43,9 +45,11 @@ export const CheckpointMenu = ({ ts, commitHash, currentCheckpointHash }: Checkp return (
- + {!checkpoint?.isFirst && ( + + )} { diff --git a/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx b/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx index d5c0050e533..d48bfaf7f7e 100644 --- a/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx +++ b/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx @@ -1,22 +1,37 @@ +import { useMemo } from "react" + import { CheckpointMenu } from "./CheckpointMenu" +import { checkpointSchema } from "./schema" type CheckpointSavedProps = { ts: number commitHash: string + checkpoint?: Record currentCheckpointHash?: string } -export const CheckpointSaved = (props: CheckpointSavedProps) => { +export const CheckpointSaved = ({ checkpoint, ...props }: CheckpointSavedProps) => { const isCurrent = props.currentCheckpointHash === props.commitHash + const metadata = useMemo(() => { + if (!checkpoint) { + return undefined + } + + const result = checkpointSchema.safeParse(checkpoint) + return result.success ? result.data : undefined + }, [checkpoint]) + + const isFirst = !!metadata?.isFirst + return (
- Checkpoint + {isFirst ? "Initial Checkpoint" : "Checkpoint"} {isCurrent && Current}
- +
) } diff --git a/webview-ui/src/components/chat/checkpoints/schema.ts b/webview-ui/src/components/chat/checkpoints/schema.ts new file mode 100644 index 00000000000..4acd32a6ab6 --- /dev/null +++ b/webview-ui/src/components/chat/checkpoints/schema.ts @@ -0,0 +1,9 @@ +import { z } from "zod" + +export const checkpointSchema = z.object({ + isFirst: z.boolean(), + from: z.string(), + to: z.string(), +}) + +export type Checkpoint = z.infer From d9ac8efb1ff3439f009b7112362206b83c74b927 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 11:19:00 -0800 Subject: [PATCH 5/6] Complete the comment --- src/services/checkpoints/CheckpointService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 94bd7b65371..15b9cb415b1 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -152,7 +152,7 @@ export class CheckpointService { ) // Escalate to a forced checkout if we can't checkout the - // main branch under nor + // main branch under normal circumstances. currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"]) if (currentBranch !== this.mainBranch) { From 94cb09f6cd94b2215f9c88994a871f350c11e6f4 Mon Sep 17 00:00:00 2001 From: cte Date: Tue, 11 Feb 2025 11:24:33 -0800 Subject: [PATCH 6/6] Log restoreCheckpoint duration --- src/services/checkpoints/CheckpointService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/services/checkpoints/CheckpointService.ts b/src/services/checkpoints/CheckpointService.ts index 15b9cb415b1..d229452021d 100644 --- a/src/services/checkpoints/CheckpointService.ts +++ b/src/services/checkpoints/CheckpointService.ts @@ -354,9 +354,12 @@ export class CheckpointService { } public async restoreCheckpoint(commitHash: string) { + const startTime = Date.now() await this.ensureBranch(this.mainBranch) await this.git.clean([CleanOptions.FORCE, CleanOptions.RECURSIVE]) await this.git.raw(["restore", "--source", commitHash, "--worktree", "--", "."]) + const duration = Date.now() - startTime + this.log(`[restoreCheckpoint] restored checkpoint ${commitHash} in ${duration}ms`) this.currentCheckpoint = commitHash }