Skip to content

fix(review): propagate configured curator fallback chains - #79750

Open
Cloudymap1e wants to merge 1 commit into
NousResearch:mainfrom
Cloudymap1e:fix/78371-curator-fallback-chain
Open

Cloudymap1e wants to merge 1 commit into
NousResearch:mainfrom
Cloudymap1e:fix/78371-curator-fallback-chain

Conversation

@Cloudymap1e

Copy link
Copy Markdown

Summary

  • resolve auxiliary-agent fallback chains as task-specific entries followed by global entries
  • deduplicate normalized routes while ignoring empty or malformed entries
  • pass the effective chain to curator and background-review forks
  • preserve the live parent fallback chain for routed and unrouted background-review forks when configuration supplies no chain
  • preserve routed-runtime and cache-parity behavior

Fixes #78371

Testing

  • fallback configuration, curator, background-review cost-control, and cache-parity suites — 48 passed
  • Ruff on touched files — passed
  • git diff --check — passed

Checklist

  • Task chain precedes global chain
  • Routed and unrouted forks preserve parent fallback
  • Malformed/empty entries fail safely
  • No credentials logged or persisted

@Halldrix

Copy link
Copy Markdown
Contributor

Hi @Cloudymap1e — I independently opened #93664 for the same bug before
noticing this PR (my fault for missing it in the duplicate search); the
triage bot correctly flagged mine, and I'll be closing it. Your design
matches what I landed independently: task chain first, global chain as
safety net, backend-identity dedupe, fresh dicts, wiring into both forks.

Three things from building the same fix that may be useful here:

  1. background_review ignores auxiliary fallback_chain config when primary provider fails #93592 is also fixed by this PR but won't auto-close. The
    background-review fork is wired here, yet the body only cites
    Fixes #78371. Adding Fixes #93592 would close the newer report too
    when this merges.

  2. transport -> api_mode aliasing. Chain entries may declare the
    wire format as transport (_resolve_fallback_entry in
    auxiliary_client.py accepts api_mode or transport), but
    _try_activate_fallback() only reads api_mode — a transport:-only
    entry silently drops to chat_completions on the conversation-loop path.
    One normalization line in get_auxiliary_fallback_chain closes it.

  3. Parent-chain inheritance is a behavior change worth surfacing.
    Keeping agent._fallback_chain when no config supplies a chain means
    previously-unconfigured installs will start failing reviews over across
    providers (full snapshot replay, cold cache). It may well be intended —
    but it goes beyond what either linked issue asks for, so a line in the
    PR body would help reviewers.

teknium1 pushed a commit that referenced this pull request Sep 19, 2026
hermes_cli/oneshot.py::_run_agent called resolve_runtime_provider() bare. When the
primary's credential pool is exhausted (quota 429), expired, or cooled down, that
raises AuthError before AIAgent exists, so the mid-session fallback_model wiring
never gets a chance and `hermes -z` dies for the whole quota window even with a
healthy fallback_providers chain configured — the gateway
(_try_resolve_fallback_provider) and the interactive CLI (_resolve_fallback_runtime)
already walk the chain at resolution time (#81209).

Add one shared hermes_cli/runtime_provider.py::resolve_runtime_with_fallback():
AuthError-only trigger (ValueError misconfiguration such as a typo'd --provider still
fails loudly instead of being rerouted), get_fallback_chain() semantics (managed
overlay, key_env, effective_runtime_provider identity), primary-error precedence when
every entry fails. Oneshot uses it and sends the chosen entry's model; the primary's
stored api_mode is dropped on a switch. The helper is the extraction point for the
auxiliary/curator ports (#82132, #79750) and the gateway loop.

Salvaged from PR #81562 (AuthError-only, injectable resolver, oneshot wiring) with the
chain-walk and error-precedence properties adjudicated from PR #81517.

Fixes #81209
pull Bot pushed a commit to TKaxv-7S/hermes-agent that referenced this pull request Sep 19, 2026
hermes_cli/oneshot.py::_run_agent called resolve_runtime_provider() bare. When the
primary's credential pool is exhausted (quota 429), expired, or cooled down, that
raises AuthError before AIAgent exists, so the mid-session fallback_model wiring
never gets a chance and `hermes -z` dies for the whole quota window even with a
healthy fallback_providers chain configured — the gateway
(_try_resolve_fallback_provider) and the interactive CLI (_resolve_fallback_runtime)
already walk the chain at resolution time (NousResearch#81209).

Add one shared hermes_cli/runtime_provider.py::resolve_runtime_with_fallback():
AuthError-only trigger (ValueError misconfiguration such as a typo'd --provider still
fails loudly instead of being rerouted), get_fallback_chain() semantics (managed
overlay, key_env, effective_runtime_provider identity), primary-error precedence when
every entry fails. Oneshot uses it and sends the chosen entry's model; the primary's
stored api_mode is dropped on a switch. The helper is the extraction point for the
auxiliary/curator ports (NousResearch#82132, NousResearch#79750) and the gateway loop.

Salvaged from PR NousResearch#81562 (AuthError-only, injectable resolver, oneshot wiring) with the
chain-walk and error-precedence properties adjudicated from PR NousResearch#81517.

Fixes NousResearch#81209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Curator review: per-task fallback_chain (auxiliary.curator.fallback_chain) is ignored

3 participants