Skip to content

Commit 21661d7

Browse files
playcationshannesrudolph
authored andcommitted
Files Changed Overview updating after file edits using tool calls.
Problem: - FCO missing last edited file (calculated at checkpoint creation before tools execute) - FCO disappears when tasks are aborted (state not preserved) - Manual user edits must remain protected during rollback (issue #4827) Solution: - Add immediate FCO updates after each file editing tool execution - Preserve FCO state during task abort and restore on resume - Maintain checkpoint timing BEFORE edits for rollback safety - Add final checkpoint on task completion to capture all changes Changes: - Add updateFCOAfterEdit helper to calculate and display changes without checkpoints - Update presentAssistantMessage to call FCO updates after file tools - Add final checkpoint in attemptCompletionTool - Preserve/restore FCO state in ClineProvider during abort/resume - Add test utilities for checkpoint functionality This separates FCO visibility (immediate updates) from checkpoint safety (before edits), solving both user experience issues while maintaining rollback protection.
1 parent 32dafd3 commit 21661d7

File tree

7 files changed

+226
-12
lines changed

7 files changed

+226
-12
lines changed

src/core/assistant-message/presentAssistantMessage.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { Task } from "../task/Task"
3737
import { codebaseSearchTool } from "../tools/codebaseSearchTool"
3838
import { experiments, EXPERIMENT_IDS } from "../../shared/experiments"
3939
import { applyDiffToolLegacy } from "../tools/applyDiffTool"
40+
import { updateFCOAfterEdit } from "../../services/file-changes/updateAfterEdit"
4041

4142
/**
4243
* Processes and presents assistant message content to the user interface.
@@ -426,6 +427,7 @@ export async function presentAssistantMessage(cline: Task) {
426427
case "write_to_file":
427428
await checkpointSaveAndMark(cline)
428429
await writeToFileTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
430+
await updateFCOAfterEdit(cline)
429431
break
430432
case "update_todo_list":
431433
await updateTodoListTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
@@ -446,6 +448,7 @@ export async function presentAssistantMessage(cline: Task) {
446448
if (isMultiFileApplyDiffEnabled) {
447449
await checkpointSaveAndMark(cline)
448450
await applyDiffTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
451+
await updateFCOAfterEdit(cline)
449452
} else {
450453
await checkpointSaveAndMark(cline)
451454
await applyDiffToolLegacy(
@@ -456,16 +459,19 @@ export async function presentAssistantMessage(cline: Task) {
456459
pushToolResult,
457460
removeClosingTag,
458461
)
462+
await updateFCOAfterEdit(cline)
459463
}
460464
break
461465
}
462466
case "insert_content":
463467
await checkpointSaveAndMark(cline)
464468
await insertContentTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
469+
await updateFCOAfterEdit(cline)
465470
break
466471
case "search_and_replace":
467472
await checkpointSaveAndMark(cline)
468473
await searchAndReplaceTool(cline, block, askApproval, handleError, pushToolResult, removeClosingTag)
474+
await updateFCOAfterEdit(cline)
469475
break
470476
case "read_file":
471477
// Check if this model should use the simplified single-file read tool
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { vitest } from "vitest"
2+
3+
export const createMockTask = (options: {
4+
taskId: string
5+
hasExistingCheckpoints?: boolean
6+
enableCheckpoints?: boolean
7+
provider?: any
8+
}) => {
9+
const mockTask = {
10+
taskId: options.taskId,
11+
instanceId: "test-instance",
12+
rootTask: undefined as any,
13+
parentTask: undefined as any,
14+
taskNumber: 1,
15+
workspacePath: "/mock/workspace",
16+
enableCheckpoints: options.enableCheckpoints ?? true,
17+
checkpointService: null as any,
18+
checkpointServiceInitializing: false,
19+
ongoingCheckpointSaves: new Map(),
20+
clineMessages: options.hasExistingCheckpoints
21+
? [{ say: "checkpoint_saved", ts: Date.now(), text: "existing-checkpoint-hash" }]
22+
: [],
23+
providerRef: {
24+
deref: () => options.provider || createMockProvider(),
25+
},
26+
fileContextTracker: {},
27+
todoList: undefined,
28+
}
29+
30+
return mockTask
31+
}
32+
33+
export const createMockProvider = () => ({
34+
getFileChangeManager: vitest.fn(),
35+
ensureFileChangeManager: vitest.fn(),
36+
log: vitest.fn(),
37+
postMessageToWebview: vitest.fn(),
38+
getGlobalState: vitest.fn(),
39+
})
40+
41+
// Mock checkpoint service for testing
42+
export const createMockCheckpointService = () => ({
43+
saveCheckpoint: vitest.fn().mockResolvedValue({
44+
commit: "mock-checkpoint-hash",
45+
message: "Mock checkpoint",
46+
}),
47+
restoreCheckpoint: vitest.fn().mockResolvedValue(true),
48+
getDiff: vitest.fn().mockResolvedValue([]),
49+
getCheckpoints: vitest.fn().mockReturnValue([]),
50+
getCurrentCheckpoint: vitest.fn().mockReturnValue("mock-current-checkpoint"),
51+
initShadowGit: vitest.fn().mockResolvedValue(true),
52+
baseHash: "mock-base-hash",
53+
})

src/core/checkpoints/index.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,13 @@ async function checkGitInstallation(
168168
// Don't throw - allow checkpoint service to continue initializing
169169
}
170170

171+
// Note: No initialization checkpoint needed - first checkpoint before file edit serves as baseline
171172
if (isCheckpointNeeded) {
172-
log("[Task#getCheckpointService] no checkpoints found, saving initial checkpoint")
173-
checkpointSave(task, true)
173+
log(
174+
"[Task#getCheckpointService] no checkpoints found, will create baseline checkpoint before first file edit",
175+
)
174176
} else {
175-
log("[Task#getCheckpointService] existing checkpoints found, skipping initial checkpoint")
177+
log("[Task#getCheckpointService] existing checkpoints found, using existing checkpoint as baseline")
176178
}
177179
} catch (err) {
178180
log("[Task#getCheckpointService] caught error in on('initialize'), disabling checkpoints")

src/core/tools/attemptCompletionTool.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,21 @@ export async function attemptCompletionTool(
8989

9090
cline.consecutiveMistakeCount = 0
9191

92+
// Create final checkpoint to capture the last file edit before completion
93+
if (cline.enableCheckpoints) {
94+
try {
95+
await cline.checkpointSave(true) // Force save to capture any final changes
96+
cline.providerRef
97+
.deref()
98+
?.log("[attemptCompletionTool] Created final checkpoint before task completion")
99+
} catch (error) {
100+
// Non-critical error, don't fail completion
101+
cline.providerRef
102+
.deref()
103+
?.log(`[attemptCompletionTool] Failed to create final checkpoint: ${error}`)
104+
}
105+
}
106+
92107
// Command execution is permanently disabled in attempt_completion
93108
// Users must use execute_command tool separately before attempt_completion
94109
await cline.say("completion_result", result, undefined, false)

src/core/tools/writeToFileTool.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,6 @@ export async function writeToFileTool(
304304
// Get the formatted response message
305305
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
306306

307-
// Track file as edited by LLM for FCO
308-
try {
309-
await cline.fileContextTracker.trackFileContext(relPath.toString(), "roo_edited")
310-
} catch (error) {
311-
console.error("Failed to track file edit in context:", error)
312-
}
313-
314307
pushToolResult(message)
315308

316309
await cline.diffViewProvider.reset()

src/core/webview/ClineProvider.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,9 @@ export class ClineProvider
818818
await this.removeClineFromStack()
819819
}
820820

821-
public async createTaskWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }) {
821+
public async createTaskWithHistoryItem(
822+
historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task; preservedFCOState?: any },
823+
) {
822824
await this.removeClineFromStack()
823825

824826
// If the history item has a saved mode, restore it and its associated API configuration.
@@ -940,6 +942,37 @@ export class ClineProvider
940942
}, 100) // Small delay to ensure task is fully ready
941943
}
942944

945+
// Restore preserved FCO state if provided (from task abort/cancel)
946+
if (historyItem.preservedFCOState) {
947+
try {
948+
const fileChangeManager = await this.ensureFileChangeManager()
949+
if (fileChangeManager && historyItem.preservedFCOState.files) {
950+
// Restore the file changes in FileChangeManager
951+
fileChangeManager.setFiles(historyItem.preservedFCOState.files)
952+
953+
// Send restored FCO state to webview
954+
const filteredChangeset = await fileChangeManager.getLLMOnlyChanges(
955+
task.taskId,
956+
task.fileContextTracker,
957+
)
958+
959+
if (filteredChangeset.files.length > 0) {
960+
this.postMessageToWebview({
961+
type: "filesChanged",
962+
filesChanged: filteredChangeset,
963+
})
964+
965+
this.log(
966+
`[createTaskWithHistoryItem] Restored FCO state with ${filteredChangeset.files.length} LLM-only file changes`,
967+
)
968+
}
969+
}
970+
} catch (error) {
971+
this.log(`[createTaskWithHistoryItem] Failed to restore FCO state: ${error}`)
972+
// Non-critical error, don't fail task creation
973+
}
974+
}
975+
943976
return task
944977
}
945978

@@ -2506,6 +2539,18 @@ export class ClineProvider
25062539
const rootTask = task.rootTask
25072540
const parentTask = task.parentTask
25082541

2542+
// Preserve FCO state before aborting task to prevent FCO from disappearing
2543+
let preservedFCOState: any = undefined
2544+
try {
2545+
const fileChangeManager = this.getFileChangeManager()
2546+
if (fileChangeManager) {
2547+
preservedFCOState = fileChangeManager.getChanges()
2548+
this.log(`[cancelTask] Preserved FCO state with ${preservedFCOState.files.length} files`)
2549+
}
2550+
} catch (error) {
2551+
this.log(`[cancelTask] Failed to preserve FCO state: ${error}`)
2552+
}
2553+
25092554
task.abortTask()
25102555

25112556
await pWaitFor(
@@ -2532,7 +2577,7 @@ export class ClineProvider
25322577
}
25332578

25342579
// Clears task again, so we need to abortTask manually above.
2535-
await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask })
2580+
await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask, preservedFCOState })
25362581
}
25372582

25382583
// Clear the current task without treating it as a subtask.
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { Task } from "../../core/task/Task"
2+
import { getCheckpointService } from "../../core/checkpoints"
3+
import { FileChangeType } from "@roo-code/types"
4+
import { FileChangeManager } from "./FileChangeManager"
5+
6+
/**
7+
* Updates FCO immediately after a file edit without changing checkpoint timing.
8+
* This provides immediate visibility of changes while preserving rollback safety.
9+
*/
10+
export async function updateFCOAfterEdit(task: Task): Promise<void> {
11+
const provider = task.providerRef.deref()
12+
if (!provider) {
13+
return
14+
}
15+
16+
try {
17+
const fileChangeManager = provider.getFileChangeManager()
18+
const checkpointService = await getCheckpointService(task)
19+
20+
if (!fileChangeManager || !checkpointService || !task.taskId || !task.fileContextTracker) {
21+
return
22+
}
23+
24+
// Get current baseline for FCO
25+
const baseline = fileChangeManager.getChanges().baseCheckpoint
26+
27+
// Calculate diff from baseline to current working directory state
28+
// We use the checkpointService to get a diff from baseline to HEAD (current state)
29+
try {
30+
const changes = await checkpointService.getDiff({
31+
from: baseline,
32+
to: "HEAD", // Current working directory state
33+
})
34+
35+
if (!changes || changes.length === 0) {
36+
// No changes detected, keep current FCO state
37+
return
38+
}
39+
40+
// Convert checkpoint service changes to FileChange format
41+
const fileChanges = changes.map((change: any) => {
42+
const type = (
43+
change.paths.newFile ? "create" : change.paths.deletedFile ? "delete" : "edit"
44+
) as FileChangeType
45+
46+
// Calculate line differences
47+
let linesAdded = 0
48+
let linesRemoved = 0
49+
50+
if (type === "create") {
51+
linesAdded = change.content.after ? change.content.after.split("\n").length : 0
52+
linesRemoved = 0
53+
} else if (type === "delete") {
54+
linesAdded = 0
55+
linesRemoved = change.content.before ? change.content.before.split("\n").length : 0
56+
} else {
57+
const lineDifferences = FileChangeManager.calculateLineDifferences(
58+
change.content.before || "",
59+
change.content.after || "",
60+
)
61+
linesAdded = lineDifferences.linesAdded
62+
linesRemoved = lineDifferences.linesRemoved
63+
}
64+
65+
return {
66+
uri: change.paths.relative,
67+
type,
68+
fromCheckpoint: baseline,
69+
toCheckpoint: "HEAD", // This represents current state, not an actual checkpoint
70+
linesAdded,
71+
linesRemoved,
72+
}
73+
})
74+
75+
// Update FileChangeManager with the new files
76+
fileChangeManager.setFiles(fileChanges)
77+
78+
// Get LLM-only changes for the webview (filters out accepted/rejected files)
79+
const filteredChangeset = await fileChangeManager.getLLMOnlyChanges(task.taskId, task.fileContextTracker)
80+
81+
// Send updated changes to webview only if there are changes to show
82+
if (filteredChangeset.files.length > 0) {
83+
provider.postMessageToWebview({
84+
type: "filesChanged",
85+
filesChanged: filteredChangeset,
86+
})
87+
88+
provider.log(
89+
`[updateFCOAfterEdit] Updated FCO with ${filteredChangeset.files.length} LLM-only file changes`,
90+
)
91+
}
92+
} catch (diffError) {
93+
// If we can't calculate diff (e.g., baseline is invalid), don't update FCO
94+
provider.log(`[updateFCOAfterEdit] Failed to calculate diff from ${baseline} to HEAD: ${diffError}`)
95+
}
96+
} catch (error) {
97+
// Non-critical error, don't throw - just log and continue
98+
provider?.log(`[updateFCOAfterEdit] Error updating FCO after edit: ${error}`)
99+
}
100+
}

0 commit comments

Comments
 (0)