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
This commit is contained in:
Joao Moura
2026-07-28 23:22:58 -07:00
parent 279c57fba3
commit e184333178
10 changed files with 339 additions and 9 deletions

View File

@@ -1030,6 +1030,16 @@ class CrewAgentExecutor(BaseAgentExecutor):
),
)
error_event_emitted = True
elif not from_cache:
# Not cached and not executable: the model asked for a tool that
# does not exist. The ReAct path reports this as a failure, so the
# native paths must too, or the same miss is silent on one and
# loud on the other.
tool_failure = ToolFailure(
message=result,
reason=ToolFailureReason.UNKNOWN_TOOL,
code=func_name,
)
after_hook_context = ToolCallHookContext(
tool_name=func_name,

View File

@@ -415,6 +415,8 @@ class EventListener(BaseEventListener):
@crewai_event_bus.on(ToolUsageFinishedEvent)
def on_tool_usage_finished(source: Any, event: ToolUsageFinishedEvent) -> None:
if not self.formatter.should_render_success_panel(event.failure):
return
if isinstance(source, LLM):
self.formatter.handle_llm_tool_usage_finished(
event.tool_name,
@@ -444,6 +446,8 @@ class EventListener(BaseEventListener):
def on_tool_failure_detected(
source: Any, event: ToolFailureDetectedEvent
) -> None:
if not self.formatter.should_render_failure_panel(event.failure):
return
self.formatter.handle_tool_failure_detected(
event.tool_name,
event.failure,

View File

@@ -12,6 +12,7 @@ from rich.live import Live
from rich.panel import Panel
from rich.text import Text
from crewai.tools.tool_failure import ToolFailureReason
from crewai.version import is_current_version_yanked, is_newer_version_available
@@ -492,6 +493,26 @@ To enable tracing, do any one of these:
content, f"✅ Tool Execution Completed (#{iteration})", "green"
)
@staticmethod
def should_render_success_panel(failure: Any) -> bool:
"""Whether a finished tool call should print the green panel.
A call that reported failure must not read as successful, so the
green panel is suppressed and the red one takes its place.
"""
return failure is None
@staticmethod
def should_render_failure_panel(failure: Any) -> bool:
"""Whether a reported failure should print its own red panel.
A tool that *raised* already produced a ``ToolUsageErrorEvent`` and
its own red panel, so printing a second one for the same exception is
pure noise. The event itself is still emitted -- only the duplicate
console output is skipped.
"""
return getattr(failure, "reason", None) is not ToolFailureReason.EXCEPTION
def handle_tool_failure_detected(
self,
tool_name: str,

View File

@@ -2046,6 +2046,8 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor):
),
)
error_event_emitted = True
else:
tool_failure = self._unknown_tool_failure(func_name, result)
elif max_usage_reached:
# Return error message when max usage limit is reached
if original_tool:
@@ -2056,6 +2058,8 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor):
tool_failure = ToolFailure(
message=result, reason=ToolFailureReason.USAGE_LIMIT
)
elif not from_cache:
tool_failure = self._unknown_tool_failure(func_name, result)
# Execute after_tool_call hooks (even if blocked, to allow logging/monitoring)
after_hook_context = ToolCallHookContext(
@@ -2109,6 +2113,19 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor):
"original_tool": original_tool,
}
@staticmethod
def _unknown_tool_failure(func_name: str, result: str) -> ToolFailure:
"""Build the failure for a tool the model asked for but we do not have.
The ReAct path reports this as a failure, so the native path must too,
or the same miss is silent on one and loud on the other.
"""
return ToolFailure(
message=result,
reason=ToolFailureReason.UNKNOWN_TOOL,
code=func_name,
)
def _extract_tool_name(self, tool_call: Any) -> str:
"""Extract tool name from various tool call formats."""
if hasattr(tool_call, "function"):

View File

@@ -59,6 +59,15 @@ class LiteAgentOutput(BaseModel):
),
)
@property
def has_tool_failures(self) -> bool:
"""Whether any tool reported a failure while producing this output.
Same name and meaning as on ``TaskOutput`` and ``CrewOutput``, so a
check written for one result type works on all three.
"""
return bool(self.tool_failures)
plan: str | None = Field(
default=None, description="The execution plan that was generated, if any"
)

View File

@@ -38,6 +38,7 @@ from crewai.tools.structured_tool import (
build_schema_hint,
format_description_for_llm,
)
from crewai.tools.tool_failure import ToolFailurePolicy
from crewai.types.callback import SerializableCallable, _resolve_dotted_path
from crewai.utilities.string_utils import sanitize_tool_name
@@ -184,6 +185,14 @@ class BaseTool(BaseModel, ABC):
default=None,
description="Maximum number of times this tool can be used. None means unlimited usage.",
)
tool_failure_policy: ToolFailurePolicy | None = Field(
default=None,
description=(
"Overrides the agent's and task's tool_failure_policy for this tool "
"only. Leave None to inherit. Use to tighten a single destructive "
"tool to 'raise', or to exempt a chatty one with 'ignore'."
),
)
current_usage_count: int = Field(
default=0,
description="Current number of times this tool has been used.",
@@ -402,6 +411,7 @@ class BaseTool(BaseModel, ABC):
max_usage_count=self.max_usage_count,
current_usage_count=self.current_usage_count,
cache_function=self.cache_function,
tool_failure_policy=self.tool_failure_policy,
)
structured_tool._original_tool = self
return structured_tool

View File

@@ -21,7 +21,7 @@ from pydantic import (
)
from typing_extensions import Self
from crewai.tools.tool_failure import ToolFailure
from crewai.tools.tool_failure import ToolFailure, ToolFailurePolicy
from crewai.utilities.logger import Logger
from crewai.utilities.pydantic_schema_utils import (
create_model_from_schema,
@@ -212,6 +212,7 @@ class CrewStructuredTool(BaseModel):
result_as_answer: bool = Field(default=False)
max_usage_count: int | None = Field(default=None)
current_usage_count: int = Field(default=0)
tool_failure_policy: ToolFailurePolicy | None = Field(default=None)
cache_function: Any = Field(default=None, exclude=True)
_logger: Logger = PrivateAttr(default_factory=Logger)
_original_tool: Any = PrivateAttr(default=None)

View File

@@ -204,8 +204,15 @@ def resolve_tool_failure_policy(
Most specific wins: tool, then task, then agent, then crew, then
:attr:`ToolFailurePolicy.WARN`.
Execution paths hand over either a :class:`~crewai.tools.base_tool.BaseTool`
or the ``CrewStructuredTool`` that wraps it, so the tool scope is read
through the wrapper as well -- otherwise a tool-scoped policy would be
silently ignored on every native function-calling path.
"""
for source in (tool, task, agent, crew):
original_tool = getattr(tool, "_original_tool", None) if tool is not None else None
for source in (tool, original_tool, task, agent, crew):
if source is None:
continue
policy = getattr(source, "tool_failure_policy", None)

View File

@@ -33,6 +33,7 @@ from crewai.tools.structured_tool import (
)
from crewai.tools.tool_failure import (
ToolFailure,
ToolFailureReason,
detect_tool_failure,
failure_from_exception,
handle_tool_failure,
@@ -1717,6 +1718,16 @@ def execute_single_native_tool_call(
),
)
error_event_emitted = True
else:
# Not cached and not executable: the model asked for a tool that
# does not exist. The ReAct path reports this as a failure, so the
# native paths must too, or the same miss is silent on one and
# loud on the other.
tool_failure = ToolFailure(
message=result,
reason=ToolFailureReason.UNKNOWN_TOOL,
code=func_name,
)
after_hook_context = ToolCallHookContext(
tool_name=func_name,

View File

@@ -1,5 +1,7 @@
"""Tests for structured tool-failure signalling and the per-agent policy."""
from datetime import datetime
from types import SimpleNamespace
from typing import Any
import pytest
@@ -66,10 +68,13 @@ class ScriptedLLM(LLM):
def _slack_steps() -> list[str]:
return [
call_step = (
"Thought: posting\n"
"Action: slackbot_send_message\n"
'Action Input: {"channel": "#joao-message"}',
+ "Action: slackbot_send_message\n"
+ 'Action Input: {"channel": "#joao-message"}'
)
return [
call_step,
"Thought: it failed\nFinal Answer: I could not post the message.",
]
@@ -320,6 +325,243 @@ class TestEndToEndPolicies:
assert tool_messages
class TestToolScopedPolicyReachesTheExecutor:
"""A tool-scoped policy must survive the CrewStructuredTool wrapper.
The executors hand ``handle_tool_failure`` the wrapper, not the authored
BaseTool, so a policy set on the tool used to be silently dropped.
"""
def test_policy_survives_to_structured_tool(self) -> None:
class StrictSlack(SlackTool):
tool_failure_policy: ToolFailurePolicy | None = ToolFailurePolicy.RAISE
wrapper = StrictSlack().to_structured_tool()
assert wrapper.tool_failure_policy is ToolFailurePolicy.RAISE
assert resolve_tool_failure_policy(tool=wrapper) is ToolFailurePolicy.RAISE
def test_policy_resolves_through_original_tool_reference(self) -> None:
"""Even a wrapper that never copied the field resolves via _original_tool."""
class StrictSlack(SlackTool):
tool_failure_policy: ToolFailurePolicy | None = ToolFailurePolicy.RAISE
wrapper = StrictSlack().to_structured_tool()
wrapper.tool_failure_policy = None
assert resolve_tool_failure_policy(tool=wrapper) is ToolFailurePolicy.RAISE
def test_tool_policy_aborts_a_warn_agent_end_to_end(self) -> None:
class StrictSlack(SlackTool):
tool_failure_policy: ToolFailurePolicy | None = ToolFailurePolicy.RAISE
agent = Agent(
role="Slack Messenger",
goal="post a message",
backstory="b",
llm=ScriptedLLM(_slack_steps()),
tools=[StrictSlack()],
tool_failure_policy=ToolFailurePolicy.WARN,
)
task = Task(description="post to slack", expected_output="c", agent=agent)
with pytest.raises(ToolExecutionFailedError):
Crew(agents=[agent], tasks=[task]).kickoff()
def test_tool_policy_can_exempt_a_raising_agent(self) -> None:
class ChattySlack(SlackTool):
tool_failure_policy: ToolFailurePolicy | None = ToolFailurePolicy.IGNORE
agent = Agent(
role="Slack Messenger",
goal="post a message",
backstory="b",
llm=ScriptedLLM(_slack_steps()),
tools=[ChattySlack()],
tool_failure_policy=ToolFailurePolicy.RAISE,
)
task = Task(description="post to slack", expected_output="c", agent=agent)
result = Crew(agents=[agent], tasks=[task]).kickoff()
assert not result.has_tool_failures
def test_plain_tools_default_to_inheriting(self) -> None:
assert SlackTool().tool_failure_policy is None
class TestConsolePanels:
"""Exactly one panel per failed call, and never a green one.
A failed call used to print the green "Tool Execution Completed" panel --
the terminal equivalent of the green checkmarks in the bug report.
"""
@staticmethod
def _formatter():
from crewai.events.utils.console_formatter import ConsoleFormatter
return ConsoleFormatter(verbose=True)
def test_success_panel_suppressed_when_the_call_failed(self) -> None:
failure = ToolFailure(message="nope", code="channel_not_found")
assert self._formatter().should_render_success_panel(failure) is False
def test_success_panel_still_shown_for_a_working_call(self) -> None:
assert self._formatter().should_render_success_panel(None) is True
def test_exception_failures_do_not_double_print(self) -> None:
"""ToolUsageErrorEvent already prints; the failure panel must not repeat it."""
failure = failure_from_exception(ValueError("kaboom"))
assert self._formatter().should_render_failure_panel(failure) is False
def test_tool_reported_failures_do_print(self) -> None:
failure = ToolFailure(message="nope", code="channel_not_found")
assert self._formatter().should_render_failure_panel(failure) is True
def test_mcp_failures_do_print(self) -> None:
failure = ToolFailure(message="nope", reason=ToolFailureReason.MCP_ERROR)
assert self._formatter().should_render_failure_panel(failure) is True
def test_failure_panel_renders_without_raising(self) -> None:
"""The real formatter must handle the payload it is given."""
self._formatter().handle_tool_failure_detected(
"slackbot_send_message",
ToolFailure(message="nope", code="channel_not_found"),
ToolFailurePolicy.WARN,
)
def test_listener_consults_the_predicates(self) -> None:
"""The listener must route through the predicates, not its own logic."""
import inspect
from crewai.events.event_listener import EventListener
source = inspect.getsource(EventListener.setup_listeners)
assert "should_render_success_panel" in source
assert "should_render_failure_panel" in source
class TestUnknownToolOnNativePaths:
"""The ReAct path reported unknown tools; the native paths did not."""
def test_native_path_records_unknown_tool(self) -> None:
from crewai.utilities.agent_utils import execute_single_native_tool_call
agent = Agent(role="r", goal="g", backstory="b")
recorded: list[ToolFailureDetectedEvent] = []
tool_call = SimpleNamespace(
id="call_1",
function=SimpleNamespace(name="does_not_exist", arguments="{}"),
)
with crewai_event_bus.scoped_handlers():
@crewai_event_bus.on(ToolFailureDetectedEvent)
def _(source: Any, event: ToolFailureDetectedEvent) -> None:
recorded.append(event)
execute_single_native_tool_call(
tool_call,
available_functions={},
original_tools=[],
structured_tools=[],
tools_handler=None,
agent=agent,
task=None,
crew=None,
event_source=agent,
printer=None,
verbose=False,
)
# emit() dispatches sync handlers on a thread pool, so drain
# before asserting on what subscribers saw.
crewai_event_bus.flush(timeout=10.0)
# The record is written synchronously, before the event is emitted.
assert len(agent.last_tool_failures) == 1
record = agent.last_tool_failures[0]
assert record.tool_name == "does_not_exist"
assert record.failure.reason is ToolFailureReason.UNKNOWN_TOOL
assert record.failure.code == "does_not_exist"
assert len(recorded) == 1
assert recorded[0].failure.reason is ToolFailureReason.UNKNOWN_TOOL
def test_unknown_tool_can_abort_under_raise(self) -> None:
from crewai.utilities.agent_utils import execute_single_native_tool_call
agent = Agent(
role="r",
goal="g",
backstory="b",
tool_failure_policy=ToolFailurePolicy.RAISE,
)
tool_call = SimpleNamespace(
id="call_1",
function=SimpleNamespace(name="does_not_exist", arguments="{}"),
)
with pytest.raises(ToolExecutionFailedError):
execute_single_native_tool_call(
tool_call,
available_functions={},
original_tools=[],
structured_tools=[],
tools_handler=None,
agent=agent,
task=None,
crew=None,
event_source=agent,
printer=None,
verbose=False,
)
class TestExceptionFailuresStillRecorded:
def test_raised_tool_produces_a_failure_record(self) -> None:
class BoomTool(BaseTool):
name: str = "boom"
description: str = "Always explodes."
def _run(self, x: str) -> Any:
raise ValueError("kaboom")
agent = Agent(
role="Breaker",
goal="break",
backstory="b",
llm=ScriptedLLM(
[
'Thought: go\nAction: boom\nAction Input: {"x": "1"}',
"Thought: it broke\nFinal Answer: it broke.",
]
),
tools=[BoomTool()],
)
task = Task(description="break it", expected_output="e", agent=agent)
result = Crew(agents=[agent], tasks=[task]).kickoff()
assert result.has_tool_failures
reasons = {f.failure.reason for f in result.tool_failures}
assert ToolFailureReason.EXCEPTION in reasons
class TestLiteAgentOutputParity:
def test_has_tool_failures_exists_on_all_output_types(self) -> None:
from crewai.crews.crew_output import CrewOutput
from crewai.lite_agent_output import LiteAgentOutput
from crewai.tasks.task_output import TaskOutput
record = ToolFailureRecord(
tool_name="t", failure=ToolFailure(message="nope")
)
assert LiteAgentOutput(agent_role="r").has_tool_failures is False
assert (
LiteAgentOutput(agent_role="r", tool_failures=[record]).has_tool_failures
is True
)
assert TaskOutput(description="d", agent="a").has_tool_failures is False
assert CrewOutput().has_tool_failures is False
class TestMCPIsErrorPlumbing:
"""An MCP server flags a failed tool with isError on a 200 response."""
@@ -359,11 +601,9 @@ class TestPlatformActionTool:
@staticmethod
def _tool() -> Any:
from crewai_tools.tools.crewai_platform_tools.crewai_platform_action_tool import ( # noqa: E501
CrewAIPlatformActionTool,
)
import crewai_tools.tools.crewai_platform_tools.crewai_platform_action_tool as mod
return CrewAIPlatformActionTool(
return mod.CrewAIPlatformActionTool(
description="Send a Slack message",
action_name="slackbot_send_message",
action_schema={