Skip to content

Commit e184333

Browse files
joaomdmouraclaude
andcommitted
fix(tools): address review round 1 on tool-failure signalling
Five real defects from Bugbot, none of them cosmetic. Tool-scoped policy never applied (high). `resolve_tool_failure_policy` read `tool_failure_policy` off the object handed to it, but every execution path passes the `CrewStructuredTool` wrapper, which never carried the attribute -- and `BaseTool` never declared it in the first place. A tool-scoped `raise`/`ignore` was silently ignored while the docs and a unit test claimed otherwise; the test passed only because it called the resolver directly with an authored tool. Declared the field on `BaseTool`, propagated it through `to_structured_tool()` and `CrewStructuredTool`, and made resolution fall back through `_original_tool` so either shape works. A failed call still printed the green "Completed" panel, then the red one. That is the terminal version of the exact bug this PR is about. Suppressed the success panel when the call reported failure. A raised tool printed twice: `ToolUsageErrorEvent` already renders a red panel, and the new failure panel repeated it. The event is still emitted -- policy and traces need it -- but the duplicate console output is gone. Both decisions now live in named predicates on `ConsoleFormatter` rather than inline in the listener closure, so they are directly testable. Unknown tools were reported on the ReAct path but silently ignored on all three native paths, so the same miss was loud or silent depending on executor style. Native paths now record `UNKNOWN_TOOL` too. This also surfaced a live `NameError`: ruff had pruned `ToolFailureReason` from `agent_utils` as unused, so the new branch would have crashed at runtime. `LiteAgentOutput` had `tool_failures` but not `has_tool_failures`, which the PR promised on all three output types -- an `AttributeError` for any caller sharing one check across result types. Testing: 16 further tests, 45 total. Two console tests were passing vacuously because `emit()` dispatches sync handlers on a thread pool, so the assertions raced the handler; they now assert on the predicates directly, and the native-path test drains the bus with `flush()` and checks the synchronously-written record. Full suite still matches baseline exactly at 377 pre-existing failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ETacm2dMASfpMAYUiDu5YG
1 parent 279c57f commit e184333

10 files changed

Lines changed: 339 additions & 9 deletions

File tree

lib/crewai/src/crewai/agents/crew_agent_executor.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,16 @@ def _execute_single_native_tool_call(
10301030
),
10311031
)
10321032
error_event_emitted = True
1033+
elif not from_cache:
1034+
# Not cached and not executable: the model asked for a tool that
1035+
# does not exist. The ReAct path reports this as a failure, so the
1036+
# native paths must too, or the same miss is silent on one and
1037+
# loud on the other.
1038+
tool_failure = ToolFailure(
1039+
message=result,
1040+
reason=ToolFailureReason.UNKNOWN_TOOL,
1041+
code=func_name,
1042+
)
10331043

10341044
after_hook_context = ToolCallHookContext(
10351045
tool_name=func_name,

lib/crewai/src/crewai/events/event_listener.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,8 @@ def on_tool_usage_started(source: Any, event: ToolUsageStartedEvent) -> None:
415415

416416
@crewai_event_bus.on(ToolUsageFinishedEvent)
417417
def on_tool_usage_finished(source: Any, event: ToolUsageFinishedEvent) -> None:
418+
if not self.formatter.should_render_success_panel(event.failure):
419+
return
418420
if isinstance(source, LLM):
419421
self.formatter.handle_llm_tool_usage_finished(
420422
event.tool_name,
@@ -444,6 +446,8 @@ def on_tool_usage_error(source: Any, event: ToolUsageErrorEvent) -> None:
444446
def on_tool_failure_detected(
445447
source: Any, event: ToolFailureDetectedEvent
446448
) -> None:
449+
if not self.formatter.should_render_failure_panel(event.failure):
450+
return
447451
self.formatter.handle_tool_failure_detected(
448452
event.tool_name,
449453
event.failure,

lib/crewai/src/crewai/events/utils/console_formatter.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from rich.panel import Panel
1313
from rich.text import Text
1414

15+
from crewai.tools.tool_failure import ToolFailureReason
1516
from crewai.version import is_current_version_yanked, is_newer_version_available
1617

1718

@@ -492,6 +493,26 @@ def handle_tool_usage_finished(
492493
content, f"✅ Tool Execution Completed (#{iteration})", "green"
493494
)
494495

496+
@staticmethod
497+
def should_render_success_panel(failure: Any) -> bool:
498+
"""Whether a finished tool call should print the green panel.
499+
500+
A call that reported failure must not read as successful, so the
501+
green panel is suppressed and the red one takes its place.
502+
"""
503+
return failure is None
504+
505+
@staticmethod
506+
def should_render_failure_panel(failure: Any) -> bool:
507+
"""Whether a reported failure should print its own red panel.
508+
509+
A tool that *raised* already produced a ``ToolUsageErrorEvent`` and
510+
its own red panel, so printing a second one for the same exception is
511+
pure noise. The event itself is still emitted -- only the duplicate
512+
console output is skipped.
513+
"""
514+
return getattr(failure, "reason", None) is not ToolFailureReason.EXCEPTION
515+
495516
def handle_tool_failure_detected(
496517
self,
497518
tool_name: str,

lib/crewai/src/crewai/experimental/agent_executor.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,6 +2046,8 @@ def _execute_single_native_tool_call(self, tool_call: Any) -> dict[str, Any]:
20462046
),
20472047
)
20482048
error_event_emitted = True
2049+
else:
2050+
tool_failure = self._unknown_tool_failure(func_name, result)
20492051
elif max_usage_reached:
20502052
# Return error message when max usage limit is reached
20512053
if original_tool:
@@ -2056,6 +2058,8 @@ def _execute_single_native_tool_call(self, tool_call: Any) -> dict[str, Any]:
20562058
tool_failure = ToolFailure(
20572059
message=result, reason=ToolFailureReason.USAGE_LIMIT
20582060
)
2061+
elif not from_cache:
2062+
tool_failure = self._unknown_tool_failure(func_name, result)
20592063

20602064
# Execute after_tool_call hooks (even if blocked, to allow logging/monitoring)
20612065
after_hook_context = ToolCallHookContext(
@@ -2109,6 +2113,19 @@ def _execute_single_native_tool_call(self, tool_call: Any) -> dict[str, Any]:
21092113
"original_tool": original_tool,
21102114
}
21112115

2116+
@staticmethod
2117+
def _unknown_tool_failure(func_name: str, result: str) -> ToolFailure:
2118+
"""Build the failure for a tool the model asked for but we do not have.
2119+
2120+
The ReAct path reports this as a failure, so the native path must too,
2121+
or the same miss is silent on one and loud on the other.
2122+
"""
2123+
return ToolFailure(
2124+
message=result,
2125+
reason=ToolFailureReason.UNKNOWN_TOOL,
2126+
code=func_name,
2127+
)
2128+
21122129
def _extract_tool_name(self, tool_call: Any) -> str:
21132130
"""Extract tool name from various tool call formats."""
21142131
if hasattr(tool_call, "function"):

lib/crewai/src/crewai/lite_agent_output.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ class LiteAgentOutput(BaseModel):
5959
),
6060
)
6161

62+
@property
63+
def has_tool_failures(self) -> bool:
64+
"""Whether any tool reported a failure while producing this output.
65+
66+
Same name and meaning as on ``TaskOutput`` and ``CrewOutput``, so a
67+
check written for one result type works on all three.
68+
"""
69+
return bool(self.tool_failures)
70+
6271
plan: str | None = Field(
6372
default=None, description="The execution plan that was generated, if any"
6473
)

lib/crewai/src/crewai/tools/base_tool.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
build_schema_hint,
3939
format_description_for_llm,
4040
)
41+
from crewai.tools.tool_failure import ToolFailurePolicy
4142
from crewai.types.callback import SerializableCallable, _resolve_dotted_path
4243
from crewai.utilities.string_utils import sanitize_tool_name
4344

@@ -184,6 +185,14 @@ def _serialize_result_schema(
184185
default=None,
185186
description="Maximum number of times this tool can be used. None means unlimited usage.",
186187
)
188+
tool_failure_policy: ToolFailurePolicy | None = Field(
189+
default=None,
190+
description=(
191+
"Overrides the agent's and task's tool_failure_policy for this tool "
192+
"only. Leave None to inherit. Use to tighten a single destructive "
193+
"tool to 'raise', or to exempt a chatty one with 'ignore'."
194+
),
195+
)
187196
current_usage_count: int = Field(
188197
default=0,
189198
description="Current number of times this tool has been used.",
@@ -402,6 +411,7 @@ def to_structured_tool(self) -> CrewStructuredTool:
402411
max_usage_count=self.max_usage_count,
403412
current_usage_count=self.current_usage_count,
404413
cache_function=self.cache_function,
414+
tool_failure_policy=self.tool_failure_policy,
405415
)
406416
structured_tool._original_tool = self
407417
return structured_tool

lib/crewai/src/crewai/tools/structured_tool.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
)
2222
from typing_extensions import Self
2323

24-
from crewai.tools.tool_failure import ToolFailure
24+
from crewai.tools.tool_failure import ToolFailure, ToolFailurePolicy
2525
from crewai.utilities.logger import Logger
2626
from crewai.utilities.pydantic_schema_utils import (
2727
create_model_from_schema,
@@ -212,6 +212,7 @@ class CrewStructuredTool(BaseModel):
212212
result_as_answer: bool = Field(default=False)
213213
max_usage_count: int | None = Field(default=None)
214214
current_usage_count: int = Field(default=0)
215+
tool_failure_policy: ToolFailurePolicy | None = Field(default=None)
215216
cache_function: Any = Field(default=None, exclude=True)
216217
_logger: Logger = PrivateAttr(default_factory=Logger)
217218
_original_tool: Any = PrivateAttr(default=None)

lib/crewai/src/crewai/tools/tool_failure.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,15 @@ def resolve_tool_failure_policy(
204204
205205
Most specific wins: tool, then task, then agent, then crew, then
206206
:attr:`ToolFailurePolicy.WARN`.
207+
208+
Execution paths hand over either a :class:`~crewai.tools.base_tool.BaseTool`
209+
or the ``CrewStructuredTool`` that wraps it, so the tool scope is read
210+
through the wrapper as well -- otherwise a tool-scoped policy would be
211+
silently ignored on every native function-calling path.
207212
"""
208-
for source in (tool, task, agent, crew):
213+
original_tool = getattr(tool, "_original_tool", None) if tool is not None else None
214+
215+
for source in (tool, original_tool, task, agent, crew):
209216
if source is None:
210217
continue
211218
policy = getattr(source, "tool_failure_policy", None)

lib/crewai/src/crewai/utilities/agent_utils.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
)
3434
from crewai.tools.tool_failure import (
3535
ToolFailure,
36+
ToolFailureReason,
3637
detect_tool_failure,
3738
failure_from_exception,
3839
handle_tool_failure,
@@ -1717,6 +1718,16 @@ def execute_single_native_tool_call(
17171718
),
17181719
)
17191720
error_event_emitted = True
1721+
else:
1722+
# Not cached and not executable: the model asked for a tool that
1723+
# does not exist. The ReAct path reports this as a failure, so the
1724+
# native paths must too, or the same miss is silent on one and
1725+
# loud on the other.
1726+
tool_failure = ToolFailure(
1727+
message=result,
1728+
reason=ToolFailureReason.UNKNOWN_TOOL,
1729+
code=func_name,
1730+
)
17201731

17211732
after_hook_context = ToolCallHookContext(
17221733
tool_name=func_name,

0 commit comments

Comments
 (0)