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
This commit is contained in:
Shawn 2025-09-01 00:12:06 -04:00 committed by Hannes Rudolph
parent b4086b1a5a
commit 3048471ae1
3 changed files with 307 additions and 84 deletions

View file

@ -13,6 +13,17 @@ vitest.mock("../../../services/checkpoints", () => ({
},
}))
// Mock the TelemetryService to prevent unhandled rejections
vitest.mock("@roo-code/telemetry", () => ({
TelemetryService: {
instance: {
captureCheckpointCreated: vitest.fn(),
captureCheckpointRestored: vitest.fn(),
captureCheckpointDiffed: vitest.fn(),
},
},
}))
import { describe, it, expect, beforeEach, afterEach, vitest } from "vitest"
import * as path from "path"
import * as fs from "fs/promises"
@ -130,6 +141,12 @@ describe("getCheckpointService orchestration", () => {
})
return Promise.resolve()
})
mockService.saveCheckpoint = vitest.fn(() => {
return Promise.resolve({
commit: "mock-checkpoint-hash",
message: "Mock checkpoint",
})
})
// Mock the service creation
;(RepoPerTaskCheckpointService.create as any).mockReturnValue(mockService)
@ -147,7 +164,7 @@ describe("getCheckpointService orchestration", () => {
hasExistingCheckpoints: false,
})
const service = getCheckpointService(task)
const service = await getCheckpointService(task)
console.log("Service returned:", service)
expect(service).toBe(mockService)
expect(RepoPerTaskCheckpointService.create).toHaveBeenCalledWith({
@ -167,7 +184,7 @@ describe("getCheckpointService orchestration", () => {
// Set existing checkpoint service
task.checkpointService = mockService
const service = getCheckpointService(task)
const service = await getCheckpointService(task)
expect(service).toBe(mockService)
// Should not create a new service
@ -181,7 +198,7 @@ describe("getCheckpointService orchestration", () => {
enableCheckpoints: false,
})
const service = getCheckpointService(task)
const service = await getCheckpointService(task)
expect(service).toBeUndefined()
})
})
@ -193,7 +210,7 @@ describe("getCheckpointService orchestration", () => {
hasExistingCheckpoints: false,
})
const service = getCheckpointService(task)
const service = await getCheckpointService(task)
expect(service).toBe(mockService)
// initShadowGit should be called

View file

@ -50,11 +50,16 @@ export class FCOMessageHandler {
task.taskId,
task.fileContextTracker,
)
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: filteredChangeset.files.length > 0 ? filteredChangeset : undefined,
})
// Only send update if there are actual changes
if (filteredChangeset.files.length > 0) {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: filteredChangeset,
})
}
// If no changes, don't send anything - keep FCO in current state
}
// If can't filter, don't send anything - keep FCO in current state
break
}
@ -189,15 +194,18 @@ export class FCOMessageHandler {
if (message.uri && acceptFileChangeManager && task?.taskId && task?.fileContextTracker) {
await acceptFileChangeManager.acceptChange(message.uri)
// Send updated state with LLM-only filtering
// Send updated state with LLM-only filtering only if there are remaining changes
const updatedChangeset = await acceptFileChangeManager.getLLMOnlyChanges(
task.taskId,
task.fileContextTracker,
)
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
})
if (updatedChangeset.files.length > 0) {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset,
})
}
// If no remaining changes, don't send anything - keep FCO in current state
}
}
@ -239,28 +247,27 @@ export class FCOMessageHandler {
// Remove from tracking since the file has been reverted
await rejectFileChangeManager.rejectChange(message.uri)
// Send updated state with LLM-only filtering
// Send updated state with LLM-only filtering only if there are remaining changes
if (currentTask?.taskId && currentTask?.fileContextTracker) {
const updatedChangeset = await rejectFileChangeManager.getLLMOnlyChanges(
currentTask.taskId,
currentTask.fileContextTracker,
)
console.log(`[FCO] After rejection, sending ${updatedChangeset.files.length} LLM-only files to webview`)
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
})
console.log(`[FCO] After rejection, found ${updatedChangeset.files.length} remaining LLM-only files`)
if (updatedChangeset.files.length > 0) {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset,
})
}
// If no remaining changes, don't send anything - keep FCO in current state
}
} catch (error) {
console.error(`[FCO] Error reverting file ${message.uri}:`, error)
// Fall back to old behavior (just remove from display) if reversion fails
await rejectFileChangeManager.rejectChange(message.uri)
const updatedChangeset = rejectFileChangeManager.getChanges()
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
})
// Don't send fallback message - just log the error and keep FCO in current state
}
}
@ -271,7 +278,7 @@ export class FCOMessageHandler {
}
await acceptAllFileChangeManager?.acceptAll()
// Clear state
// Clear FCO state - this is the one case where we DO want to clear the UI
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: undefined,
@ -345,40 +352,40 @@ export class FCOMessageHandler {
fileChangeManager = await this.provider.ensureFileChangeManager()
}
if (fileChangeManager && task?.checkpointService) {
const changeset = fileChangeManager.getChanges()
if (fileChangeManager) {
// Handle message file changes if provided
if (message.fileChanges) {
const fileChanges = message.fileChanges.map((fc: any) => ({
uri: fc.uri,
type: fc.type,
fromCheckpoint: task.checkpointService?.baseHash || "base",
fromCheckpoint: task?.checkpointService?.baseHash || "base",
toCheckpoint: "current",
}))
fileChangeManager.setFiles(fileChanges)
}
// Get filtered changeset and send to webview
const filteredChangeset = fileChangeManager.getChanges()
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: filteredChangeset.files.length > 0 ? filteredChangeset : undefined,
})
} else {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: undefined,
})
// Get LLM-only filtered changeset and send to webview only if there are changes
if (task?.taskId && task?.fileContextTracker) {
const filteredChangeset = await fileChangeManager.getLLMOnlyChanges(
task.taskId,
task.fileContextTracker,
)
// Only send update if there are actual changes
if (filteredChangeset.files.length > 0) {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: filteredChangeset,
})
}
// If no changes, don't send anything - keep FCO in current state
}
// If can't filter, don't send anything - keep FCO in current state
}
// If no fileChangeManager, don't send anything - keep FCO in current state
} catch (error) {
console.error("FCOMessageHandler: Error handling filesChangedRequest:", error)
// Send empty response to prevent FCO from hanging
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: undefined,
})
// Don't send anything on error - keep FCO in current state
}
}
@ -393,24 +400,27 @@ export class FCOMessageHandler {
// Update baseline to the specified checkpoint
await fileChangeManager.updateBaseline(message.baseline)
// Send updated state
const updatedChangeset = fileChangeManager.getChanges()
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset.files.length > 0 ? updatedChangeset : undefined,
})
} else {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: undefined,
})
// Send updated state with LLM-only filtering only if there are changes
if (task.taskId && task.fileContextTracker) {
const updatedChangeset = await fileChangeManager.getLLMOnlyChanges(
task.taskId,
task.fileContextTracker,
)
// Only send update if there are actual changes
if (updatedChangeset.files.length > 0) {
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: updatedChangeset,
})
}
// If no changes, don't send anything - keep FCO in current state
}
// If can't filter, don't send anything - keep FCO in current state
}
// If conditions not met, don't send anything - keep FCO in current state
} catch (error) {
console.error("FCOMessageHandler: Failed to update baseline:", error)
this.provider.postMessageToWebview({
type: "filesChanged",
filesChanged: undefined,
})
// Don't send anything on error - keep FCO in current state
}
}

View file

@ -205,7 +205,7 @@ describe("FCOMessageHandler", () => {
expect(mockProvider.ensureFileChangeManager).toHaveBeenCalled()
})
it("should send undefined when no LLM changes exist", async () => {
it("should not send message when no LLM changes exist", async () => {
const emptyChangeset = {
baseCheckpoint: "base123",
files: [],
@ -215,10 +215,8 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage({ type: "webviewReady" } as WebviewMessage)
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
})
// Should not send any message when no changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle missing task gracefully", async () => {
@ -227,6 +225,8 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage({ type: "webviewReady" } as WebviewMessage)
expect(mockFileChangeManager.getLLMOnlyChanges).not.toHaveBeenCalled()
// Should not send any message when no task context
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
})
@ -355,7 +355,7 @@ describe("FCOMessageHandler", () => {
})
})
it("should send undefined when no files remain after accept", async () => {
it("should not send message when no files remain after accept", async () => {
mockFileChangeManager.getLLMOnlyChanges.mockResolvedValue({
baseCheckpoint: "base123",
files: [],
@ -363,10 +363,8 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage(mockMessage)
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
})
// Should not send any message when no remaining changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle missing FileChangeManager", async () => {
@ -397,7 +395,7 @@ describe("FCOMessageHandler", () => {
mockCheckpointService.getContent.mockResolvedValue("original content")
})
it("should revert file and update changeset", async () => {
it("should revert file and not send message when no remaining changes", async () => {
const updatedChangeset = {
baseCheckpoint: "base123",
files: [],
@ -410,10 +408,8 @@ describe("FCOMessageHandler", () => {
expect(mockCheckpointService.getContent).toHaveBeenCalledWith("base123", "/test/workspace/test.txt")
expect(fs.writeFile).toHaveBeenCalledWith("/test/workspace/test.txt", "original content", "utf8")
expect(mockFileChangeManager.rejectChange).toHaveBeenCalledWith("test.txt")
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
})
// Should not send any message when no remaining changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should delete newly created files", async () => {
@ -505,10 +501,8 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.setFiles).not.toHaveBeenCalled()
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
})
// Should not send any message when no changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle errors gracefully", async () => {
@ -520,11 +514,215 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage(mockMessage)
// Should not send any message on error
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should not send message when task context is missing", async () => {
// Mock task without taskId
mockProvider.getCurrentTask.mockReturnValue({
fileContextTracker: mockFileContextTracker,
checkpointService: mockCheckpointService,
// Missing taskId
})
const mockMessage = {
type: "filesChangedRequest" as const,
}
await handler.handleMessage(mockMessage)
// Should not call getLLMOnlyChanges when taskId is missing
expect(mockFileChangeManager.getLLMOnlyChanges).not.toHaveBeenCalled()
// Should not send any message when task context is missing
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should not send message when fileContextTracker is missing", async () => {
// Mock task without fileContextTracker
mockProvider.getCurrentTask.mockReturnValue({
taskId: "test-task-id",
checkpointService: mockCheckpointService,
// Missing fileContextTracker
})
const mockMessage = {
type: "filesChangedRequest" as const,
}
await handler.handleMessage(mockMessage)
// Should not call getLLMOnlyChanges when fileContextTracker is missing
expect(mockFileChangeManager.getLLMOnlyChanges).not.toHaveBeenCalled()
// Should not send any message when fileContextTracker is missing
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
})
describe("filesChangedBaselineUpdate", () => {
it("should update baseline and send LLM-only changes", async () => {
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
const updatedChangeset = {
baseCheckpoint: "new-baseline-123",
files: [
{
uri: "updated.txt",
type: "edit" as const,
fromCheckpoint: "new-baseline-123",
toCheckpoint: "current",
linesAdded: 3,
linesRemoved: 1,
},
],
}
mockFileChangeManager.getLLMOnlyChanges.mockResolvedValue(updatedChangeset)
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).toHaveBeenCalledWith("new-baseline-123")
expect(mockFileChangeManager.getLLMOnlyChanges).toHaveBeenCalledWith("test-task-id", mockFileContextTracker)
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
filesChanged: updatedChangeset,
})
})
it("should not send message when no LLM changes remain after baseline update", async () => {
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
mockFileChangeManager.getLLMOnlyChanges.mockResolvedValue({
baseCheckpoint: "new-baseline-123",
files: [],
})
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).toHaveBeenCalledWith("new-baseline-123")
// Should not send any message when no changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should not send message when task context is missing", async () => {
// Mock task without taskId
mockProvider.getCurrentTask.mockReturnValue({
fileContextTracker: mockFileContextTracker,
checkpointService: mockCheckpointService,
// Missing taskId
})
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).toHaveBeenCalledWith("new-baseline-123")
// Should not call getLLMOnlyChanges when taskId is missing
expect(mockFileChangeManager.getLLMOnlyChanges).not.toHaveBeenCalled()
// Should not send any message when task context is missing
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should not send message when fileContextTracker is missing", async () => {
// Mock task without fileContextTracker
mockProvider.getCurrentTask.mockReturnValue({
taskId: "test-task-id",
checkpointService: mockCheckpointService,
// Missing fileContextTracker
})
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).toHaveBeenCalledWith("new-baseline-123")
// Should not call getLLMOnlyChanges when fileContextTracker is missing
expect(mockFileChangeManager.getLLMOnlyChanges).not.toHaveBeenCalled()
// Should not send any message when fileContextTracker is missing
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle missing FileChangeManager", async () => {
mockProvider.getFileChangeManager.mockReturnValue(null)
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
expect(mockProvider.ensureFileChangeManager).toHaveBeenCalled()
})
it("should not send message when no baseline provided", async () => {
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
// No baseline property
}
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).not.toHaveBeenCalled()
// Should not send any message when no baseline provided
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should not send message when task is missing", async () => {
mockProvider.getCurrentTask.mockReturnValue(null)
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).not.toHaveBeenCalled()
// Should not send any message when task is missing
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle updateBaseline errors gracefully", async () => {
mockFileChangeManager.updateBaseline.mockRejectedValue(new Error("Baseline update failed"))
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
// Should not throw and not send any message on error
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle getLLMOnlyChanges errors gracefully", async () => {
mockFileChangeManager.getLLMOnlyChanges.mockRejectedValue(new Error("Filter error"))
const mockMessage = {
type: "filesChangedBaselineUpdate" as const,
baseline: "new-baseline-123",
}
await handler.handleMessage(mockMessage)
expect(mockFileChangeManager.updateBaseline).toHaveBeenCalledWith("new-baseline-123")
// Should not send any message when filtering fails
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
})
describe("LLM Filtering Edge Cases", () => {
@ -540,10 +738,8 @@ describe("FCOMessageHandler", () => {
await handler.handleMessage({ type: "webviewReady" } as WebviewMessage)
expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
type: "filesChanged",
filesChanged: undefined,
})
// Should not send any message when no changes
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})
it("should handle mixed LLM and user-edited files", async () => {