Conversation
…lable (NousResearch#59984) When OpenRouter/Nous credentials are missing, _mark_provider_unhealthy was called with ttl=60 instead of the default 600s. The 60s TTL was designed for transient payment errors (402), but missing credentials are permanent. After 60s the unhealthy mark expires, the provider is retried, fails identically, and logs another WARNING — flooding errors.log for the lifetime of the process. Use the default 600s TTL (omit ttl=) for permanently unavailable providers so the warning is not repeated every minute.
alt-glitch
added
type/bug
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for targeting a real current-main retry path: the 60-second marks remain at agent/auxiliary_client.py:2003 and :2047.
Problems
- The PR changes the OpenRouter call but not
tests/agent/test_auxiliary_client.py:1131, which currently assertsmock_mark.assert_called_once_with("openrouter", ttl=60). The current PR diff changes no tests. - The same unavailable-credential class also exists at
agent/auxiliary_client.py:2095(Nous auth exists but no usable inference JWT), wherettl=60remains. - Commit
057f5a31d1b2358c8a1781c102a1e4401770e239introduced the short TTL specifically for missing credentials, so changing it needs an explicit regression contract for credential rediscovery in long-running processes.
Suggested changes
- Update the existing assertion and add direct coverage for both no-auth and no-usable-JWT Nous paths.
- Apply the intended TTL policy consistently across those paths, with a test that captures the desired rediscovery behavior.
Automated hermes-sweeper review.
| if not or_key: | ||
| _mark_provider_unhealthy("openrouter", ttl=60) | ||
| _mark_provider_unhealthy("openrouter") | ||
| return None, None |
Collaborator
There was a problem hiding this comment.
This changes a current tested contract: tests/agent/test_auxiliary_client.py:1131 asserts this call includes ttl=60, but this PR does not update that test. Please update the assertion and add coverage for the corresponding Nous no-credential paths.
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
3 participants
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
When OpenRouter or Nous credentials are not configured, was called with instead of the default 600s. The 60s TTL was designed for transient payment errors (402), but missing credentials are permanent configuration state — after 60s the unhealthy mark expires, the provider is retried, fails identically, and logs another WARNING. This floods with thousands of identical lines.
Fix
Remove the override when marking OpenRouter and Nous as permanently unavailable (no credentials), allowing them to use the default 600s TTL instead.
Closes #59984