fix(tui-gateway): heal or fall back when a resumed session's provider is stale
A session row persists the provider identity a chat actually used. When that provider is later renamed or removed (e.g. a custom_providers:/providers: entry deleted, or a provider renamed oldone->newone), Desktop/TUI resume restores the stale name into agent init and dies with: agent init failed: Unknown provider '<name>' while the CLI resumes the same session fine with the configured default. - runtime_provider: add is_routable_provider() (full resolution chain: built-in -> providers: -> custom_providers: -> models.dev) - _stored_session_runtime_overrides: heal a non-routable provider via canonical_custom_identity (base_url -> model -> configured provider), drop to the configured default when unrecoverable, and clear the stale base_url after healing so a dead endpoint cannot override the registry URL - _start_agent_build: gate deferred-resume overrides on provider routability; when the stored provider is gone, prefer the model the user picked for THIS session, else the configured default - tests: is_routable_provider cases, heal/fallback round-trips, gate checks Refs #75128
This commit is contained in:
lorzl
@@ -1104,6 +1104,45 @@ def canonical_custom_identity(
|
||||
return None
|
||||
|
||||
|
||||
def is_routable_provider(provider: Optional[str]) -> bool:
|
||||
"""Whether a provider name currently resolves to a routable route.
|
||||
|
||||
Empty/None is vacuously routable: agent build falls back to the
|
||||
configured default instead of failing. A name that resolves through
|
||||
the full chain (built-in -> user ``providers:`` -> ``custom_providers:``
|
||||
-> models.dev) is routable; anything else would fail agent init with
|
||||
"Unknown provider '<name>'".
|
||||
|
||||
Session resume uses this to detect a stale/renamed/removed provider
|
||||
persisted in an older session snapshot, so recovery can fall back to
|
||||
the configured default or the model the user picked instead of letting
|
||||
the agent build die.
|
||||
"""
|
||||
name = str(provider or "").strip()
|
||||
if not name or name.lower() == "auto":
|
||||
return True
|
||||
if name.lower() == "custom":
|
||||
# The bare string is the resolved billing class shared by every
|
||||
# named custom entry — not a routable identity. restore paths must
|
||||
# heal it (canonical_custom_identity) or fall back, never hand it
|
||||
# straight to agent init.
|
||||
return False
|
||||
try:
|
||||
from hermes_cli.providers import resolve_provider_full
|
||||
|
||||
config = load_config()
|
||||
return (
|
||||
resolve_provider_full(
|
||||
name,
|
||||
config.get("providers"),
|
||||
get_compatible_custom_providers(config),
|
||||
)
|
||||
is not None
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _normalize_base_url_for_match(value) -> str:
|
||||
return str(value or "").strip().rstrip("/").lower()
|
||||
|
||||
|
||||
@@ -98,3 +98,51 @@ def test_legacy_unkeyed_entry_keeps_its_name_identity(monkeypatch):
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: {})
|
||||
|
||||
assert rp.canonical_custom_identity(config_provider="Legacy Endpoint") == "custom:legacy-endpoint"
|
||||
|
||||
|
||||
class TestIsRoutableProvider:
|
||||
"""``is_routable_provider`` gates session-resume fallback: a persisted
|
||||
provider name that no longer resolves (renamed/removed) must be detected
|
||||
so recovery falls back instead of failing agent init with
|
||||
"Unknown provider '<name>'".
|
||||
"""
|
||||
|
||||
def test_empty_auto_and_builtin_are_routable(self, keyed_provider_config):
|
||||
assert rp.is_routable_provider(None) is True
|
||||
assert rp.is_routable_provider("") is True
|
||||
assert rp.is_routable_provider("auto") is True
|
||||
assert rp.is_routable_provider("openrouter") is True
|
||||
|
||||
def test_bare_custom_is_not_routable(self, keyed_provider_config):
|
||||
# The resolved billing class, not a routable identity — restore
|
||||
# paths must heal it (canonical_custom_identity) or fall back.
|
||||
assert rp.is_routable_provider("custom") is False
|
||||
|
||||
def test_registered_names_are_routable(self, keyed_provider_config):
|
||||
assert rp.is_routable_provider(PROVIDER_KEY) is True
|
||||
assert rp.is_routable_provider(CANONICAL) is True
|
||||
|
||||
def test_stale_name_is_not_routable(self, keyed_provider_config):
|
||||
# Same endpoint family, but the OLD slug no longer matches any
|
||||
# configured entry — the regression this gate exists for.
|
||||
assert rp.is_routable_provider("stale-endpoint") is False
|
||||
assert rp.is_routable_provider("custom:stale-endpoint") is False
|
||||
|
||||
def test_legacy_unkeyed_name_is_routable(self, monkeypatch):
|
||||
config = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "Legacy Endpoint",
|
||||
"base_url": "https://legacy.invalid/v1",
|
||||
"api_key": "sk-legacy",
|
||||
"models": ["legacy-model"],
|
||||
}
|
||||
]
|
||||
}
|
||||
monkeypatch.setattr(rp, "load_config", lambda *a, **k: config)
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda *a, **k: config)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: {})
|
||||
|
||||
assert rp.is_routable_provider("legacy-endpoint") is True
|
||||
assert rp.is_routable_provider("custom:legacy-endpoint") is True
|
||||
assert rp.is_routable_provider("Legacy Endpoint") is True
|
||||
|
||||
@@ -3887,7 +3887,7 @@ def test_session_cwd_set_profile_session_updates_profile_db(monkeypatch, tmp_pat
|
||||
assert "launch_update" not in captured
|
||||
|
||||
|
||||
def test_stored_session_runtime_overrides_skips_bare_billing_provider():
|
||||
def test_stored_session_runtime_overrides_skips_bare_billing_provider(monkeypatch):
|
||||
"""A bare billing bucket ("custom"/"auto") must not be restored as the provider
|
||||
identity on resume. A custom endpoint that never used `/model` persists only
|
||||
`billing_provider="custom"`; restoring that broke `session.resume` with "No LLM provider
|
||||
@@ -3909,7 +3909,23 @@ def test_stored_session_runtime_overrides_skips_bare_billing_provider():
|
||||
assert ov["provider_override"] == "anthropic"
|
||||
assert ov["model_override"]["provider"] == "anthropic"
|
||||
|
||||
# An explicit routable provider in model_config wins over the bare billing bucket.
|
||||
# An explicit ROUTABLE provider in model_config wins over the bare billing
|
||||
# bucket. It must actually resolve in the registry — a stale/renamed
|
||||
# provider is dropped (see TestStaleProviderNameFallsBack).
|
||||
cfg = {
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "myendpoint",
|
||||
"base_url": "https://myendpoint.invalid/v1",
|
||||
"api_key": "sk-test",
|
||||
"model": "m",
|
||||
}
|
||||
]
|
||||
}
|
||||
import hermes_cli.runtime_provider as rp
|
||||
|
||||
monkeypatch.setattr(rp, "load_config", lambda: cfg)
|
||||
monkeypatch.setattr("hermes_cli.config.load_config", lambda: cfg)
|
||||
ov = server._stored_session_runtime_overrides(
|
||||
{"model": "m", "billing_provider": "custom", "model_config": {"provider": "custom:myendpoint"}}
|
||||
)
|
||||
|
||||
@@ -343,3 +343,129 @@ class TestModelNameRecoversEntryIdentity:
|
||||
)
|
||||
|
||||
|
||||
class TestStaleProviderNameFallsBack:
|
||||
"""A session row stored under a provider that was renamed or removed must
|
||||
not sink agent init with "Unknown provider '<name>'": heal to the entry
|
||||
that still serves the stored model/base_url, else drop the provider so
|
||||
resume falls back to the configured default (or the user's pick)."""
|
||||
|
||||
def test_stale_bare_name_heals_via_model(self, monkeypatch):
|
||||
"""Registry serves mimo-v2.5-pro; the row still names the OLD slug —
|
||||
the exact shape of the renamed-provider report (oldone -> newone)."""
|
||||
monkeypatch.setattr(rp, "load_config", lambda: NAMED_CONFIG)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: NAMED_CONFIG["model"])
|
||||
|
||||
from tui_gateway.server import _stored_session_runtime_overrides
|
||||
|
||||
row = {
|
||||
"model": "mimo-v2.5-pro",
|
||||
"model_config": json.dumps(
|
||||
{"model": "mimo-v2.5-pro", "provider": "stale-provider"}
|
||||
),
|
||||
"billing_provider": "custom",
|
||||
}
|
||||
overrides = _stored_session_runtime_overrides(row)
|
||||
|
||||
assert overrides["provider_override"] == "custom:mimo-v2.5-pro"
|
||||
assert overrides["model_override"]["provider"] == "custom:mimo-v2.5-pro"
|
||||
|
||||
def test_stale_prefixed_name_heals_and_drops_stale_base_url(self, monkeypatch):
|
||||
"""Healing must also drop the snapshot's base_url so the registry URL
|
||||
(the renamed provider's current endpoint) is not overridden."""
|
||||
monkeypatch.setattr(rp, "load_config", lambda: NAMED_CONFIG)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: NAMED_CONFIG["model"])
|
||||
|
||||
from tui_gateway.server import _stored_session_runtime_overrides
|
||||
|
||||
row = {
|
||||
"model": "mimo-v2.5-pro",
|
||||
"model_config": json.dumps(
|
||||
{
|
||||
"model": "mimo-v2.5-pro",
|
||||
"provider": "custom:stale-provider",
|
||||
"base_url": "https://old.invalid/v1",
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
),
|
||||
"billing_provider": "custom:stale-provider",
|
||||
}
|
||||
overrides = _stored_session_runtime_overrides(row)
|
||||
|
||||
assert overrides["provider_override"] == "custom:mimo-v2.5-pro"
|
||||
assert overrides["model_override"]["base_url"] is None
|
||||
|
||||
def test_unrecoverable_provider_drops_to_default(self, monkeypatch):
|
||||
"""No entry serves the stored model AND no configured default names a
|
||||
real entry → the provider is dropped; resume falls back to the
|
||||
configured default instead of failing the build."""
|
||||
config = {"custom_providers": NAMED_CONFIG["custom_providers"]}
|
||||
monkeypatch.setattr(rp, "load_config", lambda: config)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: {})
|
||||
|
||||
from tui_gateway.server import _stored_session_runtime_overrides
|
||||
|
||||
row = {
|
||||
"model": "no-such-model",
|
||||
"model_config": json.dumps(
|
||||
{"model": "no-such-model", "provider": "dead-provider"}
|
||||
),
|
||||
"billing_provider": "custom",
|
||||
}
|
||||
overrides = _stored_session_runtime_overrides(row)
|
||||
|
||||
assert "provider_override" not in overrides
|
||||
assert overrides["model_override"]["provider"] is None
|
||||
|
||||
def test_valid_provider_is_untouched(self, monkeypatch):
|
||||
"""A live provider must round-trip unchanged — no healing, no drops."""
|
||||
monkeypatch.setattr(rp, "load_config", lambda: NAMED_CONFIG)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: NAMED_CONFIG["model"])
|
||||
|
||||
from tui_gateway.server import _stored_session_runtime_overrides
|
||||
|
||||
row = {
|
||||
"model": "mimo-v2.5-pro",
|
||||
"model_config": json.dumps(
|
||||
{
|
||||
"model": "mimo-v2.5-pro",
|
||||
"provider": "custom:mimo-v2.5-pro",
|
||||
"base_url": MIMO_URL,
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
),
|
||||
"billing_provider": "custom:mimo-v2.5-pro",
|
||||
}
|
||||
overrides = _stored_session_runtime_overrides(row)
|
||||
|
||||
assert overrides["provider_override"] == "custom:mimo-v2.5-pro"
|
||||
assert overrides["model_override"]["base_url"] == MIMO_URL
|
||||
|
||||
|
||||
class TestOverridesHaveRoutableProvider:
|
||||
def test_gate_detects_stale_provider(self, monkeypatch):
|
||||
monkeypatch.setattr(rp, "load_config", lambda: NAMED_CONFIG)
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: NAMED_CONFIG["model"])
|
||||
|
||||
from tui_gateway.server import _overrides_have_routable_provider
|
||||
|
||||
assert (
|
||||
_overrides_have_routable_provider(
|
||||
{"provider_override": "custom:mimo-v2.5-pro"}
|
||||
)
|
||||
is True
|
||||
)
|
||||
assert (
|
||||
_overrides_have_routable_provider(
|
||||
{"provider_override": "custom:stale-provider"}
|
||||
)
|
||||
is False
|
||||
)
|
||||
assert (
|
||||
_overrides_have_routable_provider(
|
||||
{"model_override": {"provider": None}}
|
||||
)
|
||||
is False
|
||||
)
|
||||
assert _overrides_have_routable_provider({}) is False
|
||||
|
||||
|
||||
|
||||
+71
-25
@@ -3225,7 +3225,11 @@ def _start_agent_build(sid: str, session: dict) -> None:
|
||||
kw["session_id"] = resume_sid
|
||||
kw["platform_override"] = _session_source(current)
|
||||
resume_overrides = current.get("resume_runtime_overrides")
|
||||
if isinstance(resume_overrides, dict) and resume_overrides:
|
||||
if (
|
||||
isinstance(resume_overrides, dict)
|
||||
and resume_overrides
|
||||
and _overrides_have_routable_provider(resume_overrides)
|
||||
):
|
||||
# Cold deferred resume: restore the full persisted runtime
|
||||
# identity (model/provider/base_url/api_mode/reasoning/tier)
|
||||
# exactly as the eager resume path's _stored_session_runtime_
|
||||
@@ -3233,9 +3237,11 @@ def _start_agent_build(sid: str, session: dict) -> None:
|
||||
# provider and fail with "No LLM provider configured".
|
||||
kw.update(resume_overrides)
|
||||
else:
|
||||
# Model/effort/fast the desktop picked for a brand-new chat
|
||||
# ride in as per-session overrides so the first build uses
|
||||
# them directly (no global config, no build-then-switch).
|
||||
# No stored runtime, or the stored provider no longer
|
||||
# resolves (renamed/removed since the row was written) —
|
||||
# never let that sink agent init with "Unknown provider".
|
||||
# Fall back to the model/effort/fast the desktop picked
|
||||
# for THIS session, else the configured default.
|
||||
if override := current.get("model_override"):
|
||||
kw["model_override"] = override
|
||||
if (reasoning := current.get("create_reasoning_override")) is not None:
|
||||
@@ -5202,6 +5208,30 @@ def _resolve_startup_runtime() -> tuple[str, str | None]:
|
||||
from hermes_state import _BARE_BILLING_PROVIDERS
|
||||
|
||||
|
||||
def _overrides_have_routable_provider(overrides: dict) -> bool:
|
||||
"""Whether persisted runtime overrides still name a routable provider.
|
||||
|
||||
A session row written under a provider that has since been renamed or
|
||||
removed would otherwise fail agent init with "Unknown provider".
|
||||
Empty provider counts as NOT routable here, so the caller falls back
|
||||
to the model the user picked for this session / the configured
|
||||
default instead of restoring a provider-less snapshot override.
|
||||
"""
|
||||
provider = str(overrides.get("provider_override") or "").strip()
|
||||
if not provider:
|
||||
provider = str(
|
||||
(overrides.get("model_override") or {}).get("provider") or ""
|
||||
).strip()
|
||||
if not provider:
|
||||
return False
|
||||
try:
|
||||
from hermes_cli.runtime_provider import is_routable_provider
|
||||
|
||||
return is_routable_provider(provider)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _stored_session_runtime_overrides(row: dict | None) -> dict:
|
||||
"""Return runtime fields persisted with a stored session.
|
||||
|
||||
@@ -5245,30 +5275,46 @@ def _stored_session_runtime_overrides(row: dict | None) -> dict:
|
||||
reasoning_config = model_config.get("reasoning_config")
|
||||
service_tier = str(model_config.get("service_tier") or "").strip()
|
||||
|
||||
# Heal a bare ``"custom"`` provider stored by an older build (or any leak
|
||||
# site that bypassed _runtime_model_config's normalization). Bare custom is
|
||||
# the resolved billing class, not a routable identity — restoring it as the
|
||||
# session's provider override routes the resume to the OpenRouter default
|
||||
# URL with no api_key, surfacing as "No LLM provider configured". Recover
|
||||
# the durable ``custom:<name>`` menu key from the stored base_url, then
|
||||
# from the entry that serves the stored model, falling back to the
|
||||
# configured provider when the row has neither (the recurring Desktop/TUI
|
||||
# regression vector). If none names a real entry,
|
||||
# drop the bare provider entirely so resume falls back to the configured
|
||||
# default rather than the broken OpenRouter route.
|
||||
if provider.strip().lower() == "custom":
|
||||
healed = None
|
||||
# Heal a stale/expired provider name persisted by an older build — not
|
||||
# just the bare ``"custom"`` billing class. A renamed or removed custom
|
||||
# provider (e.g. ``oldone`` -> ``newone``) stored in the session row
|
||||
# would otherwise fail agent init with "Unknown provider '<name>'".
|
||||
# Recover the durable ``custom:<name>`` menu key from the stored
|
||||
# base_url, then from the entry that serves the stored model, falling
|
||||
# back to the configured provider when the row has neither. When
|
||||
# nothing names a real entry, drop the provider entirely so resume
|
||||
# falls back to the configured default rather than the broken route.
|
||||
if provider:
|
||||
routable = False
|
||||
try:
|
||||
from hermes_cli.runtime_provider import canonical_custom_identity
|
||||
from hermes_cli.runtime_provider import is_routable_provider
|
||||
|
||||
healed = canonical_custom_identity(
|
||||
base_url=base_url or None, model=model or None
|
||||
)
|
||||
routable = is_routable_provider(provider)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"custom provider identity recovery failed", exc_info=True
|
||||
)
|
||||
provider = healed or ("" if not base_url else provider)
|
||||
routable = False
|
||||
if not routable:
|
||||
healed = None
|
||||
try:
|
||||
from hermes_cli.runtime_provider import canonical_custom_identity
|
||||
|
||||
healed = canonical_custom_identity(
|
||||
base_url=base_url or None, model=model or None
|
||||
)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"custom provider identity recovery failed", exc_info=True
|
||||
)
|
||||
if healed:
|
||||
logger.info(
|
||||
"healed stale session provider %r to %r", provider, healed
|
||||
)
|
||||
provider = healed
|
||||
# The healed identity owns a registered endpoint; drop the
|
||||
# snapshot's base_url so it can't override the registry URL
|
||||
# (e.g. a stale direct endpoint behind a renamed proxy).
|
||||
base_url = ""
|
||||
else:
|
||||
provider = ""
|
||||
|
||||
if model:
|
||||
# Use the same dict-shaped override that live /model switches use so a
|
||||
|
||||
Reference in New Issue
Block a user