فهرست منبع

fix(task): temporary fix for the ask error (#3471)

Co-authored-by: cte <[email protected]>
Xudong Zhang 7 ماه پیش
والد
کامیت
f6cae4df05

+ 8 - 4
src/core/checkpoints/index.ts

@@ -93,10 +93,14 @@ export function getCheckpointService(cline: Task) {
 			try {
 				provider?.postMessageToWebview({ type: "currentCheckpointUpdated", text: to })
 
-				cline.say("checkpoint_saved", to, undefined, undefined, { isFirst, from, to }).catch((err) => {
-					log("[Cline#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
-					console.error(err)
-				})
+				cline
+					.say("checkpoint_saved", to, undefined, undefined, { isFirst, from, to }, undefined, {
+						isNonInteractive: true,
+					})
+					.catch((err) => {
+						log("[Cline#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
+						console.error(err)
+					})
 			} catch (err) {
 				log("[Cline#getCheckpointService] caught unexpected error in on('checkpoint'), disabling checkpoints")
 				console.error(err)

+ 49 - 31
src/core/task/Task.ts

@@ -164,7 +164,7 @@ export class Task extends EventEmitter<ClineEvents> {
 	consecutiveMistakeCount: number = 0
 	consecutiveMistakeLimit: number
 	consecutiveMistakeCountForApplyDiff: Map<string, number> = new Map()
-	private toolUsage: ToolUsage = {}
+	toolUsage: ToolUsage = {}
 
 	// Checkpoints
 	enableCheckpoints: boolean
@@ -488,6 +488,9 @@ export class Task extends EventEmitter<ClineEvents> {
 		partial?: boolean,
 		checkpoint?: Record<string, unknown>,
 		progressStatus?: ToolProgressStatus,
+		options: {
+			isNonInteractive?: boolean
+		} = {},
 	): Promise<undefined> {
 		if (this.abort) {
 			throw new Error(`[Cline#say] task ${this.taskId}.${this.instanceId} aborted`)
@@ -495,49 +498,71 @@ export class Task extends EventEmitter<ClineEvents> {
 
 		if (partial !== undefined) {
 			const lastMessage = this.clineMessages.at(-1)
+
 			const isUpdatingPreviousPartial =
 				lastMessage && lastMessage.partial && lastMessage.type === "say" && lastMessage.say === type
+
 			if (partial) {
 				if (isUpdatingPreviousPartial) {
-					// existing partial message, so update it
+					// Existing partial message, so update it.
 					lastMessage.text = text
 					lastMessage.images = images
 					lastMessage.partial = partial
 					lastMessage.progressStatus = progressStatus
 					this.updateClineMessage(lastMessage)
 				} else {
-					// this is a new partial message, so add it with partial state
+					// This is a new partial message, so add it with partial state.
 					const sayTs = Date.now()
-					this.lastMessageTs = sayTs
+
+					if (!options.isNonInteractive) {
+						this.lastMessageTs = sayTs
+					}
+
 					await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, partial })
 				}
 			} else {
 				// New now have a complete version of a previously partial message.
+				// This is the complete version of a previously partial
+				// message, so replace the partial with the complete version.
 				if (isUpdatingPreviousPartial) {
-					// This is the complete version of a previously partial
-					// message, so replace the partial with the complete version.
-					this.lastMessageTs = lastMessage.ts
-					// lastMessage.ts = sayTs
+					if (!options.isNonInteractive) {
+						this.lastMessageTs = lastMessage.ts
+					}
+
 					lastMessage.text = text
 					lastMessage.images = images
 					lastMessage.partial = false
 					lastMessage.progressStatus = progressStatus
+
 					// Instead of streaming partialMessage events, we do a save
 					// and post like normal to persist to disk.
 					await this.saveClineMessages()
-					// More performant than an entire postStateToWebview.
+
+					// More performant than an entire `postStateToWebview`.
 					this.updateClineMessage(lastMessage)
 				} else {
 					// This is a new and complete message, so add it like normal.
 					const sayTs = Date.now()
-					this.lastMessageTs = sayTs
+
+					if (!options.isNonInteractive) {
+						this.lastMessageTs = sayTs
+					}
+
 					await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images })
 				}
 			}
 		} else {
-			// this is a new non-partial message, so add it like normal
+			// This is a new non-partial message, so add it like normal.
 			const sayTs = Date.now()
-			this.lastMessageTs = sayTs
+
+			// A "non-interactive" message is a message is one that the user
+			// does not need to respond to. We don't want these message types
+			// to trigger an update to `lastMessageTs` since they can be created
+			// asynchronously and could interrupt a pending ask.
+			if (!options.isNonInteractive) {
+				this.lastMessageTs = sayTs
+			}
+
 			await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, checkpoint })
 		}
 	}
@@ -555,8 +580,12 @@ export class Task extends EventEmitter<ClineEvents> {
 	// Start / Abort / Resume
 
 	private async startTask(task?: string, images?: string[]): Promise<void> {
-		// conversationHistory (for API) and clineMessages (for webview) need to be in sync
-		// if the extension process were killed, then on restart the clineMessages might not be empty, so we need to set it to [] when we create a new Cline client (otherwise webview would show stale messages from previous session)
+		// `conversationHistory` (for API) and `clineMessages` (for webview)
+		// need to be in sync.
+		// If the extension process were killed, then on restart the
+		// `clineMessages` might not be empty, so we need to set it to [] when
+		// we create a new Cline client (otherwise webview would show stale
+		// messages from previous session).
 		this.clineMessages = []
 		this.apiConversationHistory = []
 		await this.providerRef.deref()?.postStateToWebview()
@@ -578,28 +607,25 @@ export class Task extends EventEmitter<ClineEvents> {
 	}
 
 	public async resumePausedTask(lastMessage: string) {
-		// release this Cline instance from paused state
+		// Release this Cline instance from paused state.
 		this.isPaused = false
 		this.emit("taskUnpaused")
 
-		// fake an answer from the subtask that it has completed running and this is the result of what it has done
-		// add the message to the chat history and to the webview ui
+		// Fake an answer from the subtask that it has completed running and
+		// this is the result of what it has done  add the message to the chat
+		// history and to the webview ui.
 		try {
 			await this.say("subtask_result", lastMessage)
 
 			await this.addToApiConversationHistory({
 				role: "user",
-				content: [
-					{
-						type: "text",
-						text: `[new_task completed] Result: ${lastMessage}`,
-					},
-				],
+				content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
 			})
 		} catch (error) {
 			this.providerRef
 				.deref()
 				?.log(`Error failed to add reply from subtast into conversation of parent task, error: ${error}`)
+
 			throw error
 		}
 	}
@@ -1629,17 +1655,9 @@ export class Task extends EventEmitter<ClineEvents> {
 		}
 	}
 
-	public getToolUsage() {
-		return this.toolUsage
-	}
-
 	// Getters
 
 	public get cwd() {
 		return this.workspacePath
 	}
-
-	public getFileContextTracker(): FileContextTracker {
-		return this.fileContextTracker
-	}
 }

+ 15 - 25
src/core/tools/__tests__/readFileTool.test.ts

@@ -33,15 +33,15 @@ let mockInputContent = ""
 
 jest.mock("../../../integrations/misc/extract-text", () => {
 	const actual = jest.requireActual("../../../integrations/misc/extract-text")
-	// Create a spy on the actual addLineNumbers function
+	// Create a spy on the actual addLineNumbers function.
 	const addLineNumbersSpy = jest.spyOn(actual, "addLineNumbers")
 
 	return {
 		...actual,
-		// Expose the spy so tests can access it
+		// Expose the spy so tests can access it.
 		__addLineNumbersSpy: addLineNumbersSpy,
 		extractTextFromFile: jest.fn().mockImplementation((_filePath) => {
-			// Use the actual addLineNumbers function
+			// Use the actual addLineNumbers function.
 			const content = mockInputContent
 			return Promise.resolve(actual.addLineNumbers(content))
 		}),
@@ -87,7 +87,6 @@ describe("read_file tool with maxReadFileLine setting", () => {
 	const mockedIsBinaryFile = isBinaryFile as jest.MockedFunction<typeof isBinaryFile>
 	const mockedPathResolve = path.resolve as jest.MockedFunction<typeof path.resolve>
 
-	// Mock instances
 	const mockCline: any = {}
 	let mockProvider: any
 	let toolResult: ToolResponse | undefined
@@ -95,31 +94,26 @@ describe("read_file tool with maxReadFileLine setting", () => {
 	beforeEach(() => {
 		jest.clearAllMocks()
 
-		// Setup path resolution
 		mockedPathResolve.mockReturnValue(absoluteFilePath)
-
-		// Setup mocks for file operations
 		mockedIsBinaryFile.mockResolvedValue(false)
 
-		// Set the default content for the mock
 		mockInputContent = fileContent
 
-		// Setup the extractTextFromFile mock implementation with the current mockInputContent
+		// Setup the extractTextFromFile mock implementation with the current
+		// mockInputContent.
 		mockedExtractTextFromFile.mockImplementation((_filePath) => {
 			const actual = jest.requireActual("../../../integrations/misc/extract-text")
 			return Promise.resolve(actual.addLineNumbers(mockInputContent))
 		})
 
 		// No need to setup the extractTextFromFile mock implementation here
-		// as it's already defined at the module level
+		// as it's already defined at the module level.
 
-		// Setup mock provider
 		mockProvider = {
 			getState: jest.fn(),
 			deref: jest.fn().mockReturnThis(),
 		}
 
-		// Setup Cline instance with mock methods
 		mockCline.cwd = "/"
 		mockCline.task = "Test"
 		mockCline.providerRef = mockProvider
@@ -129,12 +123,14 @@ describe("read_file tool with maxReadFileLine setting", () => {
 		mockCline.say = jest.fn().mockResolvedValue(undefined)
 		mockCline.ask = jest.fn().mockResolvedValue(true)
 		mockCline.presentAssistantMessage = jest.fn()
-		mockCline.getFileContextTracker = jest.fn().mockReturnValue({
+
+		mockCline.fileContextTracker = {
 			trackFileContext: jest.fn().mockResolvedValue(undefined),
-		})
+		}
+
 		mockCline.recordToolUsage = jest.fn().mockReturnValue(undefined)
 		mockCline.recordToolError = jest.fn().mockReturnValue(undefined)
-		// Reset tool result
+
 		toolResult = undefined
 	})
 
@@ -433,22 +429,16 @@ describe("read_file tool XML output structure", () => {
 	beforeEach(() => {
 		jest.clearAllMocks()
 
-		// Setup path resolution
 		mockedPathResolve.mockReturnValue(absoluteFilePath)
-
-		// Setup mocks for file operations
 		mockedIsBinaryFile.mockResolvedValue(false)
 
-		// Set the default content for the mock
 		mockInputContent = fileContent
 
-		// Setup mock provider
 		mockProvider = {
 			getState: jest.fn().mockResolvedValue({ maxReadFileLine: 500 }),
 			deref: jest.fn().mockReturnThis(),
 		}
 
-		// Setup Cline instance with mock methods
 		mockCline.cwd = "/"
 		mockCline.task = "Test"
 		mockCline.providerRef = mockProvider
@@ -459,14 +449,14 @@ describe("read_file tool XML output structure", () => {
 		mockCline.ask = jest.fn().mockResolvedValue(true)
 		mockCline.presentAssistantMessage = jest.fn()
 		mockCline.sayAndCreateMissingParamError = jest.fn().mockResolvedValue("Missing required parameter")
-		// Add mock for getFileContextTracker method
-		mockCline.getFileContextTracker = jest.fn().mockReturnValue({
+
+		mockCline.fileContextTracker = {
 			trackFileContext: jest.fn().mockResolvedValue(undefined),
-		})
+		}
+
 		mockCline.recordToolUsage = jest.fn().mockReturnValue(undefined)
 		mockCline.recordToolError = jest.fn().mockReturnValue(undefined)
 
-		// Reset tool result
 		toolResult = undefined
 	})
 

+ 1 - 1
src/core/tools/applyDiffTool.ts

@@ -164,7 +164,7 @@ export async function applyDiffTool(
 
 			// Track file edit operation
 			if (relPath) {
-				await cline.getFileContextTracker().trackFileContext(relPath, "roo_edited" as RecordSource)
+				await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
 			}
 
 			// Used to determine if we should wait for busy terminal to update before sending api request

+ 3 - 3
src/core/tools/attemptCompletionTool.ts

@@ -46,7 +46,7 @@ export async function attemptCompletionTool(
 					await cline.say("completion_result", removeClosingTag("result", result), undefined, false)
 
 					telemetryService.captureTaskCompleted(cline.taskId)
-					cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.getToolUsage())
+					cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.toolUsage)
 
 					await cline.ask("command", removeClosingTag("command", command), block.partial).catch(() => {})
 				}
@@ -72,7 +72,7 @@ export async function attemptCompletionTool(
 					// Haven't sent a command message yet so first send completion_result then command.
 					await cline.say("completion_result", result, undefined, false)
 					telemetryService.captureTaskCompleted(cline.taskId)
-					cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.getToolUsage())
+					cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.toolUsage)
 				}
 
 				// Complete command message.
@@ -97,7 +97,7 @@ export async function attemptCompletionTool(
 			} else {
 				await cline.say("completion_result", result, undefined, false)
 				telemetryService.captureTaskCompleted(cline.taskId)
-				cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.getToolUsage())
+				cline.emit("taskCompleted", cline.taskId, cline.getTokenUsage(), cline.toolUsage)
 			}
 
 			if (cline.parentTask) {

+ 1 - 1
src/core/tools/insertContentTool.ts

@@ -132,7 +132,7 @@ export async function insertContentTool(
 
 		// Track file edit operation
 		if (relPath) {
-			await cline.getFileContextTracker().trackFileContext(relPath, "roo_edited" as RecordSource)
+			await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
 		}
 
 		cline.didEditFile = true

+ 1 - 1
src/core/tools/listCodeDefinitionNamesTool.ts

@@ -64,7 +64,7 @@ export async function listCodeDefinitionNamesTool(
 			}
 
 			if (relPath) {
-				await cline.getFileContextTracker().trackFileContext(relPath, "read_tool" as RecordSource)
+				await cline.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource)
 			}
 
 			pushToolResult(result)

+ 1 - 1
src/core/tools/readFileTool.ts

@@ -245,7 +245,7 @@ export async function readFileTool(
 
 			// Track file read operation
 			if (relPath) {
-				await cline.getFileContextTracker().trackFileContext(relPath, "read_tool" as RecordSource)
+				await cline.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource)
 			}
 
 			// Format the result into the required XML structure

+ 1 - 1
src/core/tools/searchAndReplaceTool.ts

@@ -215,7 +215,7 @@ export async function searchAndReplaceTool(
 
 		// Track file edit operation
 		if (relPath) {
-			await cline.getFileContextTracker().trackFileContext(relPath, "roo_edited" as RecordSource)
+			await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
 		}
 
 		cline.didEditFile = true

+ 1 - 1
src/core/tools/writeToFileTool.ts

@@ -211,7 +211,7 @@ export async function writeToFileTool(
 
 			// Track file edit operation
 			if (relPath) {
-				await cline.getFileContextTracker().trackFileContext(relPath, "roo_edited" as RecordSource)
+				await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
 			}
 
 			cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request