From f2d272da817dffe4be7fbc7b5eddcaaa4bdc6b1d Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Fri, 14 Aug 2026 13:52:08 +0800 Subject: [PATCH] fix(claude): recover OAuth tool aliases for repeated prefixes and malformed IDs - Normalize repeated `mcp____` alias prefixes during reverse remapping to resolve stacked aliases. - Add a semantic-suffix fallback when parsing fails, allowing unambiguous recovery from malformed tool IDs. Closes: #4916 --- .../executor/claude_executor_request.go | 37 +++++--- .../claude_executor_request_remap_test.go | 88 +++++++++++++++++-- 2 files changed, 106 insertions(+), 19 deletions(-) diff --git a/internal/runtime/executor/claude_executor_request.go b/internal/runtime/executor/claude_executor_request.go index d2920d5f4..32bb301a4 100644 --- a/internal/runtime/executor/claude_executor_request.go +++ b/internal/runtime/executor/claude_executor_request.go @@ -1586,9 +1586,17 @@ func (resolver claudeMCPAliasResolver) resolve(name string) (string, bool, error return "", false, nil } - repeatedServerPrefix := "mcp__" + server + "__" + server + "__" - if suffix, repeatedServer := strings.CutPrefix(name, repeatedServerPrefix); repeatedServer { - if original, exact := resolver.exact["mcp__"+server+"__"+suffix]; exact { + canonicalServerPrefix := "mcp__" + server + "__" + normalizedName := name + suffix := strings.TrimPrefix(name, canonicalServerPrefix) + for { + strippedSuffix, repeatedServer := strings.CutPrefix(suffix, server+"__") + if !repeatedServer { + break + } + suffix = strippedSuffix + normalizedName = canonicalServerPrefix + suffix + if original, exact := resolver.exact[normalizedName]; exact { return original, true, nil } } @@ -1608,21 +1616,30 @@ func (resolver claudeMCPAliasResolver) resolve(name string) (string, bool, error return "", false, fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: matched multiple declared aliases", name) } - parts, ok := parseClaudeMCPAlias(name) - if ok { + parts, validAlias := parseClaudeMCPAlias(normalizedName) + if validAlias { for _, entry := range resolver.aliases { if entry.parts.server == parts.server && entry.parts.semantic == parts.semantic { matchedOriginal = entry.original matchCount++ } } - if matchCount == 1 { - 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) + } else { + // Keep generated aliases strict while allowing a malformed response tool ID + // to recover only when its request-local semantic suffix is unambiguous. + for _, entry := range resolver.aliases { + if entry.parts.server == server && strings.HasSuffix(normalizedName, "_"+entry.parts.semantic) { + matchedOriginal = entry.original + matchCount++ + } } } + if matchCount == 1 { + 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, fmt.Errorf("cannot restore Claude OAuth MCP tool alias %q: no unique request-local match", name) } diff --git a/internal/runtime/executor/claude_executor_request_remap_test.go b/internal/runtime/executor/claude_executor_request_remap_test.go index ccc46e2cb..390fe978a 100644 --- a/internal/runtime/executor/claude_executor_request_remap_test.go +++ b/internal/runtime/executor/claude_executor_request_remap_test.go @@ -163,18 +163,83 @@ func TestReverseRemapOAuthToolNamesRecoversMangledAliases(t *testing.T) { } } -func TestReverseRemapOAuthToolNamesRecoversRepeatedServerAlias(t *testing.T) { +func TestReverseRemapOAuthToolNamesRecoversRepeatedServerAliases(t *testing.T) { + const alias = "mcp__hmzqrngkulqv__xuo7jlxlpzee_Bash" + reverseMap := map[string]string{ + alias: "Bash", + "mcp__hmzqrngkulqv__aaaaaaaaaaaa_Bash": "OtherBash", + } + tests := []struct { + name string + responseAlias string + }{ + { + name: "single repetition", + responseAlias: "mcp__hmzqrngkulqv__hmzqrngkulqv__xuo7jlxlpzee_Bash", + }, + { + name: "multiple repetitions", + responseAlias: "mcp__hmzqrngkulqv__hmzqrngkulqv__hmzqrngkulqv__xuo7jlxlpzee_Bash", + }, + } + + 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.responseAlias)) + restored, errReverse := reverseRemapOAuthToolNames(response, reverseMap) + if errReverse != nil { + t.Fatalf("reverseRemapOAuthToolNames() error = %v", errReverse) + } + if got := gjson.GetBytes(restored, "content.0.name").String(); got != "Bash" { + t.Fatalf("repeated server alias restored to %q, want Bash", got) + } + + line := []byte(fmt.Sprintf(`data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}}`, test.responseAlias)) + restoredLine, errStream := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap) + if errStream != nil { + t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %v", errStream) + } + if got := gjson.GetBytes(helps.JSONPayload(restoredLine), "content_block.name").String(); got != "Bash" { + t.Fatalf("stream repeated server alias restored to %q, want Bash", got) + } + }) + } +} + +func TestReverseRemapOAuthToolNamesRecoversMalformedToolIDBySemanticSuffix(t *testing.T) { const alias = "mcp__hmzqrngkulqv__xuo7jlxlpzee_Bash" reverseMap := map[string]string{alias: "Bash"} - responseAlias := "mcp__hmzqrngkulqv__hmzqrngkulqv__xuo7jlxlpzee_Bash" - response := []byte(fmt.Sprintf(`{"content":[{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}]}`, responseAlias)) - - restored, errReverse := reverseRemapOAuthToolNames(response, reverseMap) - if errReverse != nil { - t.Fatalf("reverseRemapOAuthToolNames() error = %v", errReverse) + tests := []struct { + name string + responseAlias string + }{ + {name: "short tool ID", responseAlias: "mcp__hmzqrngkulqv__xuo7jlxlpze_Bash"}, + {name: "long tool ID", responseAlias: "mcp__hmzqrngkulqv__xuo7jlxlpzeea_Bash"}, + {name: "invalid base32 tool ID", responseAlias: "mcp__hmzqrngkulqv__xuo7jlxlpze0_Bash"}, + {name: "substituted base32 tool ID", responseAlias: "mcp__hmzqrngkulqv__auo7jlxlpzee_Bash"}, + {name: "repeated server and short tool ID", responseAlias: "mcp__hmzqrngkulqv__hmzqrngkulqv__xuo7jlxlpze_Bash"}, } - if got := gjson.GetBytes(restored, "content.0.name").String(); got != "Bash" { - t.Fatalf("repeated server alias restored to %q, want Bash", got) + + 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.responseAlias)) + restored, errReverse := reverseRemapOAuthToolNames(response, reverseMap) + if errReverse != nil { + t.Fatalf("reverseRemapOAuthToolNames() error = %v", errReverse) + } + if got := gjson.GetBytes(restored, "content.0.name").String(); got != "Bash" { + t.Fatalf("malformed tool ID alias restored to %q, want Bash", got) + } + + line := []byte(fmt.Sprintf(`data: {"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"toolu_1","name":%q,"input":{}}}`, test.responseAlias)) + restoredLine, errStream := reverseRemapOAuthToolNamesFromStreamLine(line, reverseMap) + if errStream != nil { + t.Fatalf("reverseRemapOAuthToolNamesFromStreamLine() error = %v", errStream) + } + if got := gjson.GetBytes(helps.JSONPayload(restoredLine), "content_block.name").String(); got != "Bash" { + t.Fatalf("stream malformed tool ID alias restored to %q, want Bash", got) + } + }) } } @@ -209,6 +274,11 @@ func TestReverseRemapOAuthToolNamesRejectsUnsafeMangledAliases(t *testing.T) { alias: "mcp__" + firstParts.server + "__" + unknownToolID + "_" + firstParts.semantic, wantError: "semantic suffix matches multiple declared tools", }, + { + name: "ambiguous semantic suffix with malformed tool ID", + alias: "mcp__" + firstParts.server + "__" + unknownToolID[:len(unknownToolID)-1] + "_" + firstParts.semantic, + wantError: "semantic suffix matches multiple declared tools", + }, { name: "unrecoverable semantic suffix", alias: "mcp__" + firstParts.server + "__" + unknownToolID + "_missing_tool",