Files
crewAI/lib/crewai/tests/test_tool_cache_default.py
João Moura d729cade6b fix(openai): surface gateway errors reported inside an HTTP 200 (#7342)
* fix(openai): surface gateway errors reported inside an HTTP 200

OpenAI-compatible gateways commit `200 OK` as soon as the upstream provider
accepts a request, so a later provider failure arrives in the body as an
`error` object with no `choices`. That reached the SDK's parse helper and
surfaced as `TypeError: 'NoneType' object is not iterable`, naming neither the
provider, the status, nor the fact that a timeout happened.

The four non-streaming paths now inspect the raw body before parsing and raise
the exception the upstream code maps to, so a masked 504 is catchable exactly
like an honest one. Streaming already had this guard inside the SDK.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(openai): teach the tool-cache fake about with_raw_response

The provider now reads the raw body before parsing, so a client double that
only implements `create` no longer satisfies it. Same shape as the fixes to the
reasoning-effort retry and Snowflake doubles.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(tracing): reset the TraceCollectionListener singleton between tests

TraceCollectionListener caches a TraceBatchManager on the class and
`_initialized` short-circuits `__init__`, so batch state survives for the whole
xdist worker. `test_nested_agent_executor_flow_does_not_finalize_parent_batch`
left `trace_batch_id="debug-trace-batch"` behind, which moved every later trace
POST from /tracing/ephemeral/batches to /tracing/batches/<id>/events. The
recorded cassette then stopped matching, the agent retried, and the second call
found the cassette consumed -- surfacing as ConnectionError in an unrelated
test hundreds of tests later.

Reproduced deterministically by running the leaking test followed by
tests/tracing/test_trace_enable_disable.py::test_trace_calls_when_enabled_via_env;
fails on a68b5e903 too, so this predates the gateway fix it was blocking.

An autouse fixture now clears the cached instance after each test. Two canaries
pin the invariant and fail without the fixture.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(tracing): drop the unwritable _listeners_setup canary

Both review bots flagged that the canary read `_listeners_setup` off the class,
where it is always False, so it could never fail. Correct, and the suggested fix
does not work either: `BaseEventListener.__init__` calls `setup_listeners`
(base_event_listener.py:16), which sets the flag on the instance
(trace_listener.py:229), so reading it back through `TraceCollectionListener()`
is always True. Neither read observes a leak, so the canary is deleted rather
than replaced, with the reasoning recorded so it is not re-added.

The same finding showed the fixture was resetting two class attributes that are
never assigned at class level. Only dropping `_instance` is load-bearing, so the
fixture is now one line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tracing): correct why the _listeners_setup canary is unwritable

setup_listeners returns early when tracing is off and no override applies
(trace_listener.py:213-220), assigning the flag at :229 only when it actually
registers. Construction therefore does not always set it, as the previous note
claimed: with tracing disabled the flag never even reaches the instance dict.

The instance read reports ambient tracing state rather than isolation, which is
a better reason not to assert on it than the one recorded before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(tests): clear trace batch state in place instead of dropping the singleton

Dropping `TraceCollectionListener._instance` made the next construction re-run
`setup_listeners`, re-registering its handlers on the event bus. That broke
tests/telemetry/test_task_failure_instrumentation.py, which requires exactly one
handler per event: the re-registered `on_task_failed` made two. Verified against
a53ecc17f, where the same sequence passes -- the regression was mine.

The leak that needed fixing was batch state, not registration, so the fixture
now clears the manager's batch fields in place. Handler cleanup already belongs
to `cleanup_event_handlers`, and `first_time_handler` keeps its reference to the
same manager object.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(tests): clear tracing context vars so the listener can re-register

Bugbot flagged that keeping the singleton leaves `_listeners_setup` set, so after
`cleanup_event_handlers` wipes the bus `setup_listeners` returns early
(trace_listener.py:208) and tracing silently registers nothing for the rest of
the worker. Confirmed: after a tracing-enabled run, re-running setup restores 0
of 119 handler entries.

Dropping the singleton fixes that but previously broke
test_task_failure_instrumentation. The real cause was a third leak: the
`_tracing_enabled` context var stayed set, so the replacement listener still
believed tracing was on and re-registered `on_task_failed` next to telemetry's.
Clearing the context vars is what makes replacing the listener safe, so the
fixture now does both, and a canary pins it (fails with `assert True is False`
without the drop).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 15:12:57 -07:00

282 lines
9.9 KiB
Python

# 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.
"""
import json
from types import SimpleNamespace
import httpx
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
@property
def with_raw_response(self):
"""The provider reads the raw body to spot upstream errors that a
gateway reported inside an HTTP 200."""
return self
def create(self, **params):
choice = scripted[min(self.n, len(scripted) - 1)]
self.n += 1
payload = {
"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,
},
}
return SimpleNamespace(
text=json.dumps(payload),
parse=lambda: ChatCompletion.model_validate(payload),
http_response=httpx.Response(
200,
request=httpx.Request(
"POST", "https://api.openai.com/v1/chat/completions"
),
),
)
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