Skip to content

fix: preserve MCP embedded resource text - #29962

Closed
haha0815 wants to merge 1 commit into
NousResearch:mainfrom
haha0815:fix/mcp-embedded-resource-text
Closed

fix: preserve MCP embedded resource text#29962
haha0815 wants to merge 1 commit into
NousResearch:mainfrom
haha0815:fix/mcp-embedded-resource-text

Conversation

@haha0815

Copy link
Copy Markdown

Summary

  • Preserve text from MCP EmbeddedResource.resource.text blocks when rendering tool results.
  • Add regression coverage for embedded resource text in structured MCP output.

Tests

  • ./venv/bin/python3 -m pytest tests/tools/test_mcp_structured_content.py tests/tools/test_mcp_image_content.py tests/tools/test_mcp_tool_session_expired.py -q → 34 passed

Notes

  • Fix was verified in a fresh Hermes process for QMD get returning the expected document text instead of an empty payload.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels May 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of open PR #13736 which implements the same EmbeddedResource text extraction fix. Please coordinate with that PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The premise is confirmed on current main: tools/mcp_tool.py:3950 and tools/mcp_tool.py:3971 only consume direct block.text, so EmbeddedResource.resource.text is currently discarded. The proposed helper correctly applies the text extraction to both the normal and isError paths.

Suggested changes

This is an automated hermes-sweeper review.

@teknium1

Copy link
Copy Markdown
Contributor

This bug is now fixed on main via PR #64556 (salvaged from #64061). Your PR was the earliest report and fix for the dropped EmbeddedResource text — submitted May 21, well before the others — and that priority is acknowledged. We merged the #64061 implementation because it additionally materializes binary blob resources (PDFs etc.) to the document cache, handles ResourceLink and Audio blocks, and ships size caps + hostile-input tests. Thanks for catching this first!

@teknium1 teknium1 closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants