Skip to content

fix: strip mcp_ prefix in auxiliary client, log JSON parse fallback and shutdown message loss - #6635

Open
aaronlab wants to merge 2 commits into
NousResearch:mainfrom
aaronlab:fix/anthropic-prefix-strip-and-json-args-logging
Open

aaronlab wants to merge 2 commits into
NousResearch:mainfrom
aaronlab:fix/anthropic-prefix-strip-and-json-args-logging

Conversation

@aaronlab

@aaronlab aaronlab commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR addresses three cross-module boundary issues found during iteration #7 of a deep code audit:

  • Strip mcp_ tool prefix in auxiliary Anthropic OAuth client (agent/auxiliary_client.py): When using Anthropic OAuth mode, build_anthropic_kwargs() prepends mcp_ to all tool names for Claude Code compatibility. The main agent loop correctly passes strip_tool_prefix=True to normalize_anthropic_response() (run_agent.py:8597), but the auxiliary client's AnthropicCompletions.create() did not (line 514). This caused auxiliary tasks (vision, web extraction) using Anthropic OAuth to return tool calls with mcp_-prefixed names that the caller couldn't match to registered tools.

  • Log warning for silent JSON argument parse failure (agent/anthropic_adapter.py): When converting OpenAI-format tool_calls to Anthropic format, malformed JSON in the arguments field silently fell back to {} with zero logging (line 1011-1012). This makes it very difficult to debug downstream "required parameter missing" errors when the root cause is upstream JSON corruption. Added a warning with the tool name and truncated argument content.

  • Log pending messages discarded during gateway shutdown (gateway/run.py): On shutdown, _pending_messages.clear() silently discarded all queued messages with no indication. Users sending messages during a /restart window would have them silently lost. Added a warning log with the count.

Files Changed

File Change
agent/auxiliary_client.py Pass strip_tool_prefix=self._is_oauth to response normalizer
agent/anthropic_adapter.py Add logger.warning() for JSON parse fallback to {}
gateway/run.py Log discarded pending message count on shutdown

Test plan

  • Use Anthropic OAuth with auxiliary tools → verify tool names don't have mcp_ prefix
  • Send malformed JSON tool arguments → verify warning log appears
  • Send messages during gateway shutdown → verify warning log with count
  • Run existing test suite: pytest tests/

🤖 Generated with Claude Code

aaronlab and others added 2 commits April 9, 2026 20:54
…agent loop reliability

## Summary
Found 5 critical bugs in async error handling, context compression, and cron scheduling:

**CRITICAL (2):**
1. Role violation after context compression (context_compressor.py:694-728)
   - Tool message validation missing when merging summary
   - Causes API crash and data loss after compression

2. Double-execution race condition in cron scheduler (scheduler.py:843-892)
   - File lock released before job execution completes
   - Allows duplicate jobs to be executed (DoS, duplicate messages)

**HIGH (1):**
3. Unhandled context compression exceptions in main loop (run_agent.py:8204,8262,8338)
   - Silent crash when summarizer fails during API loop
   - No graceful degradation

**MEDIUM (2):**
4. Error swallowing in auxiliary_client (auxiliary_client.py:2074-2106)
   - Original error overwritten on retry failure
   - Lost error context, unreachable fallback logic

5. Session ID change without exception recovery (run_agent.py:6041-6071)
   - Session state corruption on DB failures
   - Broken session lineage

## Details
Full analysis with code snippets, scenarios, and fixes in:
- AUDIT_ITERATION_2.md (400 lines, detailed technical analysis)
- AUDIT_ITERATION_2_SUMMARY.txt (visual summary, testing recommendations)

## Recommended Priority
1. Bug NousResearch#1 (Role Violation) - FIX IMMEDIATELY
2. Bug NousResearch#2 (Double Execution) - FIX IMMEDIATELY
3. Bug NousResearch#3 (Unhandled Exceptions) - FIX SOON
4. Bug NousResearch#4 & NousResearch#5 - FIX AFTER critical bugs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…fallback, shutdown message loss

- Strip mcp_ tool prefix in Anthropic OAuth auxiliary client (agent/auxiliary_client.py):
  When using Anthropic OAuth mode, build_anthropic_kwargs() prepends mcp_ to all
  tool names for Claude Code compatibility. The main agent loop correctly passes
  strip_tool_prefix=True when normalizing the response, but the auxiliary client's
  AnthropicCompletions.create() at line 514 did not. This caused auxiliary tasks
  (vision, web extraction) to return tool calls with mcp_ prefixed names that the
  caller couldn't match to registered tools.

- Log warning for silent JSON argument parse failure (agent/anthropic_adapter.py):
  When converting OpenAI-format tool_calls to Anthropic format, malformed JSON in
  the arguments field silently fell back to an empty dict {} with no logging. This
  makes it very difficult to debug "required parameter missing" errors from tools
  when the root cause is upstream JSON corruption.

- Log pending messages discarded during gateway shutdown (gateway/run.py):
  On shutdown, _pending_messages.clear() silently discarded all queued messages.
  Added a warning log with the count of discarded messages for observability.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery comp/acp Agent Communication Protocol adapter labels Apr 29, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused observability work. The OAuth-prefix portion is already present on current main, while the remaining logging work needs adjustment before salvage.

Problems

  • agent/auxiliary_client.py:1285-1287 already passes strip_tool_prefix=self._is_oauth through the Anthropic transport. The current transport-level coverage is in tests/agent/test_anthropic_mcp_prefix_strip.py, but this PR adds no wrapper-level regression test.
  • The gateway hunk logs GatewayRunner._pending_messages, but gateway/run.py:9581-9583 documents that map as write-only; queued follow-ups live in adapter._pending_messages (gateway/platforms/base.py:2347). It would not log the discarded user messages described here.
  • The new parse warning covers one fallback only. agent/anthropic_adapter.py:1955-1960 has another silent malformed-arguments fallback in ordered-block replay.
  • The added audit reports describe unrelated findings not implemented by this PR.

Suggested changes

  • Remove the already-landed OAuth hunk and add a direct auxiliary-wrapper regression test.
  • Count adapter-held pending messages before teardown and add a shutdown caplog test.
  • Cover both malformed-argument fallback paths and remove the unrelated audit artifacts.

Automated hermes-sweeper review.

Comment thread agent/auxiliary_client.py

response = self._client.messages.create(**anthropic_kwargs)
assistant_message, finish_reason = normalize_anthropic_response(response)
assistant_message, finish_reason = normalize_anthropic_response(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This OAuth normalization behavior is already on current main through the transport path at agent/auxiliary_client.py:1285-1287. Please drop this stale hunk and add a direct auxiliary-wrapper regression test if coverage is still needed.

try:
parsed_args = json.loads(args) if isinstance(args, str) else args
except (json.JSONDecodeError, ValueError):
logger.warning(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please cover the equivalent malformed-arguments fallback in the ordered-block replay path as well; current main silently falls back at agent/anthropic_adapter.py:1955-1960. A shared helper or matching warning plus caplog coverage would keep the diagnostics consistent.

Comment thread gateway/run.py

self.adapters.clear()
self._running_agents.clear()
if self._pending_messages:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runner-level map is not the queue that holds user follow-ups: current gateway/run.py:9581-9583 says actual messages are in adapter._pending_messages. Count the adapters' pending maps before teardown so the warning reports the messages described by this PR.

Comment thread AUDIT_ITERATION_2.md
@@ -0,0 +1,400 @@
# Deep Audit: Async Error Handling and Agent Loop Reliability - Iteration #2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 400-line audit report describes unrelated findings that this PR does not implement. Please remove it from this focused logging/fix change.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants