Remove duplicated content when emitting structured payloads

[kargnas]

Summary

• stop returning redundant text content when array results are converted into structuredContent
• update ToolResponse to only include content when explicit text is provided
• adjust HTTP tool test expectations for the slimmer structured response

Testing

• vendor/bin/pest

---

https://chatgpt.com/codex/tasks/task_e_68eb5338a82883219302d152f503261f

[claude]

Code Review: Remove duplicated content when emitting structured payloads

Summary

This PR successfully addresses the redundancy issue where structured content was being duplicated as both JSON text in content and as structuredContent. The changes are well-implemented and align with the MCP 2025-06-18 specification.

---

Code Quality and Best Practices

Strengths:

• Clean refactoring: The ToolResponse class now properly encapsulates the logic for conditionally including content
• Good use of constructor parameters: The includeContent flag is a clear, explicit way to control behavior
• Property type declarations: Added proper @var annotations for content and metadata properties (lines 12-20 in ToolResponse.php)
• Backward compatibility: The structured() method allows explicit content when needed via the content parameter
• Simplified handler logic: Removed 9 lines of redundant code from ToolsCallHandler (lines 94-102)

Minor observations:

• Property placement: The private bool includeContent declaration (line 26) appears between the PHPDoc and constructor. While valid PHP, slightly unconventional. Consider moving it with other property declarations.

---

Potential Bugs or Issues

No critical bugs found, but two observations:

1. Unused variable in ToolsCallHandler (line 85): The result of json_encode() is no longer assigned. This is intentional (validation-only), but consider adding a comment to clarify the intent.

2. Logic in structured() method (line 94): The condition contentItems !== [] correctly ensures includeContent=true only when explicit content is provided. The constructor logic provides double-protection against empty content arrays. This is defensive and good.

---

Performance Considerations

Positive impacts:

• Reduced payload size: Removing the duplicated JSON text from content reduces response size, especially for large structured responses
• Removed unnecessary JSON encoding: The previous code encoded the result twice - once for the text field and implicitly again for structuredContent. Now it only validates encoding once
• Memory efficiency: Smaller response payloads mean less memory allocation and faster network transmission

No performance regressions identified.

---

Security Concerns

No security issues found.

• JSON encoding still uses JSON_THROW_ON_ERROR and JSON_UNESCAPED_UNICODE flags for validation
• Error handling remains intact with proper JsonRpcErrorException wrapping
• No user input validation changes
• No authentication/authorization changes

---

Test Coverage

Test updates are appropriate:

1. StreamableHttpTest.php (lines 41-43):
   • Changed from expecting 2 content items to 1 item
   • Removed assertion checking the second JSON-encoded text item
   • Still validates structuredContent is present and correct

Recommendations:
Consider adding explicit test cases for:

1. Edge case: Tool returning ToolResponse::structured() with null content (already supported, but untested)
2. Edge case: Tool returning ToolResponse::structured() with empty array content
3. Regression test: Ensure existing tools using ToolResponse::text() and ToolResponse::make() still include content

---

Migration Impact

Breaking change considerations:

This change could affect:

• External tools that rely on the duplicated content in structured responses
• Clients that parse the JSON text instead of structuredContent

However, this is the correct behavior per MCP spec. Clients should be using structuredContent for structured data, not parsing JSON from content text.

Recommendation:

• Consider documenting this change in CHANGELOG.md as a breaking change
• Update any migration guide if users have custom tools using the old pattern

---

Overall Assessment

Excellent PR!

Score: 9/10

The implementation is clean, correct, and improves both efficiency and spec compliance. The only minor suggestions are:

1. Reorder the includeContent property declaration
2. Add a clarifying comment about the validation-only JSON encoding
3. Consider adding the suggested edge case tests

Recommendation: Approve with optional minor improvements

Great work on improving the MCP server implementation!