diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index cbaf0a74c9..6bee187258 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -7025,6 +7025,40 @@ def resolve_provider_client( custom_key = build_command_token_provider( custom_key_cmd, custom_entry.get("name") or provider ) or custom_key + if not custom_key: + try: + from agent.credential_pool import ( + custom_provider_pool_key_candidates, + load_pool, + ) + + pool_name = ( + custom_entry.get("provider_key") + or custom_entry.get("name") + or provider + ) + for pool_key in custom_provider_pool_key_candidates( + custom_base, pool_name + ): + try: + pool = load_pool(pool_key) + except Exception: + continue + if not pool.has_credentials(): + continue + pool_entry = pool.select() + if pool_entry is None: + continue + pool_api_key = ( + getattr(pool_entry, "runtime_api_key", None) + or getattr(pool_entry, "access_token", "") + or "" + ) + if str(pool_api_key).strip(): + custom_key = str(pool_api_key).strip() + break + except Exception: + pass custom_key = custom_key or "no-key-required" if custom_key == "no-key-required": logger.warning( diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 93ed82f892..7678373d78 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -489,34 +489,83 @@ def _iter_custom_providers(config: Optional[dict] = None): yield _normalize_custom_pool_name(name), entry -def get_custom_provider_pool_key(base_url: Optional[str], provider_name: Optional[str] = None) -> Optional[str]: - """Look up the custom_providers list in config.yaml and return 'custom:' for a matching base_url. +def _custom_entry_name_aliases(norm_name: str, entry: Dict[str, Any]) -> set: + aliases = {norm_name} + provider_key = _normalize_custom_pool_name(str(entry.get("provider_key") or "")) + if provider_key: + aliases.add(provider_key) + return aliases - When provider_name is given, prefer matching by name first (solving the case where - multiple custom providers share the same base_url but have different API keys). - Falls back to base_url matching when no name match is found. - Returns None if no match is found. +def _requested_custom_name_aliases(provider_name: str) -> set: + normalized = _normalize_custom_pool_name(provider_name) + aliases = {normalized} if normalized else set() + if normalized.startswith(CUSTOM_POOL_PREFIX): + suffix = _normalize_custom_pool_name(normalized[len(CUSTOM_POOL_PREFIX):]) + if suffix: + aliases.add(suffix) + return aliases + + +def _pool_keys_for_custom_entry(norm_name: str, entry: Dict[str, Any]) -> List[str]: + """Durable ``providers.`` slug first, then legacy ``custom:``.""" + keys: List[str] = [] + seen = set() + + def _add(key: str) -> None: + normalized = str(key or "").strip().lower() + if normalized and normalized not in seen: + seen.add(normalized) + keys.append(normalized) + + provider_key = _normalize_custom_pool_name(str(entry.get("provider_key") or "")) + if provider_key: + _add(provider_key) + if norm_name: + _add(f"{CUSTOM_POOL_PREFIX}{norm_name}") + return keys + + +def custom_provider_pool_key_candidates( + base_url: Optional[str], + provider_name: Optional[str] = None, +) -> List[str]: + """Return pool keys to try for a custom endpoint. + + ``hermes auth add `` stores new-style ``providers.`` credentials + under the durable config slug (``b-ai``). Older rows and legacy + ``custom_providers:`` entries still live under ``custom:``. + Try the slug first, then the legacy namespace, so a populated pool is not + skipped in favour of the ``no-key-required`` placeholder. """ if not base_url: - return None + return [] normalized_url = base_url.strip().rstrip("/") + requested_aliases = ( + _requested_custom_name_aliases(provider_name) if provider_name else set() + ) - # When a provider name is given, try to match by name first. - # This fixes the P1 bug where two custom providers sharing the same - # base_url always resolve to the first one's credentials. - if provider_name: - normalized_name = _normalize_custom_pool_name(provider_name) + if requested_aliases: for norm_name, entry in _iter_custom_providers(): - if norm_name == normalized_name: - return f"{CUSTOM_POOL_PREFIX}{norm_name}" + if requested_aliases & _custom_entry_name_aliases(norm_name, entry): + return _pool_keys_for_custom_entry(norm_name, entry) - # Fall back to base_url matching (original behavior) for norm_name, entry in _iter_custom_providers(): entry_url = str(entry.get("base_url") or "").strip().rstrip("/") if entry_url and entry_url == normalized_url: - return f"{CUSTOM_POOL_PREFIX}{norm_name}" - return None + return _pool_keys_for_custom_entry(norm_name, entry) + return [] + + +def get_custom_provider_pool_key(base_url: Optional[str], provider_name: Optional[str] = None) -> Optional[str]: + """Look up the matching custom provider and return its preferred pool key. + + Prefers the durable ``providers.`` slug when present, otherwise + ``custom:``. When provider_name is given, match by name + first so two custom providers sharing a base_url keep separate keys. + """ + candidates = custom_provider_pool_key_candidates(base_url, provider_name) + return candidates[0] if candidates else None def list_custom_pool_providers() -> List[str]: @@ -557,6 +606,36 @@ def get_pool_strategy(provider: str) -> str: return STRATEGY_FILL_FIRST +def _keyed_custom_pool_matches( + pool_provider: str, + provider_norm: str, + base_url: Optional[str], +) -> bool: + """Match a durable ``providers.`` pool against runtime identities.""" + runtime_url = str(base_url or "").strip().rstrip("/") + if not runtime_url: + return False + try: + for normalized_name, entry in _iter_custom_providers(): + provider_key = _normalize_custom_pool_name( + str(entry.get("provider_key") or "") + ) + if provider_key != pool_provider: + continue + aliases = _custom_entry_name_aliases(normalized_name, entry) + aliases.add(f"{CUSTOM_POOL_PREFIX}{normalized_name}") + if provider_key: + aliases.add(f"{CUSTOM_POOL_PREFIX}{provider_key}") + configured_url = str(entry.get("base_url") or "").strip().rstrip("/") + if provider_norm == "custom": + return runtime_url == configured_url + runtime_aliases = _requested_custom_name_aliases(provider_norm) + return bool(runtime_aliases & aliases) and runtime_url == configured_url + except Exception: + return False + return False + + def credential_pool_matches_provider( pool_or_provider: Any, provider: Optional[str], @@ -567,10 +646,12 @@ def credential_pool_matches_provider( Named custom endpoints may use three identities: the live agent can retain the configured name/provider key, newer runtime paths normalize it to - ``custom``, and the pool is keyed ``custom:``. Accept those aliases - only when the runtime endpoint belongs to the same configured custom - provider. Empty identities fail closed. Legacy pool adapters without a - ``provider`` attribute remain compatible; production pools are scoped. + ``custom``, and the pool may be keyed either as the durable + ``providers.`` slug or as legacy ``custom:``. Accept those + aliases only when the runtime endpoint belongs to the same configured + custom provider. Empty identities fail closed. Legacy pool adapters + without a ``provider`` attribute remain compatible; production pools + are scoped. """ raw_pool_provider = getattr(pool_or_provider, "provider", None) if raw_pool_provider is None: @@ -586,13 +667,18 @@ def credential_pool_matches_provider( if not pool_provider or not provider_norm: return False if not pool_provider.startswith(CUSTOM_POOL_PREFIX): - return pool_provider == provider_norm + if pool_provider == provider_norm: + return True + return _keyed_custom_pool_matches(pool_provider, provider_norm, base_url) if provider_norm == "custom": try: matched_pool = get_custom_provider_pool_key(base_url or "") + if str(matched_pool or "").strip().lower() == pool_provider: + return True + candidates = custom_provider_pool_key_candidates(base_url or "") except Exception: return False - return str(matched_pool or "").strip().lower() == pool_provider + return pool_provider in {str(key).strip().lower() for key in candidates} runtime_url = str(base_url or "").strip().rstrip("/") if not runtime_url: @@ -625,9 +711,10 @@ def credential_pool_matches_provider( def resolve_runtime_pool_key(provider: Optional[str], base_url: Optional[str]) -> str: """Resolve the credential-pool key for a runtime provider identity. - Named custom runtimes retain their configured alias while their pool is - stored under ``custom:``. Return that scoped key only when the - canonical provider/endpoint boundary accepts it; otherwise preserve the + Named custom runtimes retain their configured alias while their pool may + be stored under the durable ``providers.`` slug or legacy + ``custom:``. Return that scoped key only when the canonical + provider/endpoint boundary accepts it; otherwise preserve the normalized runtime identity so callers fail closed. """ provider_norm = str(provider or "").strip().lower() @@ -644,18 +731,19 @@ def resolve_runtime_pool_key(provider: Optional[str], base_url: Optional[str]) - ): return str(candidate).strip().lower() else: - # Named and exact custom runtimes are keyed by provider identity, - # while auth storage remains keyed by display name. Search the - # configured candidates by identity before considering endpoint; - # this prevents a sibling sharing the URL from lending its pool. - for normalized_name, _entry in _iter_custom_providers(): - candidate = f"{CUSTOM_POOL_PREFIX}{normalized_name}" - if credential_pool_matches_provider( - candidate, - provider_norm, - base_url=base_url, - ): - return candidate + # Named and exact custom runtimes are keyed by provider identity. + # Auth storage prefers the durable providers. slug, with + # legacy custom: as fallback. Search configured + # candidates by identity before considering endpoint so a sibling + # sharing the URL cannot lend its pool. + for normalized_name, entry in _iter_custom_providers(): + for candidate in _pool_keys_for_custom_entry(normalized_name, entry): + if credential_pool_matches_provider( + candidate, + provider_norm, + base_url=base_url, + ): + return candidate except Exception: pass return provider_norm diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index b663a92ca5..b6899eb025 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -15,6 +15,7 @@ from agent.credential_pool import ( CredentialPool, PooledCredential, credential_pool_matches_provider, + custom_provider_pool_key_candidates, get_custom_provider_pool_key, load_pool, ) @@ -670,42 +671,59 @@ def _try_resolve_from_custom_pool( provider_name: Optional[str] = None, ) -> Optional[Dict[str, Any]]: """Check if a credential pool exists for a custom endpoint and return a runtime dict if so.""" - pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name) - if not pool_key: - return None + candidates: list[str] = [] + seen = set() + + def _add(key: Optional[str]) -> None: + normalized = str(key or "").strip().lower() + if normalized and normalized not in seen: + seen.add(normalized) + candidates.append(normalized) + try: - pool = load_pool(pool_key) - if not pool.has_credentials(): - return None - entry = pool.select() - if entry is None: - return None - pool_api_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "") - if not pool_api_key: - return None - if not has_usable_secret(pool_api_key) and _loopback_hostname(base_url_hostname(base_url)): - # Legacy configs commonly used short/placeholder keys ('123', - # 'm', ...) for local no-auth services like Ollama -- fine for - # the endpoint itself, but has_usable_secret's 4-char floor - # (added after these configs were written) now rejects them - # here with no migration path. Every OTHER resolution path in - # this file already substitutes "no-key-required" for a - # loopback endpoint with no usable secret (the config-based - # custom_providers fallback a few hundred lines below, and the - # "actual" provider's local-offline exemption further down) -- - # this pool path was the one gap (issue #86864). - pool_api_key = "no-key-required" - return { - "provider": provider_label, - "api_mode": api_mode_override or _detect_api_mode_for_url(base_url) or "chat_completions", - "base_url": base_url, - "api_key": pool_api_key, - "source": f"pool:{pool_key}", - "credential_pool": pool, - } + for key in custom_provider_pool_key_candidates(base_url, provider_name): + _add(key) except Exception: + pass + _add(get_custom_provider_pool_key(base_url, provider_name=provider_name)) + if not candidates: return None + for pool_key in candidates: + try: + pool = load_pool(pool_key) + if not pool.has_credentials(): + continue + entry = pool.select() + if entry is None: + continue + pool_api_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "") + if not pool_api_key: + continue + if not has_usable_secret(pool_api_key) and _loopback_hostname(base_url_hostname(base_url)): + # Legacy configs commonly used short/placeholder keys ('123', + # 'm', ...) for local no-auth services like Ollama -- fine for + # the endpoint itself, but has_usable_secret's 4-char floor + # (added after these configs were written) now rejects them + # here with no migration path. Every OTHER resolution path in + # this file already substitutes "no-key-required" for a + # loopback endpoint with no usable secret (the config-based + # custom_providers fallback a few hundred lines below, and the + # "actual" provider's local-offline exemption further down) -- + # this pool path was the one gap (issue #86864). + pool_api_key = "no-key-required" + return { + "provider": provider_label, + "api_mode": api_mode_override or _detect_api_mode_for_url(base_url) or "chat_completions", + "base_url": base_url, + "api_key": pool_api_key, + "source": f"pool:{pool_key}", + "credential_pool": pool, + } + except Exception: + continue + return None + def _filter_capabilities(value: Any) -> Dict[str, bool]: """Return the string-keyed boolean capabilities accepted at runtime.""" @@ -834,6 +852,11 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An "api_key": resolved_api_key, "model": entry.get("default_model", ""), } + provider_key = str(ep_name or "").strip() + if provider_key: + result["provider_key"] = provider_key + if key_env: + result["key_env"] = key_env extra_body = entry.get("extra_body") if isinstance(extra_body, dict): result["extra_body"] = dict(extra_body) @@ -1267,7 +1290,12 @@ def _resolve_named_custom_runtime( return None # Check if a credential pool exists for this custom endpoint - pool_result = _try_resolve_from_custom_pool(base_url, "custom", custom_provider.get("api_mode"), provider_name=custom_provider.get("name")) + pool_result = _try_resolve_from_custom_pool( + base_url, + "custom", + custom_provider.get("api_mode"), + provider_name=custom_provider.get("provider_key") or custom_provider.get("name"), + ) if pool_result: # Propagate the model name even when using pooled credentials — # the pool doesn't know about the custom_providers model field. diff --git a/tests/agent/test_credential_pool_provider_boundary.py b/tests/agent/test_credential_pool_provider_boundary.py index 00bbc947ad..1a5f5cfd31 100644 --- a/tests/agent/test_credential_pool_provider_boundary.py +++ b/tests/agent/test_credential_pool_provider_boundary.py @@ -89,14 +89,14 @@ def test_runtime_pool_key_resolves_all_custom_runtime_identities(): ) ] with patch("agent.credential_pool._iter_custom_providers", return_value=configured): - assert resolve_runtime_pool_key("custom", endpoint) == "custom:sibling-display" + assert resolve_runtime_pool_key("custom", endpoint) == "sibling-provider" assert ( resolve_runtime_pool_key("gemini-no-filter", endpoint) - == "custom:gemini-display" + == "gemini-no-filter" ) assert ( resolve_runtime_pool_key("custom:gemini-no-filter", endpoint) - == "custom:gemini-display" + == "gemini-no-filter" ) assert ( resolve_runtime_pool_key( @@ -127,11 +127,11 @@ def test_runtime_pool_key_resolves_modern_provider_in_mixed_config(): with patch("agent.credential_pool._load_config_safe", return_value=config): assert ( resolve_runtime_pool_key("gemini-no-filter", endpoint) - == "custom:gemini-display" + == "gemini-no-filter" ) assert ( resolve_runtime_pool_key("custom:gemini-no-filter", endpoint) - == "custom:gemini-display" + == "gemini-no-filter" ) assert ( resolve_runtime_pool_key( diff --git a/tests/run_agent/test_primary_runtime_restore.py b/tests/run_agent/test_primary_runtime_restore.py index 72ad60dff3..9826d9f06e 100644 --- a/tests/run_agent/test_primary_runtime_restore.py +++ b/tests/run_agent/test_primary_runtime_restore.py @@ -517,7 +517,7 @@ class TestRestorePrimaryRuntime: assert result is True assert agent._credential_pool is primary_pool - load_pool.assert_called_once_with("custom:gemini-display") + load_pool.assert_called_once_with("gemini-no-filter") agent._swap_credential.assert_called_once_with(primary_pool.select.return_value) def test_restore_named_custom_pool_wrong_endpoint_fails_closed(self):