Skip to content

Commit 491c8ba

Browse files
Fix:Missing Structured Content for Virtual Server in Streamable HTTP Response (#1412)
* streamblehttp output_schema Signed-off-by: rakdutta <[email protected]> * primitive_types Signed-off-by: rakdutta <[email protected]> * primitive Signed-off-by: rakdutta <[email protected]> * mcp-structure Signed-off-by: rakdutta <[email protected]> * output_schema Signed-off-by: rakdutta <[email protected]> * flake Signed-off-by: rakdutta <[email protected]> * test Signed-off-by: rakdutta <[email protected]> * ruff Signed-off-by: rakdutta <[email protected]> * remove logging Signed-off-by: rakdutta <[email protected]> * invoke_hook Signed-off-by: rakdutta <[email protected]> * plugging response Signed-off-by: rakdutta <[email protected]> * plugging Signed-off-by: rakdutta <[email protected]> * refactor: improve structured content handling and code quality - Change verbose info logging to debug level for tool responses to reduce production log noise while maintaining debug capability - Remove redundant isinstance check in plugin response handling - Add comprehensive docstring explaining structured content return types and MCP SDK behavior in call_tool function - Add test case for structured content validation with tuple returns - Improve code maintainability and documentation clarity These changes enhance code quality without altering functionality, improving pylint score from 9.64 to 9.77. Signed-off-by: Mihai Criveti <[email protected]> --------- Signed-off-by: rakdutta <[email protected]> Signed-off-by: Mihai Criveti <[email protected]> Co-authored-by: Mihai Criveti <[email protected]>
1 parent 2ad0b31 commit 491c8ba

File tree

3 files changed

+113
-13
lines changed

3 files changed

+113
-13
lines changed

mcpgateway/services/tool_service.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,15 +1240,14 @@ async def invoke_tool(self, db: Session, name: str, arguments: Dict[str, Any], r
12401240
# Don't mark as successful for error responses - success remains False
12411241
else:
12421242
result = response.json()
1243+
logger.debug(f"REST API tool response: {result}")
12431244
filtered_response = extract_using_jq(result, tool.jsonpath_filter)
12441245
tool_result = ToolResult(content=[TextContent(type="text", text=json.dumps(filtered_response, indent=2))])
12451246
success = True
1246-
12471247
# If output schema is present, validate and attach structured content
12481248
if getattr(tool, "output_schema", None):
12491249
valid = self._extract_and_validate_structured_content(tool, tool_result, candidate=filtered_response)
12501250
success = bool(valid)
1251-
12521251
elif tool.integration_type == "MCP":
12531252
transport = tool.request_type.lower()
12541253
# gateway = db.execute(select(DbGateway).where(DbGateway.id == tool.gateway_id).where(DbGateway.enabled)).scalar_one_or_none()
@@ -1405,15 +1404,18 @@ async def connect_to_streamablehttp_server(server_url: str, headers: dict = head
14051404
tool_call_result = await connect_to_sse_server(tool_gateway.url, headers=headers)
14061405
elif transport == "streamablehttp":
14071406
tool_call_result = await connect_to_streamablehttp_server(tool_gateway.url, headers=headers)
1408-
content = tool_call_result.model_dump(by_alias=True).get("content", [])
1409-
1407+
dump = tool_call_result.model_dump(by_alias=True)
1408+
logger.debug(f"Tool call result dump: {dump}")
1409+
content = dump.get("content", [])
1410+
# Accept both alias and pythonic names for structured content
1411+
structured = dump.get("structuredContent") or dump.get("structured_content")
14101412
filtered_response = extract_using_jq(content, tool.jsonpath_filter)
1411-
tool_result = ToolResult(content=filtered_response)
1412-
success = True
1413-
# If output schema is present, validate and attach structured content
1414-
if getattr(tool, "output_schema", None):
1415-
valid = self._extract_and_validate_structured_content(tool, tool_result, candidate=filtered_response)
1416-
success = bool(valid)
1413+
1414+
is_err = getattr(tool_call_result, "is_error", None)
1415+
if is_err is None:
1416+
is_err = getattr(tool_call_result, "isError", False)
1417+
tool_result = ToolResult(content=filtered_response, structured_content=structured, is_error=is_err, meta=getattr(tool_call_result, "meta", None))
1418+
logger.debug(f"Final tool_result: {tool_result}")
14171419
else:
14181420
tool_result = ToolResult(content=[TextContent(type="text", text="Invalid tool type")])
14191421

@@ -1431,7 +1433,11 @@ async def connect_to_streamablehttp_server(server_url: str, headers: dict = head
14311433
# Reconstruct ToolResult from modified result
14321434
modified_result = post_result.modified_payload.result
14331435
if isinstance(modified_result, dict) and "content" in modified_result:
1434-
tool_result = ToolResult(content=modified_result["content"])
1436+
# Safely obtain structured content using .get() to avoid KeyError when
1437+
# plugins provide only the content without structured content fields.
1438+
structured = modified_result.get("structuredContent") if "structuredContent" in modified_result else modified_result.get("structured_content")
1439+
1440+
tool_result = ToolResult(content=modified_result["content"], structured_content=structured)
14351441
else:
14361442
# If result is not in expected format, convert it to text content
14371443
tool_result = ToolResult(content=[TextContent(type="text", text=str(modified_result))])

mcpgateway/transports/streamablehttp_transport.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,25 @@ async def call_tool(name: str, arguments: dict) -> List[Union[types.TextContent,
359359
"""
360360
Handles tool invocation via the MCP Server.
361361
362+
This function supports the MCP protocol's tool calling with structured content validation.
363+
It can return either unstructured content only, or both unstructured and structured content
364+
when the tool defines an outputSchema.
365+
362366
Args:
363367
name (str): The name of the tool to invoke.
364368
arguments (dict): A dictionary of arguments to pass to the tool.
365369
366370
Returns:
367-
List of content (TextContent, ImageContent, or EmbeddedResource) from the tool response.
371+
Union[List[ContentBlock], Tuple[List[ContentBlock], Dict[str, Any]]]:
372+
- If structured content is not present: Returns a list of content blocks
373+
(TextContent, ImageContent, or EmbeddedResource)
374+
- If structured content is present: Returns a tuple of (unstructured_content, structured_content)
375+
where structured_content is a dictionary that will be validated against the tool's outputSchema
376+
377+
The MCP SDK's call_tool decorator automatically handles both return types:
378+
- List return → CallToolResult with content only
379+
- Tuple return → CallToolResult with both content and structuredContent fields
380+
368381
Logs and returns an empty list on failure.
369382
370383
Examples:
@@ -389,7 +402,31 @@ async def call_tool(name: str, arguments: dict) -> List[Union[types.TextContent,
389402
logger.warning(f"No content returned by tool: {name}")
390403
return []
391404

392-
return [types.TextContent(type=content.type, text=content.text) for content in result.content]
405+
# Normalize unstructured content to MCP SDK types
406+
unstructured = [types.TextContent(type=content.type, text=content.text) for content in result.content]
407+
408+
# If the tool produced structured content (ToolResult.structured_content / structuredContent),
409+
# return a combination (unstructured, structured) so the server can validate against outputSchema.
410+
# The ToolService may populate structured_content (snake_case) or the model may expose
411+
# an alias 'structuredContent' when dumped via model_dump(by_alias=True).
412+
structured = None
413+
try:
414+
# Prefer attribute if present
415+
structured = getattr(result, "structured_content", None)
416+
except Exception:
417+
structured = None
418+
419+
# Fallback to by-alias dump (in case the result is a pydantic model with alias fields)
420+
if structured is None:
421+
try:
422+
structured = result.model_dump(by_alias=True).get("structuredContent") if hasattr(result, "model_dump") else None
423+
except Exception:
424+
structured = None
425+
426+
if structured:
427+
return (unstructured, structured)
428+
429+
return unstructured
393430
except Exception as e:
394431
logger.exception(f"Error calling tool '{name}': {e}")
395432
return []

tests/unit/mcpgateway/transports/test_streamablehttp_transport.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ async def test_call_tool_success(monkeypatch):
165165
mock_content.type = "text"
166166
mock_content.text = "hello"
167167
mock_result.content = [mock_content]
168+
# Ensure no accidental 'structured_content' MagicMock attribute is present
169+
mock_result.structured_content = None
170+
# Prevent model_dump from returning a MagicMock with a 'structuredContent' key
171+
mock_result.model_dump = lambda by_alias=True: {}
168172

169173
monkeypatch.setattr("mcpgateway.transports.streamablehttp_transport.get_db", AsyncMock(return_value=AsyncMock(__aenter__=AsyncMock(return_value=mock_db), __aexit__=AsyncMock())))
170174
monkeypatch.setattr(tool_service, "invoke_tool", AsyncMock(return_value=mock_result))
@@ -192,6 +196,11 @@ async def test_call_tool_success(monkeypatch):
192196
async def fake_get_db():
193197
yield mock_db
194198

199+
# Ensure no accidental 'structured_content' MagicMock attribute is present
200+
mock_result.structured_content = None
201+
# Prevent model_dump from returning a MagicMock with a 'structuredContent' key
202+
mock_result.model_dump = lambda by_alias=True: {}
203+
195204
monkeypatch.setattr("mcpgateway.transports.streamablehttp_transport.get_db", fake_get_db)
196205
monkeypatch.setattr(tool_service, "invoke_tool", AsyncMock(return_value=mock_result))
197206

@@ -202,6 +211,54 @@ async def fake_get_db():
202211
assert result[0].text == "hello"
203212

204213

214+
@pytest.mark.asyncio
215+
async def test_call_tool_with_structured_content(monkeypatch):
216+
"""Test call_tool returns tuple with both unstructured and structured content."""
217+
# First-Party
218+
from mcpgateway.transports.streamablehttp_transport import call_tool, tool_service, types
219+
220+
mock_db = MagicMock()
221+
mock_result = MagicMock()
222+
mock_content = MagicMock()
223+
mock_content.type = "text"
224+
mock_content.text = '{"result": "success"}'
225+
mock_result.content = [mock_content]
226+
227+
# Simulate structured content being present
228+
mock_structured = {"status": "ok", "data": {"value": 42}}
229+
mock_result.structured_content = mock_structured
230+
mock_result.model_dump = lambda by_alias=True: {
231+
"content": [{"type": "text", "text": '{"result": "success"}'}],
232+
"structuredContent": mock_structured
233+
}
234+
235+
@asynccontextmanager
236+
async def fake_get_db():
237+
yield mock_db
238+
239+
monkeypatch.setattr("mcpgateway.transports.streamablehttp_transport.get_db", fake_get_db)
240+
monkeypatch.setattr(tool_service, "invoke_tool", AsyncMock(return_value=mock_result))
241+
242+
result = await call_tool("mytool", {"foo": "bar"})
243+
244+
# When structured content is present, result should be a tuple
245+
assert isinstance(result, tuple)
246+
assert len(result) == 2
247+
248+
# First element should be the unstructured content list
249+
unstructured, structured = result
250+
assert isinstance(unstructured, list)
251+
assert len(unstructured) == 1
252+
assert isinstance(unstructured[0], types.TextContent)
253+
assert unstructured[0].text == '{"result": "success"}'
254+
255+
# Second element should be the structured content dict
256+
assert isinstance(structured, dict)
257+
assert structured == mock_structured
258+
assert structured["status"] == "ok"
259+
assert structured["data"]["value"] == 42
260+
261+
205262
@pytest.mark.asyncio
206263
async def test_call_tool_no_content(monkeypatch, caplog):
207264
"""Test call_tool returns [] and logs warning if no content."""

0 commit comments

Comments
 (0)