fix(agent): JSON-serialize non-string tool results to prevent API 400 - #29920
fix(agent): JSON-serialize non-string tool results to prevent API 400#29920hermesagent26 wants to merge 1 commit into
Conversation
Non-multimodal tool results (especially from MCP tools and memory_search/
memory_recall) return Python dicts/lists. The OpenAI SDK rejects these
with HTTP 400 'invalid message content type: map[string]interface{}'.
Two layers of defence:
1. run_agent.py:_tool_result_content_for_active_model — serializes any
non-str result before appending to messages.
2. agent_runtime_helpers.py:sanitize_api_messages — coerces tool role
content to JSON string as a last safety-net before API call.
Closes the 'model provider failed after retries' loop caused by tool
result dicts slipping into message history.
d251ea1 to
19b8370
Compare
…esearch#29920) Two-layer fix for `HTTP 400: invalid message content type: map[string]interface{}`. 1. `_tool_result_content_for_active_model` in run_agent.py — serializes non-string, non-list results (Python dicts/lists from MCP tools or memory helpers) as JSON before appending to messages. Falls back to repr() on serialization failure. 2. `sanitize_api_messages` in agent_runtime_helpers.py — coerces tool role `content` to JSON string as a safety-net before every API call. Catches any tool results that bypass the first layer (e.g. from session restore or manual message manipulation). Fixes the 'model provider failed after retries' loop caused by a single bad tool result poisoning the entire message history.
… sessions Constraint: current main no longer has make_tool_result_message, so the fix had to land on the shared outbound sanitizer and tool-content boundary that still exist. Rejected: Reapply NousResearch#31770 verbatim | target helper is absent on current main Rejected: Reapply NousResearch#29920 verbatim | would stringify valid multimodal content-part lists Confidence: high Scope-risk: narrow Directive: Keep role=tool content coercion centralized and preserve wire-valid content-part lists when tightening future sanitizers. Tested: scripts/run_tests.sh tests/run_agent/test_agent_guardrails.py tests/tools/test_computer_use.py -q Not-tested: Full suite; live provider round-trip against strict upstreams
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real strict-provider failure: current run_agent.py:5086-5087 still returns a plain non-multimodal dict unchanged, and agent/agent_runtime_helpers.py:2462-2657 has no content-type repair despite being used by both outbound paths.
Problems
agent/agent_runtime_helpers.py:1678JSON-encodes every non-string tool content value. This also encodes valid content-part lists:run_agent.py:5089-5117deliberately returns such lists for multimodal tool results, so the added safety net would discard their structured vision payload.run_agent.py:3665-3668changes Kimi/Ollama reasoning detection but is unrelated to tool-result serialization. Currentrun_agent.py:5505-5516documents host-driven Kimi detection to avoid model-name routing; please keep this PR scoped to the serialization fix.- The PR changes only runtime files; add regression tests for a fresh dict tool result and persisted dict tool content through
sanitize_api_messages.
Suggested changes
- Preserve strings and valid content-part lists; serialize only invalid tool-content shapes, with explicit
Nonebehavior. - Cover both the construction and pre-API sanitizer paths with focused tests.
Automated hermes-sweeper review.
| # receives a dict/list in ``content`` (which OpenAI rejects with 400). | ||
| for msg in messages: | ||
| if msg.get("role") == "tool": | ||
| content = msg.get("content") |
There was a problem hiding this comment.
This blanket check also serializes valid OpenAI content-part lists. _tool_result_content_for_active_model intentionally returns such a list for multimodal results, so the final sanitizer would turn a vision payload into JSON text. Preserve valid lists and coerce only invalid tool-content shapes.
| @@ -3658,6 +3665,10 @@ def _needs_kimi_tool_reasoning(self) -> bool: | |||
| or base_url_host_matches(self.base_url, "api.kimi.com") | |||
There was a problem hiding this comment.
This Kimi/Ollama reasoning change is unrelated to the stated serialization fix. Please split it out: current main documents Kimi detection as host-driven specifically to avoid model-name routing (run_agent.py:5505-5516).
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the strict-provider HTTP 400 caused by non-string tool-result content: #29920 applies the fix at result creation and outbound sanitization, while #29937 implements similar serialization but bundles several unrelated provider, context-length, and skills-sync changes. Both sanitizers currently JSON-encode all non-string tool content, including valid structured content-part lists, so neither diff is merge-ready as written.
Related pull requests
- #29920
related— (+19/-0) — keep open and revise: The focused two-layer fix addresses fresh and persisted dict/list tool results, but the contributor keep_open review correctly identifies that the sanitizer also destroys valid multimodal content-part lists; remove the unrelated Kimi/Ollama routing change and add regression tests for fresh dict results, persisted invalid content, and preserved valid content-part lists. - #29937 [closed]
duplicate— (+106/-14) — keep closed as a superseded duplicate: Its core serialization and sanitizer changes overlap #29920 and have the same valid-content-part-list defect, while the diff additionally bundles unrelated llama.cpp context-length, system-message, custom-provider, and skills-sync changes; it remains relevant as an alternative implementation and evidence that the fix should stay narrowly scoped.
Duplicates
#29920 and #29937 substantially duplicate the same two-layer tool-result serialization change; #29937 is the broader, closed duplicate.
Suggested consolidation
Revise and then merge #29920 as the focused consolidation target: preserve strings and valid content-part lists, serialize only invalid tool-content shapes, remove the unrelated Kimi/Ollama change, and add the requested regression coverage. Keep #29937 closed as the duplicate because its relevant fix is covered by #29920 and its remaining changes belong in separate PRs.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup29920 ["PRs duplicating each other"]
P29920["PR #29920 (open)"]
P29937["PR #29937 (closed)"]
end
class P29920 open
class P29937 closed
class P29920 target
click P29920 "https://github.com/NousResearch/hermes-agent/pull/29920"
click P29937 "https://github.com/NousResearch/hermes-agent/pull/29937"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 2 kB of issue/PR text, 2 kB of discussion (3 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Two-layer fix for
HTTP 400: invalid message content type: map[string]interface{}.Root cause: MCP tools and memory helpers return Python dicts/lists as tool results. The OpenAI SDK expects tool role
contentto be a string or content-parts list, not raw objects.Changes:
run_agent.py:_tool_result_content_for_active_model— serializes non-str results before appending to messages.agent_runtime_helpers.py:sanitize_api_messages— coerces tool rolecontentto JSON string as a safety-net before every API call.Fixes the 'model provider failed after retries' loop caused by a single bad tool result poisoning the entire message history.