mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-10 08:21:54 +00:00
fix(tools): make crew-scoped policy real and close the last raise leak
Two findings, and the first was a documented feature that never worked. `resolve_tool_failure_policy` consulted a crew, and the docs advertised crew as a scope, but `Crew` had no `tool_failure_policy` field at all -- and even with one it was unreachable, because `BaseAgent` defaulted the policy to `WARN` rather than `None`, so resolution always stopped at the agent. Crew-level configuration was silently ignored. Fixed by making "inherit" the default everywhere instead of baking `warn` into one layer: `Crew` gains the field, and `BaseAgent`/`LiteAgent` default to `None` like `Task` and `BaseTool` already did. The resolver owns the single fallback, so the chain is genuinely tool > task > agent > crew > warn and the effective default with nothing configured is still `warn`. Reading `agent.tool_failure_policy` now returns `None` (meaning "inherit") rather than `WARN`. The other: `StepExecutor` re-raised `ToolExecutionFailedError` from its outer handler, but the nested handler around the native-to-text tooling fallback still caught it and returned `StepResult(success=False)`. An agent whose LLM lacked native tool calling would therefore not abort under `raise`. That is the third distinct place this exception was being downgraded; it now re-raises there too. Testing: 8 further tests, 60 total, including the full precedence chain walked one level at a time and crew-scoped `raise`/`ignore` driven end-to-end through `kickoff()` rather than only through the resolver -- the gap that let the original crew bug pass review. Full suite matches baseline exactly at 377 pre-existing failures; mypy clean on every changed file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ETacm2dMASfpMAYUiDu5YG
This commit is contained in:
@@ -388,7 +388,7 @@ tool's `max_usage_count` is spent, or when the agent calls a tool that does not
|
||||
| `raise` | Records and emits, then aborts with `ToolExecutionFailedError`. |
|
||||
|
||||
```python Code
|
||||
from crewai import Agent, Task
|
||||
from crewai import Agent, Crew, Task
|
||||
from crewai.tools.tool_failure import ToolFailurePolicy
|
||||
|
||||
agent = Agent(
|
||||
@@ -406,10 +406,18 @@ task = Task(
|
||||
agent=agent,
|
||||
tool_failure_policy=ToolFailurePolicy.RAISE,
|
||||
)
|
||||
|
||||
# Or set a baseline once for every agent in the crew.
|
||||
crew = Crew(
|
||||
agents=[agent],
|
||||
tasks=[task],
|
||||
tool_failure_policy=ToolFailurePolicy.WARN,
|
||||
)
|
||||
```
|
||||
|
||||
The most specific setting wins: tool, then task, then agent, then crew, then the
|
||||
`warn` default.
|
||||
The most specific setting wins: **tool → task → agent → crew → `warn`**. Every
|
||||
level defaults to `None`, meaning "inherit from the next one out", so the
|
||||
effective default with nothing configured anywhere is `warn`.
|
||||
|
||||
### Inspecting Failures
|
||||
|
||||
|
||||
@@ -300,8 +300,8 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
max_iter: int = Field(
|
||||
default=25, description="Maximum iterations for an agent to execute a task"
|
||||
)
|
||||
tool_failure_policy: ToolFailurePolicy = Field(
|
||||
default=ToolFailurePolicy.WARN,
|
||||
tool_failure_policy: ToolFailurePolicy | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"How to react when a tool runs to completion but reports that it "
|
||||
"failed (an upstream API rejecting the request, an MCP server "
|
||||
@@ -309,7 +309,8 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
"'ignore' restores pre-1.16 behavior and records nothing; 'warn' "
|
||||
"records the failure, emits ToolFailureDetectedEvent and keeps "
|
||||
"going; 'raise' additionally aborts with ToolExecutionFailedError. "
|
||||
"A Task or a tool may override this for a narrower scope."
|
||||
"None inherits from the crew, falling back to 'warn'. A task or a "
|
||||
"tool may override this for a narrower scope."
|
||||
),
|
||||
)
|
||||
agent_executor: Annotated[
|
||||
|
||||
@@ -224,6 +224,12 @@ class StepExecutor:
|
||||
tool_calls_made=tool_calls_made,
|
||||
execution_time=elapsed,
|
||||
)
|
||||
except ToolExecutionFailedError:
|
||||
# Same reason as the outer handler: a deliberate stop must
|
||||
# not be downgraded into StepResult(success=False), even
|
||||
# when reached through the text-tooling fallback.
|
||||
raise
|
||||
|
||||
except Exception as fallback_error:
|
||||
e = fallback_error
|
||||
|
||||
|
||||
@@ -116,6 +116,7 @@ from crewai.tasks.task_output import TaskOutput
|
||||
from crewai.tools.agent_tools.agent_tools import AgentTools
|
||||
from crewai.tools.agent_tools.read_file_tool import ReadFileTool
|
||||
from crewai.tools.base_tool import BaseTool
|
||||
from crewai.tools.tool_failure import ToolFailurePolicy
|
||||
from crewai.types.callback import SerializableCallable
|
||||
from crewai.types.streaming import CrewStreamingOutput
|
||||
from crewai.types.usage_metrics import UsageMetrics
|
||||
@@ -231,6 +232,15 @@ class Crew(FlowTrackable, BaseModel):
|
||||
"unless they set a cache_function that prevents caching."
|
||||
),
|
||||
)
|
||||
tool_failure_policy: ToolFailurePolicy | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"Baseline reaction for every agent in this crew when a tool runs "
|
||||
"to completion but reports that it failed. Leave None for the "
|
||||
"'warn' default. An agent, task, or tool may override it for a "
|
||||
"narrower scope."
|
||||
),
|
||||
)
|
||||
tasks: list[Task] = Field(default_factory=list)
|
||||
agents: Annotated[
|
||||
list[BaseAgent],
|
||||
|
||||
@@ -228,11 +228,12 @@ class LiteAgent(FlowTrackable, BaseModel):
|
||||
max_iterations: int = Field(
|
||||
default=15, description="Maximum number of iterations for tool usage"
|
||||
)
|
||||
tool_failure_policy: ToolFailurePolicy = Field(
|
||||
default=ToolFailurePolicy.WARN,
|
||||
tool_failure_policy: ToolFailurePolicy | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"How to react when a tool runs to completion but reports that it "
|
||||
"failed. See BaseAgent.tool_failure_policy."
|
||||
"failed. None falls back to 'warn'. See "
|
||||
"BaseAgent.tool_failure_policy."
|
||||
),
|
||||
)
|
||||
max_execution_time: int | None = Field(
|
||||
|
||||
@@ -173,6 +173,62 @@ class TestPolicyResolution:
|
||||
resolved = resolve_tool_failure_policy(agent=agent, task=task)
|
||||
assert resolved is ToolFailurePolicy.IGNORE
|
||||
|
||||
def test_crew_policy_used_when_agent_inherits(self) -> None:
|
||||
from crewai import Crew
|
||||
|
||||
agent = Agent(role="r", goal="g", backstory="b")
|
||||
crew = Crew(
|
||||
agents=[agent], tasks=[], tool_failure_policy=ToolFailurePolicy.RAISE
|
||||
)
|
||||
resolved = resolve_tool_failure_policy(agent=agent, crew=crew)
|
||||
assert resolved is ToolFailurePolicy.RAISE
|
||||
|
||||
def test_agent_overrides_crew(self) -> None:
|
||||
from crewai import Crew
|
||||
|
||||
agent = Agent(
|
||||
role="r",
|
||||
goal="g",
|
||||
backstory="b",
|
||||
tool_failure_policy=ToolFailurePolicy.IGNORE,
|
||||
)
|
||||
crew = Crew(
|
||||
agents=[agent], tasks=[], tool_failure_policy=ToolFailurePolicy.RAISE
|
||||
)
|
||||
resolved = resolve_tool_failure_policy(agent=agent, crew=crew)
|
||||
assert resolved is ToolFailurePolicy.IGNORE
|
||||
|
||||
def test_full_precedence_chain(self) -> None:
|
||||
"""tool > task > agent > crew > warn."""
|
||||
from crewai import Crew
|
||||
|
||||
class ScopedTool(SlackTool):
|
||||
tool_failure_policy: ToolFailurePolicy | None = None
|
||||
|
||||
tool = ScopedTool()
|
||||
agent = Agent(role="r", goal="g", backstory="b")
|
||||
task = Task(description="d", expected_output="e")
|
||||
crew = Crew(agents=[agent], tasks=[])
|
||||
|
||||
def resolved() -> ToolFailurePolicy:
|
||||
return resolve_tool_failure_policy(
|
||||
tool=tool, agent=agent, task=task, crew=crew
|
||||
)
|
||||
|
||||
assert resolved() is ToolFailurePolicy.WARN
|
||||
|
||||
crew.tool_failure_policy = ToolFailurePolicy.IGNORE
|
||||
assert resolved() is ToolFailurePolicy.IGNORE
|
||||
|
||||
agent.tool_failure_policy = ToolFailurePolicy.WARN
|
||||
assert resolved() is ToolFailurePolicy.WARN
|
||||
|
||||
task.tool_failure_policy = ToolFailurePolicy.RAISE
|
||||
assert resolved() is ToolFailurePolicy.RAISE
|
||||
|
||||
tool.tool_failure_policy = ToolFailurePolicy.IGNORE
|
||||
assert resolved() is ToolFailurePolicy.IGNORE
|
||||
|
||||
def test_invalid_policy_is_ignored_rather_than_raising(self) -> None:
|
||||
"""A bad policy value must never take down a tool call."""
|
||||
|
||||
@@ -208,14 +264,28 @@ class TestPolicyResolution:
|
||||
assert resolved is ToolFailurePolicy.RAISE
|
||||
|
||||
|
||||
class TestAgentDefault:
|
||||
def test_agent_defaults_to_warn(self) -> None:
|
||||
agent = Agent(role="r", goal="g", backstory="b")
|
||||
assert agent.tool_failure_policy is ToolFailurePolicy.WARN
|
||||
class TestDefaults:
|
||||
"""Every scope defaults to None ('inherit'); the resolver owns 'warn'."""
|
||||
|
||||
def test_task_policy_defaults_to_none_so_it_inherits(self) -> None:
|
||||
def test_agent_defaults_to_inherit(self) -> None:
|
||||
assert Agent(role="r", goal="g", backstory="b").tool_failure_policy is None
|
||||
|
||||
def test_task_defaults_to_inherit(self) -> None:
|
||||
assert Task(description="d", expected_output="e").tool_failure_policy is None
|
||||
|
||||
def test_crew_defaults_to_inherit(self) -> None:
|
||||
from crewai import Crew
|
||||
|
||||
agent = Agent(role="r", goal="g", backstory="b")
|
||||
assert Crew(agents=[agent], tasks=[]).tool_failure_policy is None
|
||||
|
||||
def test_tool_defaults_to_inherit(self) -> None:
|
||||
assert SlackTool().tool_failure_policy is None
|
||||
|
||||
def test_effective_default_is_warn(self) -> None:
|
||||
agent = Agent(role="r", goal="g", backstory="b")
|
||||
assert resolve_tool_failure_policy(agent=agent) is ToolFailurePolicy.WARN
|
||||
|
||||
|
||||
class TestEndToEndPolicies:
|
||||
def test_warn_records_and_emits_without_stopping(self) -> None:
|
||||
@@ -598,6 +668,41 @@ class TestRaisePolicySurvivesEveryWrapper:
|
||||
Crew(agents=[agent], tasks=[task]).kickoff()
|
||||
assert agent._times_executed == 0, "the abort must not trigger retries"
|
||||
|
||||
def test_crew_policy_aborts_end_to_end(self) -> None:
|
||||
"""Crew scope must actually reach the executor, not just the resolver."""
|
||||
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.RAISE,
|
||||
)
|
||||
|
||||
with pytest.raises(ToolExecutionFailedError):
|
||||
crew.kickoff()
|
||||
|
||||
def test_crew_ignore_suppresses_recording_end_to_end(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)
|
||||
result = Crew(
|
||||
agents=[agent],
|
||||
tasks=[task],
|
||||
tool_failure_policy=ToolFailurePolicy.IGNORE,
|
||||
).kickoff()
|
||||
assert not result.has_tool_failures
|
||||
|
||||
def test_passthrough_tuple_includes_the_error(self) -> None:
|
||||
from crewai.agent.core import _passthrough_exceptions
|
||||
|
||||
|
||||
Reference in New Issue
Block a user