Skip to content

Commit 55160c6

Browse files
joaomdmouraclaude
andcommitted
fix(tools): make ignore truly silent, stop caching failures, close 4 gaps
Six findings from the latest review round, all verified against the code before touching it. `ignore` was not silent. `ToolUsageFinishedEvent.failure` was set before the policy ran, so traces still saw a failed call under a policy documented as surfacing nothing. Worse, the console then showed *no* panel at all: green was suppressed because `failure` was present, red was skipped because `ignore` never emits `ToolFailureDetectedEvent`. New `reportable_failure()` resolves the policy before the finished event and drops the flag under `ignore`; wired into all four execution paths. Failures were being cached. `CacheHandler.add` stored a `ToolFailure` like any other result, so a transient error became permanent for the rest of the run and every later hit re-reported a call that never re-ran. The cache now refuses to store declared failures -- fixed at the single choke point rather than at each of the four call sites. A spent `max_usage_count` was invisible on the shared native path. `BaseTool._claim_usage` returned a bare string that only the executors recognising that exact message treated as a failure. It now returns a `ToolFailure` with `USAGE_LIMIT`, so every path records it. A guardrail returning a whole `TaskOutput` replaced the output without carrying accumulated failures over, so earlier attempts vanished from `CrewOutput.tool_failures`. New `merge_tool_failures()` combines and deduplicates, and the retry-rebuild path uses it too. A hook-blocked call inherited a cached failure and attributed it to a call that never ran. Now cleared. Not reachable through the built-in cache once failures stop being cached, so the test injects a custom cache handler that does retain them -- verified to fail without the guard. Also removed a `datetime` import left unused by the earlier console-test rewrite. Testing: 13 further tests, 73 total. Full suite matches baseline exactly at 377 pre-existing failures; the usage-limit suites that `_claim_usage` touches pass unchanged; 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
1 parent bc99c4b commit 55160c6

9 files changed

Lines changed: 395 additions & 14 deletions

File tree

lib/crewai/src/crewai/agents/cache/cache_handler.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ class CacheHandler(BaseModel):
2323
def add(self, tool: str, input: str, output: Any) -> None:
2424
"""Add a tool result to the cache.
2525
26+
Declared failures are never stored: replaying one would make a
27+
transient error permanent for the rest of the run, and every later hit
28+
would re-report a call that did not run.
29+
2630
Args:
2731
tool: Name of the tool.
2832
input: Input string used for the tool.
@@ -31,6 +35,11 @@ def add(self, tool: str, input: str, output: Any) -> None:
3135
Notes:
3236
- TODO: Rename 'input' parameter to avoid shadowing builtin.
3337
"""
38+
from crewai.tools.tool_failure import ToolFailure
39+
40+
if isinstance(output, ToolFailure):
41+
return
42+
3443
with self._lock.w_locked():
3544
self._cache[f"{tool}-{input}"] = output
3645

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
detect_tool_failure,
5757
failure_from_exception,
5858
handle_tool_failure,
59+
reportable_failure,
5960
)
6061
from crewai.types.callback import SerializableCallable
6162
from crewai.utilities.agent_utils import (
@@ -979,6 +980,9 @@ def _execute_single_native_tool_call(
979980
if hook_blocked:
980981
result = f"Tool execution blocked by hook. Tool: {func_name}"
981982
raw_tool_result = result
983+
# The blocked message replaces any cached result, so a cached
984+
# failure must not be attributed to this call.
985+
tool_failure = None
982986
elif max_usage_reached and original_tool:
983987
result = f"Tool '{func_name}' has reached its usage limit of {original_tool.max_usage_count} times and cannot be used anymore."
984988
raw_tool_result = result
@@ -1064,7 +1068,13 @@ def _execute_single_native_tool_call(
10641068
agent_key=agent_key,
10651069
started_at=started_at,
10661070
finished_at=datetime.now(),
1067-
failure=tool_failure,
1071+
failure=reportable_failure(
1072+
tool_failure,
1073+
tool=structured_tool,
1074+
agent=self.agent,
1075+
task=self.task,
1076+
crew=self.crew,
1077+
),
10681078
),
10691079
)
10701080

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
detect_tool_failure,
8181
failure_from_exception,
8282
handle_tool_failure,
83+
reportable_failure,
8384
)
8485
from crewai.utilities.agent_utils import (
8586
_llm_stop_words_applied,
@@ -2003,6 +2004,9 @@ def _execute_single_native_tool_call(self, tool_call: Any) -> dict[str, Any]:
20032004
if hook_blocked:
20042005
result = f"Tool execution blocked by hook. Tool: {func_name}"
20052006
raw_tool_result = result
2007+
# The blocked message replaces any cached result, so a cached
2008+
# failure must not be attributed to this call.
2009+
tool_failure = None
20062010
elif not from_cache and not max_usage_reached and output_tool is not None:
20072011
if func_name in self._available_functions:
20082012
try:
@@ -2087,7 +2091,13 @@ def _execute_single_native_tool_call(self, tool_call: Any) -> dict[str, Any]:
20872091
agent_key=agent_key,
20882092
started_at=started_at,
20892093
finished_at=datetime.now(),
2090-
failure=tool_failure,
2094+
failure=reportable_failure(
2095+
tool_failure,
2096+
tool=structured_tool,
2097+
agent=self.agent,
2098+
task=self.task,
2099+
crew=self.crew,
2100+
),
20912101
),
20922102
)
20932103

lib/crewai/src/crewai/task.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
ToolFailurePolicy,
5757
ToolFailureRecord,
5858
collect_tool_failures,
59+
merge_tool_failures,
5960
)
6061
from crewai.utilities.config import process_config
6162
from crewai.utilities.constants import NOT_SPECIFIED, _NotSpecified
@@ -1361,7 +1362,12 @@ def _invoke_guardrail_function(
13611362
task_output.pydantic = pydantic_output
13621363
task_output.json_dict = json_output
13631364
elif isinstance(guardrail_result.result, TaskOutput):
1365+
# A guardrail may return a whole new output; carry the
1366+
# accumulated failures over or earlier attempts vanish.
13641367
task_output = guardrail_result.result
1368+
task_output.tool_failures = merge_tool_failures(
1369+
accumulated_failures, task_output.tool_failures
1370+
)
13651371

13661372
return task_output
13671373

@@ -1423,7 +1429,9 @@ def _invoke_guardrail_function(
14231429
agent=agent.role,
14241430
output_format=self._get_output_format(),
14251431
messages=agent.last_messages, # type: ignore[attr-defined]
1426-
tool_failures=accumulated_failures + collect_tool_failures(agent),
1432+
tool_failures=merge_tool_failures(
1433+
accumulated_failures, collect_tool_failures(agent)
1434+
),
14271435
)
14281436
accumulated_failures = list(task_output.tool_failures)
14291437

@@ -1476,7 +1484,12 @@ async def _ainvoke_guardrail_function(
14761484
task_output.pydantic = pydantic_output
14771485
task_output.json_dict = json_output
14781486
elif isinstance(guardrail_result.result, TaskOutput):
1487+
# A guardrail may return a whole new output; carry the
1488+
# accumulated failures over or earlier attempts vanish.
14791489
task_output = guardrail_result.result
1490+
task_output.tool_failures = merge_tool_failures(
1491+
accumulated_failures, task_output.tool_failures
1492+
)
14801493

14811494
return task_output
14821495

@@ -1538,7 +1551,9 @@ async def _ainvoke_guardrail_function(
15381551
agent=agent.role,
15391552
output_format=self._get_output_format(),
15401553
messages=agent.last_messages, # type: ignore[attr-defined]
1541-
tool_failures=accumulated_failures + collect_tool_failures(agent),
1554+
tool_failures=merge_tool_failures(
1555+
accumulated_failures, collect_tool_failures(agent)
1556+
),
15421557
)
15431558
accumulated_failures = list(task_output.tool_failures)
15441559

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
build_schema_hint,
3939
format_description_for_llm,
4040
)
41-
from crewai.tools.tool_failure import ToolFailurePolicy
41+
from crewai.tools.tool_failure import ToolFailure, ToolFailurePolicy, ToolFailureReason
4242
from crewai.types.callback import SerializableCallable, _resolve_dotted_path
4343
from crewai.utilities.string_utils import sanitize_tool_name
4444

@@ -299,21 +299,26 @@ def _validate_kwargs(self, kwargs: dict[str, Any]) -> dict[str, Any]:
299299
) from e
300300
return kwargs
301301

302-
def _claim_usage(self) -> str | None:
302+
def _claim_usage(self) -> ToolFailure | None:
303303
"""Atomically check max usage and increment the counter.
304304
305305
Returns:
306-
None if usage was claimed successfully, or an error message
307-
string if the tool has reached its usage limit.
306+
None if usage was claimed, otherwise a :class:`ToolFailure`. A
307+
structured result rather than a bare string so every execution
308+
path records a spent limit, instead of only the ones that
309+
recognise the message.
308310
"""
309311
with self._usage_lock:
310312
if (
311313
self.max_usage_count is not None
312314
and self.current_usage_count >= self.max_usage_count
313315
):
314-
return (
315-
f"Tool '{self.name}' has reached its usage limit of "
316-
f"{self.max_usage_count} times and cannot be used anymore."
316+
return ToolFailure(
317+
message=(
318+
f"Tool '{self.name}' has reached its usage limit of "
319+
f"{self.max_usage_count} times and cannot be used anymore."
320+
),
321+
reason=ToolFailureReason.USAGE_LIMIT,
317322
)
318323
self.current_usage_count += 1
319324
return None

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,32 @@ def resolve_tool_failure_policy(
208208
return ToolFailurePolicy.WARN
209209

210210

211+
def merge_tool_failures(
212+
*groups: list[ToolFailureRecord],
213+
) -> list[ToolFailureRecord]:
214+
"""Concatenate failure lists, dropping records already present.
215+
216+
Guardrail retries rebuild the output from overlapping sources, so identity
217+
is not enough to avoid duplicates.
218+
"""
219+
merged: list[ToolFailureRecord] = []
220+
seen: set[tuple[Any, ...]] = set()
221+
for group in groups:
222+
for record in group:
223+
key = (
224+
record.tool_name,
225+
record.failure.message,
226+
record.failure.code,
227+
record.task_id,
228+
str(record.tool_args),
229+
)
230+
if key in seen:
231+
continue
232+
seen.add(key)
233+
merged.append(record)
234+
return merged
235+
236+
211237
def collect_tool_failures(agent: Any) -> list[ToolFailureRecord]:
212238
"""Return the failures recorded on an agent, tolerating custom agents.
213239
@@ -227,6 +253,26 @@ def _record_on_agent(agent: Any, record: ToolFailureRecord) -> None:
227253
failures.append(record)
228254

229255

256+
def reportable_failure(
257+
failure: ToolFailure | None,
258+
*,
259+
tool: Any = None,
260+
agent: BaseAgent | LiteAgent | None = None,
261+
task: Task | None = None,
262+
crew: Crew | None = None,
263+
) -> ToolFailure | None:
264+
"""Return the failure to attach to ``ToolUsageFinishedEvent``.
265+
266+
``None`` under :attr:`ToolFailurePolicy.IGNORE`, so that policy really does
267+
surface nothing -- neither a record, nor an event, nor a flag on the
268+
finished event that a trace UI would render as a failure.
269+
"""
270+
if failure is None:
271+
return None
272+
policy = resolve_tool_failure_policy(tool=tool, agent=agent, task=task, crew=crew)
273+
return None if policy is ToolFailurePolicy.IGNORE else failure
274+
275+
230276
def handle_tool_failure(
231277
failure: ToolFailure,
232278
*,

lib/crewai/src/crewai/tools/tool_usage.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
ToolFailureReason,
3030
detect_tool_failure,
3131
failure_from_exception,
32+
reportable_failure,
3233
)
3334
from crewai.utilities.agent_utils import (
3435
get_tool_names,
@@ -1017,7 +1018,12 @@ def on_tool_use_finished(
10171018
"finished_at": datetime.datetime.fromtimestamp(finished_at),
10181019
"from_cache": from_cache,
10191020
"output": result,
1020-
"failure": self.last_failure,
1021+
"failure": reportable_failure(
1022+
self.last_failure,
1023+
tool=tool,
1024+
agent=self.agent,
1025+
task=self.task,
1026+
),
10211027
}
10221028
)
10231029
if self.task:

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
detect_tool_failure,
3838
failure_from_exception,
3939
handle_tool_failure,
40+
reportable_failure,
4041
)
4142
from crewai.tools.tool_types import ToolResult
4243
from crewai.utilities.errors import AgentRepositoryError
@@ -1678,6 +1679,9 @@ def execute_single_native_tool_call(
16781679
if hook_blocked:
16791680
result = f"Tool execution blocked by hook. Tool: {func_name}"
16801681
raw_tool_result = result
1682+
# The blocked message replaces any cached result, so a cached failure
1683+
# must not be attributed to this call.
1684+
tool_failure = None
16811685
elif not from_cache:
16821686
if func_name in available_functions and output_tool is not None:
16831687
try:
@@ -1755,7 +1759,13 @@ def execute_single_native_tool_call(
17551759
plan_step_description=plan_step_description,
17561760
started_at=started_at,
17571761
finished_at=datetime.now(),
1758-
failure=tool_failure,
1762+
failure=reportable_failure(
1763+
tool_failure,
1764+
tool=structured_tool,
1765+
agent=agent,
1766+
task=task,
1767+
crew=crew,
1768+
),
17591769
),
17601770
)
17611771

0 commit comments

Comments
 (0)