Skip to content

fix(web): light up web tools when a registered custom provider is available but none configured - #57808

Closed
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/web-lightup-multi-custom-provider
Closed

fix(web): light up web tools when a registered custom provider is available but none configured#57808
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/web-lightup-multi-custom-provider

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

What does this PR do?

check_web_api_key() is the check_fn gate that decides whether web_search and web_extract appear in the toolset. Today's refactor (a9cd0e07c, "single registry authority for custom-provider availability") changed the plugin-provider branch of that gate to delegate to the registry's single-active resolvers:

from agent.web_search_registry import (
    get_active_search_provider,
    get_active_extract_provider,
)
return (
    get_active_search_provider() is not None
    or get_active_extract_provider() is not None
)

get_active_*_provider() calls _resolve(None, capability=...) (no config key). With explicit=None, _resolve only returns a provider when exactly one is eligible (len(eligible) == 1) or the name matches the _LEGACY_PREFERENCE order — and custom plugin names never appear in that legacy order. So when 2+ registered custom providers are available and capable and no web.backend config key is set, _resolve returns None for both capabilities, check_web_api_key() returns False, and web_search / web_extract are filtered out of the toolset entirely.

Meanwhile _get_backend() (the router) has a final fallback loop that walks _list_registered_web_providers() and returns the first available non-legacy provider — so it does resolve one of them. The gate and the router now diverge: the tools are hidden even though the router would dispatch to a custom provider without issue. This is exactly the "second resolution path that could diverge" the refactor set out to remove, reintroduced in the opposite direction. check_web_api_key's own docstring states the intended contract: a plugin-registered provider that reports is_available() must light the tools up even when no built-in backend has credentials (issues #28651, #31873).

The fix mirrors _get_backend()'s final registered-provider walk in the gate so the two agree. The single-custom-provider case already worked (and is covered by the test landed with the refactor); this closes the 2+ case.

Related Issue

Code-originated regression from a9cd0e07c — no separate issue filed.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/web_tools.py — in check_web_api_key(), replace the get_active_search_provider() / get_active_extract_provider() delegation tail with the same registered-provider walk _get_backend() uses (skip _LEGACY_WEB_BACKENDS; return True on the first provider.is_available()), so the gate and the router agree.
  • tests/tools/test_web_tools_config.py — add a regression test registering two available custom providers with no config key, asserting check_web_api_key() is True and _get_backend() resolves one of them.

How to Test

  1. Register two distinct custom WebSearchProviders (e.g. custom-a, custom-b), both is_available() == True, both search/extract-capable.
  2. Strip all built-in web env keys and set no web.backend config key.
  3. Before this change: check_web_api_key() returns False (tools hidden) while _get_backend() returns one of the two names — gate and router disagree.
  4. After this change: check_web_api_key() returns True, matching _get_backend().

Regression test test_check_web_api_key_true_for_multiple_custom_providers_none_configured fails before / passes after. Full touched-area suite (tests/tools/test_web_tools_config.py, test_web_providers.py, test_web_tools_tavily.py, test_web_tools_truncate.py) passes: 96 passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) — pure Python provider-registry logic, no platform-specific code — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Copilot AI review requested due to automatic review settings July 3, 2026 15:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in the web tools availability gate (check_web_api_key) where web_search / web_extract could be hidden when multiple plugin-registered custom providers were available but no web.backend was configured, even though the router (_get_backend) could still successfully resolve a provider.

Changes:

  • Update check_web_api_key() to mirror _get_backend()’s “registered custom providers” fallback walk so tool gating and routing agree.
  • Add a regression test covering the “2+ available custom providers, no config key” scenario.
  • Ensure the test asserts both the gate result and that _get_backend() resolves one of the custom providers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tools/web_tools.py Aligns tool availability gating with router fallback behavior when multiple custom providers are registered.
tests/tools/test_web_tools_config.py Adds regression coverage for multiple custom providers with no configured backend.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/web_tools.py Outdated
Comment on lines +982 to +991
for provider in _list_registered_web_providers():
if provider.name in _LEGACY_WEB_BACKENDS:
continue
try:
if provider.is_available():
return True
except Exception as exc: # noqa: BLE001 — a broken provider is skipped
logger.debug(
"web provider %r.is_available() raised: %s", provider.name, exc
)
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/web Web search and extraction comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: follow-up to the just-merged #57779. That refactor threaded the registry through _is_backend_available, _get_backend, and _get_capability_backend, but left check_web_api_key() resolving custom providers via get_active_search_provider()/get_active_extract_provider() (_resolve(None)), which returns None when 2+ custom providers are registered and no web.backend key is set — so the gate hides web_search/web_extract even though _get_backend() would dispatch. This PR mirrors _get_backend()'s _list_registered_web_providers() walk in the gate so the two agree. Verified on main: check_web_api_key() still uses the divergent resolver path (this is a real gap, not a duplicate). Same cluster as #36987 / #28651 / #31873. Maintainer to confirm consolidation.

@briandevans

Copy link
Copy Markdown
Contributor Author

@copilot Addressed in 3a914fe0. The custom-provider fallback walk in check_web_api_key() now routes each provider through _registered_web_provider_available(provider.name) instead of inlining its own is_available() try/except/log, so broken-provider handling has a single source of truth shared with backend selection. Behavior is unchanged: legacy names are still skipped, and a provider whose is_available() raises is treated as unavailable (the helper catches and returns False, and the truthy check skips both False and the unreachable None). tests/tools/test_web_tools_config.py + test_web_providers.py (71 tests, incl. test_check_web_api_key_true_for_multiple_custom_providers_none_configured) pass; ruff clean.

@briandevans
briandevans force-pushed the fix/web-lightup-multi-custom-provider branch from fba9d8d to 3a914fe Compare July 9, 2026 23:31
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying a real current-main regression: check_web_api_key() uses the registry's active-provider resolvers (tools/web_tools.py:1069-1085), while _get_backend() independently selects the first available registered custom provider (tools/web_tools.py:255-269). With multiple unconfigured custom providers, _resolve() only accepts exactly one eligible provider before its legacy-only fallback (agent/web_search_registry.py:203-219).

Problems

  • The proposed loop restores the duplicate resolution path that a9cd0e07c deliberately removed. That commit records that the old hand-rolled walk bypassed capability-aware registry selection. The fallback policy should be consolidated in agent/web_search_registry.py::_resolve() rather than duplicated in the gate.

Suggested changes

  • Add a deterministic, capability-aware non-legacy fallback in _resolve() after the legacy-preference walk, then keep check_web_api_key() using the active resolvers.
  • Test multiple custom providers through both active-provider resolver functions, not only _get_backend() and the gate.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 15, 2026
…ilable but none configured

check_web_api_key() gates whether web_search / web_extract appear in the
toolset. After the a9cd0e0 refactor it delegated its plugin-provider
probe to the registry's single-active get_active_search_provider() /
get_active_extract_provider(), i.e. _resolve(None, ...). With no
web.backend config key, _resolve only returns a provider when exactly one
is eligible (len(eligible) == 1) or the name matches the legacy preference
order; custom plugin names never match. So with 2+ registered custom
providers, all available and capable, and no config key, both resolvers
return None and the gate returns False — hiding the web tools entirely.

Meanwhile _get_backend() has a final fallback that walks
_list_registered_web_providers() and returns the first available non-legacy
provider, so it DOES resolve one of them. Gate and router diverge: the tools
are filtered out even though the router would happily dispatch to a custom
provider.

Mirror _get_backend()'s final registered-provider walk in the gate so the
two agree. Works for the single-custom-provider case (already covered);
this closes the 2+ case.
…lity helper

The custom-provider fallback walk in check_web_api_key() duplicated the
registry is_available() try/except/log already in
_registered_web_provider_available(). Route the loop through that helper so
broken-provider handling stays identical between backend selection and tool
gating, with a single source of truth. Behavior is unchanged: legacy names
are still skipped and a provider whose is_available() raises is treated as
unavailable.
@briandevans
briandevans force-pushed the fix/web-lightup-multi-custom-provider branch from 3a914fe to 2ec6db4 Compare July 16, 2026 07:32
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed needs-decision Awaiting maintainer decision before any implementation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 16, 2026
@briandevans

Copy link
Copy Markdown
Contributor Author

Re-verified against current main — this no longer reproduces; the behaviour is already correct on main (see 0a9d42ce40), so this change is not needed. Closing — thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/web Web search and extraction type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants