diff --git a/strix/core/execution.py b/strix/core/execution.py index dfcd39fa..4cbc9ac3 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -11,7 +11,7 @@ from functools import cache from typing import TYPE_CHECKING, Any, cast from agents import RunConfig, Runner -from agents.exceptions import AgentsException, MaxTurnsExceeded, UserError +from agents.exceptions import AgentsException, MaxTurnsExceeded, ModelRefusalError, UserError from agents.sandbox.errors import ExecTransportError from openai import ( APIConnectionError, @@ -72,6 +72,20 @@ class ProviderRefusalError(AgentsException): """Raised when a provider returns a structured refusal instead of an exception.""" +def _refusal_text(exc: BaseException) -> str | None: + """The refusal text of a provider refusal, however the SDK surfaced it. + + A refusal can reach here as this module's own `ProviderRefusalError`, built + from a structured `refusal` content item, or as the SDK's own + `ModelRefusalError`, which some providers (observed with Gemini through + LiteLLM) raise directly instead of returning refusal content. Both carry + the same information and deserve the same one-line, non-retryable outcome. + """ + if isinstance(exc, ProviderRefusalError | ModelRefusalError): + return str(exc.refusal if isinstance(exc, ModelRefusalError) else exc) + return None + + def _structured_provider_refusal(result: Any) -> str | None: for item in getattr(result, "new_items", ()) or (): raw_item = getattr(item, "raw_item", None) @@ -130,7 +144,9 @@ def _model_error_status_code(exc: BaseException) -> int | None: def _is_transient_model_error(exc: BaseException) -> bool: - if codex.is_content_guardrail_error(exc): + if codex.is_content_guardrail_error(exc) or isinstance( + exc, ProviderRefusalError | ModelRefusalError + ): return False if isinstance( exc, APITimeoutError | APIConnectionError | TimeoutError | ConnectionError | OSError @@ -790,9 +806,9 @@ async def _run_cycle( # noqa: PLR0912, PLR0915 continue if session is not None: await _salvage_stream_to_session(session, pre_run_items, stream, agent_id) - if isinstance(exc, ProviderRefusalError): - logger.warning("agent %s refused by the model provider: %s", agent_id, exc) - await coordinator.set_status(agent_id, "failed", error=str(exc)) + if (refusal := _refusal_text(exc)) is not None: + logger.warning("agent %s refused by the model provider: %s", agent_id, refusal) + await coordinator.set_status(agent_id, "failed", error=refusal) await notify_parent_on_terminal(coordinator, agent_id, "failed") return None if isinstance(exc, MaxTurnsExceeded): diff --git a/tests/test_execution.py b/tests/test_execution.py index 8fbf18ff..f141fc95 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -9,7 +9,7 @@ from typing import Any, cast from unittest.mock import MagicMock import pytest -from agents.exceptions import MaxTurnsExceeded +from agents.exceptions import MaxTurnsExceeded, ModelRefusalError from agents.items import MessageOutputItem from agents.memory import SQLiteSession from agents.tool_context import ToolContext @@ -29,6 +29,26 @@ from strix.tools.finish.tool import finish_scan _NO_STREAM_EVENTS: list[Any] = [] +class _SdkRefusalStream: + """A stream where the SDK itself raises ModelRefusalError, not a content item. + + Some providers (observed with Gemini through LiteLLM) surface a content- + filter refusal this way instead of the structured `refusal` content item + that OpenAI-compatible providers use. + """ + + def __init__(self, refusal: str) -> None: + self.run_loop_exception: BaseException | None = ModelRefusalError(refusal) + self.new_items: list[Any] = [] + + async def stream_events(self) -> Any: + for event in _NO_STREAM_EVENTS: + yield event + + def cancel(self, mode: str = "immediate") -> None: # noqa: ARG002 + return + + class _StructuredRefusalStream: def __init__(self, refusal: str) -> None: self.run_loop_exception: BaseException | None = None @@ -898,6 +918,48 @@ async def test_structured_provider_refusal_fails_noninteractive_child( session.close() +@pytest.mark.asyncio +async def test_sdk_model_refusal_fails_cleanly_without_a_traceback( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A ModelRefusalError raised by the SDK gets the same clear, non-retried + outcome as this module's own ProviderRefusalError, instead of falling + through to the generic crash path with a full traceback. + """ + refusal = "Response withheld by the provider's content filter." + stream = _SdkRefusalStream(refusal) + monkeypatch.setattr( + "strix.core.execution.Runner.run_streamed", lambda *_args, **_kwargs: stream + ) + coordinator = AgentCoordinator() + await coordinator.register("root", "strix", parent_id=None) + + with caplog.at_level("WARNING", logger="strix.core.execution"): + result = await execution._run_cycle( + MagicMock(), + coordinator, + "root", + input_data="task", + run_config=MagicMock(), + context={}, + max_turns=5, + session=None, + interactive=True, + event_sink=None, + hooks=None, + ) + + assert result is None + assert coordinator.statuses["root"] == "failed" + assert coordinator.errors["root"] == refusal + assert any( + record.levelname == "WARNING" and refusal in record.getMessage() + for record in caplog.records + ) + assert not any(record.exc_info for record in caplog.records) + + @pytest.mark.asyncio async def test_crashing_noninteractive_child_settles_and_wakes_its_parent( monkeypatch: pytest.MonkeyPatch,