fix: serialize nested dict tool output to prevent TypeError (#6267)#6332
fix: serialize nested dict tool output to prevent TypeError (#6267)#6332C1-BA-B1-F3 wants to merge 1 commit into
Conversation
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR changes tool output formatting to JSON-serialize dict/list results and displays task output before human feedback when verbose logging is disabled; no exploitable security vulnerabilities were identified.
Risk: Low risk. The changes do not introduce new authentication, authorization, external request, file path, SQL, or command execution surfaces, and the added behavior only affects internal output formatting/display.
📝 WalkthroughWalkthroughTool output formatting now serializes dict and list results to JSON in both structured and native tool paths, with string fallback for non-serializable values. The tests were updated to cover nested dicts, lists, and custom object fallbacks. ChangesTool result JSON formatting
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/crewai/tests/tools/test_structured_tool.py (1)
527-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a fallback serialization test case.
Please add one test where
json.dumps(..., default=str)still fails (e.g., unsupported dict key type) and assert fallback tostr(...). That will cover the new exception branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/tools/test_structured_tool.py` around lines 527 - 582, The new structured tool output tests cover JSON serialization for dicts, lists, and strings, but they do not exercise the fallback path when json.dumps(..., default=str) still fails. Add a test in test_structured_tool.py around the existing CrewStructuredTool.format_output_for_agent cases that returns a dict with an unsupported key type so serialization raises, then assert the fallback to str(...) is used for the result. Use the existing test helpers and CrewStructuredTool.from_function/invoke/format_output_for_agent flow to locate the right branch.lib/crewai/src/crewai/tools/tool_usage.py (1)
714-718: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct tests for
_format_resultdict/list serialization path.This branch changed behavior but isn’t directly covered by the new tests (which only exercise
CrewStructuredTool.format_output_for_agent). Please add a focused unit test for_format_resultwith dict/list inputs (including fallback cases) to lock this path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/tools/tool_usage.py` around lines 714 - 718, Add focused unit tests directly for the `_format_result` branch in `tool_usage.py` that handles `dict` and `list` inputs, rather than only testing `CrewStructuredTool.format_output_for_agent`. Cover both the successful `json.dumps(..., ensure_ascii=False, default=str)` serialization path and the fallback `str(result)` path when serialization raises `TypeError` or `ValueError`, using `_format_result` as the target symbol so the behavior is locked down independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/agents/crew_agent_executor.py`:
- Around line 244-247: The async feedback flow in CrewAgentExecutor still skips
the pre-feedback result display when verbose is false, unlike the sync invoke
path. Mirror the same visibility check and call to _show_logs(formatted_answer)
inside ainvoke before any feedback prompt, using the existing verbose condition
on self.agent.verbose and self.crew.verbose so both invoke and ainvoke behave
consistently.
---
Nitpick comments:
In `@lib/crewai/src/crewai/tools/tool_usage.py`:
- Around line 714-718: Add focused unit tests directly for the `_format_result`
branch in `tool_usage.py` that handles `dict` and `list` inputs, rather than
only testing `CrewStructuredTool.format_output_for_agent`. Cover both the
successful `json.dumps(..., ensure_ascii=False, default=str)` serialization path
and the fallback `str(result)` path when serialization raises `TypeError` or
`ValueError`, using `_format_result` as the target symbol so the behavior is
locked down independently.
In `@lib/crewai/tests/tools/test_structured_tool.py`:
- Around line 527-582: The new structured tool output tests cover JSON
serialization for dicts, lists, and strings, but they do not exercise the
fallback path when json.dumps(..., default=str) still fails. Add a test in
test_structured_tool.py around the existing
CrewStructuredTool.format_output_for_agent cases that returns a dict with an
unsupported key type so serialization raises, then assert the fallback to
str(...) is used for the result. Use the existing test helpers and
CrewStructuredTool.from_function/invoke/format_output_for_agent flow to locate
the right branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6556fb63-a45c-4ae8-b336-02589106aeab
📒 Files selected for processing (4)
lib/crewai/src/crewai/agents/crew_agent_executor.pylib/crewai/src/crewai/tools/structured_tool.pylib/crewai/src/crewai/tools/tool_usage.pylib/crewai/tests/tools/test_structured_tool.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai/tests/tools/test_structured_tool.py (1)
602-604: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the fallback contract directly.
assert "value" in formattedstill passes for several non-fallback outputs, so it doesn't actually lock in thestr(raw_result)branch this test is named after. Comparing againststr(result)or asserting thatjson.loads(formatted)fails would make this regression-proof.Suggested test tightening
result = tool.invoke(input={"query": "test"}) formatted = tool.format_output_for_agent(result) assert isinstance(formatted, str) - assert "value" in formatted + assert formatted == str(result)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/tools/test_structured_tool.py` around lines 602 - 604, The structured tool fallback test is too permissive and does not verify the actual fallback path in format_output_for_agent. Tighten the assertion in test_structured_tool by comparing the formatted result directly to str(result), or by explicitly asserting that JSON parsing fails for this output, so the test uniquely covers the raw string fallback branch instead of any output containing "value".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/tests/tools/test_structured_tool.py`:
- Around line 584-640: The new structured-tool tests cover output serialization,
but ToolUsage._format_result is a separate formatting path that still needs
coverage. Add a companion unit test in test_tool_usage.py that exercises
ToolUsage._format_result with a tool result that is a dict or list and verifies
the returned value is valid JSON. Use the existing ToolUsage test setup/helpers
in test_tool_usage.py to locate the right path and ensure the assertion checks
the serialized output, not just argument handling.
---
Nitpick comments:
In `@lib/crewai/tests/tools/test_structured_tool.py`:
- Around line 602-604: The structured tool fallback test is too permissive and
does not verify the actual fallback path in format_output_for_agent. Tighten the
assertion in test_structured_tool by comparing the formatted result directly to
str(result), or by explicitly asserting that JSON parsing fails for this output,
so the test uniquely covers the raw string fallback branch instead of any output
containing "value".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0caf6bd9-0242-4fcf-ba68-761e682f72d2
📒 Files selected for processing (2)
lib/crewai/src/crewai/agents/crew_agent_executor.pylib/crewai/tests/tools/test_structured_tool.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/crewai/src/crewai/agents/crew_agent_executor.py
| def test_dict_with_unserializable_key_falls_back_to_str(self): | ||
| """Dict with non-string keys that json.dumps(default=str) can't handle falls back to str().""" | ||
| from crewai.tools.structured_tool import CrewStructuredTool | ||
|
|
||
| class UnhashableKey: | ||
| """A key type that json.dumps cannot serialize even with default=str.""" | ||
| def __str__(self): | ||
| raise RuntimeError("cannot stringify") | ||
|
|
||
| def bad_key_tool(query: str) -> dict: | ||
| return {UnhashableKey(): "value"} | ||
|
|
||
| tool = CrewStructuredTool.from_function( | ||
| func=bad_key_tool, | ||
| name="bad_key_tool", | ||
| description="Returns a dict with unhashable keys.", | ||
| ) | ||
| result = tool.invoke(input={"query": "test"}) | ||
| formatted = tool.format_output_for_agent(result) | ||
| assert isinstance(formatted, str) | ||
| assert "value" in formatted | ||
|
|
||
| def test_dict_with_non_string_keys_serialized(self): | ||
| """Dict with int keys should be JSON-serialized with default=str.""" | ||
| import json | ||
| from crewai.tools.structured_tool import CrewStructuredTool | ||
|
|
||
| def int_key_tool(query: str) -> dict: | ||
| return {1: "one", 2: "two"} | ||
|
|
||
| tool = CrewStructuredTool.from_function( | ||
| func=int_key_tool, | ||
| name="int_key_tool", | ||
| description="Returns a dict with int keys.", | ||
| ) | ||
| result = tool.invoke(input={"query": "test"}) | ||
| formatted = tool.format_output_for_agent(result) | ||
| parsed = json.loads(formatted) | ||
| assert parsed["1"] == "one" | ||
|
|
||
| def test_nested_list_output_serialized(self): | ||
| """Deeply nested list should be JSON-serialized.""" | ||
| import json | ||
| from crewai.tools.structured_tool import CrewStructuredTool | ||
|
|
||
| def nested_list_tool(query: str) -> list: | ||
| return [[{"a": 1}], [{"b": 2}]] | ||
|
|
||
| tool = CrewStructuredTool.from_function( | ||
| func=nested_list_tool, | ||
| name="nested_list_tool", | ||
| description="Returns nested lists.", | ||
| ) | ||
| result = tool.invoke(input={"query": "test"}) | ||
| formatted = tool.format_output_for_agent(result) | ||
| parsed = json.loads(formatted) | ||
| assert parsed[0][0]["a"] == 1 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ToolUsage serialization implementation:"
sed -n '709,720p' lib/crewai/src/crewai/tools/tool_usage.py
echo
echo "Tests that appear to cover ToolUsage/_format_result:"
rg -n -C2 --type=py '\b_format_result\s*\(|\bToolUsage\b' lib/crewai/tests
echo
echo "Structured tool serialization tests in this PR:"
rg -n -C2 --type=py '\bformat_output_for_agent\s*\(' lib/crewai/tests/tools/test_structured_tool.pyRepository: crewAIInc/crewAI
Length of output: 14295
Add companion unit test for ToolUsage._format_result
The new tests in test_structured_tool.py confirm CrewStructuredTool.format_output_for_agent handles serialization correctly. However, ToolUsage._format_result in lib/crewai/src/crewai/tools/tool_usage.py is a distinct code path responsible for formatting tool results during agent execution. The existing test_tool_usage.py focuses on input validation and argument parsing, lacking a test that asserts the output is JSON-serialized when a tool returns a dict or list.
Please add a test in lib/crewai/tests/tools/test_tool_usage.py that calls ToolUsage._format_result with a dict or list and asserts the return value is valid JSON.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai/tests/tools/test_structured_tool.py` around lines 584 - 640, The
new structured-tool tests cover output serialization, but
ToolUsage._format_result is a separate formatting path that still needs
coverage. Add a companion unit test in test_tool_usage.py that exercises
ToolUsage._format_result with a tool result that is a dict or list and verifies
the returned value is valid JSON. Use the existing ToolUsage test setup/helpers
in test_tool_usage.py to locate the right path and ensure the assertion checks
the serialized output, not just argument handling.
|
Nice fix! The JSON serialization with One thought: the same serialization logic is duplicated in both |
When a custom tool returns a dict or list, the output was being converted using str() which produces Python repr format (e.g., single quotes, True/False). This is not valid JSON and can cause parsing errors in the LLM. Now uses json.dumps() for dict/list types, falling back to str() for non-serializable types. Fixes crewAIInc#6267
C1-BA-B1-F3
force-pushed
the
fix/nested-dict-tool-output
branch
from
124c1bf to
20f90a3
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/tools/structured_tool.py (1)
73-79: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the warning text to match the new fallback.
Line 78 still says this path falls back to
str(raw_result), but Line 83 now uses_serialize_to_json_string(raw_result). That warning will be misleading when debugging validation failures.Proposed fix
warnings.warn( ( f"Failed to validate or serialize output from tool " f"'{getattr(tool, 'name', '<unknown>')}' using result_schema " f"'{result_schema.__name__}': {exc.__class__.__name__}. " - "Falling back to str(raw_result)." + "Falling back to JSON serialization when possible, otherwise str(raw_result)." ), RuntimeWarning, stacklevel=2, )Also applies to: 83-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/tools/structured_tool.py` around lines 73 - 79, The warning message in the structured tool fallback path is out of sync with the actual behavior. Update the warning text in structured_tool.py near the validation/serialization failure handling so it references the JSON serialization fallback used by the code path (through _serialize_to_json_string in the structured tool logic) instead of saying it falls back to str(raw_result). Keep the message aligned with the tool name and result_schema context so debugging remains accurate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/crewai/src/crewai/tools/structured_tool.py`:
- Around line 73-79: The warning message in the structured tool fallback path is
out of sync with the actual behavior. Update the warning text in
structured_tool.py near the validation/serialization failure handling so it
references the JSON serialization fallback used by the code path (through
_serialize_to_json_string in the structured tool logic) instead of saying it
falls back to str(raw_result). Keep the message aligned with the tool name and
result_schema context so debugging remains accurate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c2c6a02c-ccd4-47b7-965a-98c72f9976ee
📒 Files selected for processing (4)
lib/crewai/src/crewai/tools/structured_tool.pylib/crewai/src/crewai/utilities/agent_utils.pylib/crewai/tests/tools/test_base_tool.pylib/crewai/tests/tools/test_structured_tool.py
|
@itxaiohanglover Thanks for the kind words and the thoughtful suggestion! You're right that the serialization logic is duplicated — extracting it to a shared helper (e.g. in Appreciate the review! |
2 participants
Summary
When a custom tool returns a nested dictionary, the agent's execution loop crashes with a
TypeError(e.g.,unhashable type: 'dict') becausestr()produces a Python repr instead of valid JSON.This fix adds
json.dumps()serialization fordictandlistoutputs in both:_format_tool_output_for_agent()instructured_tool.py_format_result()intool_usage.pyAlso fixes #6072 by showing task output before prompting for human feedback when
verbose=False.Changes
lib/crewai/src/crewai/tools/structured_tool.py: JSON serialize dict/list in_format_tool_output_for_agentlib/crewai/src/crewai/tools/tool_usage.py: JSON serialize dict/list in_format_resultlib/crewai/src/crewai/agents/crew_agent_executor.py: Show output before human feedback promptlib/crewai/tests/tools/test_structured_tool.py: Add tests for dict/list/string tool outputsTest plan
Closes #6267
Closes #6072
Summary by CodeRabbit