diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 582f5f6fcd..b9b0e7c612 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -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 ''". + + 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() diff --git a/tests/hermes_cli/test_canonical_custom_identity.py b/tests/hermes_cli/test_canonical_custom_identity.py index 80ef6c3624..46b8f07c6a 100644 --- a/tests/hermes_cli/test_canonical_custom_identity.py +++ b/tests/hermes_cli/test_canonical_custom_identity.py @@ -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 ''". + """ + + 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 diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index f8290f8442..48addce8b2 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -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"}} ) diff --git a/tests/tui_gateway/test_custom_provider_session_persistence.py b/tests/tui_gateway/test_custom_provider_session_persistence.py index 2f0e677f2c..d2fe9c0acd 100644 --- a/tests/tui_gateway/test_custom_provider_session_persistence.py +++ b/tests/tui_gateway/test_custom_provider_session_persistence.py @@ -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 ''": 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 + + diff --git a/tui_gateway/server.py b/tui_gateway/server.py index a2fe9bd5cb..a17fe98f7c 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -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:`` 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 ''". + # Recover the durable ``custom:`` 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