fix(retry): honor gateway-advertised reset window; respect api_mode on fallback entries - #76225
Open
yusharwz wants to merge 2 commits into
Open
fix(retry): honor gateway-advertised reset window; respect api_mode on fallback entries#76225yusharwz wants to merge 2 commits into
yusharwz wants to merge 2 commits into
Conversation
Aggregator gateways that proxy several upstream providers synthesize their
429 after an internal failover walk, so they have no Retry-After header to
forward and state the window in the error body instead:
[antigravity/claude-sonnet-4-6] [429]: {...} (reset after 4s)
[claude/claude-opus-5] [429]: {...} (reset after 2m 13s)
The retry loop only read the Retry-After header, so this was discarded and
the wait fell back to a generic exponential backoff. Observed windows are
mostly 2-30s, and the backoff frequently retried inside the still-closed
window, failed again, and abandoned an otherwise healthy provider for a
fallback that was seconds away from being usable.
parse_reset_after_seconds() reads the hint and the retry path prefers it
when no header is present. A real Retry-After header still wins, and
windows over 600s are ignored so a long outage falls through to the normal
failover instead of a long sleep.
Also adds a cross-session guard: those same gateways multiplex accounts, so
concurrent sessions (chat platform DMs, subagents, background review) can
each land on the same upstream and independently compute a short wait.
Publishing the cooldown per upstream tag lets them converge on one wait
instead of retrying back into the same wall.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
try_activate_fallback() recomputed fb_api_mode from scratch and never read the api_mode field of the fallback entry, so setting it had no effect. For a custom gateway speaking the Anthropic Messages API this defaulted to chat_completions and POSTed to /chat/completions, which 404s. The only way to force anthropic_messages was provider: anthropic, which ignores base_url and sends the request to api.anthropic.com instead of the configured gateway -- surfacing as a confusing 404 on a model name the real API has never heard of, against whatever credentials happen to be configured. An explicit api_mode now wins over the heuristics; everything else is unchanged when the field is absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves provider failover reliability when running Hermes against aggregator-style gateways by (1) honoring rate-limit reset windows advertised in 429 error bodies (when no Retry-After header exists), and (2) fixing fallback activation to respect an explicitly configured api_mode per fallback entry.
Changes:
- Add
parse_reset_after_seconds()and wire it into the rate-limit retry path when noRetry-Afterheader is present. - Introduce a cross-session downstream rate-limit guard keyed by upstream tags embedded in gateway error text, and merge waits across sessions.
- Update fallback activation to prefer an explicit
api_modefield over heuristics; add tests covering parser, wiring, and guard behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
agent/retry_utils.py |
Adds parsing for (reset after …) hints in 429 bodies. |
agent/downstream_rate_guard.py |
New module to persist/read per-upstream cooldowns across sessions. |
agent/conversation_loop.py |
Wires body-hint parsing and downstream cooldown merging into retry logic. |
agent/chat_completion_helpers.py |
Ensures fallback entries can force api_mode. |
tests/agent/test_reset_after_hint.py |
Unit tests for reset-window parsing from gateway error strings. |
tests/agent/test_reset_after_wiring.py |
Tests precedence of header vs body hint vs backoff for wait computation. |
tests/agent/test_downstream_rate_guard.py |
Tests persistence, expiry, isolation, and key sanitization for the new guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+75
to
+80
| try: | ||
| from hermes_constants import get_hermes_home | ||
| base = get_hermes_home() | ||
| except ImportError: | ||
| base = os.path.join(os.path.expanduser("~"), ".hermes") | ||
| return os.path.join(base, _STATE_SUBDIR, f"downstream_{safe_key}.json") |
Comment on lines
+136
to
+137
| except (FileNotFoundError, json.JSONDecodeError, KeyError, TypeError, ValueError): | ||
| return None |
Comment on lines
+5438
to
+5455
| if is_rate_limited: | ||
| from agent.downstream_rate_guard import ( | ||
| downstream_rate_limit_remaining, | ||
| extract_upstream_tag, | ||
| record_downstream_rate_limit, | ||
| ) | ||
| _upstream_tag = extract_upstream_tag(agent._summarize_api_error(api_error)) | ||
| if _upstream_tag: | ||
| _shared_rl_key = f"{getattr(agent, 'provider', 'unknown')}:{_upstream_tag}" | ||
| _shared_remaining = downstream_rate_limit_remaining(_shared_rl_key) | ||
| if _shared_remaining and _shared_remaining > wait_time: | ||
| logger.info( | ||
| "Extending retry wait for %s from %.1fs to %.1fs — another " | ||
| "session already recorded this upstream as rate-limited", | ||
| _shared_rl_key, wait_time, _shared_remaining, | ||
| ) | ||
| wait_time = _shared_remaining | ||
| record_downstream_rate_limit(_shared_rl_key, seconds=wait_time) |
alt-glitch
added
type/bug
Collaborator
Duplicate of #16346 for the explicit fallback api_mode hunk: both make fallback-entry api_mode override transport heuristics. This PR also carries separate reset-window retry work. |
teknium1
reviewed
Aug 1, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for confirming two real current-main gaps: agent/conversation_loop.py:5387-5412 only consumes a Retry-After header, and agent/chat_completion_helpers.py:1823-1867 ignores fallback-entry api_mode.
Problems
agent/downstream_rate_guard.py:136-137promises fail-open behavior but omitsOSError/PermissionError; the new call inagent/conversation_loop.pyis unguarded, so an unreadable state file can break retry recovery. The existing review correctly called this out.agent/downstream_rate_guard.py:75-80retains a hardcoded~/.hermesfallback, contrary to profile-safe path handling.- The new raw
api_modeoverride bypasses the canonical validator inhermes_cli/runtime_provider.py:394-400, unlike primary initialization atagent/agent_init.py:615-616. Open PR #16346 already carries the broader validated fallback implementation. tests/agent/test_reset_after_wiring.py:27-38mirrors production logic locally rather than executingagent/conversation_loop.py; it cannot prove the integration.
Suggested changes
- Reuse the canonical parser and reconcile this hunk with #16346.
- Make the guard fully fail-open and profile-safe.
- Test a real headerless-429 retry path, following
tests/run_agent/test_run_agent.py:1674-1716.
Automated hermes-sweeper review.
| base = get_hermes_home() | ||
| except ImportError: | ||
| base = os.path.join(os.path.expanduser("~"), ".hermes") | ||
| return os.path.join(base, _STATE_SUBDIR, f"downstream_{safe_key}.json") |
Contributor
There was a problem hiding this comment.
Please remove this fallback and always resolve state through get_hermes_home(). A hardcoded ~/.hermes path is not profile-safe and can write a profile's cooldown state into the default home.
| pass | ||
| return None | ||
| except (FileNotFoundError, json.JSONDecodeError, KeyError, TypeError, ValueError): | ||
| return None |
Contributor
There was a problem hiding this comment.
This function promises to fail open for an unreadable state file, but PermissionError/other OSError values escape this handler and then escape the unguarded caller in the retry loop. Treat filesystem errors and malformed non-dict JSON as no active cooldown.
| # ``api_mode`` on the fallback entry always wins over the heuristics | ||
| # below — without this, a fallback pointed at a custom gateway | ||
| # (provider: custom, base_url without a path suffix the heuristics | ||
| # recognize) silently defaulted to chat_completions and 404'd, and |
Contributor
There was a problem hiding this comment.
Validate and normalize this override with hermes_cli.runtime_provider._parse_api_mode rather than accepting every non-empty string. Primary initialization only permits the canonical API-mode set (agent/agent_init.py:615-616), so an invalid fallback value currently creates an unsupported runtime state.
teknium1
added
sweeper:risk-compatibility
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent fixes for provider failover, both found while running Hermes against a self-hosted multi-provider gateway (an OpenAI/Anthropic-compatible router that fans out to several upstream accounts).
1. Honor the reset window a gateway states in its 429 body
Gateways that proxy several upstreams synthesize their 429 after an internal failover walk, so there is no upstream
Retry-Afterheader to forward. They state the window in the error body instead:run_conversation()only read theRetry-Afterheader, so this was discarded and the wait fell back tojittered_backoff(). In practice the advertised windows are mostly 2–30s, and the generic backoff frequently retried inside the still-closed window, failed again, and abandoned an otherwise healthy provider for a fallback that was seconds away from being usable.parse_reset_after_seconds()(inagent/retry_utils.py) parses the4s/2m 13s/1h 5mshapes. The retry path prefers it only when no header is present:Retry-Afterheader still wins — existing behavior unchanged;500msis not misread as 500 minutes.This also adds
agent/downstream_rate_guard.py. The same gateways multiplex accounts, so concurrent sessions (chat-platform DMs, subagents, background review) can each land on the same upstream moments apart and independently compute their own short wait. Publishing the cooldown keyed by the upstream tag found in the error lets them converge on one wait instead of retrying back into the same wall. It only ever extends a wait the session computed itself, never shortens it and never blocks a request, so a stale or misparsed tag costs at most one slightly longer sleep. It fails open if the state file is unreadable.2. Respect an explicit
api_modeonfallback_modelentriestry_activate_fallback()recomputedfb_api_modefrom scratch and never read theapi_modefield of the fallback entry, so setting it had no effect.For a custom gateway speaking the Anthropic Messages API this defaulted to
chat_completionsand POSTed to/chat/completions, which 404s. The only way to forceanthropic_messageswasprovider: anthropic, which ignoresbase_urland sends the request toapi.anthropic.cominstead of the configured gateway — surfacing as a confusing 404 on a model name the real API has never heard of, against whatever credentials happen to be configured.An explicit
api_modenow wins over the heuristics. Everything else is unchanged when the field is absent.Testing
Three new test files (33 tests), using verbatim error strings captured from a real gateway:
tests/agent/test_reset_after_hint.py— parser: real gateway shapes, the 600s cap, the500msguard, zero/absent windows.tests/agent/test_reset_after_wiring.py— precedence: header beats body hint, body hint beats backoff, overlong window falls back to backoff.tests/agent/test_downstream_rate_guard.py— record/read/expire/clear, key isolation, filesystem-unsafe keys, cap.Existing retry/failover suites pass unchanged.
Notes
The reset-window path only runs when
agent.api_max_retriesis ≥ 2 — with the value at 1 the loop gives up before reaching the wait. That is pre-existing behavior and this PR does not change the default.