mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-11 01:45:10 +00:00
fix(tools)!: make tool-result caching opt-in instead of on by default (#6509)
* fix(tools)!: make tool-result caching opt-in instead of on by default Tool-result caching defaulted to on (Crew.cache=True, and standalone agents self-wired a CacheHandler at construction), so an LLM calling the same tool with identical arguments twice in one run silently got the first result back without the tool executing. For live-data tools that is a confidently stale answer; for state-mutating tools the second action is silently dropped. Caching is now opt-in with the machinery unchanged: - Crew.cache defaults to False; Crew(cache=True) restores today's behavior exactly (agents still default to participating when a crew offers its handler, and Agent(cache=False) still opts an agent out). - Standalone agents no longer self-wire a cache; Agent(cache=True) or an explicit cache_handler opts in. Previously even Crew(cache=False) agents cached via this self-wired handler. - Per-tool cache_function write gating is unchanged once opted in. Existing tests that exercised the caching machinery now opt in explicitly; new regression tests cover the default (both identical calls execute), crew-level opt-in dedup, and agent-level wiring. Fixes EPD-180. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agent): don't let copy() turn the cache default into an explicit opt-in Agent.copy() rebuilds from model_dump(), which includes the field default cache=True, so the copy's model_fields_set contained "cache" and _setup_agent_executor wired a CacheHandler the source agent never opted into (Bugbot review finding). Drop "cache" from the dump when it was not explicitly set on the source; explicit opt-ins still survive copying. Also sync the Crew and BaseAgent class docstrings with the new opt-in cache semantics (CodeRabbit review findings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agent): preserve cache_handler-only opt-in across Agent.copy() copy() excludes cache_handler from the rebuilt agent, so an agent that opted into tool-result caching solely via an explicit cache_handler lost caching after copy() (Bugbot review finding). Carry the consent as cache=True on the copy when the source has a handler wired and hasn't explicitly disabled caching — the copy wires its own fresh handler, matching pre-change copy semantics (copies never shared the source's handler instance). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(crew): offer the crew cache handler to the hierarchical manager The hierarchical manager agent is created in _create_manager_agent, outside the validation-time agents loop that offers the crew's cache handler — and managers no longer self-wire a handler — so Crew(cache=True) hierarchical runs never cached the manager's delegation tool calls (Bugbot review finding). Offer the shared crew handler when the crew opted in; a user-provided manager with cache=False stays excluded via the existing set_cache_handler gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agent): only construction-time cache opt-ins survive Agent.copy() The previous copy() fix treated any wired cache_handler as consent, but agents that merely received the crew's shared handler at kickoff (set_cache_handler from Crew(cache=True)) never opted in themselves — their copies must not become standalone cachers (Bugbot review finding). Record the opt-in signal in _setup_agent_executor, which runs at construction before any crew wiring can happen, and have copy() consult that flag instead of inspecting cache_handler after the fact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -401,10 +401,29 @@ class Agent(BaseAgent):
|
||||
return self.planning_config is not None or self.planning
|
||||
|
||||
def _setup_agent_executor(self) -> None:
|
||||
"""Initialize the agent executor with a default cache handler."""
|
||||
if not self.cache_handler:
|
||||
self.cache_handler = CacheHandler()
|
||||
self.set_cache_handler(self.cache_handler)
|
||||
"""Initialize the agent's tools handler and optional tool cache.
|
||||
|
||||
Tool-result caching is opt-in: a standalone agent gets a cache only
|
||||
when it was constructed with an explicit ``cache=True`` or a
|
||||
``cache_handler``. Agents inside a crew additionally receive the
|
||||
crew's shared handler when ``Crew(cache=True)``. Without an opt-in,
|
||||
repeated tool calls with identical arguments always re-execute the
|
||||
tool — the safe default for live-data and state-mutating tools.
|
||||
"""
|
||||
# Recorded before any crew can offer its shared handler at kickoff,
|
||||
# so copy() can distinguish a construction-time opt-in from runtime
|
||||
# crew wiring (which must not turn copies into cachers).
|
||||
self._constructor_cache_opt_in = bool(
|
||||
self.cache
|
||||
and (self.cache_handler is not None or "cache" in self.model_fields_set)
|
||||
)
|
||||
opted_in = self.cache_handler is not None or (
|
||||
"cache" in self.model_fields_set and self.cache
|
||||
)
|
||||
if opted_in:
|
||||
if not self.cache_handler:
|
||||
self.cache_handler = CacheHandler()
|
||||
self.set_cache_handler(self.cache_handler)
|
||||
|
||||
def set_knowledge(self, crew_embedder: EmbedderConfig | None = None) -> None:
|
||||
"""Initialize knowledge sources with the agent or crew embedder config."""
|
||||
|
||||
@@ -205,7 +205,11 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
role (str): Role of the agent.
|
||||
goal (str): Objective of the agent.
|
||||
backstory (str): Backstory of the agent.
|
||||
cache (bool): Whether the agent should use a cache for tool usage.
|
||||
cache (bool): Whether the agent participates in tool-result caching
|
||||
when a cache is enabled. The default (True) only permits
|
||||
participation — caching activates when the crew sets cache=True
|
||||
or the agent explicitly opts in with cache=True or a
|
||||
cache_handler; cache=False excludes the agent entirely.
|
||||
config (dict[str, Any] | None): Configuration for the agent.
|
||||
verbose (bool): Verbose mode for the Agent Execution.
|
||||
max_rpm (int | None): Maximum number of requests per minute for the agent execution.
|
||||
@@ -254,6 +258,7 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
_logger: Logger = PrivateAttr(default_factory=lambda: Logger(verbose=False))
|
||||
_rpm_controller: RPMController | None = PrivateAttr(default=None)
|
||||
_request_within_rpm_limit: SerializableCallable | None = PrivateAttr(default=None)
|
||||
_constructor_cache_opt_in: bool = PrivateAttr(default=False)
|
||||
_original_role: str | None = PrivateAttr(default=None)
|
||||
_original_goal: str | None = PrivateAttr(default=None)
|
||||
_original_backstory: str | None = PrivateAttr(default=None)
|
||||
@@ -267,7 +272,14 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
description="Configuration for the agent", default=None, exclude=True
|
||||
)
|
||||
cache: bool = Field(
|
||||
default=True, description="Whether the agent should use a cache for tool usage."
|
||||
default=True,
|
||||
description=(
|
||||
"Whether the agent participates in tool-result caching when a "
|
||||
"cache is enabled. Caching itself is opt-in: it activates only "
|
||||
"when the crew sets cache=True or the agent explicitly opts in "
|
||||
"(cache=True or a cache_handler at construction). Set False to "
|
||||
"exclude this agent even when the crew enables caching."
|
||||
),
|
||||
)
|
||||
verbose: bool = Field(
|
||||
default=False, description="Verbose mode for the Agent Execution"
|
||||
@@ -716,6 +728,19 @@ class BaseAgent(BaseModel, ABC, metaclass=AgentMeta):
|
||||
|
||||
copied_data = self.model_dump(exclude=exclude)
|
||||
copied_data = {k: v for k, v in copied_data.items() if v is not None}
|
||||
# Tool-result caching distinguishes "explicitly enabled" from the
|
||||
# field default via model_fields_set; don't let the dump turn the
|
||||
# default into an explicit opt-in on the copy. An agent that opted
|
||||
# in at construction via an explicit cache_handler (excluded from
|
||||
# the dump) must stay opted in — carry the consent as cache=True so
|
||||
# the copy wires its own fresh handler. A handler merely offered by
|
||||
# a crew at kickoff is runtime wiring, not consent, and must not
|
||||
# opt the copy in; _constructor_cache_opt_in is recorded before any
|
||||
# crew wiring can happen.
|
||||
if "cache" not in self.model_fields_set:
|
||||
copied_data.pop("cache", None)
|
||||
if self._constructor_cache_opt_in:
|
||||
copied_data["cache"] = True
|
||||
return type(self)(
|
||||
**copied_data,
|
||||
llm=existing_llm,
|
||||
|
||||
@@ -168,8 +168,11 @@ class Crew(FlowTrackable, BaseModel):
|
||||
manager_agent: Custom agent that will be used as manager.
|
||||
memory: Whether the crew should use memory to store memories of it's
|
||||
execution.
|
||||
cache: Whether the crew should use a cache to store the results of the
|
||||
tools execution.
|
||||
cache: Whether to cache tool results for the crew's agents. Off by
|
||||
default; when enabled, repeated calls to the same tool with
|
||||
identical arguments reuse the first result without re-executing —
|
||||
avoid enabling for live-data or state-mutating tools unless they
|
||||
gate writes with a cache_function.
|
||||
function_calling_llm: The language model that will run the tool calling
|
||||
for all the agents.
|
||||
process: The process flow that the crew will follow (e.g., sequential,
|
||||
@@ -216,7 +219,16 @@ class Crew(FlowTrackable, BaseModel):
|
||||
_kickoff_event_id: str | None = PrivateAttr(default=None)
|
||||
|
||||
name: str | None = Field(default="crew")
|
||||
cache: bool = Field(default=True)
|
||||
cache: bool = Field(
|
||||
default=False,
|
||||
description=(
|
||||
"Whether to cache tool results for the crew's agents. Opt-in: "
|
||||
"when enabled, repeated calls to the same tool with identical "
|
||||
"arguments return the first result without re-executing the "
|
||||
"tool — do not enable for live-data or state-mutating tools "
|
||||
"unless they set a cache_function that prevents caching."
|
||||
),
|
||||
)
|
||||
tasks: list[Task] = Field(default_factory=list)
|
||||
agents: Annotated[
|
||||
list[BaseAgent],
|
||||
@@ -1507,6 +1519,11 @@ class Crew(FlowTrackable, BaseModel):
|
||||
)
|
||||
self.manager_agent = manager
|
||||
manager.crew = self
|
||||
# The manager is created outside the agents loop that offers the
|
||||
# crew's cache handler at validation time; offer it here so an
|
||||
# opted-in crew (cache=True) also dedupes the manager's tool calls.
|
||||
if self.cache:
|
||||
manager.set_cache_handler(self._cache_handler)
|
||||
|
||||
def _get_execution_start_index(self, tasks: list[Task]) -> int | None:
|
||||
if self.checkpoint_kickoff_event_id is None:
|
||||
|
||||
@@ -859,6 +859,7 @@ def test_cache_hitting_between_agents(researcher, writer, ceo):
|
||||
crew = Crew(
|
||||
agents=[ceo, researcher],
|
||||
tasks=tasks,
|
||||
cache=True,
|
||||
)
|
||||
|
||||
with patch.object(CacheHandler, "read") as read:
|
||||
@@ -2246,7 +2247,9 @@ def test_tools_with_custom_caching():
|
||||
agent=writer2,
|
||||
)
|
||||
|
||||
crew = Crew(agents=[writer1, writer2], tasks=[task1, task2, task3, task4])
|
||||
crew = Crew(
|
||||
agents=[writer1, writer2], tasks=[task1, task2, task3, task4], cache=True
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
CacheHandler, "add", wraps=crew._cache_handler.add
|
||||
|
||||
263
lib/crewai/tests/test_tool_cache_default.py
Normal file
263
lib/crewai/tests/test_tool_cache_default.py
Normal file
@@ -0,0 +1,263 @@
|
||||
# mypy: ignore-errors
|
||||
"""Regression tests for EPD-180: tool-result caching used to be ON by default,
|
||||
so an LLM calling the same tool with identical arguments twice in one run got
|
||||
the first (possibly stale) result back without the tool executing — silently
|
||||
wrong for live-data tools, and silently dropped actions for stateful tools.
|
||||
|
||||
Caching is now opt-in: ``Crew(cache=True)`` for crews, ``Agent(cache=True)``
|
||||
(or an explicit ``cache_handler``) for standalone agents. The machinery —
|
||||
including per-tool ``cache_function`` write gating — is unchanged once opted
|
||||
in.
|
||||
|
||||
The end-to-end tests run fully offline: a fake OpenAI client scripts two
|
||||
identical tool calls followed by a final answer, mirroring the EPD-180
|
||||
clean-room repro.
|
||||
"""
|
||||
|
||||
from openai.types.chat import ChatCompletion
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from crewai import LLM, Agent, Crew, Task
|
||||
from crewai.agents.cache.cache_handler import CacheHandler
|
||||
from crewai.tools import BaseTool
|
||||
|
||||
|
||||
class LookupArgs(BaseModel):
|
||||
city: str = Field(description="City to look up.")
|
||||
|
||||
|
||||
def make_live_tool():
|
||||
"""A tool returning a different value on every real execution."""
|
||||
executions = []
|
||||
|
||||
class LiveLookupTool(BaseTool):
|
||||
name: str = "live_lookup"
|
||||
description: str = "Returns a live (time-varying) reading for a city."
|
||||
args_schema: type[BaseModel] = LookupArgs
|
||||
# cache_function deliberately NOT set — exercising the default.
|
||||
|
||||
def _run(self, city: str) -> str:
|
||||
executions.append(city)
|
||||
return f"reading #{len(executions)} for {city}"
|
||||
|
||||
return LiveLookupTool(), executions
|
||||
|
||||
|
||||
def make_scripted_llm():
|
||||
"""An offline LLM whose client scripts two identical tool calls."""
|
||||
|
||||
def tool_call_response(call_id: str):
|
||||
return {
|
||||
"index": 0,
|
||||
"finish_reason": "tool_calls",
|
||||
"message": {
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": call_id,
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "live_lookup",
|
||||
"arguments": '{"city": "paris"}',
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
scripted = [
|
||||
tool_call_response("call_1"),
|
||||
tool_call_response("call_2"), # identical name+args, new id
|
||||
{
|
||||
"index": 0,
|
||||
"finish_reason": "stop",
|
||||
"message": {"role": "assistant", "content": "Final answer: done."},
|
||||
},
|
||||
]
|
||||
|
||||
class FakeCompletions:
|
||||
def __init__(self):
|
||||
self.n = 0
|
||||
|
||||
def create(self, **params):
|
||||
choice = scripted[min(self.n, len(scripted) - 1)]
|
||||
self.n += 1
|
||||
return ChatCompletion.model_validate(
|
||||
{
|
||||
"id": f"chatcmpl-fake-{self.n}",
|
||||
"object": "chat.completion",
|
||||
"created": 1,
|
||||
"model": params.get("model", "gpt-4o"),
|
||||
"choices": [choice],
|
||||
"usage": {
|
||||
"prompt_tokens": 10,
|
||||
"completion_tokens": 5,
|
||||
"total_tokens": 15,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
class FakeClient:
|
||||
def __init__(self):
|
||||
self.chat = type("Chat", (), {"completions": FakeCompletions()})()
|
||||
|
||||
llm = LLM(model="openai/gpt-4o")
|
||||
llm._client = FakeClient()
|
||||
return llm
|
||||
|
||||
|
||||
def run_crew(**crew_kwargs):
|
||||
tool, executions = make_live_tool()
|
||||
agent = Agent(
|
||||
role="reader",
|
||||
goal="Look things up.",
|
||||
backstory="Test agent.",
|
||||
llm=make_scripted_llm(),
|
||||
tools=[tool],
|
||||
verbose=False,
|
||||
)
|
||||
task = Task(
|
||||
description="Look up paris twice and report.",
|
||||
expected_output="A report.",
|
||||
agent=agent,
|
||||
)
|
||||
crew = Crew(agents=[agent], tasks=[task], verbose=False, **crew_kwargs)
|
||||
crew.kickoff()
|
||||
return executions
|
||||
|
||||
|
||||
class TestToolCachingIsOptIn:
|
||||
def test_default_reexecutes_identical_tool_calls(self):
|
||||
"""EPD-180: with no opt-in, both identical calls must really execute."""
|
||||
executions = run_crew()
|
||||
assert len(executions) == 2
|
||||
|
||||
def test_crew_cache_true_dedupes_identical_tool_calls(self):
|
||||
"""Opting in via Crew(cache=True) restores the dedup behavior."""
|
||||
executions = run_crew(cache=True)
|
||||
assert len(executions) == 1
|
||||
|
||||
|
||||
class TestAgentCacheWiring:
|
||||
def _agent(self, **kwargs) -> Agent:
|
||||
return Agent(
|
||||
role="reader",
|
||||
goal="Look things up.",
|
||||
backstory="Test agent.",
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
def test_standalone_agent_has_no_cache_by_default(self):
|
||||
agent = self._agent()
|
||||
assert agent.tools_handler.cache is None
|
||||
assert agent.cache_handler is None
|
||||
|
||||
def test_standalone_agent_explicit_cache_true_opts_in(self):
|
||||
agent = self._agent(cache=True)
|
||||
assert agent.tools_handler.cache is not None
|
||||
assert agent.cache_handler is not None
|
||||
|
||||
def test_standalone_agent_explicit_cache_handler_opts_in(self):
|
||||
handler = CacheHandler()
|
||||
agent = self._agent(cache_handler=handler)
|
||||
assert agent.tools_handler.cache is handler
|
||||
|
||||
def test_explicit_cache_false_stays_off_even_with_handler(self):
|
||||
agent = self._agent(cache=False, cache_handler=CacheHandler())
|
||||
assert agent.tools_handler.cache is None
|
||||
|
||||
def test_agents_accept_a_crew_offered_handler_by_default(self):
|
||||
"""``Crew(cache=True)`` offers its handler via set_cache_handler at
|
||||
kickoff; agents that didn't explicitly opt out must accept it."""
|
||||
agent = self._agent()
|
||||
assert agent.tools_handler.cache is None
|
||||
|
||||
handler = CacheHandler()
|
||||
agent.set_cache_handler(handler)
|
||||
assert agent.tools_handler.cache is handler
|
||||
|
||||
def test_agents_that_opted_out_refuse_a_crew_offered_handler(self):
|
||||
agent = self._agent(cache=False)
|
||||
agent.set_cache_handler(CacheHandler())
|
||||
assert agent.tools_handler.cache is None
|
||||
|
||||
def test_copy_of_default_agent_does_not_opt_in(self):
|
||||
"""copy() rebuilds from model_dump(), which includes the field
|
||||
default cache=True — that must not read as an explicit opt-in on
|
||||
the copy (Bugbot review finding on the original PR)."""
|
||||
copied = self._agent().copy()
|
||||
assert copied.tools_handler.cache is None
|
||||
assert copied.cache_handler is None
|
||||
|
||||
def test_copy_of_opted_in_agent_stays_opted_in(self):
|
||||
copied = self._agent(cache=True).copy()
|
||||
assert copied.tools_handler.cache is not None
|
||||
|
||||
def test_copy_of_handler_opted_in_agent_stays_opted_in(self):
|
||||
"""An explicit cache_handler is an opt-in too; copy() excludes the
|
||||
handler itself, but the consent must survive — the copy wires its
|
||||
own fresh handler (Bugbot review finding on the original PR)."""
|
||||
source = self._agent(cache_handler=CacheHandler())
|
||||
copied = source.copy()
|
||||
assert copied.tools_handler.cache is not None
|
||||
assert copied.tools_handler.cache is not source.tools_handler.cache
|
||||
|
||||
def test_copy_of_explicit_cache_false_with_handler_stays_off(self):
|
||||
copied = self._agent(cache=False, cache_handler=CacheHandler()).copy()
|
||||
assert copied.tools_handler.cache is None
|
||||
|
||||
def test_copy_of_crew_wired_agent_does_not_opt_in(self):
|
||||
"""A handler offered by a crew at kickoff (set_cache_handler) is
|
||||
runtime wiring, not construction-time consent — copies of such
|
||||
agents must not become standalone cachers (Bugbot review finding
|
||||
on the original PR)."""
|
||||
agent = self._agent()
|
||||
agent.set_cache_handler(CacheHandler()) # what Crew(cache=True) does
|
||||
assert agent.tools_handler.cache is not None
|
||||
|
||||
copied = agent.copy()
|
||||
assert copied.tools_handler.cache is None
|
||||
assert copied.cache_handler is None
|
||||
|
||||
|
||||
class TestHierarchicalManagerCacheWiring:
|
||||
"""The auto-created hierarchical manager is built outside the agents
|
||||
loop that offers the crew's cache handler; an opted-in crew must wire
|
||||
the manager too (Bugbot review finding on the original PR)."""
|
||||
|
||||
def _crew(self, **crew_kwargs) -> Crew:
|
||||
from crewai.process import Process
|
||||
|
||||
agent = Agent(role="worker", goal="Do work.", backstory="Test agent.")
|
||||
task = Task(description="Do the work.", expected_output="Done.")
|
||||
return Crew(
|
||||
agents=[agent],
|
||||
tasks=[task],
|
||||
process=Process.hierarchical,
|
||||
manager_llm="gpt-4o",
|
||||
**crew_kwargs,
|
||||
)
|
||||
|
||||
def test_manager_gets_crew_handler_when_cache_enabled(self):
|
||||
crew = self._crew(cache=True)
|
||||
crew._create_manager_agent()
|
||||
assert crew.manager_agent.tools_handler.cache is crew._cache_handler
|
||||
|
||||
def test_manager_has_no_cache_when_crew_did_not_opt_in(self):
|
||||
crew = self._crew()
|
||||
crew._create_manager_agent()
|
||||
assert crew.manager_agent.tools_handler.cache is None
|
||||
|
||||
def test_user_provided_manager_with_cache_false_stays_excluded(self):
|
||||
manager = Agent(
|
||||
role="manager",
|
||||
goal="Manage.",
|
||||
backstory="Test manager.",
|
||||
cache=False,
|
||||
allow_delegation=True,
|
||||
)
|
||||
crew = self._crew(cache=True)
|
||||
crew.manager_agent = manager
|
||||
crew._create_manager_agent()
|
||||
assert manager.tools_handler.cache is None
|
||||
Reference in New Issue
Block a user