Skip to content

fix(agent): check credential pool exhaustion before restoring primary runtime (#15298) - #15434

Open
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/restore-primary-check-credential-pool
Open

Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/restore-primary-check-credential-pool

Conversation

@Tranquil-Flow

@Tranquil-Flow Tranquil-Flow commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

_restore_primary_runtime() checks a 60-second rate-limit timer (_rate_limited_until) but does not consult the credential pool's exhaustion state. After the 60-second timer expires, it attempts to restore the primary provider every turn even if the credential pool still marks that provider as exhausted. This burns retries and generates noise during extended outages.

Related Issue

Fixes #15298

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • run_agent.py — after the _rate_limited_until check in _restore_primary_runtime(), added a guard that consults self._credential_pool.has_available(). If the pool exists for the primary provider and reports all credentials exhausted, restoration is skipped. This integrates with the existing CredentialPool architecture (which already tracks per-credential exhaustion with cooldowns) rather than adding another independent timer.
  • tests/run_agent/test_primary_runtime_restore.py — 4 new tests in TestCredentialPoolExhaustionGate

How to Test

  1. pytest tests/run_agent/test_primary_runtime_restore.py::TestCredentialPoolExhaustionGate -v — 4 new tests cover:
    • Pool exhausted, same provider: restoration blocked
    • Pool has credentials: restoration proceeds
    • Pool for different provider: not blocked
    • No pool at all: not blocked
  2. Tested on macOS (Python 3.11)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS (Python 3.11)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

N/A — see commit description and PR diff.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 25, 2026
@Tranquil-Flow
Tranquil-Flow force-pushed the fix/restore-primary-check-credential-pool branch 2 times, most recently from 8c75dd0 to 287fccd Compare May 25, 2026 11:05

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the repeated primary-restore retry path. The pool-aware gap still exists on current main: after the fixed cooldown expires, agent/agent_runtime_helpers.py:1163-1199 restores the snapshot key and client; the pool is only considered when has_available() is true at :1220-1222.

Problems

  • The new direct comparison at agent/agent_runtime_helpers.py:897-900 does not cover named custom providers. Custom pools are keyed custom:<name> while the agent runtime stays custom (agent/credential_pool.py:117-120). Current main resolves this safely through get_custom_provider_pool_key() at agent/agent_runtime_helpers.py:1235-1249.
  • Current HEAD deliberately tests the inverse empty-pool behavior: tests/agent/test_restore_primary_pool_reselect.py:162-175 expects the snapshot key to be restored when no pool credential is available. This PR needs to reconcile that contract, not just add parallel mock coverage.

Suggested changes

  • Rework the gate against the current helper and reuse its canonical custom-pool matching logic.
  • Add a real exhausted-pool regression case, including custom:<name>, and update the existing empty-pool expectation if the intended contract is now to remain on fallback.

This is an automated hermes-sweeper review.

Comment thread agent/agent_runtime_helpers.py Outdated
# Also defer if the credential pool for the primary provider reports
# all credentials still exhausted — avoids burning retries while the
# provider's own cooldown hasn't elapsed.
if hasattr(agent, '_credential_pool') and agent._credential_pool is not None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This direct provider equality misses named custom pools: their pool key is custom:<name> while the runtime provider remains custom. Please reuse the canonical custom-pool matching approach now used in restore_primary_runtime so an exhausted named custom endpoint is also gated.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
… runtime (NousResearch#15298)

After the 60-second rate-limit timer expires, restore_primary_runtime()
attempted to restore the primary provider every turn even if the
credential pool still marked all credentials as exhausted. This burned
retries and generated noise during extended outages.

Now, after the _rate_limited_until check, if the agent has a
_credential_pool for the primary provider and the pool reports no
available credentials (all exhausted), restoration is skipped. This
integrates with the existing credential pool architecture (has_available /
_mark_exhausted) rather than adding another independent timer.

Adapted from the original PR: main has refactored _restore_primary_runtime
into agent.agent_runtime_helpers.restore_primary_runtime; the gate now
lives in the helper. Tests reach the helper transparently through the
forwarder on AIAgent.
@Tranquil-Flow
Tranquil-Flow force-pushed the fix/restore-primary-check-credential-pool branch from 677b3ac to 0a5ef57 Compare July 13, 2026 16:55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_restore_primary_runtime() doesn't check credential cooldown — burns retries every turn while provider is exhausted

3 participants