Skip to content
13 changes: 13 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ export const MAX_CHECKPOINT_TIMEOUT_SECONDS = 60
*/
export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15

/**
* Whether per-write checkpoints and task-start baseline are enabled by default.
* Master switch for the B cluster of checkpoint features.
* @default true
*/
export const DEFAULT_PER_WRITE_CHECKPOINTS = true
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* GlobalSettings
*/
Expand Down Expand Up @@ -200,6 +207,12 @@ export const globalSettingsSchema = z.object({
.min(MIN_CHECKPOINT_TIMEOUT_SECONDS)
.max(MAX_CHECKPOINT_TIMEOUT_SECONDS)
.optional(),
/**
* Whether to record a shadow-git checkpoint after every successful write_to_file,
* edit_file, and apply_patch (per-write checkpoints), plus a task-start baseline.
* @default true
*/
perWriteCheckpoints: z.boolean().optional(),

ttsEnabled: z.boolean().optional(),
ttsSpeed: z.number().optional(),
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/vscode-extension-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ export type ExtensionState = Pick<

enableCheckpoints: boolean
checkpointTimeout: number // Timeout for checkpoint initialization in seconds (default: 15)
perWriteCheckpoints: boolean
maxOpenTabsContext: number // Maximum number of VSCode open tabs to include in context (0-500)
maxWorkspaceFiles: number // Maximum number of files to include in current working directory details (0-500)
showRooIgnoredFiles: boolean // Whether to show .rooignore'd files in listings
Expand Down
14 changes: 14 additions & 0 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
public lastMessageTs?: number
private autoApprovalTimeoutRef?: NodeJS.Timeout

// B1: task-start baseline, recorded at most once (initiateTaskLoop also runs on resume).
private taskStartBaselineDone = false

// Tool Use
consecutiveMistakeCount: number = 0
consecutiveMistakeLimit: number
Expand Down Expand Up @@ -2492,6 +2495,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// arm needed.
void getCheckpointService(this)

// B1 task-start baseline: a suppressed pre-task root commit (default-on).
if (!this.taskStartBaselineDone) {
this.taskStartBaselineDone = true
const baselineEnabled = (await this.providerRef.deref()?.getState())?.perWriteCheckpoints
if (baselineEnabled !== false) {
// allowEmpty=true so a clean workspace still produces the baseline
// commit; awaited so the first per-write checkpoint cannot interleave.
await this.checkpointSave(true, true)
}
}

let nextUserContent = userContent
let includeFileDetails = true

Expand Down
92 changes: 92 additions & 0 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3289,6 +3289,98 @@ describe("Cline", () => {
})
})

describe("task-start baseline (B1 perWriteCheckpoints)", () => {
it("records one suppressed baseline checkpoint per Task instance at loop start", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "baseline task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
const saveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined)
const state = await mockProvider.getState()
vi.spyOn(mockProvider, "getState").mockResolvedValue(state)

task.abort = true

await taskAccess.initiateTaskLoop([])
await taskAccess.initiateTaskLoop([])

expect(saveSpy).toHaveBeenCalledOnce()
expect(saveSpy).toHaveBeenCalledWith(true, true)
})

it("records the baseline checkpoint when the setting is unset (default-on)", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "baseline unset task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
const saveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined)
const state = await mockProvider.getState()
// Unset: the property is absent from the state, so default-on applies.
const unsetState = { ...state }
Reflect.deleteProperty(unsetState, "perWriteCheckpoints")
vi.spyOn(mockProvider, "getState").mockResolvedValue(unsetState as typeof state)

task.abort = true

await taskAccess.initiateTaskLoop([])

expect(saveSpy).toHaveBeenCalledOnce()
expect(saveSpy).toHaveBeenCalledWith(true, true)
})

it("does not record a baseline checkpoint when perWriteCheckpoints is disabled", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "baseline disabled task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
const saveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined)
const state = await mockProvider.getState()
vi.spyOn(mockProvider, "getState").mockResolvedValue({ ...state, perWriteCheckpoints: false })

task.abort = true

await taskAccess.initiateTaskLoop([])

expect(saveSpy).not.toHaveBeenCalled()
})

it("awaits the baseline checkpoint before entering the request loop", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "baseline await task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
type SaveResult = Awaited<ReturnType<typeof task.checkpointSave>>
let resolveSave: (value: SaveResult | PromiseLike<SaveResult>) => void = () => {}
const saveSpy = vi
.spyOn(task, "checkpointSave")
.mockImplementation(() => new Promise<SaveResult>((resolve) => (resolveSave = resolve)))
const requestSpy = vi.spyOn(task, "recursivelyMakeClineRequests").mockResolvedValue(true)
vi.spyOn(mockProvider, "getState").mockResolvedValue({ ...(await mockProvider.getState()) })
const loopPromise = taskAccess.initiateTaskLoop([])

// The loop must not enter while the baseline checkpoint is still in flight.
await new Promise((resolve) => setTimeout(resolve, 0))
expect(saveSpy).toHaveBeenCalledOnce()
expect(requestSpy).not.toHaveBeenCalled()

resolveSave()
await loopPromise
expect(requestSpy).toHaveBeenCalled()
})
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

describe("start()", () => {
it("should be a no-op if the task was already started in the constructor", () => {
const task = new Task({
Expand Down
67 changes: 49 additions & 18 deletions src/core/tools/ApplyPatchTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { getReadablePath } from "../../utils/path"
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
import { Task } from "../task/Task"
import { checkpointSave } from "../checkpoints"
import { formatResponse } from "../prompts/responses"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { fileExistsAtPath } from "../../utils/fs"
Expand Down Expand Up @@ -102,7 +103,10 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
return
}

// Process each file change
// Process each file change. The handlers report whether their file
// operation succeeded, so a rejected approval or a failed local write
// does not get checkpointed as if the patch had succeeded.
let patchSucceeded = true
for (const change of changes) {
const relPath = change.path
const absolutePath = path.resolve(task.cwd, relPath)
Expand All @@ -120,17 +124,39 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {

if (change.type === "add") {
// Create new file
await this.handleAddFile(change, absolutePath, relPath, task, callbacks, isWriteProtected)
patchSucceeded =
(await this.handleAddFile(change, absolutePath, relPath, task, callbacks, isWriteProtected)) &&
patchSucceeded
} else if (change.type === "delete") {
// Delete file
await this.handleDeleteFile(absolutePath, relPath, task, callbacks, isWriteProtected)
patchSucceeded =
(await this.handleDeleteFile(absolutePath, relPath, task, callbacks, isWriteProtected)) &&
patchSucceeded
} else if (change.type === "update") {
// Update file
await this.handleUpdateFile(change, absolutePath, relPath, task, callbacks, isWriteProtected)
patchSucceeded =
(await this.handleUpdateFile(
change,
absolutePath,
relPath,
task,
callbacks,
isWriteProtected,
)) && patchSucceeded
}
}

task.consecutiveMistakeCount = 0

// B1: one checkpoint for the whole patch (not per file), and only when
// every file operation succeeded. Live setting with default-on
// semantics: skip only when explicitly false.
if (patchSucceeded) {
const perWriteCheckpoints = (await task.providerRef?.deref()?.getState())?.perWriteCheckpoints
if (perWriteCheckpoints !== false) {
void checkpointSave(task, false, true).catch(() => {})
}
}
} catch (error) {
await handleError("apply patch", error as Error)
await task.diffViewProvider.reset()
Expand All @@ -144,7 +170,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
task: Task,
callbacks: ToolCallbacks,
isWriteProtected: boolean,
): Promise<void> {
): Promise<boolean> {
const { askApproval, pushToolResult } = callbacks

// Check if file already exists
Expand All @@ -155,7 +181,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
const errorMessage = `File already exists: ${relPath}. Use Update File instead.`
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
return
return false
}

const newContent = change.newContent || ""
Expand Down Expand Up @@ -209,7 +235,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
}
pushToolResult("Changes were rejected by the user.")
await task.diffViewProvider.reset()
return
return false
}

// Save the changes
Expand All @@ -227,6 +253,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
pushToolResult(message)
await task.diffViewProvider.reset()
task.processQueuedMessages()
return true
}

private async handleDeleteFile(
Expand All @@ -235,7 +262,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
task: Task,
callbacks: ToolCallbacks,
isWriteProtected: boolean,
): Promise<void> {
): Promise<boolean> {
const { askApproval, pushToolResult } = callbacks

// Check if file exists
Expand All @@ -246,7 +273,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
const errorMessage = `File not found: ${relPath}. Cannot delete a non-existent file.`
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
return
return false
}

const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath)
Expand All @@ -268,7 +295,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {

if (!didApprove) {
pushToolResult("Delete operation was rejected by the user.")
return
return false
}

// Delete the file
Expand All @@ -278,12 +305,13 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
const errorMessage = `Failed to delete file '${relPath}': ${error instanceof Error ? error.message : String(error)}`
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
return
return false
}

task.didEditFile = true
pushToolResult(`Successfully deleted ${relPath}`)
task.processQueuedMessages()
return true
}

private async handleUpdateFile(
Expand All @@ -293,7 +321,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
task: Task,
callbacks: ToolCallbacks,
isWriteProtected: boolean,
): Promise<void> {
): Promise<boolean> {
const { askApproval, pushToolResult } = callbacks

// Check if file exists
Expand All @@ -304,7 +332,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
const errorMessage = `File not found: ${relPath}. Cannot update a non-existent file.`
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
return
return false
}

const originalContent = change.originalContent || ""
Expand All @@ -318,9 +346,11 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
// Generate and validate diff
const diff = formatResponse.createPrettyPatch(relPath, originalContent, newContent)
if (!diff) {
// A no-op change is not a failure: the patch processed cleanly and
// nothing was written, so the whole-patch success state is kept.
pushToolResult(`No changes needed for '${relPath}'`)
await task.diffViewProvider.reset()
return
return true
}

// Check experiment settings
Expand Down Expand Up @@ -366,7 +396,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
}
pushToolResult("Changes were rejected by the user.")
await task.diffViewProvider.reset()
return
return false
}

// Handle file move if specified
Expand All @@ -379,7 +409,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
await task.say("rooignore_error", change.movePath)
pushToolResult(formatResponse.rooIgnoreError(change.movePath))
await task.diffViewProvider.reset()
return
return false
}

// Check if destination path is write-protected
Expand All @@ -391,7 +421,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
await task.diffViewProvider.reset()
return
return false
}

// Check if destination path is outside workspace
Expand All @@ -403,7 +433,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
await task.say("error", errorMessage)
pushToolResult(formatResponse.toolError(errorMessage))
await task.diffViewProvider.reset()
return
return false
}

// Save new content to the new path
Expand Down Expand Up @@ -447,6 +477,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
pushToolResult(message)
await task.diffViewProvider.reset()
task.processQueuedMessages()
return true
}

override async handlePartial(task: Task, block: ToolUse<"apply_patch">): Promise<void> {
Expand Down
Loading
Loading