diff --git a/lib/crewai/src/crewai/llms/providers/openai/completion.py b/lib/crewai/src/crewai/llms/providers/openai/completion.py index 80b324c6d..cb4658823 100644 --- a/lib/crewai/src/crewai/llms/providers/openai/completion.py +++ b/lib/crewai/src/crewai/llms/providers/openai/completion.py @@ -431,11 +431,7 @@ class OpenAICompletion(BaseLLM): "api_key": self.api_key, "organization": self.organization, "project": self.project, - "base_url": self.base_url - or self.api_base - or os.getenv("OPENAI_BASE_URL") - or os.getenv("OPENAI_API_BASE") - or None, + "base_url": self._resolved_base_url(), "timeout": self.timeout, "max_retries": self.max_retries, "default_headers": self.default_headers, @@ -1813,6 +1809,22 @@ class OpenAICompletion(BaseLLM): ) return param, message.lower() + def _resolved_base_url(self) -> str | None: + """The endpoint this instance actually talks to, or None for OpenAI's own. + + Environment variables count: an instance can be pointed at a proxy without + either field being set. Single source of truth for both the client config + and the learned-behaviour cache keys, which must agree on what "endpoint" + means or a proxy ends up sharing api.openai.com's cache. + """ + return ( + self.base_url + or self.api_base + or os.getenv("OPENAI_BASE_URL") + or os.getenv("OPENAI_API_BASE") + or None + ) + def _model_cache_key(self, model: str | None = None) -> tuple[str, str]: """Identity for the learned-behaviour caches: the endpoint plus the model. @@ -1820,8 +1832,10 @@ class OpenAICompletion(BaseLLM): OpenAI-compatible proxy with different capabilities, so a conflict learned against one endpoint must not silently apply to the other. """ - endpoint = self.base_url or self.api_base or "https://api.openai.com/v1" - return endpoint, model if model is not None else self.model + return ( + self._resolved_base_url() or "https://api.openai.com/v1", + model if model is not None else self.model, + ) def _is_recoverable_completion_error(self, error: BaseException) -> bool: """Whether ``_call_completions`` will retry instead of surfacing this error. @@ -2130,6 +2144,12 @@ class OpenAICompletion(BaseLLM): params["messages"], content, from_agent ) except NotFoundError as e: + # `_call_completions` retries a responses-only model on the Responses + # API. Reporting a failed call here would surface an error the user + # never experiences, and the ValueError wrapper would also hide the + # NotFoundError that the retry needs in order to recognize it. + if self._is_recoverable_completion_error(e): + raise error_msg = self._model_not_found_message(e) logging.error(error_msg) self._emit_call_failed_event( @@ -2559,6 +2579,12 @@ class OpenAICompletion(BaseLLM): if usage.get("total_tokens", 0) > 0: logging.info(f"OpenAI API usage: {usage}") except NotFoundError as e: + # `_call_completions` retries a responses-only model on the Responses + # API. Reporting a failed call here would surface an error the user + # never experiences, and the ValueError wrapper would also hide the + # NotFoundError that the retry needs in order to recognize it. + if self._is_recoverable_completion_error(e): + raise error_msg = self._model_not_found_message(e) logging.error(error_msg) self._emit_call_failed_event( diff --git a/lib/crewai/tests/llms/openai/test_responses_only_models.py b/lib/crewai/tests/llms/openai/test_responses_only_models.py index 7a01ab0b8..45420dade 100644 --- a/lib/crewai/tests/llms/openai/test_responses_only_models.py +++ b/lib/crewai/tests/llms/openai/test_responses_only_models.py @@ -14,6 +14,8 @@ retried on the Responses API, then the model is remembered so the wasted round t is paid once per process. """ +from types import SimpleNamespace + import httpx import pytest from openai import NotFoundError @@ -206,6 +208,85 @@ class TestEffectiveApi: assert proxy._effective_api() == "completions" +class TestEndpointResolution: + """The cache key must agree with the endpoint the client actually uses.""" + + def test_env_configured_proxy_gets_its_own_key(self, monkeypatch): + """An instance can be pointed at a proxy purely through the environment. + + `_get_client_params` honours OPENAI_BASE_URL, so a key that stopped at the + base_url/api_base fields would file a proxy under api.openai.com -- the + exact leakage the tuple key exists to prevent. + """ + monkeypatch.setenv("OPENAI_BASE_URL", "https://proxy.internal/v1") + llm = build("gpt-5-pro") + + assert llm._model_cache_key() == ("https://proxy.internal/v1", "gpt-5-pro") + assert llm._get_client_params()["base_url"] == "https://proxy.internal/v1" + + def test_explicit_base_url_wins_over_env(self, monkeypatch): + monkeypatch.setenv("OPENAI_BASE_URL", "https://proxy.internal/v1") + llm = build("gpt-5-pro", base_url="https://explicit.internal/v1") + + assert llm._model_cache_key()[0] == "https://explicit.internal/v1" + + def test_defaults_to_openai(self, monkeypatch): + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("OPENAI_API_BASE", raising=False) + + assert build("gpt-5-pro")._model_cache_key() == ( + "https://api.openai.com/v1", + "gpt-5-pro", + ) + + def test_env_proxy_does_not_inherit_openai_learning(self, monkeypatch): + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("OPENAI_API_BASE", raising=False) + real = build("gpt-5-pro") + real._remember_responses_only_model() + assert real._effective_api() == "responses" + + # Same model string, different endpoint -> must not inherit the learning. + monkeypatch.setenv("OPENAI_BASE_URL", "https://proxy.internal/v1") + assert build("gpt-5-pro")._effective_api() == "completions" + + +class TestFailureReportingSuppressed: + """A recovered 404 must not surface as a failed call.""" + + def test_not_found_handler_defers_to_the_retry(self, monkeypatch): + """NotFoundError is caught before the generic handler. + + Without a guard there it logs, emits LLMCallFailedEvent, and re-raises as + ValueError -- which both reports a failure the user never sees and hides + the NotFoundError the retry needs to recognize. + """ + llm = build("gpt-5-pro") + emitted: list[str] = [] + monkeypatch.setattr( + llm, + "_emit_call_failed_event", + lambda **kwargs: emitted.append(kwargs.get("error", "")), + ) + error = make_not_found(RESPONSES_ONLY_MESSAGES[0]) + + class FakeCompletions: + def create(self, **kwargs): + raise error + + monkeypatch.setattr( + llm, + "_get_sync_client", + lambda: SimpleNamespace(chat=SimpleNamespace(completions=FakeCompletions())), + ) + + # The original NotFoundError must survive, not a ValueError wrapper. + with pytest.raises(NotFoundError): + llm._handle_completion({"model": "gpt-5-pro", "messages": MESSAGES}) + + assert emitted == [] + + class TestNotFoundMessage: @pytest.mark.parametrize("message", RESPONSES_ONLY_MESSAGES) def test_points_at_responses_api(self, message: str):