Skip to content

fix(hermes): serialize handle_tool_call returns to JSON string (closes #254)#255

Merged
rohitg00 merged 1 commit intomainfrom
fix/254-hermes-tool-result-json-string
May 9, 2026
Merged

fix(hermes): serialize handle_tool_call returns to JSON string (closes #254)#255
rohitg00 merged 1 commit intomainfrom
fix/254-hermes-tool-result-json-string

Conversation

@rohitg00
Copy link
Copy Markdown
Owner

@rohitg00 rohitg00 commented May 9, 2026

Closes #254.

What

Hermes stores handle_tool_call's return value verbatim as the tool result content in the session history. When Hermes is configured with an Anthropic-protocol LLM backend (provider: anthropic, plus any Anthropic-compatible proxy), the API rejects non-string content with a 400 on the next request:

Failed to deserialize the JSON body into the target type:
messages[N]: content should be a string or a list at line 1 column XXXXX

Once that happens, every subsequent request in the affected session 400s until the session JSON is hand-cleaned — a real cliff.

Fix

Wrap all four return paths in json.dumps:

  • memory_recall (empty + populated branches)
  • memory_save
  • memory_search (empty + populated branches)
  • unknown-tool fallback

Tighten the return-type annotation on both the abstract base and the concrete class from Anystr so a future regression surfaces at type-check time.

Why this is the right contract

agentmemory's own MCP server already returns JSON strings — see src/mcp/standalone.ts:

{ type: "text", text: JSON.stringify(payload, null, pretty ? 2 : 0) }

The Hermes integration is the outlier here, not the rule. After this PR, every agentmemory tool surface (REST, MCP, OpenClaw plugin, OpenCode plugin once #237 lands, Hermes plugin) hands back a string.

Test plan

  • Unit smoke test against an unreachable backend — every handle_tool_call path returns a str that round-trips through json.loads.
  • Verified against agentmemory's existing MCP server contract (src/mcp/standalone.ts) so the Hermes plugin is now consistent with native MCP behavior.
  • Real-world: trigger memory_recall in a Hermes session with provider: anthropic, continue conversation past the tool turn, no 400.

Credit to @KyoMio for the precise root-cause analysis (4 dict returns flagged with before/after diffs) and the Anthropic-protocol-specific repro that pinpointed why the bug is invisible on OpenAI-protocol backends.

Note re: KyoMio's secondary point about session_id in hook signatures

KyoMio also flagged that prefetch / queue_prefetch / sync_turn were missing *, session_id: str = "" for gateway multi-session contexts. That's already addressed by #252 (merged this morning) which added **kwargs: Any to all those hooks plus on_session_end / on_pre_compress / on_memory_write / shutdown. **kwargs is a strict superset of the keyword-only session_id proposal — accepts that key plus any other future Hermes-passed context kwargs without further patches.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced consistency in how memory tool results are stored and managed within session history.

Review Change Stack

…#254)

Hermes stores handle_tool_call's return value as the tool result
`content` field in the session history. Anthropic-protocol LLM providers
reject non-string content on the next request with:

  Failed to deserialize the JSON body into the target type:
  messages[N]: content should be a string or a list at line 1 column XXXXX

Once triggered, every subsequent request in that session 400s until the
session JSON is manually cleaned.

The plugin's handle_tool_call returned raw Python dicts in all 4 paths
(memory_recall, memory_save, memory_search, unknown-tool fallback). Wrap
each return in json.dumps so the result is always a string.

This matches the contract that agentmemory's own MCP server already
honors in src/mcp/standalone.ts:

  { type: "text", text: JSON.stringify(payload, null, ...) }

Tightens the return-type annotation on both the abstract base and the
concrete class from Any -> str so mypy / type checkers catch any future
regression.

Reported by @KyoMio with full Anthropic-protocol repro.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentmemory Ready Ready Preview, Comment May 9, 2026 10:47am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f2a9a347-e717-4bc4-9554-a5594c2f39a3

📥 Commits

Reviewing files that changed from the base of the PR and between c2f2618 and 2959230.

📒 Files selected for processing (1)
  • integrations/hermes/__init__.py

📝 Walkthrough

Walkthrough

The PR updates the handle_tool_call method contract and implementation to ensure tool results are returned as JSON-serialized strings instead of raw Python dicts. The base class abstract method signature changes return type from Any to str, and all four tool handlers (memory_recall, memory_save, memory_search, error case) now serialize their results via json.dumps().

Changes

Tool Result String Serialization

Layer / File(s) Summary
Contract Signature Update
integrations/hermes/__init__.py
MemoryProvider.handle_tool_call return type annotation changes from Any to str.
Tool Handler Implementation
integrations/hermes/__init__.py
All tool handlers (memory_recall, memory_save, memory_search) and error case now serialize results to JSON strings using json.dumps(). Default fallback values are also JSON-serialized.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through dicts turned strings,
JSON wrapping all the things—
Tool calls now serialize with care,
No more dicts floating in the air! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: serializing handle_tool_call returns to JSON strings and references the closed issue #254.
Linked Issues check ✅ Passed All primary coding requirements from #254 are met: handle_tool_call returns JSON strings via json.dumps, return-type annotations changed from Any to str for both abstract and concrete classes.
Out of Scope Changes check ✅ Passed Changes are focused on the stated objectives: serializing handle_tool_call results and tightening type annotations. No extraneous modifications detected outside the scope of issue #254.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/254-hermes-tool-result-json-string

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rohitg00 rohitg00 merged commit eaecc65 into main May 9, 2026
5 checks passed
rohitg00 added a commit that referenced this pull request May 9, 2026
Bug-fix patch focused on search recall correctness and plugin
compatibility. Pins iii-engine to v0.11.2 because v0.11.6 introduces
a new sandbox-everything-via-`iii worker add` model that agentmemory
hasn't been refactored for yet — pin lifts once that refactor lands.
Adds a hard guard against silent vector-index corruption, fixes BM25
indexing for memories saved via memory_save, and lands four Hermes
plugin fixes.

Per AGENTS.md release checklist:
- package.json version 0.9.4 -> 0.9.5
- src/version.ts VERSION constant
- src/types.ts ExportData version union
- src/functions/export-import.ts supportedVersions Set
- test/export-import.test.ts assertion
- plugin/.claude-plugin/plugin.json version
- CHANGELOG.md detailed entries with contributor shoutouts

Headlines (full detail in CHANGELOG):

Fixed:
- BM25 search now indexes memories saved via memory_save (#258, #257)
  Thanks @Nizar-BenHamida for the precise repro.
- Embedding providers no longer silently corrupt the vector index when
  an API returns wrong-dimension vectors (#248, #247, #256)
  Thanks @AmmarSaleh50 for issue + fix + tests.
- Hermes handle_tool_call returns JSON strings, not raw dicts (#255, #254)
  Thanks @KyoMio for the Anthropic-protocol repro.
- Hermes status reflects real service state on systemd installs (#253, #250)
  Thanks @OptionalCoin for tracing it to env-source divergence.
- Hermes hooks accept passthrough kwargs (#252, #249)
  Thanks @OptionalCoin again for the log analysis.
- agentmemory demo now seeds observations correctly (#251, #229)
  Thanks @seishonagon for root-cause analysis.
- LLM compression / summarization timeouts increased (#213)
  Thanks @xuli500177.
- Pi / OpenClaw / Hermes integration plugin fixes (#230)
  Thanks @deepmroot.

Changed:
- iii-engine pinned to v0.11.2 across every install path (#260).
  v0.11.6 introduces a new `iii worker add` sandbox model that
  agentmemory still pre-dates; pin lifts when we refactor agentmemory
  to register as a sandboxed worker. Override with
  AGENTMEMORY_III_VERSION=<version> for users who've migrated manually.
- README documents iii worker add extension surface (#242).
- README iii Console install/launch commands corrected (#243).

Validated: 852/852 tests pass, npm run build clean.
rohitg00 added a commit that referenced this pull request May 9, 2026
Bug-fix patch focused on search recall correctness and plugin
compatibility. Pins iii-engine to v0.11.2 because v0.11.6 introduces
a new sandbox-everything-via-`iii worker add` model that agentmemory
hasn't been refactored for yet — pin lifts once that refactor lands.
Adds a hard guard against silent vector-index corruption, fixes BM25
indexing for memories saved via memory_save, and lands four Hermes
plugin fixes.

Per AGENTS.md release checklist:
- package.json version 0.9.4 -> 0.9.5
- src/version.ts VERSION constant
- src/types.ts ExportData version union
- src/functions/export-import.ts supportedVersions Set
- test/export-import.test.ts assertion
- plugin/.claude-plugin/plugin.json version
- CHANGELOG.md detailed entries with contributor shoutouts

Headlines (full detail in CHANGELOG):

Fixed:
- BM25 search now indexes memories saved via memory_save (#258, #257)
  Thanks @Nizar-BenHamida for the precise repro.
- Embedding providers no longer silently corrupt the vector index when
  an API returns wrong-dimension vectors (#248, #247, #256)
  Thanks @AmmarSaleh50 for issue + fix + tests.
- Hermes handle_tool_call returns JSON strings, not raw dicts (#255, #254)
  Thanks @KyoMio for the Anthropic-protocol repro.
- Hermes status reflects real service state on systemd installs (#253, #250)
  Thanks @OptionalCoin for tracing it to env-source divergence.
- Hermes hooks accept passthrough kwargs (#252, #249)
  Thanks @OptionalCoin again for the log analysis.
- agentmemory demo now seeds observations correctly (#251, #229)
  Thanks @seishonagon for root-cause analysis.
- LLM compression / summarization timeouts increased (#213)
  Thanks @xuli500177.
- Pi / OpenClaw / Hermes integration plugin fixes (#230)
  Thanks @deepmroot.

Changed:
- iii-engine pinned to v0.11.2 across every install path (#260).
  v0.11.6 introduces a new `iii worker add` sandbox model that
  agentmemory still pre-dates; pin lifts when we refactor agentmemory
  to register as a sandboxed worker. Override with
  AGENTMEMORY_III_VERSION=<version> for users who've migrated manually.
- README documents iii worker add extension surface (#242).
- README iii Console install/launch commands corrected (#243).

Validated: 852/852 tests pass, npm run build clean.
rohitg00 added a commit that referenced this pull request May 9, 2026
Bug-fix patch focused on search recall correctness and plugin
compatibility. Pins iii-engine to v0.11.2 because v0.11.6 introduces
a new sandbox-everything-via-`iii worker add` model that agentmemory
hasn't been refactored for yet — pin lifts once that refactor lands.
Adds a hard guard against silent vector-index corruption, fixes BM25
indexing for memories saved via memory_save, and lands four Hermes
plugin fixes.

Per AGENTS.md release checklist:
- package.json version 0.9.4 -> 0.9.5
- src/version.ts VERSION constant
- src/types.ts ExportData version union
- src/functions/export-import.ts supportedVersions Set
- test/export-import.test.ts assertion
- plugin/.claude-plugin/plugin.json version
- CHANGELOG.md detailed entries with contributor shoutouts

Headlines (full detail in CHANGELOG):

Fixed:
- BM25 search now indexes memories saved via memory_save (#258, #257)
  Thanks @Nizar-BenHamida for the precise repro.
- Embedding providers no longer silently corrupt the vector index when
  an API returns wrong-dimension vectors (#248, #247, #256)
  Thanks @AmmarSaleh50 for issue + fix + tests.
- Hermes handle_tool_call returns JSON strings, not raw dicts (#255, #254)
  Thanks @KyoMio for the Anthropic-protocol repro.
- Hermes status reflects real service state on systemd installs (#253, #250)
  Thanks @OptionalCoin for tracing it to env-source divergence.
- Hermes hooks accept passthrough kwargs (#252, #249)
  Thanks @OptionalCoin again for the log analysis.
- agentmemory demo now seeds observations correctly (#251, #229)
  Thanks @seishonagon for root-cause analysis.
- LLM compression / summarization timeouts increased (#213)
  Thanks @xuli500177.
- Pi / OpenClaw / Hermes integration plugin fixes (#230)
  Thanks @deepmroot.

Changed:
- iii-engine pinned to v0.11.2 across every install path (#260).
  v0.11.6 introduces a new `iii worker add` sandbox model that
  agentmemory still pre-dates; pin lifts when we refactor agentmemory
  to register as a sandboxed worker. Override with
  AGENTMEMORY_III_VERSION=<version> for users who've migrated manually.
- README documents iii worker add extension surface (#242).
- README iii Console install/launch commands corrected (#243).

Validated: 852/852 tests pass, npm run build clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool result serialization: handle_tool_call should return JSON string, not dict

1 participant