Skip to content

fix(agent): preserve tool_name on sanitizer-injected stub tool results - #29185

Open
soynchux wants to merge 1 commit into
NousResearch:mainfrom
soynchux:fix/agent-preserve-tool-name-sanitizer-stub
Open

fix(agent): preserve tool_name on sanitizer-injected stub tool results#29185
soynchux wants to merge 1 commit into
NousResearch:mainfrom
soynchux:fix/agent-preserve-tool-name-sanitizer-stub

Conversation

@soynchux

Copy link
Copy Markdown
Contributor

What

Use make_tool_result_message() when _sanitize_api_messages() injects a stub tool result for an assistant tool call whose result is missing.

This keeps sanitizer-generated tool messages consistent with normal tool execution messages by including both:

  • name for provider/wire-format compatibility
  • tool_name for internal session DB and JSON session log persistence

Why

Recent fixes made tool result construction populate tool_name so state.db, JSON session logs, and tool-aware search do not get blank tool names.

The pre-call sanitizer still had one manual stub-message path that only set name, content, and tool_call_id. If that path ran after a missing/dropped tool result, the generated stub could still persist with an empty tool_name.

Testing

Ran targeted tests on Windows with thread-based pytest timeout override:

uv --cache-dir .uv-cache run --extra dev pytest tests/run_agent/test_agent_guardrails.py::TestSanitizeApiMessages -q -n0 --timeout-method=thread --basetemp .pytest-tmp
uv --cache-dir .uv-cache run --extra dev pytest tests/run_agent/test_tool_name_db_persistence.py -q -n0 --timeout-method=thread --basetemp .pytest-tmp

- 7
- 1

PASSED

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 20, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the remaining manual stub construction.

Problems

  • agent/conversation_loop.py:792-905 builds and sanitizes an api_messages copy, while run_agent.py:1692-1695 persists the canonical messages list. The proposed tool_name therefore cannot reach the session DB or JSON log.
  • More importantly, agent/chat_completion_helpers.py:1705-1708 strips tool_name before agent/chat_completion_helpers.py:1728 runs this sanitizer, and then sends the result directly at agent/chat_completion_helpers.py:1852. Replacing the stub with make_tool_result_message() would re-add tool_name; agent/transports/chat_completions.py:159-163 documents that strict providers reject it.

Suggested changes

  • Keep this API-bound stub schema-only, or strip tool_name again after sanitization on direct-request paths. If persistence is the target, trace the canonical message construction path and add an end-to-end persistence regression instead.

Automated hermes-sweeper review.

"content": "[Result unavailable — see context summary above]",
"tool_call_id": cid,
})
patched.append(make_tool_result_message(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sanitize_api_messages() operates on the per-request API copy, not the persisted history. In the iteration-summary path, tool_name is deliberately stripped before this sanitizer and the request then bypasses ChatCompletionsTransport; this helper would add tool_name back and strict providers reject that schema-foreign field. Keep this stub API-schema-only rather than using make_tool_result_message() here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants