mirror of
https://github.com/router-for-me/CLIProxyAPI.git
synced 2026-09-03 06:35:00 +08:00
fix(claude): restore tool_search_tool_result references and recognize advisor/agent server tools (#5044) (#5082)
- Add bidirectional remapping for nested tool_references inside tool_search_tool_result across non-stream, SSE stream, and multi-turn message history. - Add advisor_ and agent_toolset_ to IsClaudeServerToolType to prevent schema stripping and MCP aliasing on native Anthropic server tools. - Wrap MCP alias restoration errors in claudeMCPAliasRestoreError with IsRequestScoped() bool to avoid cooling down healthy OAuth credentials. - Add unit tests for tool_search_tool_result remapping, error variants, server tool recognition, and error scoping.
This commit is contained in:
@@ -1392,6 +1392,25 @@ func remapOAuthToolNamesWithBatchedEdits(body []byte, mcpAliases claudeMCPAliasO
|
||||
return true
|
||||
})
|
||||
}
|
||||
case "tool_search_tool_result":
|
||||
toolRefs := part.Get("content.tool_references")
|
||||
if toolRefs.Exists() && toolRefs.IsArray() {
|
||||
toolRefs.ForEach(func(_, refPart gjson.Result) bool {
|
||||
if refPart.Get("type").String() != "tool_reference" {
|
||||
return true
|
||||
}
|
||||
nameResult := refPart.Get("tool_name")
|
||||
refToolName := nameResult.String()
|
||||
if newName, renamed := rewriteName(refToolName); renamed {
|
||||
if !appendStringEdit(nameResult, newName) {
|
||||
validOffsets = false
|
||||
return false
|
||||
}
|
||||
recordRename(refToolName, newName)
|
||||
}
|
||||
return true
|
||||
})
|
||||
}
|
||||
}
|
||||
return validOffsets
|
||||
})
|
||||
@@ -1620,6 +1639,21 @@ func remapOAuthToolNamesWithOptionsLegacy(body []byte, mcpAliases claudeMCPAlias
|
||||
return true
|
||||
})
|
||||
}
|
||||
case "tool_search_tool_result":
|
||||
toolRefs := part.Get("content.tool_references")
|
||||
if toolRefs.Exists() && toolRefs.IsArray() {
|
||||
toolRefs.ForEach(func(refIndex, refPart gjson.Result) bool {
|
||||
if refPart.Get("type").String() == "tool_reference" {
|
||||
refToolName := refPart.Get("tool_name").String()
|
||||
if newName, renamed := rewriteName(refToolName); renamed {
|
||||
refPath := fmt.Sprintf("messages.%d.content.%d.content.tool_references.%d.tool_name", msgIndex.Int(), contentIndex.Int(), refIndex.Int())
|
||||
body, _ = sjson.SetBytes(body, refPath, newName)
|
||||
recordRename(refToolName, newName)
|
||||
}
|
||||
}
|
||||
return true
|
||||
})
|
||||
}
|
||||
}
|
||||
return true
|
||||
})
|
||||
@@ -1648,6 +1682,18 @@ type claudeMCPAliasResolver struct {
|
||||
servers map[string]struct{}
|
||||
}
|
||||
|
||||
type claudeMCPAliasRestoreError struct {
|
||||
error
|
||||
}
|
||||
|
||||
func (e claudeMCPAliasRestoreError) Unwrap() error {
|
||||
return e.error
|
||||
}
|
||||
|
||||
func (claudeMCPAliasRestoreError) IsRequestScoped() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func newClaudeMCPAliasResolver(reverseMap map[string]string) claudeMCPAliasResolver {
|
||||
resolver := claudeMCPAliasResolver{
|
||||
exact: reverseMap,
|
||||
@@ -1761,7 +1807,7 @@ func (resolver claudeMCPAliasResolver) resolve(name string) (string, bool, error
|
||||
return matchedOriginal, true, nil
|
||||
}
|
||||
if matchCount > 1 {
|
||||
return "", false, fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: matched multiple declared aliases", name)
|
||||
return "", false, claudeMCPAliasRestoreError{fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: matched multiple declared aliases", name)}
|
||||
}
|
||||
|
||||
parts, validAlias := parseClaudeMCPAlias(normalizedName)
|
||||
@@ -1816,10 +1862,10 @@ func (resolver claudeMCPAliasResolver) resolve(name string) (string, bool, error
|
||||
return matchedOriginal, true, nil
|
||||
}
|
||||
if matchCount > 1 {
|
||||
return "", false, fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: semantic suffix matches multiple declared tools", name)
|
||||
return "", false, claudeMCPAliasRestoreError{fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: semantic suffix matches multiple declared tools", name)}
|
||||
}
|
||||
|
||||
return "", false, fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: no unique request-local match", name)
|
||||
return "", false, claudeMCPAliasRestoreError{fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: no unique request-local match", name)}
|
||||
}
|
||||
|
||||
// reverseRemapOAuthToolNames reverses the tool name mapping for non-stream responses
|
||||
@@ -1880,6 +1926,26 @@ func reverseRemapOAuthToolNames(body []byte, reverseMap map[string]string) ([]by
|
||||
return true
|
||||
})
|
||||
}
|
||||
case "tool_search_tool_result":
|
||||
toolRefs := part.Get("content.tool_references")
|
||||
if toolRefs.Exists() && toolRefs.IsArray() {
|
||||
toolRefs.ForEach(func(refIndex, refPart gjson.Result) bool {
|
||||
if refPart.Get("type").String() != "tool_reference" {
|
||||
return true
|
||||
}
|
||||
toolName := refPart.Get("tool_name").String()
|
||||
origName, matched, errResolve := resolver.resolve(toolName)
|
||||
if errResolve != nil {
|
||||
resolveErr = errResolve
|
||||
return false
|
||||
}
|
||||
if matched {
|
||||
path := fmt.Sprintf("content.%d.content.tool_references.%d.tool_name", index.Int(), refIndex.Int())
|
||||
body, _ = sjson.SetBytes(body, path, origName)
|
||||
}
|
||||
return true
|
||||
})
|
||||
}
|
||||
}
|
||||
return resolveErr == nil
|
||||
})
|
||||
@@ -1928,6 +1994,44 @@ func reverseRemapOAuthToolNamesFromStreamLine(line []byte, reverseMap map[string
|
||||
return line, nil
|
||||
}
|
||||
updated, err = sjson.SetBytes(payload, "content_block.tool_name", origName)
|
||||
case "tool_search_tool_result":
|
||||
toolRefs := contentBlock.Get("content.tool_references")
|
||||
if !toolRefs.Exists() || !toolRefs.IsArray() {
|
||||
return line, nil
|
||||
}
|
||||
updatedPayload := payload
|
||||
var resolveErr error
|
||||
hasChange := false
|
||||
toolRefs.ForEach(func(refIndex, refPart gjson.Result) bool {
|
||||
if refPart.Get("type").String() != "tool_reference" {
|
||||
return true
|
||||
}
|
||||
toolName := refPart.Get("tool_name").String()
|
||||
origName, matched, errResolve := resolver.resolve(toolName)
|
||||
if errResolve != nil {
|
||||
resolveErr = errResolve
|
||||
return false
|
||||
}
|
||||
if matched {
|
||||
path := fmt.Sprintf("content_block.content.tool_references.%d.tool_name", refIndex.Int())
|
||||
updatedPayload, err = sjson.SetBytes(updatedPayload, path, origName)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
hasChange = true
|
||||
}
|
||||
return true
|
||||
})
|
||||
if resolveErr != nil {
|
||||
return line, resolveErr
|
||||
}
|
||||
if err != nil {
|
||||
return line, fmt.Errorf("rewrite Claude OAuth MCP tool alias: %w", err)
|
||||
}
|
||||
if !hasChange {
|
||||
return line, nil
|
||||
}
|
||||
updated = updatedPayload
|
||||
default:
|
||||
return line, nil
|
||||
}
|
||||
|
||||
@@ -2,12 +2,14 @@ package executor
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"maps"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/router-for-me/CLIProxyAPI/v7/internal/runtime/executor/helps"
|
||||
cliproxyexecutor "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/executor"
|
||||
"github.com/tidwall/gjson"
|
||||
)
|
||||
|
||||
@@ -343,14 +345,24 @@ func TestReverseRemapOAuthToolNamesRejectsUnsafeMangledAliases(t *testing.T) {
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
response := []byte(fmt.Sprintf(`{"content":[{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}]}`, test.alias))
|
||||
if _, errReverse := reverseRemapOAuthToolNames(response, reverseMap); errReverse == nil || !strings.Contains(errReverse.Error(), test.wantError) {
|
||||
_, errReverse := reverseRemapOAuthToolNames(response, reverseMap)
|
||||
if errReverse == nil || !strings.Contains(errReverse.Error(), test.wantError) {
|
||||
t.Fatalf("reverseRemapOAuthToolNames() error = %v, want %q", errReverse, test.wantError)
|
||||
}
|
||||
var requestErr cliproxyexecutor.RequestScopedError
|
||||
if !errors.As(errReverse, &requestErr) || !requestErr.IsRequestScoped() {
|
||||
t.Fatalf("reverseRemapOAuthToolNames() error = %T %v, want request-scoped", errReverse, errReverse)
|
||||
}
|
||||
|
||||
line := []byte(fmt.Sprintf(`data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}}`, test.alias))
|
||||
if _, errStream := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap); errStream == nil || !strings.Contains(errStream.Error(), test.wantError) {
|
||||
_, errStream := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap)
|
||||
if errStream == nil || !strings.Contains(errStream.Error(), test.wantError) {
|
||||
t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %v, want %q", errStream, test.wantError)
|
||||
}
|
||||
requestErr = nil
|
||||
if !errors.As(errStream, &requestErr) || !requestErr.IsRequestScoped() {
|
||||
t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %T %v, want request-scoped", errStream, errStream)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -542,3 +554,194 @@ func TestRemapKeepsReverseMapEmptyWhenOnlyCallerMCPToolsArePresent(t *testing.T)
|
||||
t.Fatalf("body = %s, want unchanged %s", out, body)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReverseRemapOAuthToolNamesMarksTrailingMarkupFailureRequestScoped(t *testing.T) {
|
||||
const alias = "mcp__hmzqrngkulqv__xuo7jlxlpzee_clear_thinking"
|
||||
malformedAlias := alias + "</parameter>\n<parameter name=\"merge\""
|
||||
reverseMap := map[string]string{alias: "clear_thinking"}
|
||||
|
||||
response := []byte(fmt.Sprintf(`{"content":[{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}]}`, malformedAlias))
|
||||
restored, errReverse := reverseRemapOAuthToolNames(response, reverseMap)
|
||||
if errReverse == nil {
|
||||
t.Fatal("reverseRemapOAuthToolNames() error = nil, want fail-closed alias error")
|
||||
}
|
||||
if !bytes.Equal(restored, response) {
|
||||
t.Fatalf("reverseRemapOAuthToolNames() returned modified response: %s", restored)
|
||||
}
|
||||
var requestErr cliproxyexecutor.RequestScopedError
|
||||
if !errors.As(errReverse, &requestErr) || !requestErr.IsRequestScoped() {
|
||||
t.Fatalf("reverseRemapOAuthToolNames() error = %T %v, want request-scoped", errReverse, errReverse)
|
||||
}
|
||||
|
||||
line := []byte(fmt.Sprintf(`data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}}`, malformedAlias))
|
||||
restoredLine, errStream := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap)
|
||||
if errStream == nil {
|
||||
t.Fatal("reverseRemapOAuthToolNamesFromStreamLine() error = nil, want fail-closed alias error")
|
||||
}
|
||||
if !bytes.Equal(restoredLine, line) {
|
||||
t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() returned modified line: %s", restoredLine)
|
||||
}
|
||||
requestErr = nil
|
||||
if !errors.As(errStream, &requestErr) || !requestErr.IsRequestScoped() {
|
||||
t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %T %v, want request-scoped", errStream, errStream)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReverseRemapOAuthToolNames_ToolSearchResult_NonStream(t *testing.T) {
|
||||
body := []byte(`{"tools":[{"name":"task_list","input_schema":{"type":"object"}},{"name":"fetch_url","input_schema":{"type":"object"}}]}`)
|
||||
remapped, reverseMap := remapOAuthToolNamesWithOptions(body, claudeMCPAliasOptions{secret: "tool-search-caller"})
|
||||
taskAlias := gjson.GetBytes(remapped, "tools.0.name").String()
|
||||
fetchAlias := gjson.GetBytes(remapped, "tools.1.name").String()
|
||||
|
||||
if taskAlias == "task_list" || fetchAlias == "fetch_url" {
|
||||
t.Fatalf("tools were not aliased: task=%q, fetch=%q", taskAlias, fetchAlias)
|
||||
}
|
||||
|
||||
// 1. Successful search result with multiple tool_references
|
||||
resp := []byte(fmt.Sprintf(`{
|
||||
"id": "msg_01HczuyKgD1KCUuVzvZEH3WN",
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_search_tool_result",
|
||||
"tool_use_id": "srvtoolu_01HczuyKgD1KCUuVzvZEH3WN",
|
||||
"content": {
|
||||
"type": "tool_search_tool_search_result",
|
||||
"tool_references": [
|
||||
{"type": "tool_reference", "tool_name": %q},
|
||||
{"type": "tool_reference", "tool_name": %q}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"type": "tool_use",
|
||||
"id": "toolu_01",
|
||||
"name": %q,
|
||||
"input": {"action": "list"}
|
||||
}
|
||||
]
|
||||
}`, taskAlias, fetchAlias, taskAlias))
|
||||
|
||||
restored, err := reverseRemapOAuthToolNames(resp, reverseMap)
|
||||
if err != nil {
|
||||
t.Fatalf("reverseRemapOAuthToolNames() error = %v", err)
|
||||
}
|
||||
|
||||
ref0 := gjson.GetBytes(restored, "content.0.content.tool_references.0.tool_name").String()
|
||||
ref1 := gjson.GetBytes(restored, "content.0.content.tool_references.1.tool_name").String()
|
||||
toolUseName := gjson.GetBytes(restored, "content.1.name").String()
|
||||
|
||||
if ref0 != "task_list" {
|
||||
t.Errorf("ref0 = %q, want task_list", ref0)
|
||||
}
|
||||
if ref1 != "fetch_url" {
|
||||
t.Errorf("ref1 = %q, want fetch_url", ref1)
|
||||
}
|
||||
if toolUseName != "task_list" {
|
||||
t.Errorf("toolUseName = %q, want task_list", toolUseName)
|
||||
}
|
||||
|
||||
// 2. Error variant (tool_search_tool_result_error) without tool_references should pass cleanly
|
||||
errorResp := []byte(`{
|
||||
"id": "msg_02",
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_search_tool_result",
|
||||
"tool_use_id": "srvtoolu_02",
|
||||
"content": {
|
||||
"type": "tool_search_tool_result_error",
|
||||
"error_code": "regex_compilation_failed"
|
||||
}
|
||||
}
|
||||
]
|
||||
}`)
|
||||
|
||||
restoredErr, err := reverseRemapOAuthToolNames(errorResp, reverseMap)
|
||||
if err != nil {
|
||||
t.Fatalf("reverseRemapOAuthToolNames(errorResp) error = %v", err)
|
||||
}
|
||||
if !bytes.Equal(restoredErr, errorResp) {
|
||||
t.Fatalf("errorResp altered: %s", string(restoredErr))
|
||||
}
|
||||
}
|
||||
|
||||
func TestReverseRemapOAuthToolNamesFromStreamLine_ToolSearchResult_Stream(t *testing.T) {
|
||||
body := []byte(`{"tools":[{"name":"task_list","input_schema":{"type":"object"}}]}`)
|
||||
remapped, reverseMap := remapOAuthToolNamesWithOptions(body, claudeMCPAliasOptions{secret: "tool-search-stream-caller"})
|
||||
taskAlias := gjson.GetBytes(remapped, "tools.0.name").String()
|
||||
|
||||
line := []byte(fmt.Sprintf(`data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_search_tool_result","tool_use_id":"srvtoolu_01","content":{"type":"tool_search_tool_search_result","tool_references":[{"type":"tool_reference","tool_name":%q}]}}}`, taskAlias))
|
||||
|
||||
restoredLine, err := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap)
|
||||
if err != nil {
|
||||
t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %v", err)
|
||||
}
|
||||
|
||||
if !bytes.HasPrefix(restoredLine, []byte("data: ")) {
|
||||
t.Fatalf("restoredLine lost data: prefix: %s", string(restoredLine))
|
||||
}
|
||||
gotToolName := gjson.GetBytes(helps.JSONPayload(restoredLine), "content_block.content.tool_references.0.tool_name").String()
|
||||
if gotToolName != "task_list" {
|
||||
t.Fatalf("stream restored tool_name = %q, want task_list", gotToolName)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemapOAuthToolNames_ToolSearchResultInMessageHistory(t *testing.T) {
|
||||
body := []byte(`{
|
||||
"tools": [
|
||||
{"name": "task_list", "input_schema": {"type": "object"}},
|
||||
{"type": "advisor_20260301", "name": "advisor", "model": "claude-haiku-4-5-20251001"},
|
||||
{"type": "agent_toolset_20260401"}
|
||||
],
|
||||
"messages": [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_search_tool_result",
|
||||
"tool_use_id": "srvtoolu_01",
|
||||
"content": {
|
||||
"type": "tool_search_tool_search_result",
|
||||
"tool_references": [
|
||||
{"type": "tool_reference", "tool_name": "task_list"}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}`)
|
||||
|
||||
remapped, reverseMap := remapOAuthToolNamesWithOptions(body, claudeMCPAliasOptions{secret: "history-caller"})
|
||||
taskAlias := reverseMapKeyFor(reverseMap, "task_list")
|
||||
if taskAlias == "" {
|
||||
t.Fatal("task_list was not aliased")
|
||||
}
|
||||
|
||||
// Verify server tools were preserved
|
||||
tools := gjson.GetBytes(remapped, "tools")
|
||||
if tools.Get("1.type").String() != "advisor_20260301" || tools.Get("1.name").String() != "advisor" {
|
||||
t.Fatalf("advisor server tool was altered: %s", tools.Get("1").Raw)
|
||||
}
|
||||
if tools.Get("2.type").String() != "agent_toolset_20260401" {
|
||||
t.Fatalf("agent_toolset was altered: %s", tools.Get("2").Raw)
|
||||
}
|
||||
|
||||
// Verify tool_search_tool_result in messages was remapped
|
||||
gotHistoryToolName := gjson.GetBytes(remapped, "messages.0.content.0.content.tool_references.0.tool_name").String()
|
||||
if gotHistoryToolName != taskAlias {
|
||||
t.Fatalf("history tool_name = %q, want aliased %q", gotHistoryToolName, taskAlias)
|
||||
}
|
||||
}
|
||||
|
||||
func reverseMapKeyFor(m map[string]string, val string) string {
|
||||
for k, v := range m {
|
||||
if v == val && k != val {
|
||||
return k
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
@@ -27,6 +27,8 @@ func newClaudeBuiltinToolRegistry() map[string]bool {
|
||||
func IsClaudeServerToolType(toolType string) bool {
|
||||
toolType = strings.ToLower(strings.TrimSpace(toolType))
|
||||
for _, prefix := range []string{
|
||||
"advisor_",
|
||||
"agent_toolset_",
|
||||
"bash_",
|
||||
"code_execution_",
|
||||
"computer_",
|
||||
|
||||
@@ -32,7 +32,18 @@ func TestClaudeBuiltinToolRegistry_AugmentsKnownTypedBuiltinsFromBody(t *testing
|
||||
}
|
||||
|
||||
func TestIsClaudeServerToolType(t *testing.T) {
|
||||
for _, toolType := range []string{"web_search_20250305", "code_execution_20250522", "tool_search_tool_regex_20251119"} {
|
||||
for _, toolType := range []string{
|
||||
"web_search_20250305",
|
||||
"code_execution_20250522",
|
||||
"tool_search_tool_regex_20251119",
|
||||
"advisor_20260301",
|
||||
"agent_toolset_20260401",
|
||||
"bash_20250124",
|
||||
"text_editor_20250728",
|
||||
"memory_20250818",
|
||||
"computer_20241022",
|
||||
"web_fetch_20260209",
|
||||
} {
|
||||
if !IsClaudeServerToolType(toolType) {
|
||||
t.Fatalf("IsClaudeServerToolType(%q) = false, want true", toolType)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user