added validation and debug for invalid call tool result

[lifeizhou-ap]

Summary

Root Cause
When the route llm_search tool was called, it return the empty content in the call_tool_result payload, and saved into session.

payload:

[{"type":"toolResponse","id":"tooluse_EhzKFCKORv6dLIKcEtOKxQ","toolResult":{"status":"success","value":{"content":[],"isError":false}}}]

When loading the session and deserialized call_tool_result via rmcp, it returns error as the call_tool_result because of result.content.is_empty() && result.structured_content.is_none(). This is based on the mcp standard that it should either have at least one content or structured_content.

Fix

• Validated call_tool_result and set it to errorData if it is invalid. This will avoid set invalid call_tool_result in the message and session
• Added debug the tool result data when deserialising tool_result fails to make it easy to debug.

Type of Change

• Feature
• Bug fix
• Refactor / Code quality
• Performance improvement
• Documentation
• Tests
• Security fix
• Build / Release
• Other (specify below)

AI Assistance

• This PR was created or reviewed with AI assistance

Testing

Unit test and manual testing

Related Issues

#6345

[Copilot]

Pull request overview

This PR addresses an issue where invalid CallToolResult objects with empty content were being saved to sessions, causing deserialization failures when loading. The fix adds validation to convert invalid tool results into error responses and improves debugging with additional logging.

Key changes:

• Added validate() function to detect invalid CallToolResult objects and convert them to errors before saving
• Enhanced deserialization error handling with debug logging that includes the original data
• Applied validation in the agent tool execution flow to prevent invalid results from being stored

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/goose/src/conversation/tool_result_serde.rs	Added validation function and debug logging for deserialization failures; includes comprehensive test coverage
crates/goose/src/conversation/mod.rs	Changed module visibility from private to public to expose validation function
crates/goose/src/agents/agent.rs	Integrated validation into tool result processing pipeline

[Copilot]

This validation performs an unnecessary serialize->deserialize round-trip. Since CallToolResult is from the rmcp crate which performs validation during deserialization, you can validate directly by checking if content is empty and structured_content is None, matching the MCP standard constraint mentioned in the PR description.

Suggested change

	Ok(call_tool_result) => match serde_json::to_string(call_tool_result) {
	Ok(json_str) => match serde_json::from_str::<CallToolResult>(&json_str) {
	Ok(_) => result,
	Err(e) => {
	tracing::error!("CallToolResult failed validation by deserialization: {}. Original data: {}", e, json_str);
	Err(ErrorData {
	code: ErrorCode::INTERNAL_ERROR,
	message: Cow::from(format!("Tool result validation failed: {}", e)),
	data: None,
	})
	}
	},
	Err(e) => {
	tracing::error!("CallToolResult failed serialization: {}", e);
	Err(ErrorData {
	code: ErrorCode::INTERNAL_ERROR,
	message: Cow::from(format!("Tool result serialization failed: {}", e)),
	data: None,
	})
	}
	},
	Ok(call_tool_result) => {
	let has_empty_content = call_tool_result.content.is_empty();
	let has_no_structured_content = call_tool_result.structured_content.is_none();
	if has_empty_content && has_no_structured_content {
	tracing::error!(
	"CallToolResult failed validation: content must not be empty when structured_content is None"
	);
	Err(ErrorData {
	code: ErrorCode::INTERNAL_ERROR,
	message: Cow::from(
	"Tool result validation failed: content must not be empty when structured_content is None",
	),
	data: None,
	})
	} else {
	result
	}
	}

[lifeizhou-ap]

we could, but it introduced duplicated logic in goose and rmcp. So it would be good to reuse the deserialization function in rmcp to use it as source of truth

[Copilot]

Function name has grammatical error: should be "returns_error" not "return_error".

Suggested change

	fn test_validate_return_error_for_invalid_calltoolresult() {
	fn test_validate_returns_error_for_invalid_calltoolresult() {

[michaelneale]

nice one - would be great if you could make this a branch and then it runs through the test_providers.sh end to end etc with live providers

[lifeizhou-ap]

nice one - would be great if you could make this a branch and then it runs through the test_providers.sh end to end etc with live providers

Thanks for the review! The empty content is only returned by router_llm_search tool and most of the tools return valid result . It is hard to create the test data for smoke test. Therefore we use unit test to test this scenario

[lifeizhou-ap]

Hi @alexhancock,

For the fix with the validation of CallToolResult, do you think it is worthwhile to fix in the rmcp? Either to check during CallToolResult serialization or in the constructor of CallToolResult. I feel check during serialization might be safer