Skip to content

fix(web-tools): lazy plugin discovery in registry access functions - #71791

Open
troyrowe-resource wants to merge 1 commit into
NousResearch:mainfrom
troyrowe-resource:fix/web-extract-plugin-discovery-order
Open

fix(web-tools): lazy plugin discovery in registry access functions#71791
troyrowe-resource wants to merge 1 commit into
NousResearch:mainfrom
troyrowe-resource:fix/web-extract-plugin-discovery-order

Conversation

@troyrowe-resource

@troyrowe-resource troyrowe-resource commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Problem

web_extract_tool() called _get_extract_backend() before _ensure_web_plugins_loaded(), so plugin-registered backends (e.g. a custom crawl4ai extract provider) were not yet in the web_search_registry when availability was checked. _is_backend_available() returned None for the configured backend, silently falling back to the wrong provider (e.g. ddgs or firecrawl instead of crawl4ai).

The symptom: web.extract_backend: crawl4ai set in config, web-crawl4ai plugin enabled, container running — but web_extract returned "ddgs is a search-only backend and cannot extract URL content".

Root Cause

The registry access functions (_registered_web_provider, _list_registered_web_providers) assumed plugin discovery had already been triggered by the caller. This made the entire backend selection chain fragile and order-dependent:

_get_extract_backend()
  → _get_capability_backend("extract")
    → _is_backend_available("crawl4ai")
      → _registered_web_provider_available("crawl4ai")
        → _registered_web_provider("crawl4ai")
          → registry lookup → None (discovery hasn't run yet!)
      → returns None → falls through to _get_backend() → wrong backend

web_search_tool() happened to have the correct call order (_ensure_web_plugins_loaded() before _get_search_backend()), but web_extract_tool() had them reversed — a copy-paste regression.

Fix

Added _ensure_web_plugins_loaded_once() — a lazy discovery helper that calls the idempotent _ensure_plugins_discovered(). Injected it into the two registry access functions that all backend selection chains through:

  • _registered_web_provider(backend) — calls _ensure_web_plugins_loaded_once() before registry lookup
  • _list_registered_web_providers() — calls _ensure_web_plugins_loaded_once() before listing

This makes _is_backend_available → _get_capability_backend → _get_extract_backend fully order-independent. Any caller that queries the registry gets discovery for free, regardless of call order.

The existing explicit _ensure_web_plugins_loaded() calls in web_search_tool() and web_extract_tool() are left in place as belt-and-suspenders (0ms overhead after first call due to the _discovered re-entrancy guard in PluginManager.discover_and_load()).

Verification

# Without any prior plugin discovery call:
>>> _is_backend_available("crawl4ai")
True
>>> _get_extract_backend()
'crawl4ai'

# Idempotency: 100 calls take 0.000s (cached via _discovered flag)
# No circular import (try/except guards; hermes_cli.plugins does not import web_tools)

End-to-end test:

web_search("e-waste recycling Victoria")  → 3 results via ddgs ✅
web_extract("https://www.sustainability.vic.gov.au/...")  → 3488 chars via crawl4ai ✅

Checklist

  • Root cause identified (order-dependent registry access)
  • Fix targets the registry access layer, not a single call site
  • Idempotent (relies on existing _discovered re-entrancy guard)
  • No circular import risk (verified at runtime)
  • No performance impact (0ms per call after first)
  • Existing explicit _ensure_web_plugins_loaded() calls preserved as belt-and-suspenders
  • web_extract_tool() code unchanged from upstream (no ordering hack)

@troyrowe-resource
fix(web-tools): lazy plugin discovery in registry access functions
c60c00b 
web_extract_tool() called _get_extract_backend() before
_ensure_web_plugins_loaded(), so plugin-registered backends (e.g.
crawl4ai) were not yet in the web_search_registry when availability
was checked. _is_backend_available() returned None for the configured
backend, silently falling back to the wrong provider.

Root cause: the registry access functions (_registered_web_provider,
_list_registered_web_providers) assumed plugin discovery had already
been triggered by the caller, making the entire backend selection
chain fragile and order-dependent.

Fix: add _ensure_web_plugins_loaded_once() - a lazy discovery helper
that calls the idempotent _ensure_plugins_discovered(). Inject it
into _registered_web_provider() and _list_registered_web_providers()
so every registry query self-initialises, regardless of call order.

This makes _is_backend_available -> _get_capability_backend ->
_get_extract_backend fully order-independent. The existing explicit
_ensure_web_plugins_loaded() calls in web_search_tool and
web_extract_tool remain as belt-and-suspenders (0ms overhead after
first call due to the _discovered re-entrancy guard).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets comp/plugins Plugin system and bundled plugins tool/web Web search and extraction labels Jul 26, 2026
@alt-glitch

alt-glitch commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #34563 and #69144. This patch places discovery before registry-backed backend selection; discovery performed after _get_extract_backend() cannot repair that ordering.

@GottZ GottZ mentioned this pull request Jul 28, 2026
Open
teknium1
teknium1 reviewed Jul 30, 2026
View reviewed changes

@teknium1 teknium1 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.

Thanks for isolating the backend-selection ordering gap. The premise is still present on current main: web_extract_tool() selects at tools/web_tools.py:857 before discovery at tools/web_tools.py:866, while configured non-legacy backends pass through _is_backend_available() at tools/web_tools.py:323-327.

Problems

  • No test exercises the new cold-registry selection guarantee. The existing dispatcher test at tests/tools/test_web_providers.py:249-332 verifies only that discovery eventually runs and uses legacy firecrawl; it does not call _get_extract_backend() with an empty registry and configured custom provider.
  • check_web_api_key() still resolves active registry providers directly at tools/web_tools.py:1073-1080, bypassing both helpers changed here. That remains a cold-start path for an otherwise available custom provider when no backend is configured.

Suggested changes

  • Add a focused cold-registry test for _get_extract_backend() and one for check_web_api_key().
  • Reuse existing _ensure_web_plugins_loaded() from the registry helpers instead of maintaining a second discovery wrapper with different logging behavior.

Automated hermes-sweeper review.

Comment thread tools/web_tools.py

Lazily triggers plugin discovery first so the registry is populated
even when this is called before any tool has run
``_ensure_web_plugins_loaded()``.

@teknium1 teknium1 Jul 30, 2026

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.

Please reuse the existing _ensure_web_plugins_loaded() helper here. It already performs this exact idempotent discovery and preserves the warning-level diagnostic for a failed plugin load; maintaining a second wrapper risks divergent behavior.

@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 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Twenty-four PRs address this complex across three causes: cold-registry plugin discovery, hardcoded provider selection/tool gates, and non-actionable failure reporting. The discovery and allowlist classes have merged reference implementations in #34563 and #57779, while #58359 addresses genuine failure guidance and #67110/#67309/#71791/#72646 address the remaining selection-before-discovery ordering gap.

Related pull requests

Duplicates

#27584, #27700, #28202, and #69144 overlap on discovery-before-dispatch, with #34563 as the merged reference; #31829, #31887, #32465, #33516, #33902, #36094, #38375, #52057, #56201, and #56761 overlap with the provider-gate class implemented by #57779. #67110, #67309, #71791, and the web-selection portion of #72646 overlap on discovery-before-selection, while #58359 remains the distinct failure-guidance change.

Suggested consolidation

Keep #71791 open with a salvage path: reuse the existing _ensure_web_plugins_loaded() diagnostic behavior, add cold-registry tests for configured extraction and check_web_api_key(), and reconcile its helper placement with the broader tested #67309 approach rather than duplicating it. Keep #58359 and #72646 open only for their separately salvageable guidance and display work after their respective contributor reviews are addressed; the older discovery and allowlist variants can remain closed as duplicates or superseded references to #34563 and #57779.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I27683(["issue #27683 (closed)"])
    subgraph Dup67309 ["PRs duplicating each other"]
        P67309["PR #67309 (open)"]
        P71791["PR #71791 (open)"]
    end
    P71791 -.->|partial| I27683
    class I27683 closed
    class P67309 open
    class P71791 open
    class P71791 target
    click I27683 "https://github.com/NousResearch/hermes-agent/issues/27683"
    click P67309 "https://github.com/NousResearch/hermes-agent/pull/67309"
    click P71791 "https://github.com/NousResearch/hermes-agent/pull/71791"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 24 pull requests and 8 issues in this complex. Each diff was read against this issue; Assessment working set: 147 kB of PR diffs, 118 kB of issue/PR text, 64 kB of discussion (79 comments), 95 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

This was referenced Aug 3, 2026
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

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 sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/web Web search and extraction type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@troyrowe-resource @alt-glitch @GottZ @teknium1