Skip to content

Commit 3048471

Browse files
playcationshannesrudolph
authored andcommitted
Ignore empty tool usage
Added more test cases missing and removed cases in case nothing was changed. and made it so those get ignored by the fco
1 parent b4086b1 commit 3048471

File tree

3 files changed

+307
-84
lines changed

3 files changed

+307
-84
lines changed

src/core/checkpoints/__tests__/index.spec.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@ vitest.mock("../../../services/checkpoints", () => ({
1313
},
1414
}))
1515

16+
// Mock the TelemetryService to prevent unhandled rejections
17+
vitest.mock("@roo-code/telemetry", () => ({
18+
TelemetryService: {
19+
instance: {
20+
captureCheckpointCreated: vitest.fn(),
21+
captureCheckpointRestored: vitest.fn(),
22+
captureCheckpointDiffed: vitest.fn(),
23+
},
24+
},
25+
}))
26+
1627
import { describe, it, expect, beforeEach, afterEach, vitest } from "vitest"
1728
import * as path from "path"
1829
import * as fs from "fs/promises"
@@ -130,6 +141,12 @@ describe("getCheckpointService orchestration", () => {
130141
})
131142
return Promise.resolve()
132143
})
144+
mockService.saveCheckpoint = vitest.fn(() => {
145+
return Promise.resolve({
146+
commit: "mock-checkpoint-hash",
147+
message: "Mock checkpoint",
148+
})
149+
})
133150

134151
// Mock the service creation
135152
;(RepoPerTaskCheckpointService.create as any).mockReturnValue(mockService)
@@ -147,7 +164,7 @@ describe("getCheckpointService orchestration", () => {
147164
hasExistingCheckpoints: false,
148165
})
149166

150-
const service = getCheckpointService(task)
167+
const service = await getCheckpointService(task)
151168
console.log("Service returned:", service)
152169
expect(service).toBe(mockService)
153170
expect(RepoPerTaskCheckpointService.create).toHaveBeenCalledWith({
@@ -167,7 +184,7 @@ describe("getCheckpointService orchestration", () => {
167184
// Set existing checkpoint service
168185
task.checkpointService = mockService
169186

170-
const service = getCheckpointService(task)
187+
const service = await getCheckpointService(task)
171188
expect(service).toBe(mockService)
172189

173190
// Should not create a new service
@@ -181,7 +198,7 @@ describe("getCheckpointService orchestration", () => {
181198
enableCheckpoints: false,
182199
})
183200

184-
const service = getCheckpointService(task)
201+
const service = await getCheckpointService(task)
185202
expect(service).toBeUndefined()
186203
})
187204
})
@@ -193,7 +210,7 @@ describe("getCheckpointService orchestration", () => {
193210
hasExistingCheckpoints: false,
194211
})
195212

196-
const service = getCheckpointService(task)
213+
const service = await getCheckpointService(task)
197214
expect(service).toBe(mockService)
198215

199216
// initShadowGit should be called

src/services/file-changes/FCOMessageHandler.ts

Lines changed: 66 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,16 @@ export class FCOMessageHandler {
5050
task.taskId,
5151
task.fileContextTracker,
5252
)
53-
this.provider.postMessageToWebview({
54-
type: "filesChanged",
55-
filesChanged: filteredChangeset.files.length > 0 ? filteredChangeset : undefined,
56-
})
53+
// Only send update if there are actual changes
54+
if (filteredChangeset.files.length > 0) {
55+
this.provider.postMessageToWebview({
56+
type: "filesChanged",
57+
filesChanged: filteredChangeset,
58+
})
59+
}
60+
// If no changes, don't send anything - keep FCO in current state
5761
}
62+
// If can't filter, don't send anything - keep FCO in current state
5863
break
5964
}
6065

@@ -189,15 +194,18 @@ export class FCOMessageHandler {
189194
if (message.uri && acceptFileChangeManager && task?.taskId && task?.fileContextTracker) {
190195
await acceptFileChangeManager.acceptChange(message.uri)
191196

192-
// Send updated state with LLM-only filtering
197+
// Send updated state with LLM-only filtering only if there are remaining changes
193198
const updatedChangeset = await acceptFileChangeManager.getLLMOnlyChanges(
194199
task.taskId,
195200
task.fileContextTracker,
196201
)
197-
this.provider.postMessageToWebview({
198-
type: "filesChanged",
199-
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
200-
})
202+
if (updatedChangeset.files.length > 0) {
203+
this.provider.postMessageToWebview({
204+
type: "filesChanged",
205+
filesChanged: updatedChangeset,
206+
})
207+
}
208+
// If no remaining changes, don't send anything - keep FCO in current state
201209
}
202210
}
203211

@@ -239,28 +247,27 @@ export class FCOMessageHandler {
239247
// Remove from tracking since the file has been reverted
240248
await rejectFileChangeManager.rejectChange(message.uri)
241249

242-
// Send updated state with LLM-only filtering
250+
// Send updated state with LLM-only filtering only if there are remaining changes
243251
if (currentTask?.taskId && currentTask?.fileContextTracker) {
244252
const updatedChangeset = await rejectFileChangeManager.getLLMOnlyChanges(
245253
currentTask.taskId,
246254
currentTask.fileContextTracker,
247255
)
248-
console.log(`[FCO] After rejection, sending ${updatedChangeset.files.length} LLM-only files to webview`)
249-
this.provider.postMessageToWebview({
250-
type: "filesChanged",
251-
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
252-
})
256+
console.log(`[FCO] After rejection, found ${updatedChangeset.files.length} remaining LLM-only files`)
257+
if (updatedChangeset.files.length > 0) {
258+
this.provider.postMessageToWebview({
259+
type: "filesChanged",
260+
filesChanged: updatedChangeset,
261+
})
262+
}
263+
// If no remaining changes, don't send anything - keep FCO in current state
253264
}
254265
} catch (error) {
255266
console.error(`[FCO] Error reverting file ${message.uri}:`, error)
256267
// Fall back to old behavior (just remove from display) if reversion fails
257268
await rejectFileChangeManager.rejectChange(message.uri)
258269

259-
const updatedChangeset = rejectFileChangeManager.getChanges()
260-
this.provider.postMessageToWebview({
261-
type: "filesChanged",
262-
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
263-
})
270+
// Don't send fallback message - just log the error and keep FCO in current state
264271
}
265272
}
266273

@@ -271,7 +278,7 @@ export class FCOMessageHandler {
271278
}
272279
await acceptAllFileChangeManager?.acceptAll()
273280

274-
// Clear state
281+
// Clear FCO state - this is the one case where we DO want to clear the UI
275282
this.provider.postMessageToWebview({
276283
type: "filesChanged",
277284
filesChanged: undefined,
@@ -345,40 +352,40 @@ export class FCOMessageHandler {
345352
fileChangeManager = await this.provider.ensureFileChangeManager()
346353
}
347354

348-
if (fileChangeManager && task?.checkpointService) {
349-
const changeset = fileChangeManager.getChanges()
350-
355+
if (fileChangeManager) {
351356
// Handle message file changes if provided
352357
if (message.fileChanges) {
353358
const fileChanges = message.fileChanges.map((fc: any) => ({
354359
uri: fc.uri,
355360
type: fc.type,
356-
fromCheckpoint: task.checkpointService?.baseHash || "base",
361+
fromCheckpoint: task?.checkpointService?.baseHash || "base",
357362
toCheckpoint: "current",
358363
}))
359364

360365
fileChangeManager.setFiles(fileChanges)
361366
}
362367

363-
// Get filtered changeset and send to webview
364-
const filteredChangeset = fileChangeManager.getChanges()
365-
this.provider.postMessageToWebview({
366-
type: "filesChanged",
367-
filesChanged: filteredChangeset.files.length > 0 ? filteredChangeset : undefined,
368-
})
369-
} else {
370-
this.provider.postMessageToWebview({
371-
type: "filesChanged",
372-
filesChanged: undefined,
373-
})
368+
// Get LLM-only filtered changeset and send to webview only if there are changes
369+
if (task?.taskId && task?.fileContextTracker) {
370+
const filteredChangeset = await fileChangeManager.getLLMOnlyChanges(
371+
task.taskId,
372+
task.fileContextTracker,
373+
)
374+
// Only send update if there are actual changes
375+
if (filteredChangeset.files.length > 0) {
376+
this.provider.postMessageToWebview({
377+
type: "filesChanged",
378+
filesChanged: filteredChangeset,
379+
})
380+
}
381+
// If no changes, don't send anything - keep FCO in current state
382+
}
383+
// If can't filter, don't send anything - keep FCO in current state
374384
}
385+
// If no fileChangeManager, don't send anything - keep FCO in current state
375386
} catch (error) {
376387
console.error("FCOMessageHandler: Error handling filesChangedRequest:", error)
377-
// Send empty response to prevent FCO from hanging
378-
this.provider.postMessageToWebview({
379-
type: "filesChanged",
380-
filesChanged: undefined,
381-
})
388+
// Don't send anything on error - keep FCO in current state
382389
}
383390
}
384391

@@ -393,24 +400,27 @@ export class FCOMessageHandler {
393400
// Update baseline to the specified checkpoint
394401
await fileChangeManager.updateBaseline(message.baseline)
395402

396-
// Send updated state
397-
const updatedChangeset = fileChangeManager.getChanges()
398-
this.provider.postMessageToWebview({
399-
type: "filesChanged",
400-
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
401-
})
402-
} else {
403-
this.provider.postMessageToWebview({
404-
type: "filesChanged",
405-
filesChanged: undefined,
406-
})
403+
// Send updated state with LLM-only filtering only if there are changes
404+
if (task.taskId && task.fileContextTracker) {
405+
const updatedChangeset = await fileChangeManager.getLLMOnlyChanges(
406+
task.taskId,
407+
task.fileContextTracker,
408+
)
409+
// Only send update if there are actual changes
410+
if (updatedChangeset.files.length > 0) {
411+
this.provider.postMessageToWebview({
412+
type: "filesChanged",
413+
filesChanged: updatedChangeset,
414+
})
415+
}
416+
// If no changes, don't send anything - keep FCO in current state
417+
}
418+
// If can't filter, don't send anything - keep FCO in current state
407419
}
420+
// If conditions not met, don't send anything - keep FCO in current state
408421
} catch (error) {
409422
console.error("FCOMessageHandler: Failed to update baseline:", error)
410-
this.provider.postMessageToWebview({
411-
type: "filesChanged",
412-
filesChanged: undefined,
413-
})
423+
// Don't send anything on error - keep FCO in current state
414424
}
415425
}
416426

0 commit comments

Comments
 (0)