fix(debug): show correct URL and API key in request dumps for all api_modes - #54221
fix(debug): show correct URL and API key in request dumps for all api_modes#54221liuhao1024 wants to merge 3 commits into
Conversation
…_modes The dump_api_request_debug function had two bugs when api_mode was anthropic_messages or bedrock_converse: 1. URL construction only handled codex_responses (/responses) vs the default (/chat/completions). anthropic_messages requests actually go to /v1/messages, and bedrock_converse to /converse, but the dump always showed /chat/completions for non-codex modes. 2. API key extraction read from agent.client.api_key, but in anthropic_messages mode agent.client is None (the Anthropic SDK client lives at agent._anthropic_client). This caused the dump to show "Bearer None" even when a valid key was present. Fix both by dispatching on api_mode for URL construction and falling back to agent._anthropic_api_key (Anthropic) or a literal "aws-sdk" (Bedrock) when agent.client is absent. Also include api_mode in the dump payload so the active transport is visible without guessing. Fixes NousResearch#54206
tonydwb
left a comment
There was a problem hiding this comment.
Fixes debug dump accuracy across all transport modes (85 additions). Correct URL and API key resolution for anthropic_messages, bedrock_converse, and codex_responses modes. Includes tests for anthropic and chat_completions modes. Good observability improvement.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the misleading Anthropic dump; the current-head helper still hardcodes non-Codex modes to /chat/completions and reads only agent.client.api_key (agent/agent_runtime_helpers.py:1443-1457), while Anthropic initialization deliberately sets agent.client = None (agent/agent_init.py:817-831).
Problems
- The Bedrock branch in this diff reports
Bearer aws-sdkand<base>/converse. Current Bedrock execution uses boto3's AWS credential chain (agent/bedrock_adapter.py:91-101) and callsclient.converse(**kwargs)withmodelId(agent/chat_completion_helpers.py:274-278;agent/bedrock_adapter.py:944-950), so those fields do not faithfully describe the transport. - The new tests cover Anthropic and default chat-completions only; no test exercises the changed Bedrock branch.
Suggested changes
- Label Bedrock authentication as AWS SDK/SigV4 rather than a Bearer key, and report an accurately derived request target or an explicitly labeled SDK operation.
- Add a Bedrock regression test for that representation.
Automated hermes-sweeper review.
| if agent.api_mode == "anthropic_messages": | ||
| api_key = getattr(agent, "_anthropic_api_key", None) | ||
| elif agent.api_mode == "bedrock_converse": | ||
| api_key = "aws-sdk" |
There was a problem hiding this comment.
Bedrock authenticates through boto3's AWS credential chain (agent/bedrock_adapter.py:91-101), not a bearer API key. Please record an explicitly labeled AWS SDK/SigV4 auth mode instead of fabricating Bearer aws-sdk in a request dump.
| elif agent.api_mode == "anthropic_messages": | ||
| _url = f"{_base}/v1/messages" | ||
| elif agent.api_mode == "bedrock_converse": | ||
| _url = f"{_base}/converse" |
There was a problem hiding this comment.
The live Bedrock path invokes boto3 client.converse(**kwargs) with a modelId (agent/chat_completion_helpers.py:274-278, agent/bedrock_adapter.py:944-950); <base>/converse is not enough to claim it is the actual outbound request URL. Please emit a labeled SDK operation or derive the complete target.
| assert payload["request"]["url"] == "http://127.0.0.1:9208/v1/chat/completions" | ||
|
|
||
|
|
||
| def test_dump_api_request_debug_uses_anthropic_messages_url(monkeypatch, tmp_path): |
There was a problem hiding this comment.
Please add a companion Bedrock-mode regression test. This PR changes the Bedrock URL/auth branch, but this test only exercises the Anthropic branch.
What does this PR do?
Fixes the
dump_api_request_debugfunction so that request dumps show the correct URL and API key for allapi_modevalues (anthropic_messages,bedrock_converse,codex_responses,chat_completions).Previously, the dump function had two bugs:
URL: Only
codex_responsesgot/responses; everything else got/chat/completions— evenanthropic_messages(which actually hits/v1/messages) andbedrock_converse(/converse).API key: Always read from
agent.client.api_key. Inanthropic_messagesmodeagent.clientisNone(the Anthropic SDK client lives atagent._anthropic_client), so the dump showedBearer Noneeven when a valid key was present.This made debugging provider-side errors extremely confusing — issue #54206 reports a
hermes -zuser seeingPOST https://api.anthropic.com/chat/completionswithBearer Nonein the dump and concluding the request was misrouted, when in fact the Anthropic SDK was sending to/v1/messageswith the correct key.Related Issue
Fixes #54206
Type of Change
Changes Made
agent/agent_runtime_helpers.py: Dispatch URL construction onapi_mode(/v1/messagesfor anthropic,/conversefor bedrock,/responsesfor codex,/chat/completionsdefault). Fall back toagent._anthropic_api_keyoragent.api_keywhenagent.clientis absent. Addapi_modefield to dump payload.tests/run_agent/test_run_agent_codex_responses.py: Addtest_dump_api_request_debug_uses_anthropic_messages_url(verifies URL and key for anthropic_messages mode) andtest_dump_api_request_debug_includes_api_mode(verifies api_mode field in dump payload).How to Test
python -m pytest tests/run_agent/test_run_agent_codex_responses.py::test_dump_api_request_debug_uses_anthropic_messages_url tests/run_agent/test_run_agent_codex_responses.py::test_dump_api_request_debug_includes_api_mode tests/run_agent/test_run_agent_codex_responses.py::test_dump_api_request_debug_uses_responses_url tests/run_agent/test_run_agent_codex_responses.py::test_dump_api_request_debug_uses_chat_completions_url tests/run_agent/test_run_agent_codex_responses.py::test_dump_api_request_debug_redacts_request_and_error_secrets -q— should passurl == "https://api.anthropic.com/v1/messages"andAuthorizationdoes not contain "None".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/A