From 4fdb7f2bfbbd696927130e2839ea48a3c79ad2ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moura?= Date: Fri, 10 Jul 2026 21:37:36 -0300 Subject: [PATCH] fix(tools)!: make tool-result caching opt-in instead of on by default (#6509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * 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 * 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 --------- Co-authored-by: Claude Fable 5 --- lib/crewai/src/crewai/agent/core.py | 27 +- .../crewai/agents/agent_builder/base_agent.py | 29 +- lib/crewai/src/crewai/crew.py | 23 +- lib/crewai/tests/test_crew.py | 5 +- lib/crewai/tests/test_tool_cache_default.py | 263 ++++++++++++++++++ 5 files changed, 337 insertions(+), 10 deletions(-) create mode 100644 lib/crewai/tests/test_tool_cache_default.py diff --git a/lib/crewai/src/crewai/agent/core.py b/lib/crewai/src/crewai/agent/core.py index 4f4c2368c..4c20693a8 100644 --- a/lib/crewai/src/crewai/agent/core.py +++ b/lib/crewai/src/crewai/agent/core.py @@ -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.""" diff --git a/lib/crewai/src/crewai/agents/agent_builder/base_agent.py b/lib/crewai/src/crewai/agents/agent_builder/base_agent.py index a12a4c18b..4530f9527 100644 --- a/lib/crewai/src/crewai/agents/agent_builder/base_agent.py +++ b/lib/crewai/src/crewai/agents/agent_builder/base_agent.py @@ -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, diff --git a/lib/crewai/src/crewai/crew.py b/lib/crewai/src/crewai/crew.py index 7d4a5107d..696d831c8 100644 --- a/lib/crewai/src/crewai/crew.py +++ b/lib/crewai/src/crewai/crew.py @@ -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: diff --git a/lib/crewai/tests/test_crew.py b/lib/crewai/tests/test_crew.py index 90d648711..0195112cb 100644 --- a/lib/crewai/tests/test_crew.py +++ b/lib/crewai/tests/test_crew.py @@ -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 diff --git a/lib/crewai/tests/test_tool_cache_default.py b/lib/crewai/tests/test_tool_cache_default.py new file mode 100644 index 000000000..7a503d269 --- /dev/null +++ b/lib/crewai/tests/test_tool_cache_default.py @@ -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