fix(anthropic): alias session search on OAuth wire - #76669
konsisumer wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the narrowly isolated OAuth request-shape failure documented in #65365. Current remote main still injects SESSION_SEARCH_GUIDANCE when the tool is enabled (agent/system_prompt.py:230-233), and its OAuth transform has no aliasing step (agent/anthropic_adapter.py:2783-2833).
Problems
- The new schema alias at
agent/anthropic_adapter.py:2823makessession_searchadvertise asmcp__chat_history_lookup, but namedtool_choiceremains canonical atagent/anthropic_adapter.py:2858.agent/auxiliary_client.py:1438accepts a named function choice and forwards it atagent/auxiliary_client.py:1461, so this valid OAuth request shape can name a tool that is no longer advertised.
Suggested changes
- Normalize a named OAuth
tool_choicethrough_to_oauth_wire_name, and add OAuth/non-OAuth assertions fortool_choice="session_search".
Automated hermes-sweeper review.
| # classifier. normalize_response reverses both forms via registry | ||
| # lookup so the dispatcher still sees the original name. GH-25255. | ||
| def _to_oauth_wire_name(name: str) -> str: | ||
| name = _OAUTH_TOOL_NAME_ALIASES.get(name, name) |
There was a problem hiding this comment.
This aliases the advertised schema name, but the later named tool_choice branch still emits the canonical session_search string. Normalize named OAuth choices through _to_oauth_wire_name too, then add a regression test for tool_choice="session_search".
Related: this patch aliases only |
|
Thanks for the review. I confirmed the mismatch: the OAuth schema advertises I would keep this PR limited to the bisection-backed I could not verify #24250's current state or contents because the GitHub API is unreachable from this worker. Should this PR proceed with the focused OAuth/non-OAuth |
|
Salvaged in #76807 — your commit was cherry-picked with authorship preserved (rebase-merge), so it lands on main under your name. Good catch on the mechanism: aliasing the tool schema on the OAuth wire and reversing it in Three follow-up commits on top:
One honest note on verification: nobody in this review could confirm the billing lane changed, only that the request shape no longer carries the fingerprinted tokens — HTTP 200 doesn't prove which bucket was charged, only the Anthropic dashboard does. If you're on a subscription account and can watch plan-vs-overage utilization after this lands, that datapoint would be genuinely useful. Closing this in favor of the merged salvage. Thanks for the contribution — the diagnosis was the hard part. |
What does this PR do?
Fixes the deterministic Anthropic OAuth billing-classifier failure in #65365. OAuth requests now replace the
session_searchtoken withchat_history_lookupin the system prompt and tool schemas, while response normalization restoressession_searchbefore Hermes dispatches the call. API-key requests remain unchanged.Related Issue
Fixes #65365
Type of Change
Changes Made
agent/anthropic_adapter.py: aliassession_searchin OAuth-only system text, tool names, descriptions, and replayed tool-use names.agent/transports/anthropic.py: reverse the OAuth alias before generic registry lookup so calls reach the canonicalsession_searchhandler.tests/agent/test_anthropic_mcp_prefix_strip.py: cover the OAuth request shape, response round-trip, and non-OAuth passthrough.How to Test
/opt/homebrew/bin/timeout -k 30 480 sh -c 'pytest tests/ -q -x --timeout=60 "$@"' sh./opt/homebrew/bin/timeout -k 30 480 pytest tests/agent/test_anthropic_mcp_prefix_strip.py -q --timeout=60.python scripts/check-windows-footguns.py.What platforms tested on
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
Not applicable.
Screenshots / Logs
Not applicable.