fix(claude): add dynamic snakeCaseToTitleCase fallback for OAuth tool rename#2839
fix(claude): add dynamic snakeCaseToTitleCase fallback for OAuth tool rename#2839WilsonLiu95 wants to merge 1 commit into
Conversation
… rename The static oauthToolRenameMap only covers 14 built-in Claude Code tools. Third-party harnesses (OpenCode, OpenClaw, etc.) register custom tools with snake_case names (e.g. sessions_list, agents_list, feishu_calendar_event) that pass through to Anthropic unchanged, triggering third-party client detection and extra-usage billing on OAuth traffic. This commit adds a dynamic fallback that automatically converts any snake_case or kebab-case tool name to TitleCase when the static map has no entry. A per-request reverse map (dynReverse) is threaded through the response path so dynamically renamed tools are correctly restored to their original names before returning to the downstream client. Changes: - Add snakeCaseToTitleCase() with MCP tool exclusion (__ separator) - Add dynamicOAuthToolRename() with static map collision check - Modify remapOAuthToolNames() to return per-request dynReverse map - Modify reverseRemapOAuthToolNames() and stream variant to accept dynReverse - Fix non-streaming translation path: when stream=true internally (format translation), apply line-by-line SSE reverse instead of JSON body reverse - Update default fingerprint to claude-cli/2.1.108 (external, sdk-cli) Closes router-for-me#2599 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This pull request targeted The base branch has been automatically changed to |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cee7f7d63e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if newName, ok := resolveNewName(tcName); ok { | ||
| body, _ = sjson.SetBytes(body, "tool_choice.name", newName) |
There was a problem hiding this comment.
Exempt builtin tool names from dynamic OAuth remapping
This branch now applies resolveNewName to tool_choice.name, which will convert built-in names like web_search to WebSearch for OAuth requests. In the same function, typed built-ins in tools[] are explicitly kept unchanged, so a payload can become internally inconsistent (tools[].name stays web_search while tool_choice.name becomes WebSearch), which can break forced-tool requests against Anthropic. Please skip dynamic remapping for known/typed built-ins before rewriting tool_choice (and similarly for message tool references).
Useful? React with 👍 / 👎.
| if newName, ok := dynamicOAuthToolRename(name); ok { | ||
| dynReverse[newName] = name | ||
| log.Debugf("claude: dynamic tool rename: %q -> %q", name, newName) |
There was a problem hiding this comment.
Guard against dynamic tool-name collisions in dynReverse
The dynamic reverse map is updated with dynReverse[newName] = name without checking whether newName was already produced by another tool in the same request. If two distinct names collapse to the same TitleCase (for example, foo_bar and foo-bar), the second write silently overwrites the first, which can both create duplicate upstream tool names and make response remapping nondeterministic (last writer wins). Add a per-request collision check and avoid renaming colliding tools.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic tool name remapping for Claude OAuth tokens, converting snake_case or kebab-case names to TitleCase when static mappings are missing. It also updates the Claude fingerprint versions and improves response handling for streamed data by applying reverse remapping line-by-line. Feedback includes optimizing collision checks for dynamic renames, reducing code duplication in name resolution logic, and adding test coverage for the new dynamic renaming functionality.
| for _, v := range oauthToolRenameMap { | ||
| if v == newName { | ||
| return name, false | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop iterates over all values in oauthToolRenameMap for every dynamic rename check. This has a linear time complexity (O(n) where n is the size of the map). For better performance and maintainability, especially if the map grows, consider creating a map[string]struct{} from the values of oauthToolRenameMap once at initialization. This would allow for an O(1) collision check.
| // reverseRemapOAuthToolNamesFromStreamLine reverses the tool name mapping for SSE stream lines. | ||
| func reverseRemapOAuthToolNamesFromStreamLine(line []byte) []byte { | ||
| // dynReverse contains per-request mappings for dynamically renamed tools. | ||
| func reverseRemapOAuthToolNamesFromStreamLine(line []byte, dynReverse map[string]string) []byte { |
There was a problem hiding this comment.
The resolveOrigName closure defined within this function is identical to the one in reverseRemapOAuthToolNames. To improve maintainability and avoid code duplication, consider extracting this logic into a shared helper function that returns the resolver closure.
For example:
func newOAuthToolNameResolver(dynReverse map[string]string) func(string) (string, bool) {
return func(name string) (string, bool) {
if origName, ok := oauthToolRenameReverseMap[name]; ok {
return origName, true
}
if dynReverse != nil {
if origName, ok := dynReverse[name]; ok {
return origName, true
}
}
return name, false
}
}Then both reverseRemapOAuthToolNames and reverseRemapOAuthToolNamesFromStreamLine can use it: resolveOrigName := newOAuthToolNameResolver(dynReverse).
| } | ||
| } | ||
|
|
||
| func TestRemapOAuthToolNames_Lowercase_ReverseApplied(t *testing.T) { |
There was a problem hiding this comment.
While the existing tests are correctly updated, the new dynamic renaming functionality for snake_case and kebab-case tool names is not covered. To ensure robustness and prevent regressions, please add a new test case that specifically verifies this dynamic fallback.
Here is an example test you could add:
func TestRemapOAuthToolNames_DynamicSnakeCase(t *testing.T) {
body := []byte(`{"tools":[{"name":"my_custom_tool"}]}`)
out, renamed, dynReverse := remapOAuthToolNames(body)
if !renamed {
t.Fatal("renamed = false, want true for dynamic rename")
}
if got := gjson.GetBytes(out, "tools.0.name").String(); got != "MyCustomTool" {
t.Fatalf("tools.0.name = %q, want %q", got, "MyCustomTool")
}
if len(dynReverse) != 1 || dynReverse["MyCustomTool"] != "my_custom_tool" {
t.Fatalf("dynReverse map is incorrect: got %v", dynReverse)
}
resp := []byte(`{"content":[{"type":"tool_use","name":"MyCustomTool"}]}`)
reversed := reverseRemapOAuthToolNames(resp, dynReverse)
if got := gjson.GetBytes(reversed, "content.0.name").String(); got != "my_custom_tool" {
t.Fatalf("reversed content.0.name = %q, want %q", got, "my_custom_tool")
}
}
luispater
left a comment
There was a problem hiding this comment.
Summary
This PR extends Claude OAuth tool renaming by adding a dynamic snake_case/kebab-case → TitleCase fallback (with a per-request dynReverse map to restore names in responses). It also fixes the non-stream executor path when it internally uses stream=true for format translation by reversing tool names line-by-line on SSE data, and updates the default Claude fingerprint (UA/package version).
Key findings
- No blocking issues found.
- Non-blocking: Please consider adding unit tests for the dynamic fallback +
dynReverserestoration, including the SSE reverse path.
Test plan
- Did not checkout/run locally (per review constraints).
- CI checks are green.
This is an automated Codex review result and still requires manual verification by a human reviewer.
Summary
The static
oauthToolRenameMaponly covers 14 built-in Claude Code tools. Third-party harnesses (OpenCode, OpenClaw, etc.) register custom tools withsnake_casenames (e.g.sessions_list,agents_list,feishu_calendar_event) that pass through to Anthropic unchanged, triggering third-party client detection and extra-usage billing on OAuth traffic.This PR adds a dynamic fallback that automatically converts any
snake_caseorkebab-casetool name to TitleCase when the static map has no entry, with a per-request reverse map for response decoding.Changes
snakeCaseToTitleCase()— convertssnake_case/kebab-caseto TitleCase, skips MCP tools (__separator) and single-word namesdynamicOAuthToolRename()— dynamic fallback with collision check against static map valuesremapOAuthToolNames()— now returns a per-requestdynReversemap alongside renamed bodyreverseRemapOAuthToolNames()/ stream variant — acceptdynReversefor two-tier lookup (static + dynamic)stream=trueinternally for format translation, apply line-by-line SSE reverse instead of single JSON body reverse (fixes tool names not being restored in OpenAI-format responses)claude-cli/2.1.108 (external, sdk-cli), package version to0.81.0Examples
bashBashBashsessions_listSessionsListSessionsListfeishu_calendar_eventFeishuCalendarEventFeishuCalendarEventmcp__server__toolmcp__server__toolmessagemessageRelation to other PRs
Test plan
TestRemapOAuthToolNames_*tests passsessions_list→SessionsList,agents_list→AgentsList,feishu_calendar_event→FeishuCalendarEventmcp__server__tool) correctly skippedmessage,Bash) correctly skippedCloses #2599