fix(gateway): preserve tool_call_id in api_server conversation history#17422
fix(gateway): preserve tool_call_id in api_server conversation history#17422inkccc wants to merge 1 commit into
Conversation
The API server was stripping tool_call_id, tool_calls, and name fields when reconstructing conversation history. This caused strict providers (xiaomimimo, stepfun) to return HTTP 400 'tool_call_id is required' for WebUI sessions with tool calls. Fix by preserving all message fields in the three conversation_history.append locations (lines 1747, 2409, 2435). Evidence: - Error log: 'tool_call_id is not set' in ~/.hermes/logs/errors.log - Session analysis: 7/7 tool messages missing tool_call_id in WebUI session - Comparison: CLI sessions work (platform=cli), WebUI sessions fail (platform=api_server) - Both use same provider (mimo-v2.5-pro), confirming API server issue Fixes NousResearch#16800
There was a problem hiding this comment.
Pull request overview
Fixes API server conversation history reconstruction so tool-related fields aren’t dropped, preventing strict providers from rejecting follow-up WebUI requests that include prior tool turns.
Changes:
- Preserve
tool_call_id,tool_calls, andnamewhen rebuildingconversation_historyfor/v1/responses. - Preserve the same fields for
/v1/runs, including the “input-as-history” fallback path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if entry.get("tool_call_id"): | ||
| msg["tool_call_id"] = entry["tool_call_id"] | ||
| # Preserve tool_calls for assistant messages | ||
| if entry.get("tool_calls"): | ||
| msg["tool_calls"] = entry["tool_calls"] | ||
| if entry.get("name"): |
There was a problem hiding this comment.
Now that conversation_history can pass through tool-related fields, these values should be type-validated (e.g., tool_call_id/name as strings, tool_calls as a list) before forwarding to the agent. Without validation, a malformed client payload (tool_calls as dict/string) can lead to incorrect downstream behavior or server errors; consider returning 400 when the optional fields are present but of the wrong type. Also, if the intent is to preserve fields, checking key presence (vs truthiness) avoids silently dropping empty values supplied by the client.
| if entry.get("tool_call_id"): | |
| msg["tool_call_id"] = entry["tool_call_id"] | |
| # Preserve tool_calls for assistant messages | |
| if entry.get("tool_calls"): | |
| msg["tool_calls"] = entry["tool_calls"] | |
| if entry.get("name"): | |
| if "tool_call_id" in entry: | |
| if not isinstance(entry["tool_call_id"], str): | |
| return web.json_response( | |
| _openai_error(f"conversation_history[{i}].tool_call_id must be a string"), | |
| status=400, | |
| ) | |
| msg["tool_call_id"] = entry["tool_call_id"] | |
| # Preserve tool_calls for assistant messages | |
| if "tool_calls" in entry: | |
| if not isinstance(entry["tool_calls"], list): | |
| return web.json_response( | |
| _openai_error(f"conversation_history[{i}].tool_calls must be an array"), | |
| status=400, | |
| ) | |
| msg["tool_calls"] = entry["tool_calls"] | |
| if "name" in entry: | |
| if not isinstance(entry["name"], str): | |
| return web.json_response( | |
| _openai_error(f"conversation_history[{i}].name must be a string"), | |
| status=400, | |
| ) |
| msg = {"role": str(entry["role"]), "content": str(entry["content"])} | ||
| # Preserve tool_call_id for tool messages — required by strict providers | ||
| if entry.get("tool_call_id"): | ||
| msg["tool_call_id"] = entry["tool_call_id"] | ||
| # Preserve tool_calls for assistant messages | ||
| if entry.get("tool_calls"): | ||
| msg["tool_calls"] = entry["tool_calls"] | ||
| if entry.get("name"): | ||
| msg["name"] = entry["name"] | ||
| conversation_history.append(msg) |
There was a problem hiding this comment.
In this endpoint, conversation_history is declared as List[Dict[str, str]], but the newly preserved fields (e.g., tool_calls as list/dict, tool_call_id/name) make the dict values non-string. Updating the type annotation to Dict[str, Any] (and ideally validating the optional fields) will prevent misleading typing and make it clearer what shapes agent.run_conversation can receive here.
| msg_out = {"role": msg["role"], "content": str(content)} | ||
| # Preserve tool_call_id for tool messages — required by strict providers | ||
| if msg.get("tool_call_id"): | ||
| msg_out["tool_call_id"] = msg["tool_call_id"] | ||
| # Preserve tool_calls for assistant messages | ||
| if msg.get("tool_calls"): | ||
| msg_out["tool_calls"] = msg["tool_calls"] | ||
| if msg.get("name"): | ||
| msg_out["name"] = msg["name"] | ||
| conversation_history.append(msg_out) |
There was a problem hiding this comment.
The raw_input multi-message fallback only includes prior messages when msg.get("content") is truthy, which will drop common assistant tool-call messages that have content=None/missing but do have tool_calls. That means even with the new preservation logic, tool_calls-only assistant messages won’t make it into conversation_history, potentially leaving subsequent tool results orphaned (and still triggering strict-provider validation errors). Consider relaxing the guard to accept messages with role plus (content OR tool_calls OR tool_call_id) and then normalizing content to "" when absent.
| msg = {"role": str(entry["role"]), "content": entry_content} | ||
| # Preserve tool_call_id for tool messages — required by strict providers | ||
| if entry.get("tool_call_id"): | ||
| msg["tool_call_id"] = entry["tool_call_id"] | ||
| # Preserve tool_calls for assistant messages | ||
| if entry.get("tool_calls"): | ||
| msg["tool_calls"] = entry["tool_calls"] | ||
| if entry.get("name"): | ||
| msg["name"] = entry["name"] | ||
| conversation_history.append(msg) |
There was a problem hiding this comment.
Add a regression test that posts conversation_history containing (1) an assistant message with tool_calls (often content=None/missing) and (2) a tool message with tool_call_id, and assert the adapter forwards both fields unchanged. This would have caught the original stripping bug and will prevent future regressions across both /v1/responses and /v1/runs paths.
alt-glitch
added
type/bug
What does this PR do?
Fix a bug where
api_server.pystripstool_call_id,tool_calls, andnamefields when reconstructing conversation history. This causes strict providers (xiaomimimo, stepfun) to return HTTP 400 "tool_call_id is required" for API clients that send conversation history with tool messages.Related Issue
Fixes #16800
Type of Change
Changes Made
gateway/platforms/api_server.py: Preservetool_call_id,tool_calls, andnamefields in conversation history reconstruction at three locations (lines 1747, 2409, 2435)Root Cause
The API server's
/v1/runsand/v1/responseshandlers reconstruct conversation history by only keepingroleandcontentfields, discardingtool_call_id,tool_calls, andname. When API clients (including third-party WebUIs like hermes-web-ui) send follow-up messages with full conversation history, strict providers reject the request because tool messages lack the requiredtool_call_idfield.The gateway's
run.pyalready has correct logic to preserve these fields (lines 10737-10743), butapi_server.pywas missing this handling when rebuilding conversation history from client requests.Impact
conversation_historyvia/v1/runsor/v1/responsesgateway/run.py)Evidence
Error Log
Session Analysis
eph_mojx6dbl_itkoil(platform=api_server) — 7/7 tool messages missingtool_call_id20260429_183619_6f3307(platform=cli) — 0/13 tool messages missingComparison
Both sessions use the same provider (
mimo-v2.5-pro), confirming this is an API server issue, not a provider issue.How to Test
Checklist
Code
pytest tests/ -qand all tests pass (no dedicated api_server tests exist)Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture — or N/A