Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 15 additions & 3 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@
ApiStreamToolCallDeltaChunk,
ApiStreamToolCallEndChunk,
} from "../../api/transform/stream"
import { MCP_TOOL_PREFIX, MCP_TOOL_SEPARATOR, parseMcpToolName, normalizeMcpToolName } from "../../utils/mcp-name"
import {
MAX_TOOL_NAME_LENGTH,
MCP_TOOL_PREFIX,
MCP_TOOL_SEPARATOR,
buildMcpToolName,
parseMcpToolName,
normalizeMcpToolName,
} from "../../utils/mcp-name"

/**
* Helper type to extract properly typed native arguments for a given tool.
Expand Down Expand Up @@ -1141,8 +1148,13 @@
const result: McpToolUse = {
type: "mcp_tool_use" as const,
id: toolCall.id,
// Keep the original tool name (e.g., "mcp--serverName--toolName") for API history
name: toolCall.name,
// Keep the original tool name (e.g., "mcp--serverName--toolName") for API history.
// A name over the provider limit can only be the uncut name; providers reject it in
// history, so record it under the name that was declared to the model instead.
name:
toolCall.name.length > MAX_TOOL_NAME_LENGTH

Check warning on line 1155 in src/core/assistant-message/NativeToolCallParser.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/assistant-message/NativeToolCallParser.ts:1155: 2 mutation test gaps; example: Survived ConditionalExpression mutant (replacement: true). See the job summary for the complete list and resolution guidance.
? buildMcpToolName(serverName, toolName)
: toolCall.name,
serverName,
toolName,
arguments: args,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { buildMcpToolName, resolveMcpToolByName, toolNamesMatch } from "../../../utils/mcp-name"
import { NativeToolCallParser } from "../NativeToolCallParser"

const SERVER = "company-gateway-tools"
const TOOL = "confluence-confluence_get_space_page_tree"

describe("NativeToolCallParser.parseDynamicMcpTool name for API history", () => {
it("records an over-long uncut name under the declared name", () => {
const result = NativeToolCallParser.parseDynamicMcpTool({
id: "toolu_1",
name: `mcp--${SERVER}--${TOOL}`,
arguments: "{}",
})

expect(result?.name).toBe(buildMcpToolName(SERVER, TOOL))
expect(result?.name.length).toBeLessThanOrEqual(64)
})

it("resolves an over-long uncut name whose hyphens the model turned into underscores", () => {
const mangled = `mcp--${SERVER}--${TOOL}`.replace(/-/g, "_")

const result = NativeToolCallParser.parseDynamicMcpTool({ id: "toolu_5", name: mangled, arguments: "{}" })

expect(result).not.toBeNull()
expect(result!.name.length).toBeLessThanOrEqual(64)
// The recorded name is the declared one, up to the hyphen/underscore spelling.
expect(toolNamesMatch(result!.name, buildMcpToolName(SERVER, TOOL))).toBe(true)
expect(resolveMcpToolByName([{ name: SERVER, tools: [{ name: TOOL }] }], result!.name)).toEqual({
serverName: SERVER,
toolName: TOOL,
})
})

it("keeps the declared name the model returned", () => {
const declared = buildMcpToolName(SERVER, TOOL)

const result = NativeToolCallParser.parseDynamicMcpTool({ id: "toolu_2", name: declared, arguments: "{}" })

expect(result?.name).toBe(declared)
})

it("parses the declared name of a tool on a server with a very long name", () => {
const declared = buildMcpToolName("s".repeat(60), TOOL)

const result = NativeToolCallParser.parseDynamicMcpTool({ id: "toolu_4", name: declared, arguments: "{}" })

expect(result).toMatchObject({ type: "mcp_tool_use", name: declared })
})

it("keeps a short name verbatim", () => {
const result = NativeToolCallParser.parseDynamicMcpTool({
id: "toolu_3",
name: "mcp--weather--get_forecast",
arguments: "{}",
})

expect(result).toMatchObject({ name: "mcp--weather--get_forecast", serverName: "weather" })
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ interface MockTask {
providerRef: {
deref: () => {
getState: ReturnType<typeof vi.fn>
getMcpHub?: () => { findServerNameBySanitizedName: (name: string) => string | undefined }
getMcpHub?: () => {
findServerNameBySanitizedName: (name: string) => string | undefined
getAllServers: () => Array<{ name: string; tools?: Array<{ name: string }> }>
}
}
}
say: ReturnType<typeof vi.fn>
Expand Down Expand Up @@ -368,7 +371,7 @@ describe("presentAssistantMessage - automatic (policy) denials", () => {
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined, getAllServers: () => [] }),
}),
}

Expand Down Expand Up @@ -428,7 +431,7 @@ describe("presentAssistantMessage - automatic (policy) denials", () => {
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined, getAllServers: () => [] }),
}),
}

Expand Down Expand Up @@ -541,7 +544,7 @@ describe("presentAssistantMessage - automatic (policy) denials", () => {
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined, getAllServers: () => [] }),
}),
}

Expand Down Expand Up @@ -581,7 +584,7 @@ describe("presentAssistantMessage - automatic (policy) denials", () => {
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined, getAllServers: () => [] }),
}),
}

Expand Down Expand Up @@ -642,7 +645,7 @@ describe("presentAssistantMessage - automatic (policy) denials", () => {
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }),
getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined, getAllServers: () => [] }),
}),
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "test-server",
getAllServers: () => [],
executeToolCall: vi.fn().mockResolvedValue({ content: [{ type: "text", text: "result" }] }),
}),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { getModeBySlug } from "../../../shared/modes"
import type { Task } from "../../task/Task"
import type { RequestPolicySnapshot } from "../../prompts/tools/effective-tool-policy"
import { useMcpToolTool } from "../../tools/UseMcpToolTool"
import { buildMcpToolName, parseMcpToolName } from "../../../utils/mcp-name"

vi.mock("../../task/Task")
vi.mock("../../../shared/modes", async (importOriginal) => {
Expand Down Expand Up @@ -80,7 +81,10 @@ interface MockTask {
deref: () =>
| {
getState: ReturnType<typeof vi.fn>
getMcpHub?: () => { findServerNameBySanitizedName: (name: string) => string | undefined }
getMcpHub?: () => {
findServerNameBySanitizedName: (name: string) => string | undefined
getAllServers: () => Array<{ name: string; tools?: Array<{ name: string }> }>
}
}
| undefined
}
Expand Down Expand Up @@ -379,6 +383,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down Expand Up @@ -548,6 +553,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down Expand Up @@ -638,6 +644,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down Expand Up @@ -680,6 +687,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down Expand Up @@ -787,6 +795,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
}),
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down Expand Up @@ -932,6 +941,43 @@ describe("presentAssistantMessage - tool usage attribution", () => {
expect(handleSpy).toHaveBeenCalledTimes(1)
})

it("resolves a hash-shortened MCP tool name to its server and tool before executing", async () => {
const serverName = "company-gateway-tools"
const realToolName = "confluence-confluence_get_space_page_tree"
const declaredName = buildMcpToolName(serverName, realToolName)
const mockHub = {
findServerNameBySanitizedName: () => serverName,
getAllServers: () => [{ name: serverName, tools: [{ name: realToolName, enabledForPrompt: true }] }],
}
mockTask.providerRef = {
deref: () => ({
getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [], disabledTools: [] }),
getMcpHub: () => mockHub,
}),
}
const handleSpy = vi.spyOn(useMcpToolTool, "handle").mockResolvedValue(undefined)
// The parser can only recover a cut tool name from the shortened one.
const parsed = parseMcpToolName(declaredName)!
mockTask.assistantMessageContent = [
{
type: "mcp_tool_use",
id: "call_hashed_name",
name: declaredName,
serverName: parsed.serverName,
toolName: parsed.toolName,
arguments: {},
partial: false,
},
]

await presentAssistantMessage(mockTask as unknown as Task, baseSnapshot)

expect(handleSpy).toHaveBeenCalledTimes(1)
expect(handleSpy.mock.calls[0][1]).toMatchObject({
nativeArgs: { server_name: serverName, tool_name: realToolName },
})
})

it("performs no provider-state read on the native validation path", async () => {
// Handler mocked so the only reachable pre-handler code is the
// validation arm itself; any getState call inside the presenter's
Expand All @@ -947,6 +993,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
getState,
getMcpHub: () => ({
findServerNameBySanitizedName: () => "my_server",
getAllServers: () => [],
}),
}),
}
Expand Down
24 changes: 22 additions & 2 deletions src/core/assistant-message/presentAssistantMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

import { formatResponse } from "../prompts/responses"
import { sanitizeToolUseId } from "../../utils/tool-id"
import { resolveMcpToolByName, toolNamesMatch } from "../../utils/mcp-name"

/**
* Maps a raw, potentially model-controlled tool name to a safe analytics key.
Expand Down Expand Up @@ -329,6 +330,25 @@
}
}

// A name shortened to fit the provider limit cannot be parsed back into its tool, so
// when the parsed names match nothing, look the called name up among the declared ones.
let resolvedToolName = mcpBlock.toolName
if (mcpHub && !mcpBlock.partial) {
const servers = mcpHub.getAllServers()
const parsedServer = servers.find((server) => server.name === resolvedServerName)

Check warning on line 338 in src/core/assistant-message/presentAssistantMessage.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/assistant-message/presentAssistantMessage.ts:338: 4 mutation test gaps; example: Survived ArrowFunction mutant (replacement: () => undefined). See the job summary for the complete list and resolution guidance.
const parsedToolExists = parsedServer?.tools?.some((tool) =>

Check warning on line 339 in src/core/assistant-message/presentAssistantMessage.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/assistant-message/presentAssistantMessage.ts:339: 3 mutation test gaps; example: Survived MethodExpression mutant (replacement: parsedServer?.tools?.every(tool => toolNamesMatch(tool.name, resolvedToolName))). See the job summary for the complete list and resolution guidance.
toolNamesMatch(tool.name, resolvedToolName),
)

if (!parsedToolExists) {

Check warning on line 343 in src/core/assistant-message/presentAssistantMessage.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/assistant-message/presentAssistantMessage.ts:343: Survived ConditionalExpression mutant (replacement: true). See the job summary for the complete list and resolution guidance.
const resolved = resolveMcpToolByName(servers, mcpBlock.name)
if (resolved) {
resolvedServerName = resolved.serverName
resolvedToolName = resolved.toolName
}
}
}

// Execute the MCP tool using the same handler as use_mcp_tool
// Create a synthetic ToolUse block that the useMcpToolTool can handle
const syntheticToolUse: ToolUse<"use_mcp_tool"> = {
Expand All @@ -337,13 +357,13 @@
name: "use_mcp_tool",
params: {
server_name: resolvedServerName,
tool_name: mcpBlock.toolName,
tool_name: resolvedToolName,
arguments: JSON.stringify(mcpBlock.arguments),
},
partial: mcpBlock.partial,
nativeArgs: {
server_name: resolvedServerName,
tool_name: mcpBlock.toolName,
tool_name: resolvedToolName,
arguments: mcpBlock.arguments,
},
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ import { type AutoDenyDetail, AutoApprovalHandler, checkAutoApproval } from "../
import { MessageManager } from "../message-manager"
import { validateAndFixToolResultIds } from "./validateToolResultIds"
import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages"
import { capMcpToolUseNames } from "./capMcpToolUseNames"
import { prepareApiConversationMessage } from "./apiConversationHistory"
import { shouldAddUserMessageToHistory } from "./messageCounting"
import { type TaskExecutionContext } from "./providerHandoff"
Expand Down Expand Up @@ -5831,7 +5832,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// For API only: merge consecutive user messages (excludes summary messages per
// mergeConsecutiveApiMessages implementation) without mutating stored history.
const mergedForApi = mergeConsecutiveApiMessages(messagesSinceLastSummary, { roles: ["user"] })
const messagesWithoutImages = maybeRemoveImageBlocks(mergedForApi, this.api)
const messagesWithoutImages = maybeRemoveImageBlocks(capMcpToolUseNames(mergedForApi), this.api)
const cleanConversationHistory = this.buildCleanConversationHistory(
messagesWithoutImages as ApiMessage[],
requestModelInfo,
Expand Down
46 changes: 46 additions & 0 deletions src/core/task/__tests__/capMcpToolUseNames.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { Anthropic } from "@anthropic-ai/sdk"

import type { ApiMessage } from "../../task-persistence"
import { buildMcpToolName } from "../../../utils/mcp-name"
import { capMcpToolUseNames } from "../capMcpToolUseNames"

type Block = Anthropic.Messages.ContentBlockParam

const SERVER = "company-gateway-tools"
const TOOL = "confluence-confluence_get_space_page_tree"
const UNCUT = `mcp--${SERVER}--${TOOL}`

const use = (name: string): Block => ({ type: "tool_use", id: "toolu_1", name, input: {} })
const assistant = (...content: Block[]): ApiMessage => ({ role: "assistant", content })

describe("capMcpToolUseNames", () => {
it("renames an over-long MCP tool_use to the declared name", () => {
const [message] = capMcpToolUseNames([assistant({ type: "text", text: "calling" }, use(UNCUT))])

expect(message.content).toEqual([{ type: "text", text: "calling" }, use(buildMcpToolName(SERVER, TOOL))])
})

it("leaves short names, non-MCP names and other roles untouched", () => {
const messages: ApiMessage[] = [
{ role: "user", content: "hi" },
assistant(use("read_file"), use("mcp--weather--get_forecast")),
]

expect(capMcpToolUseNames(messages)).toEqual(messages)
})

it("leaves an over-long name it cannot parse as an MCP tool", () => {
const name = "x".repeat(70)

expect(capMcpToolUseNames([assistant(use(name))])[0].content).toEqual([use(name)])
})

it("does not mutate its input", () => {
const input = [assistant(use(UNCUT))]
const snapshot = structuredClone(input)

capMcpToolUseNames(input)

expect(input).toEqual(snapshot)
})
})
Loading
Loading