feat(Tools): Bucket invalid tool names under a static analytics key

Author: edelauna

packages/types/src/tool.ts

@@ -46,6 +46,7 @@ export const toolNames = [
 	"skill",
 	"generate_image",
 	"custom_tool",
+	"invalid_tool_call",
 ] as const
 
 export const toolNamesSchema = z.enum(toolNames)

src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts

@@ -101,11 +101,13 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
 		// Verify consecutiveMistakeCount was incremented
 		expect(mockTask.consecutiveMistakeCount).toBe(1)
 
-		// Verify recordToolError was called
+		// Verify recordToolError was called, bucketed under a static key rather than the raw
+		// model-supplied tool name (which must never become an arbitrary analytics property key).
 		expect(mockTask.recordToolError).toHaveBeenCalledWith(
-			"nonexistent_tool",
+			"invalid_tool_call",
 			expect.stringContaining("Unknown tool"),
 		)
+		expect(mockTask.recordToolError).not.toHaveBeenCalledWith("nonexistent_tool", expect.anything())
 
 		// Verify error message was shown to user (uses i18n key)
 		expect(mockTask.say).toHaveBeenCalledWith("error", "unknownToolError")
@@ -217,21 +219,24 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
 	it("does not record raw model-supplied tool names in tool usage analytics", async () => {
 		// block.name comes straight from the model's tool-call output and is only checked
 		// against isValidToolName() *after* recordToolUsage() would otherwise be called.
-		// An arbitrary/malicious model-supplied name must never become a toolsUsed key.
+		// An arbitrary/malicious model-supplied name must never become a toolsUsed key,
+		// whether via recordToolUsage (success path) or recordToolError (failure path).
 		const toolCallId = "tool_call_analytics_test"
+		const maliciousName = "'; DROP TABLE users; --"
 		mockTask.assistantMessageContent = [
 			{
 				type: "tool_use",
 				id: toolCallId,
-				name: "'; DROP TABLE users; --",
+				name: maliciousName,
 				params: {},
 				partial: false,
 			},
 		]
 
 		await presentAssistantMessage(mockTask)
 
-		expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith("'; DROP TABLE users; --")
+		expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith(maliciousName)
+		expect(mockTask.recordToolError).not.toHaveBeenCalledWith(maliciousName, expect.anything())
 	})
 
 	it("should still work with didRejectTool flag for unknown tool", async () => {

src/core/assistant-message/presentAssistantMessage.ts

@@ -303,14 +303,10 @@ export async function presentAssistantMessage(cline: Task) {
 			if (!toolCallId) {
 				const errorMessage =
 					"Invalid tool call: missing tool_use.id. XML tool calls are no longer supported. Remove any XML tool markup (e.g. <read_file>...</read_file>) and use native tool calling instead."
-				// Record a tool error for visibility/telemetry. Use the reported tool name if present.
+				// Record a tool error for visibility/telemetry. block.name is model-controlled and
+				// must never reach analytics unvalidated, so bucket it under a static key.
 				try {
-					if (
-						typeof (cline as any).recordToolError === "function" &&
-						typeof (block as any).name === "string"
-					) {
-						;(cline as any).recordToolError((block as any).name as ToolName, errorMessage)
-					}
+					cline.recordToolError("invalid_tool_call", errorMessage)
 				} catch {
 					// Best-effort only
 				}
@@ -901,10 +897,11 @@ export async function presentAssistantMessage(cline: Task) {
 						break
 					}
 
-					// Not a custom tool - handle as unknown tool error
+					// Not a custom tool - handle as unknown tool error. block.name is model-controlled
+					// and must never reach analytics unvalidated, so bucket it under a static key.
 					const errorMessage = `Unknown tool "${block.name}". This tool does not exist. Please use one of the available tools.`
 					cline.consecutiveMistakeCount++
-					cline.recordToolError(block.name as ToolName, errorMessage)
+					cline.recordToolError("invalid_tool_call", errorMessage)
 					await cline.say("error", t("tools:unknownToolError", { toolName: block.name }))
 					// Push tool_result directly WITHOUT setting didAlreadyUseTool
 					// This prevents the stream from being interrupted with "Response interrupted by tool use result"

src/shared/tools.ts

@@ -290,6 +290,7 @@ export const TOOL_DISPLAY_NAMES: Record<ToolName, string> = {
 	skill: "load skill",
 	generate_image: "generate images",
 	custom_tool: "use custom tools",
+	invalid_tool_call: "use an unrecognized tool",
 } as const
 
 // Define available tool groups.