fix(anthropic): normalize forced tool_choice name to mcp__ on the OAuth wire - #48192
fix(anthropic): normalize forced tool_choice name to mcp__ on the OAuth wire#48192ly-wang19 wants to merge 1 commit into
Conversation
|
Duplicate of #23361 — that open PR makes the identical fix at the same site in |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Normalizes forced tool_choice name to mcp__ prefix on the OAuth wire in the Anthropic adapter.
Looks Good
- Clean, well-scoped fix
- Consistent naming convention
Reviewed by Hermes Agent
|
Thanks @alt-glitch — you're right that #23361 (by @aldoeliacim, opened earlier) makes the same core change: lifting The one thing this PR adds on top is a dedicated regression test for the forced- I'm happy to defer to #23361 and close this if maintainers prefer to land that one — or, if it's easier, I can port my regression test over to #23361 so the fix lands with coverage. Whichever keeps it moving; just let me know. |
|
Confirming this gap is real and complementary to #47738 (system-prompt relocation), not a duplicate. With elif isinstance(tool_choice, str):
kwargs["tool_choice"] = {"type": "tool", "name": tool_choice} # raw — no mcp__ normalizationOn the OAuth wire that name no longer matches the encoded entry in Verified the rest of the OAuth path on a live Max subscription while testing #47738 (opus-4-8 ran tools, billed to plan limits). This |
|
Verified a real current-main OAuth wire-shape defect. The proposed change reuses the existing idempotent encoder, retains the non-OAuth path unchanged, and the added tests assert the relevant relationship between Automated hermes-sweeper review. |
Re-triage: removing the |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address the Anthropic OAuth tool-name billing failure or its remaining forced-tool edge. #33570 proposed stripping prefixes plus broader payload restrictions, #47723 implemented the current mcp__ wire normalization and registry-backed round trip, and #48192 applies that same normalization to forced tool_choice names.
Related pull requests
- #33570 [closed]
related— (+608/-44) — superseded: This broader alternative strippedmcp_, filtered tools, capped schemas and system prompts, and rewrote history, but merged #47723 fixed the reported classifier trigger with consistentmcp__normalization; it remains relevant as the superseded investigation, while its environment-based behavioral setting also conflicts with repository configuration policy. - #47723 [merged]
related— (+144/-108) — merged reference implementation: It fixes the root cause by ensuring neither native nor MCP-server tools retain a single-underscoremcp_prefix on the OAuth wire, and reversesmcp__names through registry lookup on response. - #48192
related— (+75/-10) — merge: It closes the sibling-site gap left by #47723 by applying the same idempotentmcp__encoder to forcedtool_choice, preventing both atools[]name mismatch and a single-underscore classifier leak while preserving non-OAuth behavior. This matches the visible keep_open review on #48192, which verified the defect on current main and the reachable caller path; the earlier same-core predecessor #23361 is closed unmerged.
Suggested consolidation
Merge #48192 as the narrow, tested follow-up to merged #47723. Keep #33570 closed as superseded by #47723; no listed open PR should be closed as a duplicate, because #48192 fixes the distinct forced-tool_choice site that #47723 did not cover.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 68 kB of PR diffs, 9 kB of issue/PR text, 5 kB of discussion (8 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
724bcf5 to
3e6de3e
Compare
|
Heads up on the red check: the only failing job is That assertion is in the test's setup (before |
…th wire build_anthropic_kwargs rewrites every tool name to the double-underscore mcp__ form under is_oauth — for tools[] and replayed tool_use history — to clear Anthropic's OAuth billing classifier (a single-underscore mcp_ name is treated as a third-party-app fingerprint and rejected with HTTP 400; NousResearchGH-25255). The forced tool_choice mapping was the one sibling site left on the raw caller name, so any OAuth request that forces a specific tool: - sends tool_choice.name="read_file" while tools[] holds "mcp__read_file" — a mismatch the Messages API rejects with HTTP 400; and - with an MCP-server tool (mcp_linear_get_issue), leaks the single-underscore mcp_ name onto the wire, the exact fingerprint the transform removes. Hoist _to_oauth_wire_name to module scope and apply it to the forced name when is_oauth. The non-OAuth path is unchanged. Adds 3 tests; the two OAuth cases fail without the fix.
3e6de3e to
f01a1d6
Compare
Summary
Follow-up to #25255 /
b70a4e753("normalize MCP-server tool names tomcp__on OAuth wire").Under
is_oauth=True,build_anthropic_kwargsrewrites every tool name to the double-underscoremcp__wire form so it clears Anthropic's subscription/OAuth billing classifier — which treats a single-underscoremcp_name as a third-party-app fingerprint and rejects the request with HTTP 400. That rename is applied totools[]and to replayedtool_usehistory:But the forced
tool_choicemapping is a sibling site that was left on the raw caller name:So any OAuth request that forces a specific tool breaks two ways:
tools[]becomesmcp__read_filebuttool_choice.namestaysread_file. The Messages API requirestool_choice.nameto match a name present intools[], so the request is rejected.tool_choice="mcp_linear_get_issue"), the single-underscoremcp_name reaches the OAuth wire viatool_choice— the exact invariant this transform exists to enforce ("ZERO single-underscoremcp_names on the wire").Reachable through
auxiliary_client.py/ the Anthropic transport, which forward a caller-suppliedtool_choice(incl. the OpenAI{"type":"function","function":{"name":…}}form, normalized to a bare string) together withis_oauth.Fix
Hoist
_to_oauth_wire_namefrom a closure to module scope and apply it to the forced name whenis_oauth:The non-OAuth path is untouched.
Tests
Added to
TestAnthropicOAuthOutgoingPrefixintests/agent/test_anthropic_mcp_prefix_strip.py:test_oauth_tool_choice_matches_renamed_bare_tool—read_file→tool_choice.name == "mcp__read_file"and equals thetools[]entry;test_oauth_tool_choice_promotes_single_underscore_mcp—mcp_linear_get_issue→mcp__linear_get_issue, no single-underscore on the wire;test_non_oauth_tool_choice_untouched— non-OAuth keeps the bare name.The two OAuth cases fail without the fix; all 12 existing prefix tests still pass.