Skip to content

Commit 28cd4eb

Browse files
ctecte
authored
Merge pull request RooCodeInc#945 from RooVetGit/cte/checkpoints-logging
Checkpoints logging tweaks + defensive catches
2 parents 351d461 + 94cb09f commit 28cd4eb

File tree

8 files changed

+156
-65
lines changed

8 files changed

+156
-65
lines changed

‎src/core/Cline.ts‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,13 @@ export class Cline {
370370
this.askResponseImages = images
371371
}
372372

373-
async say(type: ClineSay, text?: string, images?: string[], partial?: boolean): Promise<undefined> {
373+
async say(
374+
type: ClineSay,
375+
text?: string,
376+
images?: string[],
377+
partial?: boolean,
378+
checkpoint?: Record<string, unknown>,
379+
): Promise<undefined> {
374380
if (this.abort) {
375381
throw new Error("Roo Code instance aborted")
376382
}
@@ -423,7 +429,7 @@ export class Cline {
423429
// this is a new non-partial message, so add it like normal
424430
const sayTs = Date.now()
425431
this.lastMessageTs = sayTs
426-
await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images })
432+
await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, checkpoint })
427433
await this.providerRef.deref()?.postStateToWebview()
428434
}
429435
}
@@ -2747,6 +2753,13 @@ export class Cline {
27472753
// get previous api req's index to check token usage and determine if we need to truncate conversation history
27482754
const previousApiReqIndex = findLastIndex(this.clineMessages, (m) => m.say === "api_req_started")
27492755

2756+
// Save checkpoint if this is the first API request.
2757+
const isFirstRequest = this.clineMessages.filter((m) => m.say === "api_req_started").length === 0
2758+
2759+
if (isFirstRequest) {
2760+
await this.checkpointSave()
2761+
}
2762+
27502763
// 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
27512764
// for the best UX we show a placeholder api_req_started message with a loading spinner as this happens
27522765
await this.say(
@@ -3288,12 +3301,7 @@ export class Cline {
32883301
]),
32893302
)
32903303
} catch (err) {
3291-
this.providerRef
3292-
.deref()
3293-
?.log(
3294-
`[checkpointDiff] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`,
3295-
)
3296-
3304+
this.providerRef.deref()?.log("[checkpointDiff] disabling checkpoints for this task")
32973305
this.checkpointsEnabled = false
32983306
}
32993307
}
@@ -3304,6 +3312,7 @@ export class Cline {
33043312
}
33053313

33063314
try {
3315+
const isFirst = !this.checkpointService
33073316
const service = await this.getCheckpointService()
33083317
const commit = await service.saveCheckpoint(`Task: ${this.taskId}, Time: ${Date.now()}`)
33093318

@@ -3312,15 +3321,17 @@ export class Cline {
33123321
.deref()
33133322
?.postMessageToWebview({ type: "currentCheckpointUpdated", text: service.currentCheckpoint })
33143323

3315-
await this.say("checkpoint_saved", commit.commit)
3324+
// Checkpoint metadata required by the UI.
3325+
const checkpoint = {
3326+
isFirst,
3327+
from: service.baseCommitHash,
3328+
to: commit.commit,
3329+
}
3330+
3331+
await this.say("checkpoint_saved", commit.commit, undefined, undefined, checkpoint)
33163332
}
33173333
} catch (err) {
3318-
this.providerRef
3319-
.deref()
3320-
?.log(
3321-
`[checkpointSave] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`,
3322-
)
3323-
3334+
this.providerRef.deref()?.log("[checkpointSave] disabling checkpoints for this task")
33243335
this.checkpointsEnabled = false
33253336
}
33263337
}
@@ -3390,12 +3401,7 @@ export class Cline {
33903401
// Cline instance.
33913402
this.providerRef.deref()?.cancelTask()
33923403
} catch (err) {
3393-
this.providerRef
3394-
.deref()
3395-
?.log(
3396-
`[restoreCheckpoint] Encountered unexpected error: $${err instanceof Error ? err.message : String(err)}`,
3397-
)
3398-
3404+
this.providerRef.deref()?.log("[checkpointRestore] disabling checkpoints for this task")
33993405
this.checkpointsEnabled = false
34003406
}
34013407
}

‎src/services/checkpoints/CheckpointService.ts‎

Lines changed: 85 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -132,22 +132,64 @@ export class CheckpointService {
132132
stashSha: string
133133
force?: boolean
134134
}) {
135-
if (force) {
136-
await this.git.checkout(["-f", this.mainBranch])
137-
} else {
138-
await this.git.checkout(this.mainBranch)
135+
let currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"])
136+
137+
if (currentBranch !== this.mainBranch) {
138+
if (force) {
139+
try {
140+
await this.git.checkout(["-f", this.mainBranch])
141+
} catch (err) {
142+
this.log(
143+
`[restoreMain] failed to force checkout ${this.mainBranch}: ${err instanceof Error ? err.message : String(err)}`,
144+
)
145+
}
146+
} else {
147+
try {
148+
await this.git.checkout(this.mainBranch)
149+
} catch (err) {
150+
this.log(
151+
`[restoreMain] failed to checkout ${this.mainBranch}: ${err instanceof Error ? err.message : String(err)}`,
152+
)
153+
154+
// Escalate to a forced checkout if we can't checkout the
155+
// main branch under normal circumstances.
156+
currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"])
157+
158+
if (currentBranch !== this.mainBranch) {
159+
await this.git.checkout(["-f", this.mainBranch]).catch(() => {})
160+
}
161+
}
162+
}
163+
}
164+
165+
currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"])
166+
167+
if (currentBranch !== this.mainBranch) {
168+
throw new Error(`Unable to restore ${this.mainBranch}`)
139169
}
140170

141171
if (stashSha) {
142172
this.log(`[restoreMain] applying stash ${stashSha}`)
143-
await this.git.raw(["stash", "apply", "--index", stashSha])
173+
174+
try {
175+
await this.git.raw(["stash", "apply", "--index", stashSha])
176+
} catch (err) {
177+
this.log(`[restoreMain] Failed to apply stash: ${err instanceof Error ? err.message : String(err)}`)
178+
}
144179
}
145180

146-
this.log(`[restoreMain] restoring from ${branch}`)
147-
await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."])
181+
this.log(`[restoreMain] restoring from ${branch} branch`)
182+
183+
try {
184+
await this.git.raw(["restore", "--source", branch, "--worktree", "--", "."])
185+
} catch (err) {
186+
this.log(`[restoreMain] Failed to restore branch: ${err instanceof Error ? err.message : String(err)}`)
187+
}
148188
}
149189

150190
public async saveCheckpoint(message: string) {
191+
const startTime = Date.now()
192+
151193
await this.ensureBranch(this.mainBranch)
152194

153195
const stashSha = (await this.git.raw(["stash", "create"])).trim()
@@ -172,15 +214,13 @@ export class CheckpointService {
172214
*/
173215
try {
174216
await this.git.add(["-A"])
175-
const status = await this.git.status()
176-
this.log(`[saveCheckpoint] status: ${JSON.stringify(status)}`)
177217
} catch (err) {
178-
await this.git.checkout(["-f", this.mainBranch])
179-
await this.git.branch(["-D", stashBranch]).catch(() => {})
180-
181-
throw new Error(
182-
`[saveCheckpoint] Failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`,
218+
this.log(
219+
`[saveCheckpoint] failed in stage stash phase: ${err instanceof Error ? err.message : String(err)}`,
183220
)
221+
await this.restoreMain({ branch: stashBranch, stashSha, force: true })
222+
await this.git.branch(["-D", stashBranch]).catch(() => {})
223+
throw err
184224
}
185225

186226
/**
@@ -192,17 +232,25 @@ export class CheckpointService {
192232
* - UNDO: Create branch
193233
* - UNDO: Change branch
194234
*/
235+
let stashCommit
236+
195237
try {
196-
// TODO: Add a test to see if empty commits break this.
197-
const tempCommit = await this.git.commit(message, undefined, { "--no-verify": null })
198-
this.log(`[saveCheckpoint] tempCommit: ${message} -> ${JSON.stringify(tempCommit)}`)
238+
stashCommit = await this.git.commit(message, undefined, { "--no-verify": null })
239+
this.log(`[saveCheckpoint] stashCommit: ${message} -> ${JSON.stringify(stashCommit)}`)
199240
} catch (err) {
200-
await this.git.checkout(["-f", this.mainBranch])
241+
this.log(
242+
`[saveCheckpoint] failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`,
243+
)
244+
await this.restoreMain({ branch: stashBranch, stashSha, force: true })
201245
await this.git.branch(["-D", stashBranch]).catch(() => {})
246+
throw err
247+
}
202248

203-
throw new Error(
204-
`[saveCheckpoint] Failed in stash commit phase: ${err instanceof Error ? err.message : String(err)}`,
205-
)
249+
if (!stashCommit) {
250+
this.log("[saveCheckpoint] no stash commit")
251+
await this.restoreMain({ branch: stashBranch, stashSha })
252+
await this.git.branch(["-D", stashBranch])
253+
return undefined
206254
}
207255

208256
/**
@@ -219,12 +267,10 @@ export class CheckpointService {
219267
try {
220268
diff = await this.git.diff([latestSha, stashBranch])
221269
} catch (err) {
270+
this.log(`[saveCheckpoint] failed in diff phase: ${err instanceof Error ? err.message : String(err)}`)
222271
await this.restoreMain({ branch: stashBranch, stashSha, force: true })
223272
await this.git.branch(["-D", stashBranch]).catch(() => {})
224-
225-
throw new Error(
226-
`[saveCheckpoint] Failed in diff phase: ${err instanceof Error ? err.message : String(err)}`,
227-
)
273+
throw err
228274
}
229275

230276
if (!diff) {
@@ -249,12 +295,10 @@ export class CheckpointService {
249295
await this.git.reset(["--hard", this.mainBranch])
250296
this.log(`[saveCheckpoint] reset ${this.hiddenBranch}`)
251297
} catch (err) {
298+
this.log(`[saveCheckpoint] failed in reset phase: ${err instanceof Error ? err.message : String(err)}`)
252299
await this.restoreMain({ branch: stashBranch, stashSha, force: true })
253300
await this.git.branch(["-D", stashBranch]).catch(() => {})
254-
255-
throw new Error(
256-
`[saveCheckpoint] Failed in reset phase: ${err instanceof Error ? err.message : String(err)}`,
257-
)
301+
throw err
258302
}
259303

260304
/**
@@ -289,25 +333,33 @@ export class CheckpointService {
289333
this.currentCheckpoint = commit
290334
this.log(`[saveCheckpoint] cherry-pick commit = ${commit}`)
291335
} catch (err) {
336+
this.log(
337+
`[saveCheckpoint] failed in cherry pick phase: ${err instanceof Error ? err.message : String(err)}`,
338+
)
292339
await this.git.reset(["--hard", latestSha]).catch(() => {})
293340
await this.restoreMain({ branch: stashBranch, stashSha, force: true })
294341
await this.git.branch(["-D", stashBranch]).catch(() => {})
295-
296-
throw new Error(
297-
`[saveCheckpoint] Failed in cherry pick phase: ${err instanceof Error ? err.message : String(err)}`,
298-
)
342+
throw err
299343
}
300344

301345
await this.restoreMain({ branch: stashBranch, stashSha })
302346
await this.git.branch(["-D", stashBranch])
303347

348+
// We've gotten reports that checkpoints can be slow in some cases, so
349+
// we'll log the duration of the checkpoint save.
350+
const duration = Date.now() - startTime
351+
this.log(`[saveCheckpoint] saved checkpoint ${commit} in ${duration}ms`)
352+
304353
return { commit }
305354
}
306355

307356
public async restoreCheckpoint(commitHash: string) {
357+
const startTime = Date.now()
308358
await this.ensureBranch(this.mainBranch)
309359
await this.git.clean([CleanOptions.FORCE, CleanOptions.RECURSIVE])
310360
await this.git.raw(["restore", "--source", commitHash, "--worktree", "--", "."])
361+
const duration = Date.now() - startTime
362+
this.log(`[restoreCheckpoint] restored checkpoint ${commitHash} in ${duration}ms`)
311363
this.currentCheckpoint = commitHash
312364
}
313365

‎src/services/checkpoints/__tests__/CheckpointService.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,12 @@ describe("CheckpointService", () => {
210210
})
211211

212212
it("does not create a checkpoint if there are no pending changes", async () => {
213+
const commit0 = await service.saveCheckpoint("Zeroth checkpoint")
214+
expect(commit0?.commit).toBeFalsy()
215+
213216
await fs.writeFile(testFile, "Ahoy, world!")
214-
const commit = await service.saveCheckpoint("First checkpoint")
215-
expect(commit?.commit).toBeTruthy()
217+
const commit1 = await service.saveCheckpoint("First checkpoint")
218+
expect(commit1?.commit).toBeTruthy()
216219

217220
const commit2 = await service.saveCheckpoint("Second checkpoint")
218221
expect(commit2?.commit).toBeFalsy()

‎src/shared/ExtensionMessage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ export interface ClineMessage {
139139
partial?: boolean
140140
reasoning?: string
141141
conversationHistoryIndex?: number
142+
checkpoint?: Record<string, unknown>
142143
}
143144

144145
export type ClineAsk =

‎webview-ui/src/components/chat/ChatRow.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,7 @@ export const ChatRowContent = ({
761761
<CheckpointSaved
762762
ts={message.ts!}
763763
commitHash={message.text!}
764+
checkpoint={message.checkpoint}
764765
currentCheckpointHash={currentCheckpoint}
765766
/>
766767
)

‎webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
import { useState, useEffect, useCallback } from "react"
22
import { CheckIcon, Cross2Icon } from "@radix-ui/react-icons"
33

4-
import { vscode } from "../../../utils/vscode"
5-
64
import { Button, Popover, PopoverContent, PopoverTrigger } from "@/components/ui"
75

6+
import { vscode } from "../../../utils/vscode"
7+
import { Checkpoint } from "./schema"
8+
89
type CheckpointMenuProps = {
910
ts: number
1011
commitHash: string
12+
checkpoint?: Checkpoint
1113
currentCheckpointHash?: string
1214
}
1315

14-
export const CheckpointMenu = ({ ts, commitHash, currentCheckpointHash }: CheckpointMenuProps) => {
16+
export const CheckpointMenu = ({ ts, commitHash, checkpoint, currentCheckpointHash }: CheckpointMenuProps) => {
1517
const [portalContainer, setPortalContainer] = useState<HTMLElement>()
1618
const [isOpen, setIsOpen] = useState(false)
1719
const [isConfirming, setIsConfirming] = useState(false)
@@ -43,9 +45,11 @@ export const CheckpointMenu = ({ ts, commitHash, currentCheckpointHash }: Checkp
4345

4446
return (
4547
<div className="flex flex-row gap-1">
46-
<Button variant="ghost" size="icon" onClick={onCheckpointDiff} title="View Diff">
47-
<span className="codicon codicon-diff-single" />
48-
</Button>
48+
{!checkpoint?.isFirst && (
49+
<Button variant="ghost" size="icon" onClick={onCheckpointDiff} title="View Diff">
50+
<span className="codicon codicon-diff-single" />
51+
</Button>
52+
)}
4953
<Popover
5054
open={isOpen}
5155
onOpenChange={(open) => {

0 commit comments

Comments
 (0)