From 17ba4a42999356d15b09203e13e34dc98cea6c3d Mon Sep 17 00:00:00 2001 From: Joao Moura Date: Wed, 29 Jul 2026 09:27:03 -0700 Subject: [PATCH] fix(tools): keep a failed tool out of the final answer, finish crew scope Three more findings, all confirmed against the code. A failed `result_as_answer` tool still became the task's output. The native paths already excluded raised errors and hook blocks from short-circuiting, but not declared failures -- so an error message silently became the answer, which is the exact shape of bug this PR exists to prevent. Fixed on all paths, and there were three independent override points, not one: `ToolResult.result_as_answer` in tool_utils, the `execution_result` finality checks in both executors, and `process_tool_results()`, which reads `agent.tools_results` back separately. The first two fixes alone left the behavior unchanged; only the third made the test pass. `ToolUsage` never received a crew, so a crew-level `ignore` half-applied: recording and `ToolFailureDetectedEvent` stayed quiet, but the flag was still attached to `ToolUsageFinishedEvent`. It now takes and stores `crew`. `CrewAgentExecutor.invoke`/`ainvoke` routed a deliberate stop through `handle_unknown_error`, printing "An unknown error occurred" on verbose runs. LiteAgent already special-cased this; both now do. Testing: 5 further tests, 79 total, including that a *successful* `result_as_answer` tool still short-circuits. Full suite matches baseline exactly at 377 pre-existing failures; mypy clean on every changed file. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ETacm2dMASfpMAYUiDu5YG --- .../src/crewai/agents/crew_agent_executor.py | 9 ++ .../src/crewai/experimental/agent_executor.py | 5 + lib/crewai/src/crewai/tools/tool_usage.py | 10 ++ .../src/crewai/utilities/agent_utils.py | 3 + lib/crewai/src/crewai/utilities/tool_utils.py | 10 +- lib/crewai/tests/tools/test_tool_failure.py | 113 ++++++++++++++++++ 6 files changed, 148 insertions(+), 2 deletions(-) diff --git a/lib/crewai/src/crewai/agents/crew_agent_executor.py b/lib/crewai/src/crewai/agents/crew_agent_executor.py index 6a29f4253..729409ef6 100644 --- a/lib/crewai/src/crewai/agents/crew_agent_executor.py +++ b/lib/crewai/src/crewai/agents/crew_agent_executor.py @@ -245,6 +245,9 @@ class CrewAgentExecutor(BaseAgentExecutor): color="red", ) raise + except ToolExecutionFailedError: + # A deliberate stop, not an unknown error. + raise except Exception as e: handle_unknown_error(PRINTER, e, verbose=self.agent.verbose) raise @@ -1097,6 +1100,7 @@ class CrewAgentExecutor(BaseAgentExecutor): "result": result, "from_cache": from_cache, "original_tool": original_tool, + "tool_failure": tool_failure, } def _append_tool_result_and_check_finality( @@ -1127,6 +1131,8 @@ class CrewAgentExecutor(BaseAgentExecutor): original_tool and hasattr(original_tool, "result_as_answer") and original_tool.result_as_answer + # A failed tool must not become the final answer. + and execution_result.get("tool_failure") is None ): return AgentFinish( thought="Tool result is the final answer", @@ -1166,6 +1172,9 @@ class CrewAgentExecutor(BaseAgentExecutor): color="red", ) raise + except ToolExecutionFailedError: + # A deliberate stop, not an unknown error. + raise except Exception as e: handle_unknown_error(PRINTER, e, verbose=self.agent.verbose) raise diff --git a/lib/crewai/src/crewai/experimental/agent_executor.py b/lib/crewai/src/crewai/experimental/agent_executor.py index 4f1051a7e..9ad771a0c 100644 --- a/lib/crewai/src/crewai/experimental/agent_executor.py +++ b/lib/crewai/src/crewai/experimental/agent_executor.py @@ -1817,6 +1817,8 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor): original_tool and hasattr(original_tool, "result_as_answer") and original_tool.result_as_answer + # A failed tool must not become the final answer. + and execution_result.get("tool_failure") is None ): self.state.current_answer = AgentFinish( thought="Tool result is the final answer", @@ -1855,6 +1857,8 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor): original_tool and hasattr(original_tool, "result_as_answer") and original_tool.result_as_answer + # A failed tool must not become the final answer. + and execution_result.get("tool_failure") is None ): # Set the result as the final answer self.state.current_answer = AgentFinish( @@ -2124,6 +2128,7 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor): "result": result, "from_cache": from_cache, "original_tool": original_tool, + "tool_failure": tool_failure, } @staticmethod diff --git a/lib/crewai/src/crewai/tools/tool_usage.py b/lib/crewai/src/crewai/tools/tool_usage.py index b7a70f0ba..fa9fb69a7 100644 --- a/lib/crewai/src/crewai/tools/tool_usage.py +++ b/lib/crewai/src/crewai/tools/tool_usage.py @@ -43,6 +43,7 @@ from crewai.utilities.string_utils import sanitize_tool_name if TYPE_CHECKING: from crewai.agents.agent_builder.base_agent import BaseAgent from crewai.agents.tools_handler import ToolsHandler + from crewai.crew import Crew from crewai.lite_agent import LiteAgent from crewai.llm import LLM from crewai.task import Task @@ -102,6 +103,7 @@ class ToolUsage: agent: BaseAgent | LiteAgent | None = None, action: Any = None, fingerprint_context: dict[str, str] | None = None, + crew: Crew | None = None, ) -> None: self._telemetry: Telemetry = Telemetry() self._run_attempts: int = 1 @@ -113,6 +115,7 @@ class ToolUsage: self.tools_handler = tools_handler self.tools = tools self.task = task + self.crew = crew self.action = action self.function_calling_llm = function_calling_llm self.fingerprint_context = fingerprint_context or {} @@ -403,6 +406,9 @@ class ToolUsage: if ( hasattr(available_tool, "result_as_answer") and available_tool.result_as_answer + # A failed tool must not become the final answer; + # process_tool_results() reads this back independently. + and self.last_failure is None ): result_as_answer = available_tool.result_as_answer data["result_as_answer"] = result_as_answer @@ -651,6 +657,9 @@ class ToolUsage: if ( hasattr(available_tool, "result_as_answer") and available_tool.result_as_answer + # A failed tool must not become the final answer; + # process_tool_results() reads this back independently. + and self.last_failure is None ): result_as_answer = available_tool.result_as_answer data["result_as_answer"] = result_as_answer @@ -1023,6 +1032,7 @@ class ToolUsage: tool=tool, agent=self.agent, task=self.task, + crew=self.crew, ), } ) diff --git a/lib/crewai/src/crewai/utilities/agent_utils.py b/lib/crewai/src/crewai/utilities/agent_utils.py index b59a69fb4..7e526d376 100644 --- a/lib/crewai/src/crewai/utilities/agent_utils.py +++ b/lib/crewai/src/crewai/utilities/agent_utils.py @@ -1802,6 +1802,9 @@ def execute_single_native_tool_call( and original_tool.result_as_answer and not error_event_emitted and not hook_blocked + # A declared failure is excluded for the same reason a raised one is: + # an error must not silently become the task's answer. + and tool_failure is None ) return NativeToolCallResult( diff --git a/lib/crewai/src/crewai/utilities/tool_utils.py b/lib/crewai/src/crewai/utilities/tool_utils.py index 280b5de00..755df4d25 100644 --- a/lib/crewai/src/crewai/utilities/tool_utils.py +++ b/lib/crewai/src/crewai/utilities/tool_utils.py @@ -86,6 +86,7 @@ async def aexecute_tool_and_check_finality( task=task, agent=agent, action=agent_action, + crew=crew, ) tool_calling = tool_usage.parse_tool_calling(agent_action.text) @@ -159,7 +160,9 @@ async def aexecute_tool_and_check_finality( return ToolResult( modified_result if modified_result is not None else tool_result, - tool.result_as_answer, + # A failed tool must not become the final answer -- the same + # exclusion the native paths already apply to raised errors. + tool.result_as_answer and tool_usage.last_failure is None, ) tool_result = I18N_DEFAULT.errors("wrong_tool_name").format( @@ -233,6 +236,7 @@ def execute_tool_and_check_finality( task=task, agent=agent, action=agent_action, + crew=crew, ) tool_calling = tool_usage.parse_tool_calling(agent_action.text) @@ -306,7 +310,9 @@ def execute_tool_and_check_finality( return ToolResult( modified_result if modified_result is not None else tool_result, - tool.result_as_answer, + # A failed tool must not become the final answer -- the same + # exclusion the native paths already apply to raised errors. + tool.result_as_answer and tool_usage.last_failure is None, ) tool_result = I18N_DEFAULT.errors("wrong_tool_name").format( diff --git a/lib/crewai/tests/tools/test_tool_failure.py b/lib/crewai/tests/tools/test_tool_failure.py index 6eb24c85f..39e590ec6 100644 --- a/lib/crewai/tests/tools/test_tool_failure.py +++ b/lib/crewai/tests/tools/test_tool_failure.py @@ -1079,6 +1079,119 @@ class TestHookBlockDoesNotInheritCachedFailure: assert agent.last_tool_failures == [] +class TestCrewScopeReachesTheFinishedEvent: + """`ToolUsage` needs the crew, or crew-level ignore only half applies.""" + + def test_crew_ignore_suppresses_the_finished_event_flag(self) -> None: + agent = Agent( + role="Slack Messenger", + goal="post a message", + backstory="b", + llm=ScriptedLLM(_slack_steps()), + tools=[SlackTool()], + ) + task = Task(description="post to slack", expected_output="c", agent=agent) + crew = Crew( + agents=[agent], + tasks=[task], + tool_failure_policy=ToolFailurePolicy.IGNORE, + ) + + finished: list[ToolUsageFinishedEvent] = [] + with crewai_event_bus.scoped_handlers(): + + @crewai_event_bus.on(ToolUsageFinishedEvent) + def _(source: Any, event: ToolUsageFinishedEvent) -> None: + finished.append(event) + + crew.kickoff() + crewai_event_bus.flush(timeout=10.0) + + slack = [e for e in finished if e.tool_name == "slackbot_send_message"] + assert slack + assert all(e.failure is None for e in slack) + + def test_tool_usage_accepts_and_stores_crew(self) -> None: + from crewai.tools.tool_usage import ToolUsage + + agent = Agent(role="r", goal="g", backstory="b") + crew = Crew(agents=[agent], tasks=[]) + usage = ToolUsage( + tools_handler=None, + tools=[], + task=None, + function_calling_llm=None, # type: ignore[arg-type] + agent=agent, + crew=crew, + ) + assert usage.crew is crew + + +class TestFailedToolIsNotTheFinalAnswer: + """result_as_answer must not turn an error into the task's output.""" + + @staticmethod + def _agent(policy: ToolFailurePolicy) -> Agent: + class AnswerSlack(SlackTool): + result_as_answer: bool = True + + return Agent( + role="Slack Messenger", + goal="post a message", + backstory="b", + llm=ScriptedLLM(_slack_steps()), + tools=[AnswerSlack()], + tool_failure_policy=policy, + ) + + def test_failure_does_not_short_circuit_under_warn(self) -> None: + agent = self._agent(ToolFailurePolicy.WARN) + task = Task(description="post to slack", expected_output="c", agent=agent) + result = Crew(agents=[agent], tasks=[task]).kickoff() + + assert "Slack rejected the message" not in result.raw + assert result.raw == "I could not post the message." + assert result.has_tool_failures + + def test_successful_result_as_answer_still_short_circuits(self) -> None: + class AnswerEcho(WorkingTool): + result_as_answer: bool = True + + agent = Agent( + role="Echoer", + goal="echo", + backstory="b", + llm=ScriptedLLM( + [ + 'Thought: go\nAction: echo\nAction Input: {"text": "hi"}', + "Thought: done\nFinal Answer: unused.", + ] + ), + tools=[AnswerEcho()], + ) + task = Task(description="echo", expected_output="c", agent=agent) + result = Crew(agents=[agent], tasks=[task]).kickoff() + + assert result.raw == "echoed: hi" + assert not result.has_tool_failures + + +class TestDeliberateStopIsNotAnUnknownError: + def test_executor_loops_do_not_route_it_to_handle_unknown_error(self) -> None: + """Verbose runs must not print 'An unknown error occurred' for a stop.""" + import inspect + + from crewai.agents.crew_agent_executor import CrewAgentExecutor + + for func in (CrewAgentExecutor.invoke, CrewAgentExecutor.ainvoke): + source = inspect.getsource(func) + if "handle_unknown_error" not in source: + continue + assert "ToolExecutionFailedError" in source, ( + f"{func.__qualname__} would report a deliberate stop as unknown" + ) + + class TestMCPIsErrorPlumbing: """An MCP server flags a failed tool with isError on a 200 response."""