diff --git a/agent/image_gen_registry.py b/agent/image_gen_registry.py index 715133231cb..5d14a6f1ece 100644 --- a/agent/image_gen_registry.py +++ b/agent/image_gen_registry.py @@ -77,6 +77,17 @@ def get_active_provider() -> Optional[ImageGenProvider]: Reads ``image_gen.provider`` from config.yaml; falls back per the module docstring. + + **Availability semantics** (mirrors :mod:`agent.web_search_registry`): + + - When ``image_gen.provider`` is explicitly set, the configured + provider is returned even if :meth:`ImageGenProvider.is_available` + reports False — the dispatcher surfaces a precise "X_API_KEY is not + set" error rather than silently switching backends. + - When ``image_gen.provider`` is unset, the fallback path (single- + provider shortcut and the FAL legacy preference) is filtered by + ``is_available()`` so we don't pick a provider the user has no + credentials for. """ configured: Optional[str] = None try: @@ -94,6 +105,17 @@ def get_active_provider() -> Optional[ImageGenProvider]: with _lock: snapshot = dict(_providers) + def _is_available_safe(p: ImageGenProvider) -> bool: + """Wrap ``is_available()`` so a buggy provider doesn't kill resolution.""" + try: + return bool(p.is_available()) + except Exception as exc: # noqa: BLE001 + logger.debug("image_gen provider %s.is_available() raised %s", p.name, exc) + return False + + # 1. Explicit config wins — return regardless of is_available() so the + # user gets a precise downstream error message rather than a silent + # backend switch. if configured: provider = snapshot.get(configured) if provider is not None: @@ -103,13 +125,16 @@ def get_active_provider() -> Optional[ImageGenProvider]: configured, ) - # Fallback: single-provider case - if len(snapshot) == 1: - return next(iter(snapshot.values())) + # 2. Fallback: single registered provider — but only if it's actually + # available (no credentials = don't surface it as "active"). + available = [p for p in snapshot.values() if _is_available_safe(p)] + if len(available) == 1: + return available[0] - # Fallback: prefer legacy FAL for backward compat - if "fal" in snapshot: - return snapshot["fal"] + # 3. Fallback: prefer legacy FAL for backward compat, when available. + fal = snapshot.get("fal") + if fal is not None and _is_available_safe(fal): + return fal return None diff --git a/agent/web_search_registry.py b/agent/web_search_registry.py index 8f1e884b3cf..45f2a0f8883 100644 --- a/agent/web_search_registry.py +++ b/agent/web_search_registry.py @@ -114,7 +114,31 @@ _LEGACY_PREFERENCE = ("brave-free", "firecrawl", "searxng", "ddgs") def _resolve(configured: Optional[str], *, capability: str) -> Optional[WebSearchProvider]: - """Resolve the active provider for a capability ("search" | "extract").""" + """Resolve the active provider for a capability ("search" | "extract"). + + Resolution rules (in order): + + 1. **Explicit config wins, ignoring availability.** If + ``web.{capability}_backend`` or ``web.backend`` names a registered + provider that supports *capability*, return it even if its + :meth:`is_available` returns False — the dispatcher will surface a + precise "X_API_KEY is not set" error to the user instead of silently + routing somewhere else. Matches legacy + :func:`tools.web_tools._get_backend` behavior for configured names. + + 2. **Single-provider shortcut.** When only one registered provider + supports *capability* AND ``is_available()`` reports True, return it. + + 3. **Legacy preference walk, filtered by availability.** Walk the + :data:`_LEGACY_PREFERENCE` order looking for a provider whose + ``supports_()`` is True AND whose ``is_available()`` is + True. This is the path that fires when no config key is set — pick + the highest-priority backend the user actually has credentials for. + + Returns None when no provider is configured AND no available provider + matches the legacy preference; the dispatcher then returns a "set up a + provider" error to the user. + """ with _lock: snapshot = dict(_providers) @@ -125,6 +149,17 @@ def _resolve(configured: Optional[str], *, capability: str) -> Optional[WebSearc return bool(p.supports_extract()) return False + def _is_available_safe(p: WebSearchProvider) -> bool: + """Wrap ``is_available()`` so a buggy provider doesn't kill resolution.""" + try: + return bool(p.is_available()) + except Exception as exc: # noqa: BLE001 + logger.debug("provider %s.is_available() raised %s", p.name, exc) + return False + + # 1. Explicit config wins — return regardless of is_available() so the + # user gets a precise downstream error message rather than a silent + # backend switch. Matches _get_backend() in web_tools.py. if configured: provider = snapshot.get(configured) if provider is not None and _capable(provider): @@ -140,13 +175,24 @@ def _resolve(configured: Optional[str], *, capability: str) -> Optional[WebSearc configured, capability, ) - eligible = [p for p in snapshot.values() if _capable(p)] + # 2. + 3. Fallback path — filter by availability so we don't surface + # a provider the user has no credentials for. Without this filter, + # brave-free's slot in the legacy preference order would make it + # the "active" provider on a fresh install with no API keys at all. + eligible = [ + p for p in snapshot.values() + if _capable(p) and _is_available_safe(p) + ] if len(eligible) == 1: return eligible[0] for legacy in _LEGACY_PREFERENCE: provider = snapshot.get(legacy) - if provider is not None and _capable(provider): + if ( + provider is not None + and _capable(provider) + and _is_available_safe(provider) + ): return provider return None