Skip to content

fix(tui): fall back on primary AuthError during agent creation - #43861

Closed
VrtxOmega wants to merge 3 commits into
NousResearch:mainfrom
VrtxOmega:fix/desktop-auth-fallback-43588
Closed

fix(tui): fall back on primary AuthError during agent creation#43861
VrtxOmega wants to merge 3 commits into
NousResearch:mainfrom
VrtxOmega:fix/desktop-auth-fallback-43588

Conversation

@VrtxOmega

Copy link
Copy Markdown
Contributor

Fixes #43588

Problem

When model.provider is an OAuth provider with invalid/revoked credentials (e.g. xai-oauth) but fallback_providers is configured, the CLI and messaging gateway already fall through to the fallback chain. Desktop/TUI hard-errored on the first message because _make_agent() called resolve_runtime_provider() directly with no AuthError → fallback handling.

Fix

Add _resolve_runtime_with_auth_fallback() in tui_gateway/server.py, mirroring gateway/run.py::_resolve_runtime_agent_kwargs and CLI _ensure_runtime_credentials. _make_agent() now tries the configured fallback chain on primary AuthError and switches to the fallback model when resolution succeeds.

Tests

  • test_make_agent_falls_back_on_primary_auth_error — dead xAI OAuth → openrouter fallback
  • test_make_agent_auth_error_without_fallback_raises — no fallback configured still surfaces error
.venv/bin/python3 -m pytest tests/test_tui_gateway_server.py::test_make_agent_falls_back_on_primary_auth_error tests/test_tui_gateway_server.py::test_make_agent_auth_error_without_fallback_raises tests/gateway/test_auth_fallback.py -q

@liuhao1024

Copy link
Copy Markdown
Contributor

Two test assertions will fail when CI runs:

1. captured["api_key"] mismatch in both tests

fake_resolve returns "api_key": "***", but both test_make_agent_falls_back_on_primary_auth_error and test_make_agent_auth_fallback_skips_stale_session_overrides assert:

assert captured["api_key"] == "fallback-key"

The mock resolution for requested="openrouter" returns "***", so this assertion should be:

assert captured["api_key"] == "***"

2. captured["fallback_model"] type mismatch

_resolve_runtime_with_auth_fallback returns (runtime, fb_model) where fb_model = entry.get("model") — a string like "meta-llama/llama-4-maverick". But the test asserts:

assert captured["fallback_model"] == fallback_chain

where fallback_chain is a list of dicts. Unless _make_agent re-serializes the chain into the fallback_model kwarg (which the diff doesn't show), this would also fail.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/auth Authentication, OAuth, credential pools labels Jun 11, 2026
@VrtxOmega

Copy link
Copy Markdown
Contributor Author

Thanks @liuhao1024 — took another pass on the assertions:

  1. api_keyfake_resolve in these tests returns "fallback-key" (not "***"; that's the gateway test style). Assertions now derive from a shared fallback_runtime dict so they stay tied to the mock output.

  2. fallback_model_make_agent() still passes fallback_model=_load_fallback_model() (the chain list) into AIAgent. The helper's second return value (fb_model string) only updates the active model= kwarg. Added test_resolve_runtime_with_auth_fallback_returns_fallback_entry_model to document the split.

.venv/bin/python3 -m pytest \
  tests/test_tui_gateway_server.py::test_resolve_runtime_with_auth_fallback_returns_fallback_entry_model \
  tests/test_tui_gateway_server.py::test_make_agent_falls_back_on_primary_auth_error \
  tests/test_tui_gateway_server.py::test_make_agent_auth_fallback_skips_stale_session_overrides \
  tests/gateway/test_auth_fallback.py -q

7 passed locally.

…esearch#43588)

Desktop/TUI gateway called resolve_runtime_provider() directly in _make_agent
without trying fallback_providers, so a dead xAI OAuth primary hard-errored
on the first message while CLI and messaging gateway already fell through.

Mirror gateway/CLI auth-fallback resolution at TUI agent build time and add
regression tests for the happy and no-fallback paths.
When _make_agent falls back from a dead primary, do not re-apply persisted
model_override credentials from the failed provider — they would clobber the
healthy fallback runtime.
Add a direct _resolve_runtime_with_auth_fallback unit test documenting that
the helper's second return value is the config entry model string, while
AIAgent.fallback_model still receives the full chain from _load_fallback_model().

Derive api_key assertions from the mock runtime dict so expectations stay tied
to fake_resolve output.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. Current main contains an overlapping init-time AuthError fallback from f44415e71a (tui_gateway/server.py:4444), so this cannot be cleanly cherry-picked. However, it does not fully implement this PR's model-selection behavior: it resolves a fallback using only requested at tui_gateway/server.py:4469, then still constructs AIAgent(model=model) at tui_gateway/server.py:4604. The current integration test explicitly preserves the primary gpt-5.5 model despite configuring deepseek-v4-pro as the fallback (tests/test_tui_gateway_server.py:8839-8856).

Problems

  • The current implementation fixes provider credential resolution but leaves the configured fallback model unused during agent construction.

Suggested changes

  • Salvage the fallback-entry model propagation and stale-session-override protection into the existing _resolve_runtime_with_fallback / _make_agent structure.
  • Update the integration assertion to require both fallback provider and fallback model.

Automated hermes-sweeper review.

@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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation and removed P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 14, 2026
teknium1 pushed a commit that referenced this pull request Jul 16, 2026
A fallback chain entry can name its API key via key_env (or the
api_key_env alias) per the fallback-providers docs, but only the gateway
path resolved it — TUI/desktop, cron, and CLI setup fallbacks ignored it,
so a fallback provider whose key lives in a non-standard env var never
resolved on those surfaces.

Centralize the inline-api_key-then-key_env lookup in
hermes_cli/fallback_config.resolve_entry_api_key() and use it at all four
fallback resolution sites (tui_gateway, cron scheduler, gateway runner,
CLI setup mixin); the CLI mixin also gains the base_url passthrough the
other surfaces already had.

Salvaged from PR #43861 (surgical reapply — the original branch predates
the #65264 fallback restructuring).
teknium1 pushed a commit that referenced this pull request Jul 16, 2026
A fallback chain entry can name its API key via key_env (or the
api_key_env alias) per the fallback-providers docs, but only the gateway
path resolved it — TUI/desktop, cron, and CLI setup fallbacks ignored it,
so a fallback provider whose key lives in a non-standard env var never
resolved on those surfaces.

Centralize the inline-api_key-then-key_env lookup in
hermes_cli/fallback_config.resolve_entry_api_key() and use it at all four
fallback resolution sites (tui_gateway, cron scheduler, gateway runner,
CLI setup mixin); the CLI mixin also gains the base_url passthrough the
other surfaces already had.

Salvaged from PR #43861 (surgical reapply — the original branch predates
the #65264 fallback restructuring).
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #65682 with you credited as commit author (998e35313). Your provider/model fallback pairing landed earlier via PR #65656's salvage of #65264 — but the key_env API-key resolution in your TUI fix was a real gap that fix didn't cover, so we extracted it, centralized it in hermes_cli/fallback_config.resolve_entry_api_key(), and applied it to all four fallback surfaces (TUI, cron, gateway, CLI setup). Your original branch predated the fallback restructuring so a direct cherry-pick wasn't possible; the reapplied commit carries your authorship. Thanks for spotting both halves of this!

Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
A fallback chain entry can name its API key via key_env (or the
api_key_env alias) per the fallback-providers docs, but only the gateway
path resolved it — TUI/desktop, cron, and CLI setup fallbacks ignored it,
so a fallback provider whose key lives in a non-standard env var never
resolved on those surfaces.

Centralize the inline-api_key-then-key_env lookup in
hermes_cli/fallback_config.resolve_entry_api_key() and use it at all four
fallback resolution sites (tui_gateway, cron scheduler, gateway runner,
CLI setup mixin); the CLI mixin also gains the base_url passthrough the
other surfaces already had.

Salvaged from PR NousResearch#43861 (surgical reapply — the original branch predates
the NousResearch#65264 fallback restructuring).
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
A fallback chain entry can name its API key via key_env (or the
api_key_env alias) per the fallback-providers docs, but only the gateway
path resolved it — TUI/desktop, cron, and CLI setup fallbacks ignored it,
so a fallback provider whose key lives in a non-standard env var never
resolved on those surfaces.

Centralize the inline-api_key-then-key_env lookup in
hermes_cli/fallback_config.resolve_entry_api_key() and use it at all four
fallback resolution sites (tui_gateway, cron scheduler, gateway runner,
CLI setup mixin); the CLI mixin also gains the base_url passthrough the
other surfaces already had.

Salvaged from PR NousResearch#43861 (surgical reapply — the original branch predates
the NousResearch#65264 fallback restructuring).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

Bug: Desktop session hard-errors on dead default-model auth; CLI falls back gracefully

4 participants