Consolidate checkpoint creation to single location

Addresses @cte's feedback in PR #4840 comment:
- Remove existing logic that saves checkpoints AFTER changes in presentAssistantMessage.ts
- Consolidate checkpoint creation to askApproval function for file-editing tools
- Remove individual checkpoint calls from writeToFileTool, applyDiffTool, insertContentTool, and searchAndReplaceTool
- Handle special case for multiApplyDiffTool batch operations
- Maintains fix for #4827: checkpoints are created BEFORE file modifications

This reduces code duplication and centralizes checkpoint logic in a single place.

Author: roomote

roo-code-messages.log

@@ -289,6 +289,17 @@ export async function presentAssistantMessage(cline: Task) {
 					pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images))
 				}
 
+				// Create checkpoint BEFORE file modifications for file-editing tools (fixes #4827)
+				const isFileEditingTool = [
+					"write_to_file",
+					"apply_diff",
+					"insert_content",
+					"search_and_replace",
+				].includes(block.name)
+				if (isFileEditingTool && cline.enableCheckpoints) {
+					await cline.checkpointSave()
+				}
+
 				return true
 			}
 
@@ -521,15 +532,8 @@ export async function presentAssistantMessage(cline: Task) {
 			break
 	}
 
-	// Note: Checkpoints are now created BEFORE file modifications in individual tools
-	// This section is kept for any edge cases where files might be modified outside of tools
-	const recentlyModifiedFiles = cline.fileContextTracker.getAndClearCheckpointPossibleFile()
-
-	if (recentlyModifiedFiles.length > 0) {
-		// This should rarely be triggered now since checkpoints are created before modifications
-		console.log("[presentAssistantMessage] Creating checkpoint after file modification (fallback)")
-		await checkpointSave(cline)
-	}
+	// Checkpoints are now created BEFORE file modifications in the askApproval function above
+	// No longer need to create checkpoints after file modifications
 
 	// Seeing out of bounds is fine, it means that the next too call is being
 	// built up and ready to add to assistantMessageContent to present.

src/core/assistant-message/presentAssistantMessage.ts

@@ -165,10 +165,7 @@ export async function applyDiffToolLegacy(
 				return
 			}
 
-			// Create checkpoint BEFORE making changes (fixes #4827)
-			if (cline.enableCheckpoints) {
-				await cline.checkpointSave()
-			}
+			// Checkpoint is now created in askApproval function before this point (fixes #4827)
 
 			// Call saveChanges to update the DiffViewProvider properties
 			await cline.diffViewProvider.saveChanges()

src/core/tools/applyDiffTool.ts

@@ -140,10 +140,7 @@ export async function insertContentTool(
 			return
 		}
 
-		// Create checkpoint BEFORE making changes (fixes #4827)
-		if (cline.enableCheckpoints) {
-			await cline.checkpointSave()
-		}
+		// Checkpoint is now created in askApproval function before this point (fixes #4827)
 
 		// Call saveChanges to update the DiffViewProvider properties
 		await cline.diffViewProvider.saveChanges()

src/core/tools/insertContentTool.ts

@@ -301,6 +301,11 @@ Original error: ${errorMessage}`
 				operationsToApprove.forEach((opResult) => {
 					updateOperationResult(opResult.path, { status: "approved" })
 				})
+
+				// Create checkpoint BEFORE processing batch operations (fixes #4827)
+				if (cline.enableCheckpoints) {
+					await cline.checkpointSave()
+				}
 			} else if (response === "noButtonClicked") {
 				// Deny all files
 				if (text) {
@@ -321,12 +326,14 @@ Original error: ${errorMessage}`
 					if (parsedResponse.action === "applyDiff" && parsedResponse.approvedFiles) {
 						const approvedFiles = parsedResponse.approvedFiles
 						let hasAnyDenial = false
+						let hasAnyApproval = false
 
 						operationsToApprove.forEach((opResult) => {
 							const approved = approvedFiles[opResult.path] === true
 
 							if (approved) {
 								updateOperationResult(opResult.path, { status: "approved" })
+								hasAnyApproval = true
 							} else {
 								hasAnyDenial = true
 								updateOperationResult(opResult.path, {
@@ -339,17 +346,24 @@ Original error: ${errorMessage}`
 						if (hasAnyDenial) {
 							cline.didRejectTool = true
 						}
+
+						// Create checkpoint BEFORE processing approved operations (fixes #4827)
+						if (hasAnyApproval && cline.enableCheckpoints) {
+							await cline.checkpointSave()
+						}
 					} else {
 						// Legacy individual permissions format
 						const individualPermissions = parsedResponse
 						let hasAnyDenial = false
+						let hasAnyApproval = false
 
 						batchDiffs.forEach((batchDiff, index) => {
 							const opResult = operationsToApprove[index]
 							const approved = individualPermissions[batchDiff.key] === true
 
 							if (approved) {
 								updateOperationResult(opResult.path, { status: "approved" })
+								hasAnyApproval = true
 							} else {
 								hasAnyDenial = true
 								updateOperationResult(opResult.path, {
@@ -362,6 +376,11 @@ Original error: ${errorMessage}`
 						if (hasAnyDenial) {
 							cline.didRejectTool = true
 						}
+
+						// Create checkpoint BEFORE processing approved operations (fixes #4827)
+						if (hasAnyApproval && cline.enableCheckpoints) {
+							await cline.checkpointSave()
+						}
 					}
 				} catch (error) {
 					// Fallback: if JSON parsing fails, deny all files
@@ -381,11 +400,7 @@ Original error: ${errorMessage}`
 			updateOperationResult(opResult.path, { status: "approved" })
 		}
 
-		// Create checkpoint BEFORE processing any approved operations (fixes #4827)
-		const approvedOperations = operationResults.filter((op) => op.status === "approved")
-		if (approvedOperations.length > 0 && cline.enableCheckpoints) {
-			await cline.checkpointSave()
-		}
+		// Checkpoint is now created in askApproval function before this point (fixes #4827)
 
 		// Process approved operations
 		const results: string[] = []

src/core/tools/multiApplyDiffTool.ts

@@ -226,10 +226,7 @@ export async function searchAndReplaceTool(
 			return
 		}
 
-		// Create checkpoint BEFORE making changes (fixes #4827)
-		if (cline.enableCheckpoints) {
-			await cline.checkpointSave()
-		}
+		// Checkpoint is now created in askApproval function before this point (fixes #4827)
 
 		// Call saveChanges to update the DiffViewProvider properties
 		await cline.diffViewProvider.saveChanges()

src/core/tools/searchAndReplaceTool.ts

@@ -212,10 +212,7 @@ export async function writeToFileTool(
 				return
 			}
 
-			// Create checkpoint BEFORE making changes (fixes #4827)
-			if (cline.enableCheckpoints) {
-				await cline.checkpointSave()
-			}
+			// Checkpoint is now created in askApproval function before this point (fixes #4827)
 
 			// Call saveChanges to update the DiffViewProvider properties
 			await cline.diffViewProvider.saveChanges()