feat(tools): per-write checkpoint + change card for apply_diff (B3a v2-8a, epic #1375)

src/core/tools/ApplyDiffTool.ts

@@ -13,6 +13,8 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization"
 import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
 import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
 import type { ToolUse } from "../../shared/tools"
+import { checkAutoApproval } from "../auto-approval"
+import { checkpointSave } from "../checkpoints"
 
 import { BaseTool, ToolCallbacks } from "./BaseTool"
 
@@ -144,16 +146,19 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 				diff: diffContent,
 			}
 
+			// Both save branches present the same approval message, and the B3a
+			// per-write checkpoint (below) reuses it for auto-approval parity.
+			const completeMessage = JSON.stringify({
+				...sharedMessageProps,
+				diff: diffContent,
+				content: unifiedPatch,
+				originalContent,
+				diffStats,
+				isProtected: isWriteProtected,
+			} satisfies ClineSayTool)
+
 			if (isPreventFocusDisruptionEnabled) {
 				// Direct file write without diff view
-				const completeMessage = JSON.stringify({
-					...sharedMessageProps,
-					diff: diffContent,
-					content: unifiedPatch,
-					originalContent,
-					diffStats,
-					isProtected: isWriteProtected,
-				} satisfies ClineSayTool)
 
 				let toolProgressStatus
 
@@ -191,15 +196,6 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 				await task.diffViewProvider.update(diffResult.content, true)
 				task.diffViewProvider.scrollToFirstDiff()
 
-				const completeMessage = JSON.stringify({
-					...sharedMessageProps,
-					diff: diffContent,
-					content: unifiedPatch,
-					originalContent,
-					diffStats,
-					isProtected: isWriteProtected,
-				} satisfies ClineSayTool)
-
 				let toolProgressStatus
 
 				if (task.diffStrategy && task.diffStrategy.getProgressStatus) {
@@ -224,6 +220,36 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 				await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
 			}
 
+			// B3a: per-write checkpoint + change card for the apply_diff write, with
+			// the same parity as write_to_file / edit_file / apply_patch (this tool
+			// previously wrote with no checkpoint and no card, so its edits had no
+			// per-file rollback surface in chat). The applied unified diff and its
+			// stats were already computed above for the tool message and are reused
+			// verbatim; auto-approved steps always get the compact card. Live setting
+			// with default-on semantics: skip only when explicitly false. apply_diff
+			// only ever modifies an existing file (a missing file errors out before
+			// the diff is applied), so the operation is always "update".
+			const perWriteCheckpoints = state?.perWriteCheckpoints
+			if (perWriteCheckpoints !== false) {
+				const autoApproved =
+					(
+						await checkAutoApproval({
+							state,
+							cwd: task.cwd,
+							ask: "tool",
+							text: completeMessage,
+							isProtected: isWriteProtected,
+						})
+					).decision === "approve"
+				await checkpointSave(task, false, true, {
+					path: relPath,
+					operation: "update",
+					...(diffStats ? { diffStats: { additions: diffStats.added, deletions: diffStats.removed } } : {}),
+					...(unifiedPatch ? { diff: unifiedPatch } : {}),
+					...(autoApproved ? { autoApproved: true } : {}),
+				}).catch(() => {})
+			}
+
 			// Track file edit operation
 			if (relPath) {
 				await task.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)

src/core/tools/__tests__/ApplyDiffTool.changeCard.spec.ts

@@ -0,0 +1,242 @@
+// npx vitest run core/tools/__tests__/ApplyDiffTool.changeCard.spec.ts
+
+import type { MockedFunction } from "vitest"
+
+import { fileExistsAtPath } from "../../../utils/fs"
+import { checkAutoApproval } from "../../auto-approval"
+import { checkpointSave } from "../../checkpoints"
+import type { Task } from "../../task/Task"
+import { ApplyDiffTool } from "../ApplyDiffTool"
+
+vi.mock("fs/promises", () => ({
+	default: {
+		readFile: vi.fn().mockResolvedValue("original file content\n"),
+		// The trial aggregate adds the S4b stat-pair self-observation around this
+		// read (ApplyDiffTool records its own read under the ReadFileTool contract);
+		// its `.catch(() => undefined)` fallback keeps the observation no-op when
+		// stat rejects, so this spec stays green on both the component branch and
+		// the trial aggregate.
+		stat: vi.fn().mockRejectedValue(new Error("stat unavailable in this spec")),
+	},
+}))
+
+vi.mock("../../../utils/fs", () => ({
+	fileExistsAtPath: vi.fn().mockResolvedValue(true),
+}))
+
+vi.mock("../../prompts/responses", () => ({
+	formatResponse: {
+		toolError: vi.fn((msg: string) => `Error: ${msg}`),
+		rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`),
+		createPrettyPatch: vi.fn(() => "mock-diff"),
+	},
+}))
+
+vi.mock("../../diff/stats", () => ({
+	// The real DiffStats shape is { added, removed } (the tool maps it to the
+	// change-card { additions, deletions } pair).
+	sanitizeUnifiedDiff: vi.fn((diff: string) => diff),
+	computeDiffStats: vi.fn(() => ({ added: 1, removed: 1 })),
+}))
+
+vi.mock("../../checkpoints", () => ({
+	checkpointSave: vi.fn().mockResolvedValue(undefined),
+}))
+
+vi.mock("../../auto-approval", () => ({
+	checkAutoApproval: vi.fn().mockResolvedValue({ decision: "ask" }),
+}))
+
+describe("ApplyDiffTool.execute - per-write checkpoint and change card (B3a, epic #1375)", () => {
+	const mockedFileExistsAtPath = fileExistsAtPath as MockedFunction<typeof fileExistsAtPath>
+	const mockCheckpointSave = checkpointSave as MockedFunction<typeof checkpointSave>
+	const mockCheckAutoApproval = checkAutoApproval as MockedFunction<typeof checkAutoApproval>
+
+	let tool: ApplyDiffTool
+	let mockTask: Pick<
+		Task,
+		| "cwd"
+		| "consecutiveMistakeCount"
+		| "consecutiveMistakeCountForApplyDiff"
+		| "recordToolError"
+		| "rooIgnoreController"
+		| "rooProtectedController"
+		| "processQueuedMessages"
+		| "didEditFile"
+		| "api"
+		| "diffStrategy"
+		| "diffViewProvider"
+		| "providerRef"
+		| "fileContextTracker"
+	>
+	let mockSaveDirectly: MockedFunction<(...args: unknown[]) => Promise<unknown>>
+	let mockGetState: MockedFunction<() => Promise<Record<string, unknown>>>
+	let mockAskApproval: MockedFunction<(...args: unknown[]) => Promise<boolean>>
+	let mockHandleError: MockedFunction<(...args: unknown[]) => Promise<void>>
+	let mockPushToolResult: MockedFunction<(...args: unknown[]) => void>
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+
+		mockedFileExistsAtPath.mockResolvedValue(true)
+
+		// Structural stubs for the prevent-focus-disruption save path: the real
+		// DiffViewProvider is out of scope here, so vi.fn() doubles stand in for
+		// the members the tool touches.
+		mockSaveDirectly = vi.fn().mockResolvedValue({
+			newProblemsMessage: "",
+			userEdits: undefined,
+			finalContent: "new content",
+		})
+		const diffViewProviderStub = {
+			editType: undefined as "create" | "modify" | undefined,
+			originalContent: undefined as string | undefined,
+			saveDirectly: mockSaveDirectly,
+			open: vi.fn().mockResolvedValue(undefined),
+			update: vi.fn().mockResolvedValue(undefined),
+			scrollToFirstDiff: vi.fn(),
+			saveChanges: vi.fn().mockResolvedValue(undefined),
+			pushToolWriteResult: vi.fn().mockResolvedValue("Saved file"),
+			reset: vi.fn().mockResolvedValue(undefined),
+		}
+
+		mockGetState = vi.fn().mockResolvedValue({
+			diagnosticsEnabled: true,
+			writeDelayMs: 1000,
+			// Exercise the focus-disruption (saveDirectly) save path.
+			experiments: { preventFocusDisruption: true },
+		})
+
+		mockTask = {
+			cwd: "/workspace/project",
+			consecutiveMistakeCount: 0,
+			consecutiveMistakeCountForApplyDiff: new Map(),
+			recordToolError: vi.fn(),
+			rooIgnoreController: {
+				validateAccess: vi.fn().mockReturnValue(true),
+			} as unknown as Task["rooIgnoreController"],
+			rooProtectedController: {
+				isWriteProtected: vi.fn().mockReturnValue(false),
+			} as unknown as Task["rooProtectedController"],
+			processQueuedMessages: vi.fn(),
+			didEditFile: false,
+			api: {
+				getModel: () => ({ id: "claude-sonnet-4-5" }),
+			} as unknown as Task["api"],
+			diffStrategy: {
+				applyDiff: vi.fn().mockResolvedValue({ success: true, content: "modified file content\n" }),
+			} as unknown as Task["diffStrategy"],
+			diffViewProvider: diffViewProviderStub as unknown as Task["diffViewProvider"],
+			providerRef: {
+				deref: vi.fn().mockReturnValue({
+					getState: mockGetState,
+				}),
+			} as unknown as Task["providerRef"],
+			fileContextTracker: {
+				trackFileContext: vi.fn().mockResolvedValue(undefined),
+			} as unknown as Task["fileContextTracker"],
+		}
+
+		mockAskApproval = vi.fn().mockResolvedValue(true)
+		mockHandleError = vi.fn().mockResolvedValue(undefined)
+		mockPushToolResult = vi.fn()
+
+		tool = new ApplyDiffTool()
+	})
+
+	it("records a per-write checkpoint with the applied diff after a successful write", async () => {
+		await tool.execute({ path: "src/thing.ts", diff: "unified diff" }, mockTask as Task, {
+			askApproval: mockAskApproval,
+			handleError: mockHandleError,
+			pushToolResult: mockPushToolResult,
+		})
+
+		expect(mockCheckpointSave).toHaveBeenCalledTimes(1)
+		expect(mockCheckpointSave).toHaveBeenCalledWith(mockTask, false, true, {
+			path: "src/thing.ts",
+			operation: "update",
+			diffStats: { additions: 1, deletions: 1 },
+			diff: "mock-diff",
+		})
+		// The card builder is fed the tool's own auto-approval decision, so the
+		// call must carry the live state, the tool's ask channel, the approval
+		// message text, and the protection flag (pins the call-argument shape).
+		expect(mockCheckAutoApproval).toHaveBeenCalledTimes(1)
+		expect(mockCheckAutoApproval).toHaveBeenCalledWith(
+			expect.objectContaining({
+				state: expect.objectContaining({ diagnosticsEnabled: true }),
+				cwd: "/workspace/project",
+				ask: "tool",
+				text: expect.stringContaining("appliedDiff"),
+				isProtected: false,
+			}),
+		)
+	})
+
+	it("marks auto-approved steps so the card renders compact", async () => {
+		mockCheckAutoApproval.mockResolvedValueOnce({ decision: "approve" })
+
+		await tool.execute({ path: "src/thing.ts", diff: "unified diff" }, mockTask as Task, {
+			askApproval: mockAskApproval,
+			handleError: mockHandleError,
+			pushToolResult: mockPushToolResult,
+		})
+
+		expect(mockCheckpointSave).toHaveBeenCalledTimes(1)
+		expect(mockCheckpointSave.mock.calls[0]?.[3]).toEqual(expect.objectContaining({ autoApproved: true }))
+	})
+
+	it("skips the checkpoint when perWriteCheckpoints is explicitly disabled", async () => {
+		mockGetState.mockResolvedValueOnce({
+			diagnosticsEnabled: true,
+			writeDelayMs: 1000,
+			experiments: { preventFocusDisruption: true },
+			perWriteCheckpoints: false,
+		})
+
+		await tool.execute({ path: "src/thing.ts", diff: "unified diff" }, mockTask as Task, {
+			askApproval: mockAskApproval,
+			handleError: mockHandleError,
+			pushToolResult: mockPushToolResult,
+		})
+
+		expect(mockCheckpointSave).not.toHaveBeenCalled()
+		// The write itself still happens (the setting gates the checkpoint only),
+		// and auto-approval is never consulted when there is no card to build.
+		expect(mockSaveDirectly).toHaveBeenCalled()
+		expect(mockCheckAutoApproval).not.toHaveBeenCalled()
+	})
+
+	it("keeps the checkpoint path alive when the provider state is unavailable", async () => {
+		// providerRef.deref() -> undefined: state is undefined, so
+		// state?.perWriteCheckpoints must short-circuit (not throw) and the
+		// default-on semantics still record the checkpoint.
+		mockTask.providerRef = {
+			deref: vi.fn().mockReturnValue(undefined),
+		} as unknown as Task["providerRef"]
+
+		await tool.execute({ path: "src/thing.ts", diff: "unified diff" }, mockTask as Task, {
+			askApproval: mockAskApproval,
+			handleError: mockHandleError,
+			pushToolResult: mockPushToolResult,
+		})
+
+		expect(mockCheckpointSave).toHaveBeenCalledTimes(1)
+		expect(mockCheckAutoApproval).toHaveBeenCalledWith(
+			expect.objectContaining({ state: undefined, ask: "tool", cwd: "/workspace/project" }),
+		)
+	})
+
+	it("records nothing when the approval is declined", async () => {
+		mockAskApproval.mockResolvedValue(false)
+
+		await tool.execute({ path: "src/thing.ts", diff: "unified diff" }, mockTask as Task, {
+			askApproval: mockAskApproval,
+			handleError: mockHandleError,
+			pushToolResult: mockPushToolResult,
+		})
+
+		expect(mockCheckpointSave).not.toHaveBeenCalled()
+		expect(mockSaveDirectly).not.toHaveBeenCalled()
+	})
+})

src/core/tools/apply-patch/apply.ts

@@ -29,6 +29,15 @@ export interface ApplyPatchFileChange {
 	originalContent?: string
 	/** New content (for add/update) */
 	newContent?: string
+	/**
+	 * B3a: the unified approval diff for this file (computed by the tool
+	 * handler), reused by the per-step change card.
+	 */
+	diff?: string
+	/** B3a: { added, removed } stats of the approval diff, when computable. */
+	diffStats?: { added: number; removed: number }
+	/** B3a: whether this file's approval was auto-approved. */
+	autoApproved?: boolean
 }
 
 /**