From e7c311acb9a5f7bfe12848f05aaac04565ffcb6c Mon Sep 17 00:00:00 2001 From: Samir Hanna Verza Date: Wed, 26 Aug 2026 09:33:45 -0300 Subject: [PATCH 1/4] fix(retry): treat provider usage-limit errors as terminal, not transient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A plan/subscription usage-limit rejection (e.g. ChatGPT's 5-hour or weekly cap, HTTP 429 `usage_limit_reached`) was classified as an ordinary transient throttle and retried by both retry layers — the SDK ModelSettings retry (`DEFAULT_MODEL_RETRY`, up to 5x) and Strix's turn replay (`_is_transient_model_error` -> `_run_cycle`, up to 5x). Retrying cannot succeed until the window resets (often hours away); it only burns more of the same exhausted quota and spends minutes thrashing exponential backoff, flooding the log with `openai.agents: Error streaming response` before the run finally stops. Classify usage-limit errors as terminal so the run fails fast and lands on the existing resumable stop path (`run_strix_scan` catches the RateLimitError, logs the `strix --resume ` hint, and stops the root): - add `codex.is_usage_limit_error()` (text-marker predicate, mirroring `is_content_guardrail_error`); - exclude it from `_is_transient_model_error` (Strix replay layer); - exclude it from the SDK retry policy via a small wrapper (`_default_model_retry_policy`). Ordinary 429 throttles are unchanged and still retried. Adds regression tests. Co-Authored-By: Claude Opus 4.8 --- strix/config/codex.py | 15 +++++++++++++++ strix/config/models.py | 22 +++++++++++++++++----- strix/core/execution.py | 2 ++ tests/test_execution_transient_retry.py | 20 ++++++++++++++++++++ tests/test_model_retry.py | 9 +++++++++ 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/strix/config/codex.py b/strix/config/codex.py index cf34f003d3..1f87e600e0 100644 --- a/strix/config/codex.py +++ b/strix/config/codex.py @@ -160,6 +160,21 @@ def is_content_guardrail_error(exc: BaseException) -> bool: return any(marker in text for marker in _GUARDRAIL_MARKERS) +_USAGE_LIMIT_MARKERS = ( + "usage_limit_reached", + "usage limit has been reached", +) + + +def is_usage_limit_error(exc: BaseException) -> bool: + """A plan/subscription usage window is exhausted (e.g. ChatGPT's 5-hour or + weekly cap), not an ordinary throttle. Terminal for the run: retrying only + burns more of the same exhausted quota and cannot succeed until it resets, + so the run should stop resumably instead of thrashing the backoff.""" + text = str(exc).lower() + return any(marker in text for marker in _USAGE_LIMIT_MARKERS) + + def _b64url(raw: bytes) -> str: return base64.urlsafe_b64encode(raw).rstrip(b"=").decode("ascii") diff --git a/strix/config/models.py b/strix/config/models.py index f6848ca4ce..a7b5d51f32 100644 --- a/strix/config/models.py +++ b/strix/config/models.py @@ -496,6 +496,15 @@ def get_model(self, model_name: str | None) -> Model: ) +def _not_usage_limit_error(context: RetryPolicyContext) -> bool: + # A usage-limit rejection (a plan/subscription window is exhausted, e.g. + # ChatGPT's 5-hour or weekly cap) is terminal for the run: retrying only + # burns more of the same exhausted quota and cannot succeed until it resets. + # Composed with ``all`` below so the run fails fast and stops resumably + # instead of thrashing the backoff on a limit that will not clear for hours. + return not codex.is_usage_limit_error(context.error) + + DEFAULT_MODEL_RETRY = ModelRetrySettings( max_retries=5, backoff=ModelRetryBackoffSettings( @@ -504,11 +513,14 @@ def get_model(self, model_name: str | None) -> Model: multiplier=2.0, jitter=False, ), - policy=retry_policies.any( - retry_policies.provider_suggested(), - retry_policies.network_error(), - retry_policies.http_status((429, 500, 502, 503, 504)), - _retry_statusless_provider_errors, + policy=retry_policies.all( + retry_policies.any( + retry_policies.provider_suggested(), + retry_policies.network_error(), + retry_policies.http_status((429, 500, 502, 503, 504)), + _retry_statusless_provider_errors, + ), + _not_usage_limit_error, ), ) diff --git a/strix/core/execution.py b/strix/core/execution.py index 9f2674d2c4..b61cd205ef 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -132,6 +132,8 @@ def _model_error_status_code(exc: BaseException) -> int | None: def _is_transient_model_error(exc: BaseException) -> bool: if codex.is_content_guardrail_error(exc): return False + if codex.is_usage_limit_error(exc): + return False if isinstance( exc, APITimeoutError | APIConnectionError | TimeoutError | ConnectionError | OSError ): diff --git a/tests/test_execution_transient_retry.py b/tests/test_execution_transient_retry.py index d81e4e7003..c64472fbb3 100644 --- a/tests/test_execution_transient_retry.py +++ b/tests/test_execution_transient_retry.py @@ -172,3 +172,23 @@ async def test_run_cycle_does_not_retry_permanent_error( streams = [_FakeStream(exc=bad_request), _FakeStream()] with pytest.raises(BadRequestError): await _run_once(monkeypatch, streams) + + +def test_usage_limit_error_is_terminal_not_transient() -> None: + usage_limited = RateLimitError( + "Error code: 429 - {'error': {'type': 'usage_limit_reached', " + "'message': 'The usage limit has been reached'}}", + response=httpx.Response(429, request=_request()), + body={"type": "usage_limit_reached"}, + ) + assert codex.is_usage_limit_error(usage_limited) is True + # A usage-limit rejection must not be retried like an ordinary throttle. + assert execution._is_transient_model_error(usage_limited) is False + + +def test_ordinary_rate_limit_is_not_usage_limit() -> None: + rate_limited = RateLimitError( + "slow down", response=httpx.Response(429, request=_request()), body=None + ) + assert codex.is_usage_limit_error(rate_limited) is False + assert execution._is_transient_model_error(rate_limited) is True diff --git a/tests/test_model_retry.py b/tests/test_model_retry.py index 4cbffbcd18..60ffe69d6a 100644 --- a/tests/test_model_retry.py +++ b/tests/test_model_retry.py @@ -88,3 +88,12 @@ def test_policy_helper_matches_statusless_only() -> None: ) is False ) + + +def test_usage_limit_error_is_not_retried() -> None: + # A usage-limit rejection carries a 429, but the window will not clear for + # hours; retrying only burns more of the same quota, so it must be terminal + # even though an ordinary 429 is retried. + usage_limited = RuntimeError("Error code: 429 - {'error': {'type': 'usage_limit_reached'}}") + assert codex.is_usage_limit_error(usage_limited) is True + assert _retries(ModelRetryNormalizedError(status_code=429), error=usage_limited) is False From 8598f6a735218414dd57789fe228ddca539aeea1 Mon Sep 17 00:00:00 2001 From: Samir Hanna Verza Date: Wed, 26 Aug 2026 10:49:24 -0300 Subject: [PATCH 2/4] fix(retry): route non-OpenAI usage-limit errors to the resumable stop Follow-up to the usage-limit terminal classification (review feedback): retry suppression is provider-agnostic (text-marker based), but run_strix_scan only mapped openai.RateLimitError to the resumable stop. A LiteLLM-routed provider that surfaces a usage-limit as a different exception type therefore reached the generic failure path (root marked failed, re-raised) instead of the promised resumable stop with the `strix --resume` hint. Route any error where codex.is_usage_limit_error(exc) is true to the same resumable stop as RateLimitError. Adds a regression test covering a non-OpenAI (LiteLLM-routed) usage-limit exception. Co-Authored-By: Claude Opus 4.8 --- strix/core/runner.py | 38 +++++++++++------ tests/test_runner_rate_limit.py | 72 +++++++++++++++++++++++++-------- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/strix/core/runner.py b/strix/core/runner.py index ee996cd3ef..cc09a0830b 100644 --- a/strix/core/runner.py +++ b/strix/core/runner.py @@ -18,7 +18,7 @@ from strix.agents.factory import build_strix_agent, make_child_factory from strix.agents.prompt import render_system_prompt -from strix.config import load_settings +from strix.config import codex, load_settings from strix.config.models import ( StrixProvider, configure_sdk_model_defaults, @@ -521,24 +521,36 @@ async def spawn_child_agent(**kwargs: Any) -> dict[str, Any]: with contextlib.suppress(Exception): await coordinator.set_status(root_id, "stopped") return None - except RateLimitError as exc: - logger.warning( - "Scan %s stopped: persistent rate limit from the LLM provider (%s). " - "Resume with 'strix --resume %s' once the limit clears.", - scan_id, - exc, - scan_id, - ) - if root_id is not None: - with contextlib.suppress(Exception): - await coordinator.set_status(root_id, "stopped") - return None except (asyncio.CancelledError, KeyboardInterrupt): logger.info("Scan %s interrupted by the user", scan_id) if root_id is not None: with contextlib.suppress(Exception): await coordinator.set_status(root_id, "running") raise + except Exception as exc: + # A usage-limit rejection is resumable regardless of the provider's + # exception class: OpenAI raises RateLimitError, but a LiteLLM-routed + # provider may surface the same exhausted-window error as a different + # type. Both retry layers already stop retrying it (see + # codex.is_usage_limit_error), so route it to the same resumable stop + # instead of the generic failure path. + if isinstance(exc, RateLimitError) or codex.is_usage_limit_error(exc): + logger.warning( + "Scan %s stopped: persistent rate limit from the LLM provider (%s). " + "Resume with 'strix --resume %s' once the limit clears.", + scan_id, + exc, + scan_id, + ) + if root_id is not None: + with contextlib.suppress(Exception): + await coordinator.set_status(root_id, "stopped") + return None + logger.exception("Strix scan %s failed", scan_id) + if root_id is not None: + with contextlib.suppress(Exception): + await coordinator.set_status(root_id, "failed") + raise except BaseException: logger.exception("Strix scan %s failed", scan_id) if root_id is not None: diff --git a/tests/test_runner_rate_limit.py b/tests/test_runner_rate_limit.py index 3110ae2cb0..b9eb504dd8 100644 --- a/tests/test_runner_rate_limit.py +++ b/tests/test_runner_rate_limit.py @@ -1,4 +1,4 @@ -"""Tests for graceful handling of persistent RateLimitError in run_strix_scan.""" +"""Tests for graceful handling of persistent usage/rate-limit errors in run_strix_scan.""" from __future__ import annotations @@ -24,11 +24,22 @@ def _make_rate_limit_error() -> RateLimitError: return RateLimitError("rate limited", response=response, body=None) -@pytest.mark.asyncio -async def test_persistent_rate_limit_stops_gracefully( - monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture -) -> None: - """A persistent RateLimitError stops the scan (root -> 'stopped') without raising.""" +class _ProviderUsageLimitError(Exception): + """A non-OpenAI (e.g. LiteLLM-routed) provider exception whose text carries the + usage-limit marker but which is *not* an ``openai.RateLimitError``.""" + + +def _make_litellm_usage_limit_error() -> Exception: + return _ProviderUsageLimitError( + "litellm.RateLimitError: 429 - {'error': {'type': 'usage_limit_reached', " + "'message': 'The usage limit has been reached'}}" + ) + + +async def _run_scan_raising( + monkeypatch: pytest.MonkeyPatch, tmp_path: Any, error: BaseException +) -> tuple[Any, AgentCoordinator]: + """Drive run_strix_scan with a fully mocked scan whose agent loop raises ``error``.""" monkeypatch.setattr(runner, "run_dir_for", lambda _scan_id: tmp_path) monkeypatch.setattr(runner, "runtime_state_dir", lambda _run_dir: tmp_path) monkeypatch.setattr(runner, "setup_scan_logging", lambda _run_dir: lambda: None) @@ -70,21 +81,24 @@ async def _cleanup(*_args: Any, **_kwargs: Any) -> None: monkeypatch.setattr(runner, "make_child_factory", lambda **_kwargs: lambda **_k: object()) monkeypatch.setattr(runner, "open_agent_session", lambda _root_id, _db: object()) - async def _raise_rate_limit(*_args: Any, **_kwargs: Any) -> None: - raise _make_rate_limit_error() + async def _raise(*_args: Any, **_kwargs: Any) -> None: + raise error - monkeypatch.setattr(runner, "run_agent_loop", _raise_rate_limit) + monkeypatch.setattr(runner, "run_agent_loop", _raise) coordinator = AgentCoordinator() + result = await runner.run_strix_scan( + scan_config={"targets": [], "scan_mode": "deep"}, + scan_id="scan-test", + image="img", + coordinator=coordinator, + ) + return result, coordinator - with caplog.at_level(logging.WARNING): - result = await runner.run_strix_scan( - scan_config={"targets": [], "scan_mode": "deep"}, - scan_id="scan-test", - image="img", - coordinator=coordinator, - ) +def _assert_stopped_resumably( + result: Any, coordinator: AgentCoordinator, caplog: pytest.LogCaptureFixture +) -> None: assert result is None root_ids = [aid for aid, parent in coordinator.parent_of.items() if parent is None] assert len(root_ids) == 1 @@ -92,3 +106,29 @@ async def _raise_rate_limit(*_args: Any, **_kwargs: Any) -> None: # the resume hint must carry the real scan id, not a literal placeholder assert "strix --resume scan-test" in caplog.text assert "" not in caplog.text + + +@pytest.mark.asyncio +async def test_persistent_rate_limit_stops_gracefully( + monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture +) -> None: + """A persistent OpenAI RateLimitError stops the scan (root -> 'stopped') without raising.""" + with caplog.at_level(logging.WARNING): + result, coordinator = await _run_scan_raising( + monkeypatch, tmp_path, _make_rate_limit_error() + ) + _assert_stopped_resumably(result, coordinator, caplog) + + +@pytest.mark.asyncio +async def test_persistent_usage_limit_non_openai_stops_gracefully( + monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture +) -> None: + """A usage-limit error from a non-OpenAI (LiteLLM-routed) provider — not an + ``openai.RateLimitError`` — must also land on the resumable stop path, not the + generic failure path.""" + with caplog.at_level(logging.WARNING): + result, coordinator = await _run_scan_raising( + monkeypatch, tmp_path, _make_litellm_usage_limit_error() + ) + _assert_stopped_resumably(result, coordinator, caplog) From 7570363318deb8e6875fa7bb2fc3c72204151685 Mon Sep 17 00:00:00 2001 From: Samir Hanna Verza <123646616+samirhvbr@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:02:38 -0300 Subject: [PATCH 3/4] test(retry): include telemetry settings in runner fixture The current upstream runner reads run_record_timings while constructing usage hooks. Its rate-limit test fixture predates that setting and raises AttributeError before reaching the mocked agent loop. Complete the shared fixture so both the existing rate-limit test and the non-OpenAI usage-limit regression exercise their intended paths. --- tests/test_runner_rate_limit.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_runner_rate_limit.py b/tests/test_runner_rate_limit.py index 362b3c2572..c54a085aff 100644 --- a/tests/test_runner_rate_limit.py +++ b/tests/test_runner_rate_limit.py @@ -57,6 +57,7 @@ async def _run_scan_raising( extra_body=None, ), runtime=types.SimpleNamespace(max_context_images=3), + telemetry=types.SimpleNamespace(run_record_timings=False), ) monkeypatch.setattr(runner, "load_settings", lambda: settings) monkeypatch.setattr(runner, "configure_sdk_model_defaults", lambda _settings: None) From 2872fc3234157c23dfad3ebf1eba2c4d954df4fb Mon Sep 17 00:00:00 2001 From: Samir Hanna Verza <123646616+samirhvbr@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:07:19 -0300 Subject: [PATCH 4/4] fix(retry): use the validated root ID in the usage-limit handler --- strix/core/runner.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/strix/core/runner.py b/strix/core/runner.py index 3aea5deb38..6ee6c67df7 100644 --- a/strix/core/runner.py +++ b/strix/core/runner.py @@ -718,14 +718,12 @@ async def spawn_child_agent(**kwargs: Any) -> dict[str, Any]: scan_id, ) _note_exit_reason("rate_limited") - if root_id is not None: - with contextlib.suppress(Exception): - await coordinator.set_status(root_id, "stopped") + with contextlib.suppress(Exception): + await coordinator.set_status(root_id, "stopped") return None logger.exception("Strix scan %s failed", scan_id) - if root_id is not None: - with contextlib.suppress(Exception): - await coordinator.set_status(root_id, "failed") + with contextlib.suppress(Exception): + await coordinator.set_status(root_id, "failed") raise except BaseException: logger.exception("Strix scan %s failed", scan_id)