Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 42 additions & 10 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
toolParamNames,
type NativeToolArgs,
} from "../../shared/tools"
import { resolveToolAlias } from "../prompts/tools/filter-tools-for-mode"
import { parseJSON } from "partial-json"
import type {
ApiStreamToolCallStartChunk,
Expand Down Expand Up @@ -246,12 +247,18 @@ export class NativeToolCallParser {
try {
const partialArgs = parseJSON(toolCall.argumentsAccumulator)

// Resolve tool alias to canonical name
const resolvedName = resolveToolAlias(toolCall.name) as ToolName
// Preserve original name if it differs from resolved (i.e., it was an alias)
const originalName = toolCall.name !== resolvedName ? toolCall.name : undefined

// Create partial ToolUse with extracted values
return this.createPartialToolUse(
toolCall.id,
toolCall.name as ToolName,
resolvedName,
partialArgs || {},
true, // partial
originalName,
)
} catch {
// Even partial-json-parser can fail on severely malformed JSON
Expand Down Expand Up @@ -327,12 +334,14 @@ export class NativeToolCallParser {
/**
* Create a partial ToolUse from currently parsed arguments.
* Used during streaming to show progress.
* @param originalName - The original tool name as called by the model (if different from canonical name)
*/
private static createPartialToolUse(
id: string,
name: ToolName,
partialArgs: Record<string, any>,
partial: boolean,
originalName?: string,
): ToolUse | null {
// Build legacy params for display
// NOTE: For streaming partial updates, we MUST populate params even for complex types
Expand Down Expand Up @@ -505,18 +514,33 @@ export class NativeToolCallParser {
}
break

// Add other tools as needed
case "search_and_replace":
if (partialArgs.path !== undefined || partialArgs.operations !== undefined) {
nativeArgs = {
path: partialArgs.path,
operations: partialArgs.operations,
}
}
break

default:
break
}

return {
const result: ToolUse = {
type: "tool_use" as const,
name,
params,
partial,
nativeArgs,
}

// Preserve original name for API history when an alias was used
if (originalName) {
result.originalName = originalName
}

return result
}

/**
Expand All @@ -535,9 +559,12 @@ export class NativeToolCallParser {
return this.parseDynamicMcpTool(toolCall)
}

// Validate tool name
if (!toolNames.includes(toolCall.name as ToolName)) {
console.error(`Invalid tool name: ${toolCall.name}`)
// Resolve tool alias to canonical name (e.g., "edit_file" -> "apply_diff", "temp_edit_file" -> "search_and_replace")
const resolvedName = resolveToolAlias(toolCall.name as string) as TName

// Validate tool name (after alias resolution)
if (!toolNames.includes(resolvedName as ToolName)) {
console.error(`Invalid tool name: ${toolCall.name} (resolved: ${resolvedName})`)
console.error(`Valid tool names:`, toolNames)
return null
}
Expand All @@ -554,13 +581,13 @@ export class NativeToolCallParser {
// Skip complex parameters that have been migrated to nativeArgs.
// For read_file, the 'files' parameter is a FileEntry[] array that can't be
// meaningfully stringified. The properly typed data is in nativeArgs instead.
if (toolCall.name === "read_file" && key === "files") {
if (resolvedName === "read_file" && key === "files") {
continue
}

// Validate parameter name
if (!toolParamNames.includes(key as ToolParamName)) {
console.warn(`Unknown parameter '${key}' for tool '${toolCall.name}'`)
console.warn(`Unknown parameter '${key}' for tool '${resolvedName}'`)
console.warn(`Valid param names:`, toolParamNames)
continue
}
Expand All @@ -580,7 +607,7 @@ export class NativeToolCallParser {
// will fall back to legacy parameter parsing if supported.
let nativeArgs: NativeArgsFor<TName> | undefined = undefined

switch (toolCall.name) {
switch (resolvedName) {
case "read_file":
if (args.files && Array.isArray(args.files)) {
nativeArgs = { files: this.convertFileEntries(args.files) } as NativeArgsFor<TName>
Expand Down Expand Up @@ -761,12 +788,17 @@ export class NativeToolCallParser {

const result: ToolUse<TName> = {
type: "tool_use" as const,
name: toolCall.name,
name: resolvedName,
params,
partial: false, // Native tool calls are always complete when yielded
nativeArgs,
}

// Preserve original name for API history when an alias was used
if (toolCall.name !== resolvedName) {
result.originalName = toolCall.name
}

return result
} catch (error) {
console.error(
Expand Down
6 changes: 5 additions & 1 deletion src/core/assistant-message/presentAssistantMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,11 @@ export async function presentAssistantMessage(cline: Task) {
// potentially causing the stream to appear frozen.
if (!block.partial) {
const modelInfo = cline.api.getModel()
const includedTools = modelInfo?.info?.includedTools
// Resolve aliases in includedTools before validation
// e.g., "edit_file" should resolve to "apply_diff"
const rawIncludedTools = modelInfo?.info?.includedTools
const { resolveToolAlias } = await import("../prompts/tools/filter-tools-for-mode")
const includedTools = rawIncludedTools?.map((tool) => resolveToolAlias(tool))

try {
validateToolUse(
Expand Down
98 changes: 62 additions & 36 deletions src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ describe("filterMcpToolsForMode", () => {
it("should return original tools when modelInfo is undefined", () => {
const tools = new Set(["read_file", "write_to_file", "apply_diff"])
const result = applyModelToolCustomization(tools, codeMode, undefined)
expect(result).toEqual(tools)
expect(result.allowedTools).toEqual(tools)
})

it("should exclude tools specified in excludedTools", () => {
Expand All @@ -498,9 +498,9 @@ describe("filterMcpToolsForMode", () => {
excludedTools: ["apply_diff"],
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.has("apply_diff")).toBe(false)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("apply_diff")).toBe(false)
})

it("should exclude multiple tools", () => {
Expand All @@ -511,10 +511,10 @@ describe("filterMcpToolsForMode", () => {
excludedTools: ["apply_diff", "write_to_file"],
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("execute_command")).toBe(true)
expect(result.has("write_to_file")).toBe(false)
expect(result.has("apply_diff")).toBe(false)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("execute_command")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(false)
expect(result.allowedTools.has("apply_diff")).toBe(false)
})

it("should include tools only if they belong to allowed groups", () => {
Expand All @@ -525,9 +525,9 @@ describe("filterMcpToolsForMode", () => {
includedTools: ["write_to_file", "apply_diff"], // Both in edit group
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.has("apply_diff")).toBe(true)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("apply_diff")).toBe(true)
})

it("should NOT include tools from groups not allowed by mode", () => {
Expand All @@ -539,9 +539,9 @@ describe("filterMcpToolsForMode", () => {
}
// Architect mode doesn't have edit group
const result = applyModelToolCustomization(tools, architectMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(false) // Not in allowed groups
expect(result.has("apply_diff")).toBe(false) // Not in allowed groups
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(false) // Not in allowed groups
expect(result.allowedTools.has("apply_diff")).toBe(false) // Not in allowed groups
})

it("should apply both exclude and include operations", () => {
Expand All @@ -553,10 +553,10 @@ describe("filterMcpToolsForMode", () => {
includedTools: ["search_and_replace"], // Another edit tool (customTool)
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.has("apply_diff")).toBe(false) // Excluded
expect(result.has("search_and_replace")).toBe(true) // Included
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("apply_diff")).toBe(false) // Excluded
expect(result.allowedTools.has("search_and_replace")).toBe(true) // Included
})

it("should handle empty excludedTools and includedTools arrays", () => {
Expand All @@ -568,7 +568,7 @@ describe("filterMcpToolsForMode", () => {
includedTools: [],
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result).toEqual(tools)
expect(result.allowedTools).toEqual(tools)
})

it("should ignore excluded tools that are not in the original set", () => {
Expand All @@ -579,9 +579,9 @@ describe("filterMcpToolsForMode", () => {
excludedTools: ["apply_diff", "nonexistent_tool"],
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.size).toBe(2)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.size).toBe(2)
})

it("should NOT include customTools by default", () => {
Expand All @@ -594,8 +594,8 @@ describe("filterMcpToolsForMode", () => {
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
// customTools should not be in the result unless explicitly included
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
})

it("should NOT include tools that are not in any TOOL_GROUPS", () => {
Expand All @@ -606,8 +606,8 @@ describe("filterMcpToolsForMode", () => {
includedTools: ["my_custom_tool"], // Not in any tool group
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("my_custom_tool")).toBe(false)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("my_custom_tool")).toBe(false)
})

it("should NOT include undefined tools even with allowed groups", () => {
Expand All @@ -619,8 +619,8 @@ describe("filterMcpToolsForMode", () => {
}
// Even though architect mode has read group, undefined tools are not added
const result = applyModelToolCustomization(tools, architectMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("custom_edit_tool")).toBe(false)
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("custom_edit_tool")).toBe(false)
})

describe("with customTools defined in TOOL_GROUPS", () => {
Expand All @@ -647,9 +647,9 @@ describe("filterMcpToolsForMode", () => {
includedTools: ["special_edit_tool"], // customTool from edit group
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.has("special_edit_tool")).toBe(true) // customTool should be included
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("special_edit_tool")).toBe(true) // customTool should be included
})

it("should NOT include customTools when not specified in includedTools", () => {
Expand All @@ -660,9 +660,9 @@ describe("filterMcpToolsForMode", () => {
// No includedTools specified
}
const result = applyModelToolCustomization(tools, codeMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("write_to_file")).toBe(true)
expect(result.has("special_edit_tool")).toBe(false) // customTool should NOT be included by default
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("write_to_file")).toBe(true)
expect(result.allowedTools.has("special_edit_tool")).toBe(false) // customTool should NOT be included by default
})

it("should NOT include customTools from groups not allowed by mode", () => {
Expand All @@ -674,8 +674,8 @@ describe("filterMcpToolsForMode", () => {
}
// Architect mode doesn't have edit group
const result = applyModelToolCustomization(tools, architectMode, modelInfo)
expect(result.has("read_file")).toBe(true)
expect(result.has("special_edit_tool")).toBe(false) // customTool should NOT be included
expect(result.allowedTools.has("read_file")).toBe(true)
expect(result.allowedTools.has("special_edit_tool")).toBe(false) // customTool should NOT be included
})
})
})
Expand Down Expand Up @@ -822,5 +822,31 @@ describe("filterMcpToolsForMode", () => {
expect(toolNames).toContain("search_and_replace") // Included
expect(toolNames).not.toContain("apply_diff") // Excluded
})

it("should rename tools to alias names when model includes aliases", () => {
const codeMode: ModeConfig = {
slug: "code",
name: "Code",
roleDefinition: "Test",
groups: ["read", "edit", "browser", "command", "mcp"] as const,
}

const modelInfo: ModelInfo = {
contextWindow: 100000,
supportsPromptCache: false,
includedTools: ["edit_file", "write_file"],
}

const filtered = filterNativeToolsForMode(mockNativeTools, "code", [codeMode], {}, undefined, {
modelInfo,
})

const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))

expect(toolNames).toContain("edit_file")
expect(toolNames).toContain("write_file")
expect(toolNames).not.toContain("apply_diff")
expect(toolNames).not.toContain("write_to_file")
})
})
})
Loading
Loading