Skip to content

fix(agent): notify when credential-pool exhaustion activates fallback - #10716

Closed
cloudyun888 wants to merge 2 commits into
NousResearch:mainfrom
cloudyun888:fix/credential-pool-fallback-notify-10476
Closed

cloudyun888 wants to merge 2 commits into
NousResearch:mainfrom
cloudyun888:fix/credential-pool-fallback-notify-10476

Conversation

@cloudyun888

Copy link
Copy Markdown

Summary

  • activate the existing fallback chain when credential-pool rotation is exhausted during billing recovery
  • preserve existing lifecycle notification behavior by reusing _try_activate_fallback()
  • add a regression test covering exhausted-pool -> fallback activation

Why

Issue #10476 reports that provider fallback triggered via credential-pool exhaustion is silent, unlike the normal fallback_model path. Today _recover_with_credential_pool() returns failure immediately when billing rotation has no next credential, so the existing fallback notifier never runs.

Testing

  • added tests/agent/test_credential_pool_routing.py::test_402_exhausted_pool_activates_fallback

Fixes #10476

@drousselbot

Copy link
Copy Markdown

QA Summary

Result: QA FAILED for PR #10716 / Issue #10476.

Why this PR was selected

  • Selected by the local deterministic workflow dispatcher for lane=qa.
  • The dispatcher returned an unlabeled PR, so I treated this as ownerless intake triage rather than a normal labeled QA handoff.

Scope checked

Evidence reviewed

Validation run

  • git diff origin/main...HEAD
  • pytest -q tests/agent/test_credential_pool_routing.py15 passed
  • gh pr checks 10716 --repo NousResearch/hermes-agent
  • gh run list --repo NousResearch/hermes-agent --branch fix/credential-pool-fallback-notify-10476 --limit 10
  • gh run view 24490792666 --repo NousResearch/hermes-agent --json status,conclusion,jobs,url,name,workflowName
  • Independent reviewer verdict: no blocking logic/security concerns in the code diff

Environment limits / honesty note

  • I could validate the narrow local regression path and the code change matches the issue scope.
  • I could not honestly mark QA passed because GitHub Actions never actually executed: the PR currently shows workflow runs with conclusion=action_required and no jobs, so repository CI evidence is still missing.

Verdict

  • The code change itself looks correct and the focused local test passed.
  • QA still fails for workflow/ownership reasons: this PR entered QA without a normal label handoff, and CI for the PR is not running yet (action_required, zero jobs). That means the required repo-level verification for this change has not completed.

Follow-up / fixes

  • Please approve/enable the PR workflows so CI can run, then hand it back through the intended workflow labels.
  • If this unlabeled intake path is expected, the deterministic workflow configuration should be updated so QA ownership is explicit instead of silent.

@drousselbot drousselbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

QA failed for workflow reasons: focused local regression test passed, but GitHub Actions for this PR are still action_required with no jobs, so repo-level verification has not actually run. This PR also arrived unlabeled through the dispatcher, so I am handing it back to development/intake until CI is enabled and ownership is explicit.

@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
@alt-glitch

Copy link
Copy Markdown
Contributor

Competing fix with #10718 — both address #10476 (silent fallback on credential pool exhaustion). This PR reuses _try_activate_fallback(); #10718 adds _emit_status() instead.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused fallback-notification fix. This is an automated hermes-sweeper review: current main now provides the requested exhausted-pool fallback and durable user-visible notification through a later unified implementation.

  • agent/agent_runtime_helpers.py:796-807 returns unsuccessful recovery when a billing-exhausted credential pool has no next entry; agent/conversation_loop.py:2617-2624 then continues to shared failure handling.
  • agent/conversation_loop.py:3166-3207 activates the configured fallback when the pool cannot recover, and synchronizes the failover system message.
  • agent/chat_completion_helpers.py:1544-1557 records a one-shot old-provider/model → new-provider/model notice; agent/conversation_loop.py:5065-5070 and run_agent.py:984-1002 emit it exactly once after successful fallback recovery.
  • Regression coverage exists at tests/run_agent/test_retry_status_buffer.py:138-164.
  • Commit 3fe7f6d27a005a0a75644c1d1d42cef8c8750c5f (fix: preserve fallback switch notice on successful fallback) introduced this behavior and is an ancestor of current main.

The member comment identifying #10718 as a competing fix is consistent with this later broader implementation.

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:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Credential pool provider fallback is silent — inconsistent with fallback_model notification behavior

4 participants