Skip to content

refactor(web): all web providers as image_gen-style plugins#25182

Closed
kshitijk4poor wants to merge 24 commits into
NousResearch:mainfrom
kshitijk4poor:spike/web-providers-plugin
Closed

refactor(web): all web providers as image_gen-style plugins#25182
kshitijk4poor wants to merge 24 commits into
NousResearch:mainfrom
kshitijk4poor:spike/web-providers-plugin

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates every web provider (search + extract + crawl) out of tools/web_tools.py into the bundled-plugin architecture, matching the image_gen template. All seven providers now live under plugins/web/<vendor>/, registering through agent.web_search_registry instead of hardcoded if backend == ... chains in the dispatcher.

This unblocks user-added web providers (drop a directory under ~/.hermes/plugins/web/ and they appear in hermes tools automatically — same UX as image_gen / spotify / google_meet plugins).

Scope — what moved

Provider Capabilities Async extract? Async crawl? Auth
brave-free search sync BRAVE_SEARCH_API_KEY
ddgs search sync none (pip ddgs)
searxng search sync SEARXNG_URL
exa search + extract sync EXA_API_KEY
parallel search + extract async PARALLEL_API_KEY
tavily search + extract + crawl sync sync TAVILY_API_KEY
firecrawl search + extract + crawl async async direct (FIRECRAWL_API_KEY / FIRECRAWL_API_URL) or Nous Tool Gateway

Each plugin is plugins/web/<vendor>/{plugin.yaml,__init__.py,provider.py}.

What changes for users

Nothing. The setup wizard, hermes tools picker, and per-provider env vars all behave identically. The PR is a pure refactor preserving the legacy response shapes bit-for-bit. The only visible difference is that user-installed web plugins now light up in the picker the same way image_gen plugins do.

Architecture

Before:

  • 9 hardcoded TOOL_CATEGORIES["web"] rows in hermes_cli/tools_config.py
  • Per-vendor inline helpers in tools/web_tools.py (_get_firecrawl_client, _tavily_request, _parallel_search, etc. — ~600 LoC)
  • Per-vendor if backend == "exa": ... elif backend == "tavily": ... chains in all three dispatchers
  • Legacy in-tree ABCs at tools/web_providers/base.py

After:

  • 2 hardcoded TOOL_CATEGORIES["web"] rows ("Nous Subscription", "Firecrawl Self-Hosted" — the two non-provider UX setup flows for firecrawl); the other 7 come from _plugin_web_search_providers()
  • All vendor code lives in plugins/web/<vendor>/provider.py
  • Each dispatcher is a single registry lookup + inspect.iscoroutinefunction async/sync detection
  • One ABC at agent/web_search_provider.py with supports_search/extract/crawl capability flags

Registry resolution semantics (Option B — conservative smart fallback)

_resolve() in agent/web_search_registry.py follows three rules in order:

  1. Explicit config wins, ignoring availability. If web.<capability>_backend (or web.backend) names a registered + capable provider, return it — even if is_available() is False. The dispatcher then surfaces a precise "X_API_KEY is not set" error instead of silently switching backends.
  2. Single-provider shortcut. When only one registered provider matches the capability and is available, pick it.
  3. Legacy preference walk, filtered by availability. Walk _LEGACY_PREFERENCE (firecrawl → parallel → tavily → exa → searxng → brave-free → ddgs) and pick the first one that's capable AND available. Matches the historic tools.web_tools._get_backend() candidate order so installs that never set a config key keep landing on the same provider they did before the plugin migration.

When the configured name is registered but capability-incompatible (e.g. web.extract_backend: brave-free), the dispatcher surfaces a typed "X is a search-only backend and cannot extract URL content" error matching the legacy wording.

Async-or-sync extract dispatch

parallel and firecrawl declare async def extract(); exa and tavily are sync. The dispatcher detects via inspect.iscoroutinefunction(provider.extract) and either awaits the result or runs the sync call in asyncio.to_thread(...) so the gateway event loop is never blocked.

Backward compatibility

The names in tools.web_tools.* that tests/integration code reaches for are preserved as re-export shims from the plugins:

  • Firecrawl (lazy proxy), check_firecrawl_api_key, _get_firecrawl_client, _is_tool_gateway_ready, _has_direct_firecrawl_config, _get_direct_firecrawl_config, _get_firecrawl_gateway_url, _firecrawl_backend_help_suffix, _raise_web_backend_configuration_error, _extract_web_search_results, _extract_scrape_payload, _to_plain_object, _normalize_result_list
  • _tavily_request, _normalize_tavily_search_results, _normalize_tavily_documents
  • _get_parallel_client, _get_async_parallel_client, _get_exa_client
  • The cached client globals (_firecrawl_client, _parallel_client, _async_parallel_client, _exa_client) live on tools.web_tools so unit tests that reset them between cases keep working

The plugin implementations use import tools.web_tools as _wt indirection to read helpers like _wt._read_nous_access_token, _wt.Firecrawl, _wt.prefers_gateway so test patches that target the legacy paths reach the plugin code transparently.

Diff stat

39 files changed, 3459 insertions(+), 1534 deletions(-)

The big wins:

  • tools/web_tools.py: 2227 → 1511 lines (−716 lines / −32%)
  • tools/web_providers/ directory deleted entirely (−168 lines)
  • hermes_cli/tools_config.py: 7 hardcoded provider rows removed (−91 lines)

The added lines are split roughly: 60% new plugin code (one provider.py per vendor, ranges from ~90 lines for brave-free to ~750 lines for firecrawl which now also handles crawl), 25% new tests (tests/plugins/web/test_web_search_provider_plugins.py = 472 lines, 45 tests), 15% new ABC + registry + plugin glue (agent/web_search_provider.py + agent/web_search_registry.py + hermes_cli/plugins.py web context hook).

Testing

210/210 plugin + dispatcher tests pass (208 original + 2 new regression tests from behavior-parity verification):

  • tests/tools/test_web_providers.py (ABC contract — rewritten to test the new unified ABC)
  • tests/tools/test_web_providers_brave_free.py / _ddgs.py / _searxng.py
  • tests/tools/test_web_tools_config.py (backend selection, client config, error responses)
  • tests/tools/test_web_tools_tavily.py
  • tests/tools/test_website_policy.py (per-URL gate + redirect re-check)
  • tests/tools/test_config_null_guard.py
  • tests/plugins/web/test_web_search_provider_plugins.py (45 new tests covering ABC, registry, async detection, error shapes)

Plugin test classes (45 tests):

  • TestBundledPluginsRegister (16) — all seven plugins register, capability flags correct, picker schema valid
  • TestIsAvailable (7) — every plugin's is_available() correctly reflects env-var presence
  • TestRegistryResolution (4) — explicit-config-wins-when-unavailable, typo-falls-back, capability-incompatible-falls-back, no-config behavior
  • TestAsyncExtractDispatch (4) — inspect.iscoroutinefunction correctly distinguishes parallel/firecrawl (async) from exa/tavily (sync)
  • TestErrorResponseShapes (8) — plugins return typed error dicts when credentials are missing, never raise (including the new firecrawl crawl error test)

Full tests/tools/ regression sweep: 4910 passed, 44 skipped, 0 failures.

Behavior-parity verification (local E2E)

To make sure the plugin migration is a pure refactor, the three dispatchers were exercised in subprocesses pinned to (a) origin/main and (b) this PR's worktree, with all web-provider credentials cleared, across 14 config scenarios (no-config, plus every search_backend / extract_backend / crawl_backend override for each registered provider). For each scenario the JSON envelope shape is reduced to {success, has_error_top, has_data, n_results, per_result_has_error, error_category} and compared.

Initial run surfaced two observable behavior changes from the migration, both fixed in commit af2a7bfe6:

  1. web_search_tool on an unconfigured system. Main returns {"error": "Error searching web: Web tools are not configured..."} (via tool_error(), no success key). The migrated dispatcher was returning {"success": False, "error": "Web tools are not configured..."} because firecrawl.search() was catching ValueError from _get_firecrawl_client() itself instead of letting it propagate. Fix: pull _get_firecrawl_client() out of the broad try in the plugin's search(). Pre-flight errors propagate to the dispatcher's top-level except handler, matching main's legacy envelope. In-flight SDK errors still get wrapped per-result.

  2. web_crawl_tool on an unconfigured system. Main short-circuits on check_firecrawl_api_key() with {"success": False, "error": "web_crawl requires Firecrawl..."} at the top level. The migrated dispatcher was returning {"results": [{"url": "...", "error": "Web tools are not configured..."}]} — the firecrawl plugin's crawl() except ValueError was wrapping the pre-flight error as a per-page failure inside results[]. Models that check result.get("error") would miss the failure. Fix: mirror main's upstream availability gate in the dispatcher — when the resolved crawl provider is is_available()==False, short-circuit BEFORE delegating to the plugin with the same top-level error shape main emits.

Two regression tests in tests/tools/test_web_providers.py::TestUnconfiguredErrorEnvelopeParity lock the new behavior in so future plugin work can't re-introduce per-result error wrapping on pre-flight config errors.

Final state: 14/14 dispatcher scenarios match origin/main shape exactly. Plugin/dispatcher tests now 210/210 (208 + 2 new regression tests).

Self-review fix (commit 0dec60f7a)

Catches up the registry's _LEGACY_PREFERENCE order with the legacy tools.web_tools._get_backend() candidate tuple so direct callers of agent.web_search_registry.get_active_*_provider() (no _get_backend() interpose) don't return None for users who have credentials but no explicit web.backend config key. Verified empirically: TAVILY+EXA → tavily; EXA+PARALLEL → parallel; BRAVE+FIRECRAWL → firecrawl, all matching main. Plus four stale-docstring cleanups (crawl claims, ABC capabilities, registry comment block) and the removal of three dead plugin-level cache globals + their no-op reset helpers (rewritten to clear the canonical _wt._<name> slots, matching the pattern exa already used correctly).

Out of scope (clean follow-ups)

Commits

24 focused commits on kshitijk4poor/hermes-agent:spike/web-providers-plugin:

ABC + registry foundation
  c28f2df  feat(web): add WebSearchProvider ABC mirroring image_gen template
  7a92840  feat(web): add web search provider registry mirroring image_gen pattern
  4a49015  feat(plugins): add ctx.register_web_search_provider() facade

Per-plugin migrations (one commit each, in dependency order)
  ab968d0  feat(web): brave_free plugin
  724ce34  feat(web): ddgs plugin
  8f1aa2b  feat(web): searxng plugin
  bc58e8d  feat(web): exa plugin
  30998b6  feat(web): parallel plugin (first async-extract)
  674307Z  feat(web): tavily plugin (first three-capability incl. crawl)
  6dc81fa  feat(web): firecrawl plugin (largest migration; lazy proxy + dual auth)

Bridges / fixes
  6a34d6c  fix(plugins): filter resolution by is_available() in web + image_gen registries
  2f28b6d  feat(web): extend ABC with supports_crawl + async-extract semantics
  d873596  refactor(web): dispatch brave-free/ddgs/searxng via web_search_registry (interim)
  63829c7  feat(tools): mirror image_gen plugin-injection in Web Search picker
  dfaa611  refactor(web): remove legacy in-tree provider modules

Dispatcher cutover + cleanup
  a1c33bf  refactor(web): dispatch all three tools through web_search_registry
  cd7e142  fix(web): preserve firecrawl crawl + website-policy gate after migration
  d82fb49  refactor(web): delete inline vendor helpers, re-export from plugins (-614 LOC)
  1bb9bdb  refactor(tools): drop hardcoded web picker rows + skiplist
  3f07fda  refactor(web): delete legacy tools/web_providers/ directory + migrate ABC tests
  8114924  test(plugins): tests/plugins/web/ — 44-test coverage for the migration
  601a174  feat(web): firecrawl plugin natively supports crawl; delete legacy inline path (-254 LOC)

Self-review fixes
  0dec60f  fix(web): align _LEGACY_PREFERENCE with legacy 7-provider order + doc cleanup
  af2a7bf  fix(web): preserve top-level error envelope on unconfigured systems

Adds plugins/web/brave_free/ as the first plugin built against the new
WebSearchProvider ABC. Mirrors the plugins/image_gen/openai/ layout exactly:

  plugins/web/brave_free/
    plugin.yaml      kind: backend, provides_web_providers: [brave-free]
    __init__.py      register(ctx) -> ctx.register_web_search_provider(...)
    provider.py      BraveFreeWebSearchProvider(WebSearchProvider)

Behavior preserved: same name ("brave-free" with hyphen), same env var
(BRAVE_SEARCH_API_KEY), same HTTP request shape, same response normalization.

The legacy tools/web_providers/brave_free.py is left in place — the
dispatcher in tools/web_tools.py still references it. Task 7 cuts over the
dispatcher to the new registry; Task 10 deletes the legacy file.

E2E verified:
  HERMES_PLUGINS_DEBUG=1 python -c "
  from hermes_cli.plugins import _ensure_plugins_discovered
  _ensure_plugins_discovered()
  from agent.web_search_registry import list_providers
  print([p.name for p in list_providers()])
  "
  # -> ['brave-free']
Adds plugins/web/ddgs/ following the same plugins/image_gen/ pattern as
brave_free. DuckDuckGo search via the community ddgs package; no API key,
package is an optional dep gated by is_available().

E2E verified — registry now has ['brave-free', 'ddgs'].
Adds plugins/web/searxng/. SearXNG aggregates results from upstream engines
via its JSON API (/search?format=json) — search-only, no extract capability
(supports_extract() returns False).

E2E verified — registry now has ['brave-free', 'ddgs', 'searxng'].
The three migrated providers (brave-free, ddgs, searxng) are now dispatched
through agent.web_search_registry.get_provider() instead of importing
their concrete classes directly. The four inline providers (parallel, exa,
tavily, firecrawl) keep their existing branches — they live in
tools/web_tools.py itself and aren't part of this spike's plugin extraction.

The legacy tools/web_providers/{brave_free,ddgs,searxng}.py modules are
still in place (untouched by this commit) — Task 10 deletes them once the
real migration PR is ready. Keeping them alive during the spike means
revertibility is trivial.

E2E verified:
  1. Plugin discovery registers ['brave-free','ddgs','searxng']
  2. Config web.search_backend: brave-free resolves to the plugin instance
  3. Dispatch result matches the original {success, data.web[]} contract
  4. compile OK; no new LSP errors beyond pre-existing ones in web_tools.py
Adds _plugin_web_search_providers() and wires it into _visible_providers()
for the "Web Search & Extract" category. Mirrors the existing image_gen
pattern at the same site exactly.

Spike scope: while the three migrated providers (brave-free, ddgs, searxng)
still have hardcoded TOOL_CATEGORIES rows, _WEB_PLUGIN_SKIPLIST excludes
them so the picker doesn't show duplicates. The migration PR drops the
hardcoded rows and the skip-list both — then this helper is the only
source of web-provider picker rows.

E2E verified: helper returns [] today (skip-list covers all 3 migrated
providers); injection point is sound and ready for the post-migration state.
Deletes tools/web_providers/{brave_free,ddgs,searxng}.py — the three
providers that moved to plugins/web/ in prior commits. tools/web_tools.py
no longer imports them (registry dispatch as of d873596), so removing
them is purely a cleanup pass.

Also migrates the existing tests to the new import paths:
  tests/tools/test_web_providers_brave_free.py
  tests/tools/test_web_providers_ddgs.py
  tests/tools/test_web_providers_searxng.py

Mechanical rewrites:
  - `from tools.web_providers.X import YSearchProvider`
      -> `from plugins.web.X.provider import YWebSearchProvider`
  - `.is_configured()` -> `.is_available()`        (legacy method  -> new method)
  - `.provider_name()` -> `.name`                  (legacy method  -> new property)
  - `from tools.web_providers.base import WebSearchProvider`
      -> `from agent.web_search_provider import WebSearchProvider`
      (the subclass-check asserts membership in the new plugin-facing ABC)
  - `sys.modules.delitem("tools.web_providers.ddgs")` updated to point at
    `plugins.web.ddgs.provider` (cache-busting for lazy ddgs imports)

The TestXBackendWiring / TestXSearchOnlyErrors classes (covering
_is_backend_available, _get_backend, check_web_api_key, and the
"search-only" error paths in web_extract/web_crawl) are untouched —
those still test web_tools.py's backend-selection logic, which continues
to recognize the names "brave-free" / "ddgs" / "searxng" even after the
modules behind them moved to plugins.

tools/web_providers/base.py is intentionally NOT deleted by this commit
— it's the parent ABC of the legacy modules and shares its name with
agent/web_search_provider.py::WebSearchProvider. Removing it surfaces the
naming collision (see PR description Finding 0); the real migration PR
deletes it in the same commit that drops the _WEB_PLUGIN_SKIPLIST
guards in hermes_cli/tools_config.py.

Test results:
  bash scripts/run_tests.sh tests/tools/test_web_providers_*.py
  -> 65 passed in 3.41s (all rewritten unit tests + unchanged integration tests)
  bash scripts/run_tests.sh tests/tools/test_web_*.py
  -> 141 passed in 4.70s (full web test set, post-deletion)
…registries

Both web_search_registry._resolve() and image_gen_registry.get_active_provider()
walked their registered providers and returned the first one matching the
capability flag — without checking whether that provider was actually
usable. On a fresh install with no credentials at all, this meant
get_active_search_provider() returned `brave-free` (legacy preference
order) even though BRAVE_SEARCH_API_KEY was unset, leading the
dispatcher to surface a "BRAVE_SEARCH_API_KEY is not set" error for a
provider the user never chose. Same bug shape in image_gen for FAL.

Resolution semantics now match tools.web_tools._get_backend():

  1. Explicit config name wins, ignoring is_available() — the dispatcher
     surfaces a precise "X_API_KEY is not set" error rather than silently
     switching backends. Matches user expectation: "I configured X, tell
     me what's wrong with X."
  2. Fallback (no explicit config) walks the legacy preference order
     filtered by is_available() — pick the highest-priority backend the
     user actually has credentials for.

is_available() is wrapped in a try/except so a buggy provider doesn't
brick resolution.

E2E verified:
  - No creds + no config: get_active_search_provider() -> None
  - Explicit brave-free + no key: get_active_search_provider() -> brave-free
    (and .is_available() correctly reports False)

This fix was identified during the spike (NousResearch#25182 finding #1) and is
fold-in to the same PR rather than a follow-up.
Two ABC additions to cover the surface area of the remaining four
providers (exa, parallel, tavily, firecrawl) which were untouched by the
initial spike:

1. supports_crawl() + crawl() — Tavily natively crawls a seed URL via
   its /crawl endpoint. Exposing supports_crawl=True lets the crawl
   tool's dispatcher route to Tavily when configured, falling back to
   the auxiliary-model summarization path otherwise. Firecrawl could
   add this in a follow-up (the SDK supports it; we just don't surface
   it as a tool today).

2. Async-or-sync extract() — Parallel's SDK is natively async
   (AsyncParallel.beta.extract); Exa and Tavily are sync; Firecrawl is
   sync but called inside asyncio.to_thread() with a 60s timeout. The
   ABC docstring now permits either shape: implementations declare
   their own sync/async signature and the dispatcher uses
   inspect.iscoroutinefunction to detect and await.

Also adds get_active_crawl_provider() to web_search_registry mirroring
the search/extract resolvers, with web.crawl_backend as the explicit
override config key.

No behavior change on its own — these are scaffolds for the four
remaining provider migrations.
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/web Web search and extraction labels May 13, 2026
…tract)

Migrates Exa from the inline `_exa_search()` / `_exa_extract()` helpers in
tools/web_tools.py to a bundled plugin at plugins/web/exa/.

This is the first plugin in this PR to advertise supports_extract=True,
exercising the multi-capability ABC path that the initial three migrations
(brave_free, ddgs, searxng — all search-only) did not cover.

Both Exa methods are sync — the SDK is sync-only. The web_extract_tool
dispatcher in tools/web_tools.py will continue to call them inline until
Task "dispatch-extract-all" cuts it over to the registry.

Behaviour preserved bit-for-bit aside from the ABC method-name change:
  - is_configured()  -> is_available()
  - provider_name()  -> name (property)
  - "exa" stays as the registered name
  - Module-level `_exa_client` cache + lazy `from exa_py import Exa`
    preserved at the new location.
  - Errors (ValueError for missing API key, ImportError for missing SDK,
    generic Exception) caught and surfaced as {"success": False, "error": ...}
    instead of raising.

Adds "exa" to _WEB_PLUGIN_SKIPLIST in hermes_cli/tools_config.py so the
hardcoded TOOL_CATEGORIES["web"] row and the plugin-injected row don't
duplicate during the spike. The skip-list goes away in the cleanup phase
along with the hardcoded row.

The legacy inline `_exa_search` / `_exa_extract` / `_get_exa_client` /
`_exa_client` in tools/web_tools.py are NOT deleted yet — the dispatcher
still references them. They go away in the next dispatcher-cutover commit.

E2E verified:
  - Plugin discovers + registers
  - .supports_search/.supports_extract/.supports_crawl = (True, True, False)
  - .get_setup_schema() returns the picker row shape
  - resolve(): explicit exa + EXA_API_KEY -> exa; without key -> exa (registered
    but unavailable, dispatcher surfaces "EXA_API_KEY not set" error)
Migrates Parallel.ai from inline `_parallel_search()` / `_parallel_extract()`
in tools/web_tools.py to a bundled plugin at plugins/web/parallel/.

First plugin in the codebase to expose an async :meth:`extract`:

  - search() is sync — Parallel.beta.search
  - extract() is **async def** — AsyncParallel.beta.extract

The ABC's docstring on supports_extract() already permits sync-or-async;
this commit is the first to exercise the async path. The web_extract_tool
dispatcher (next commit) detects coroutines via
inspect.iscoroutinefunction and awaits accordingly.

Behavior preserved:
  - PARALLEL_API_KEY required (raises ValueError if missing → surfaced
    as {"success": False, "error": "..."} instead)
  - PARALLEL_SEARCH_MODE env var honored (agentic|fast|one-shot, default
    agentic), validated via _resolve_search_mode()
  - Limit capped at 20 server-side via min(limit, 20)
  - Per-URL failure mode preserved: response.errors[] each become a
    result dict with an "error" field rather than raising
  - Module-level _parallel_client / _async_parallel_client caches kept
    (mirrors legacy singleton pattern)

Adds "parallel" to _WEB_PLUGIN_SKIPLIST in hermes_cli/tools_config.py so
the picker doesn't double-list.

The legacy inline _parallel_search, _parallel_extract, _get_parallel_client,
_get_async_parallel_client in tools/web_tools.py are NOT deleted yet — the
dispatcher still calls them. They go away when the dispatcher cuts over.

E2E verified:
  - inspect.iscoroutinefunction(p.search) -> False
  - inspect.iscoroutinefunction(p.extract) -> True
  - extract() returns a coroutine (not a list)
  - 5 providers register correctly (brave-free, ddgs, exa, parallel, searxng)
…tract + crawl)

Migrates Tavily from inline _tavily_request() / _normalize_tavily_*
helpers in tools/web_tools.py to a bundled plugin at plugins/web/tavily/.

First plugin in the codebase to advertise supports_crawl=True. Tavily is
unique among built-in backends in offering a native /crawl endpoint that
walks linked pages from a seed URL with optional natural-language
instructions and depth ("basic" or "advanced").

Capabilities:
  - supports_search()  -> True (Tavily /search)
  - supports_extract() -> True (Tavily /extract)
  - supports_crawl()   -> True (Tavily /crawl)
  All sync (httpx.post under the hood).

The crawl method accepts forward-compat kwargs (instructions, depth,
limit) and is gated against unsafe URLs/policy by the dispatcher in
web_crawl_tool — exactly as before.

Behavior preserved:
  - TAVILY_API_KEY required (ValueError → typed error response)
  - TAVILY_BASE_URL env override honored
  - /crawl requires both body auth AND Bearer header — preserved
  - failed_results[] and failed_urls[] response keys mapped to per-URL
    items with error fields rather than raising
  - max_results capped at 20 server-side

Adds "tavily" to _WEB_PLUGIN_SKIPLIST.

The legacy inline _tavily_request / _normalize_tavily_search_results /
_normalize_tavily_documents / _TAVILY_BASE_URL in tools/web_tools.py are
NOT deleted yet — search/extract dispatch and the entire web_crawl_tool
function still reference them. They go away when those dispatchers are
cut over to the registry.

E2E verified:
  - Tavily registers with all 3 capabilities
  - Provider list now: brave-free, ddgs, exa, parallel, searxng, tavily
…ct + dual auth)

Migrates Firecrawl from inline code in tools/web_tools.py to a bundled
plugin at plugins/web/firecrawl/. By line count this is the largest of
the seven provider migrations: the firecrawl path captured most of the
file's vendor-specific complexity.

What moved into the plugin (all previously in tools/web_tools.py):

  Lazy Firecrawl SDK proxy
    - _load_firecrawl_cls() — caches the imported SDK class
    - _FirecrawlProxy + Firecrawl singleton — defers ~200ms of SDK
      imports until first construction or isinstance check.

  Client construction (dual auth)
    - _get_direct_firecrawl_config()  — direct FIRECRAWL_API_KEY/URL path
    - _get_firecrawl_gateway_url()    — managed Nous tool-gateway URL
    - _is_tool_gateway_ready()        — gateway URL + Nous token check
    - _has_direct_firecrawl_config()  — direct config present?
    - _get_firecrawl_client()         — combined client construction
                                        honoring web.use_gateway
    - check_firecrawl_api_key()       — top-level "is firecrawl usable"
    - _firecrawl_backend_help_suffix() — managed-gateway help string
    - _raise_web_backend_configuration_error() — typed misconfig error

  Response shape normalization (vendor-specific)
    - _to_plain_object(), _normalize_result_list() — SDK→dict helpers
    - _extract_web_search_results() — handles SDK/direct/gateway shapes
    - _extract_scrape_payload()     — nested-data unwrap for scrape

  Per-URL extract loop
    - 60s asyncio.wait_for timeout per URL
    - Pre-scrape website-policy gate
    - Post-scrape redirect-aware SSRF re-check
    - Format-aware content selection (markdown / html / auto)
    - Per-URL errors returned as {"error": str} entries, no raises

Extract is declared `async def` — each URL is scraped in
asyncio.to_thread(...). This is the second async-extract plugin after
parallel.

The plugin re-exports `Firecrawl` (the lazy proxy) and
`check_firecrawl_api_key()` so existing tests doing
`patch("tools.web_tools.Firecrawl")` or
`monkeypatch.setattr(web_tools, "check_firecrawl_api_key", ...)` keep
working — tools/web_tools.py re-exports both names in the next
dispatcher-cutover commit.

Note: web_crawl_tool still has its own Firecrawl crawl path inline
(separate from extract); the Firecrawl SDK supports /crawl but we don't
expose supports_crawl=True on this plugin yet. Tavily handles crawl
today. Adding Firecrawl crawl is a clean follow-up.

Adds "firecrawl" to _WEB_PLUGIN_SKIPLIST.

E2E verified:
  - All 7 providers register: brave-free, ddgs, exa, firecrawl,
    parallel, searxng, tavily
  - inspect.iscoroutinefunction(firecrawl.extract) -> True
  - Firecrawl proxy is a callable lazy proxy at module level
  - check_firecrawl_api_key reflects FIRECRAWL_API_KEY presence
Cuts over web_search_tool, web_extract_tool, and web_crawl_tool in
tools/web_tools.py to dispatch through agent.web_search_registry
instead of the legacy hardcoded if-elif backend chains.

Per-tool changes:

  web_search_tool (sync)
    Replace 5 backend branches (parallel, exa, registry-3-providers,
    tavily, firecrawl-fallthrough) with a single registry path:
      1. _get_search_backend() resolves the configured name
      2. _wsp_get_provider(name) for explicit-config-wins semantics
      3. get_active_search_provider() fallback for typo / unknown name
      4. provider.search(query, limit) — sync for all 7 providers

  web_extract_tool (async)
    Replace 4 backend branches (parallel-async, exa-sync, tavily-sync,
    search-only-error, firecrawl-perurl-loop) with:
      1. Same provider resolution as search.
      2. When configured backend IS registered but doesn't support
         extract (search-only providers like brave-free), surface a
         typed "search-only" error matching the legacy text — tests
         assert that wording.
      3. inspect.iscoroutinefunction(provider.extract) detects sync vs
         async: parallel + firecrawl are async; exa + tavily are sync.
         Sync extracts run in asyncio.to_thread() so we don't block.

  web_crawl_tool (async)
    Replace tavily-specific branch + search-only-error block with:
      1. _wsp_get_provider(backend) — explicit config first
      2. Search-only typed error when the configured name doesn't
         support crawl (matches legacy phrasing)
      3. get_active_crawl_provider() fallback otherwise
      4. provider.crawl(url, **kwargs) — async-or-sync dispatch as above
      5. Response post-processing (LLM summarization, trimming) stays
         unchanged — it's not provider-specific.
    When no plugin advertises supports_crawl, falls through to the
    existing Firecrawl-via-web-summarize path below (unchanged).

Test updates (2 tests in tests/tools/test_web_tools_config.py):
  - test_web_search_clamps_limit_before_backend_call:
      patch("tools.web_tools._parallel_search") -> patch the registry
      provider returned by agent.web_search_registry.get_provider
  - test_search_error_response_does_not_expose_diagnostics:
      patch("tools.web_tools._get_firecrawl_client") -> same pattern

Tests unchanged (still pass):
  - All TestXBackendWiring classes (test _get_backend / _is_backend_available
    config-resolution, independent of dispatch)
  - All TestXSearchOnlyErrors classes (test the search-only error path
    via web_extract_tool / web_crawl_tool — error text preserved)
  - 141 passing web tests total, 0 regressions.

Dead-code cleanup deferred to a follow-up commit so this diff stays
focused on the cutover. After this commit:
  - tools.web_tools._exa_search / _exa_extract / _parallel_search /
    _parallel_extract / _tavily_request / _normalize_tavily_* /
    _get_firecrawl_client / _extract_web_search_results /
    _extract_scrape_payload / _to_plain_object / _normalize_result_list
    are no longer called by the dispatchers, but still exist.
  - The config-resolution layer (_get_backend, _is_backend_available,
    _is_tool_gateway_ready, _has_direct_firecrawl_config) IS still in
    use and must stay.
  - The Firecrawl proxy and check_firecrawl_api_key are still imported
    by integration tests and patched by unit tests — must stay (or be
    re-exported from the plugin).
Two regressions discovered by running the full tests/tools/ suite after
the dispatcher cutover, both fixed in this commit:

1. web_crawl_tool incorrectly errored "search-only" for firecrawl
---------------------------------------------------------------------
The cutover treated any provider with supports_crawl()==False as a
search-only backend and returned the typed search-only error. But
firecrawl can crawl via the legacy multi-page-extract path inside
web_crawl_tool — it just doesn't expose supports_crawl on the plugin
(adding native firecrawl crawl is a clean follow-up).

Fix: only emit the search-only error when the provider supports
NEITHER crawl NOR extract (brave-free / ddgs / searxng). When the
provider supports extract but not crawl (firecrawl), fall through to
the legacy firecrawl-via-extract path below.

2. firecrawl plugin's check_website_access wasn't patchable
---------------------------------------------------------------------
The plugin imported `from tools.website_policy import check_website_access`
INSIDE the extract() function body, so monkeypatching the name on
plugins.web.firecrawl.provider had no effect — the inner import re-bound
the name on every call.

Fix: hoist the import to module level. Cheap (website_policy itself
has no heavy deps) and makes the standard
monkeypatch.setattr(firecrawl_provider, "check_website_access", ...)
pattern work.

Test updates (tests/tools/test_website_policy.py — 4 tests):
  - test_web_extract_short_circuits_blocked_url
  - test_web_extract_blocks_redirected_final_url
    Both: patch the gate at plugins.web.firecrawl.provider (where it
    runs after migration) and force the firecrawl plugin to be the
    active extract provider via FIRECRAWL_API_KEY.
  - test_web_crawl_short_circuits_blocked_url
  - test_web_crawl_blocks_redirected_final_url
    Both: unchanged — the dispatcher-level gate at tools.web_tools.py
    line 1651 still uses the imported `check_website_access` name and
    the firecrawl-fallthrough path is exercised as before.

Verified: 22/22 tests/tools/test_website_policy.py pass.
Removes ~580 lines of dead code from tools/web_tools.py that were
superseded by the plugin migration but kept around in the cutover commit
to keep the diff focused. Replaces them with thin re-export shims so
existing tests and external callers that reach for the legacy
``tools.web_tools.<name>`` paths continue to work transparently.

Deleted from tools/web_tools.py
--------------------------------
- Lazy Firecrawl SDK proxy (_load_firecrawl_cls, _FirecrawlProxy,
  _FIRECRAWL_CLS_CACHE, the Firecrawl singleton)
- Firecrawl client section (_get_direct_firecrawl_config,
  _get_firecrawl_gateway_url, _is_tool_gateway_ready,
  _has_direct_firecrawl_config, _raise_web_backend_configuration_error,
  _firecrawl_backend_help_suffix, _get_firecrawl_client)
- Parallel client section (_get_parallel_client,
  _get_async_parallel_client, _parallel_client, _async_parallel_client)
- Tavily client section (_TAVILY_BASE_URL, _tavily_request,
  _normalize_tavily_search_results, _normalize_tavily_documents)
- Generic SDK normalizers (_to_plain_object, _normalize_result_list,
  _extract_web_search_results, _extract_scrape_payload)
- Exa client section (_get_exa_client, _exa_client, _exa_search,
  _exa_extract)
- Parallel helpers (_parallel_search, _parallel_extract)
- Duplicate inline check_firecrawl_api_key

Net: tools/web_tools.py drops from 2227 → 1613 lines (-614 lines).

Re-exports added at top of tools/web_tools.py
---------------------------------------------
- From plugins.web.firecrawl.provider:
  Firecrawl, _FirecrawlProxy, _FIRECRAWL_CLS_CACHE, _load_firecrawl_cls,
  _get_direct_firecrawl_config, _get_firecrawl_gateway_url,
  _is_tool_gateway_ready, _has_direct_firecrawl_config,
  _firecrawl_backend_help_suffix, _raise_web_backend_configuration_error,
  _get_firecrawl_client, _to_plain_object, _normalize_result_list,
  _extract_web_search_results, _extract_scrape_payload,
  check_firecrawl_api_key
- From plugins.web.tavily.provider:
  _tavily_request, _normalize_tavily_search_results,
  _normalize_tavily_documents
- From plugins.web.parallel.provider:
  _get_parallel_client, _get_async_parallel_client
- From plugins.web.exa.provider:
  _get_exa_client

Plus retained module-level imports for backward-compat with tests:
- httpx (tests patch tools.web_tools.httpx for tavily request mocking)
- build_vendor_gateway_url, _read_nous_access_token,
  resolve_managed_tool_gateway, managed_nous_tools_enabled,
  prefers_gateway (tests patch tools.web_tools.<name>)

Plugin indirection pattern (key technique)
------------------------------------------
For functions inside the firecrawl/parallel/exa plugins to honor
unit-test patches that target ``tools.web_tools.<name>``, the plugin
implementations now do ``import tools.web_tools as _wt`` at call time
and read helper names through that module (``_wt._read_nous_access_token``,
``_wt.Firecrawl``, ``_wt.prefers_gateway``, etc.). This makes the
existing test patches transparently reach the plugin code without any
test changes.

The cached client globals (_firecrawl_client, _firecrawl_client_config,
_parallel_client, _async_parallel_client, _exa_client) also now live on
tools.web_tools so existing test setup_method handlers that reset
``tools.web_tools._<vendor>_client = None`` between cases keep working.
The plugins read/write the cache via getattr/setattr on the web_tools
module.

Verified
--------
- 173/173 targeted web tests pass:
  test_web_providers.py, test_web_providers_brave_free.py,
  test_web_providers_ddgs.py, test_web_providers_searxng.py,
  test_web_tools_config.py, test_web_tools_tavily.py,
  test_website_policy.py, test_config_null_guard.py
- Compile-clean (py_compile.compile passes)
- All inline implementations now exist in exactly one place
  (plugins.web.<vendor>.provider)

Follow-up clean-up
------------------
- Drop _WEB_PLUGIN_SKIPLIST + hardcoded TOOL_CATEGORIES["web"] rows
  (next commit)
- Delete tools/web_providers/ directory entirely
- Add tests/plugins/web/ coverage
- Full tests/tools/ + tests/gateway/ regression sweep before promoting PR
…re sole source

Removes the seven hardcoded TOOL_CATEGORIES["web"] provider rows that
duplicated the plugin-registered providers, and deletes the
_WEB_PLUGIN_SKIPLIST that existed to prevent duplicate picker rows
during the migration. The Web Search & Extract category now derives its
provider rows entirely from agent.web_search_registry via
_plugin_web_search_providers(), matching how Spotify, Google Meet, and
the image_gen plugins are surfaced.

Removed (deduplicated against plugin schemas):
  - Firecrawl Cloud         → plugins.web.firecrawl
  - Exa                     → plugins.web.exa
  - Parallel                → plugins.web.parallel
  - Tavily                  → plugins.web.tavily
  - SearXNG                 → plugins.web.searxng
  - Brave Search (Free Tier) → plugins.web.brave_free
  - DuckDuckGo (ddgs)       → plugins.web.ddgs (post_setup hook preserved)

Retained in TOOL_CATEGORIES["web"]:
  - Nous Subscription   — requires requires_nous_auth +
                          managed_nous_feature + override_env_vars
                          to drive the managed-gateway UX. Not a
                          provider — a different *setup flow* for the
                          firecrawl backend.
  - Firecrawl Self-Hosted — points firecrawl at a private Docker URL
                            via FIRECRAWL_API_URL only. Same reason:
                            UX setup-flow row, not a provider.

These two rows describe alternative auth/billing paths for the
firecrawl backend; they intentionally share web_backend="firecrawl"
with the plugin row but light up different env-var prompts.

Plugin schema extensions
------------------------
- ddgs plugin's get_setup_schema() now emits `post_setup: "ddgs"` so
  selection still triggers the pip-install hook in _run_post_setup().
- _plugin_web_search_providers() passes `post_setup` through verbatim
  when present in the schema (other future plugins like camofox / a
  hypothetical playwright-web plugin can opt in the same way).
- Picker rows now carry both `web_backend` (legacy field consumed by
  setup + selection helpers) and `web_search_plugin_name`
  (informational marker), so behavior is identical between hardcoded
  and plugin-registered rows.

Net diff
--------
- hermes_cli/tools_config.py: -141/+50 lines (~91 lines net)
- plugins/web/ddgs/provider.py: +7/-4 (post_setup field + badge polish)

Verified
--------
- Compile-clean for both files
- Picker shows: 2 hardcoded rows (Nous Subscription, Firecrawl
  Self-Hosted) + 7 plugin rows (alphabetically: Brave Search,
  DuckDuckGo, Exa, Firecrawl, Parallel, SearXNG, Tavily). DuckDuckGo
  row carries post_setup="ddgs" for first-time install.
- 173 web-specific tests still pass.
… ABC tests

Removes the legacy in-tree provider scaffolding that PR NousResearch#25182 fully
replaced with the plugin architecture:

  tools/web_providers/__init__.py        (6 lines)
  tools/web_providers/base.py            (89 lines — old ABCs)
  tools/web_providers/ARCHITECTURE.md    (73 lines — old design doc)

These were the staging-ground ABCs and provider modules that the
plugin migration absorbed. All seven web providers now implement the
single :class:`agent.web_search_provider.WebSearchProvider` ABC and
live under ``plugins/web/<vendor>/``. Nothing else in the tree imports
``tools.web_providers`` — verified via grep before deletion.

Test migration (tests/tools/test_web_providers.py)
--------------------------------------------------
Rewrote ``TestWebProviderABCs`` to test the new unified ABC at
:mod:`agent.web_search_provider`:

  - test_cannot_instantiate_abc_directly — abstract ``name`` + ``is_available``
  - test_concrete_search_only_provider_works — exercise default
    ``supports_extract=False`` / ``supports_crawl=False`` flags
  - test_concrete_multi_capability_provider_works — exercise all three
    capabilities, async extract supported (declared sync here for
    simplicity; real plugins like parallel + firecrawl use async)
  - test_search_only_provider_skips_extract_and_crawl — verify
    ``supports_*()`` flags default to False so search-only providers
    don't have to implement extract() or crawl()

The 9 other tests in the file (per-capability backend selection,
DEFAULT_CONFIG merge, dispatcher routing) test public helpers in
``tools.web_tools`` that still exist and pass unchanged.

agent/web_search_provider.py docstring updated to reflect that the
legacy ABCs no longer exist; the response-shape contract is preserved
bit-for-bit so external consumers see no behavioral change.

Net diff
--------
- tools/web_providers/ removed (-168 lines)
- tests/tools/test_web_providers.py rewritten ABC section (+78/-30 net,
  same coverage, new API)
- agent/web_search_provider.py docstring (-3/+5 lines)

Verified
--------
- 173/173 targeted web tests pass
- 12/12 ABC contract tests pass with the new interface
- No remaining grep hits for ``tools.web_providers`` outside of
  intentional historical references in plugin docstrings.
Adds 44 focused tests under tests/plugins/web/ covering the surface that
the PR NousResearch#25182 web-provider migration introduced. Complements the
existing tests/tools/ coverage which is dispatcher-centric; this file is
plugin-centric and tests each plugin + the registry directly.

Test classes (44 tests, ~1.1s on 4 workers)
-------------------------------------------

TestBundledPluginsRegister (16 tests)
  - All seven plugins present in the registry after
    _ensure_plugins_discovered()
  - Per-plugin parametrized capability-flag assertions
    (brave-free / ddgs / searxng: search-only;
     exa / parallel / firecrawl: search + extract;
     tavily: search + extract + crawl)
  - Every plugin exposes name + display_name properties
  - Every plugin returns a picker-compatible get_setup_schema() dict

TestIsAvailable (7 tests)
  - Each premium plugin reports is_available()==False when its env var is
    absent and True once set (brave-free / searxng / tavily / exa /
    parallel)
  - firecrawl recognizes either FIRECRAWL_API_KEY or FIRECRAWL_API_URL
    as a "configured" signal
  - ddgs is the always-on fallback and must not raise from is_available()

TestRegistryResolution (4 tests)
  - Option B semantics validated end-to-end:
    1. Explicit configured provider wins even when is_available()==False
       (dispatcher surfaces typed credential errors, no silent switch)
    2. Unknown/typo name falls back to first available legacy-preference
       provider
    3. Asking for extract via a search-only backend falls back to an
       extract-capable available provider (capability-incompatible
       branch in _resolve())
    4. No config + no credentials → None (or ddgs if installed)

TestAsyncExtractDispatch (4 tests)
  - parallel + firecrawl extract() are coroutine functions (async path
    in dispatcher uses await)
  - exa + tavily extract() are sync (dispatcher wraps in
    asyncio.to_thread)

TestErrorResponseShapes (7 tests)
  - Plugins return typed error dicts (success=False + "error" key) when
    credentials are missing, never raise
  - async extract() returns list of per-URL error dicts
  - tavily crawl() returns {"results": [{"error": ...}]} on missing
    credentials

Design notes
------------
- All tests use real imports of plugin modules — no mocking of provider
  classes themselves — so they catch drift in the ABC, registry, and
  glue layer simultaneously. Per the hermes-agent-dev skill's E2E
  testing guidance.
- The autouse _isolate_env fixture clears every web-provider env var
  before each test so is_available() reflects the test's setup.
- Resolution tests use the lower-level _resolve() directly rather than
  rebuilding the HERMES_HOME config dance — same observable behavior,
  no sys.modules.pop side-effects that would break the ABC isinstance
  check inside ctx.register_web_search_provider().
@kshitijk4poor kshitijk4poor changed the title spike(web): web providers as image_gen-style plugins refactor(web): all web providers as image_gen-style plugins May 13, 2026
@kshitijk4poor kshitijk4poor marked this pull request as ready for review May 13, 2026 19:57
…line path

The web-provider migration originally left firecrawl crawl as the only
provider-specific code remaining inline in tools/web_tools.py (~250
lines of Firecrawl-specific crawl orchestration that didn't fit the
plugin's existing surface). This commit closes that gap.

What this adds
--------------
1. plugins/web/firecrawl/provider.py: implement async ``crawl(url, **kwargs)``
   - Accepts the same kwargs as the dispatcher passes to any crawl
     provider (``instructions``, ``depth``, ``limit``); Firecrawl's
     /crawl endpoint ignores ``instructions`` and ``depth`` so we log
     and drop with a clear info message.
   - Wraps the sync SDK ``crawl()`` call in asyncio.to_thread so the
     gateway event loop isn't blocked on a multi-page crawl.
   - Preserves the response-shape normalization across pydantic /
     typed-object / dict variants that the legacy inline code did.
   - Preserves per-page website-policy re-check (catches blocked
     redirects after the SDK returns).
   - Returns the same {"results": [...]} shape so the dispatcher's
     shared LLM-summarization post-processing path works unchanged.
   - Sets supports_crawl() to True so the dispatcher routes through
     the plugin instead of the legacy fallthrough.

2. tools/web_tools.py: delete the entire legacy firecrawl crawl block
   that used to run after "No registered provider supports crawl" —
   ~270 lines including:
   - check_firecrawl_api_key gate + typed error
   - inline SSRF + website-policy seed-URL gate (dispatcher already
     does this)
   - Firecrawl client setup with crawl_params
   - 100+ lines of pydantic/dict/typed-object normalization
   - Per-page LLM-processing loop (kept in the dispatcher's shared
     post-processing path; that's where it always belonged)
   - trimming + base64 image cleanup (still done in the dispatcher's
     shared path)

   Replaced with a single typed-error branch when no crawl-capable
   provider is available: "web_crawl has no available backend. Set
   FIRECRAWL_API_KEY (or FIRECRAWL_API_URL for self-hosted), or set
   TAVILY_API_KEY for Tavily."

Test updates
------------
- tests/tools/test_website_policy.py:
  - test_web_crawl_short_circuits_blocked_url: dispatcher seed-URL
    gate still runs on web_tools.check_website_access (no change to
    that patch), but the firecrawl client lockdown moved to the
    plugin module — patch firecrawl_provider._get_firecrawl_client
    instead of web_tools._get_firecrawl_client. The dispatcher
    short-circuits before the plugin runs, so the test still passes.
  - test_web_crawl_blocks_redirected_final_url: patch the per-page
    policy gate at plugins.web.firecrawl.provider.check_website_access
    (where it now runs) AND on web_tools (where the seed-URL gate
    still runs). Patch firecrawl_provider._get_firecrawl_client for
    the FakeCrawlClient injection. Both checks flow through the same
    fake_check function.
- tests/plugins/web/test_web_search_provider_plugins.py:
  - Update parametrized capability-flag spec: firecrawl supports_crawl
    is now True.
  - Add test_firecrawl_crawl_returns_error_dict_when_unconfigured —
    verifies inspect.iscoroutinefunction(p.crawl) is True and that
    the async crawl returns a per-page error dict (not a raise) when
    FIRECRAWL_API_KEY is missing.

Verified
--------
- 218/218 web tests pass (was 173, +44 plugin tests + 1 new firecrawl
  crawl test from this commit = 218 with the test deduplication).
- Compile-clean (py_compile passes on both files).
- Provider capabilities matrix confirmed end-to-end:
    name        search  extract  crawl   async-extract?  async-crawl?
    firecrawl   True    True     True    True            True
    tavily      True    True     True    False           False
  Both crawl-capable providers exercise the dispatcher's
  inspect.iscoroutinefunction async-or-sync detection.

Net diff
--------
- tools/web_tools.py: -254 lines (legacy inline crawl gone)
- plugins/web/firecrawl/provider.py: +185 lines (crawl method)
- test_website_policy.py: +14/-9 lines (patch locations)
- test_web_search_provider_plugins.py: +22/-1 lines (capability flag
  + new firecrawl crawl test)
- Total: -32 net LoC; tools/web_tools.py is now 1509 lines (was 1763
  before this commit, 2227 before the migration started).
… cleanup

Self-review of the plugin migration surfaced one warning and a handful of
doc/dead-code cleanups. None affect production behaviour through the main
dispatcher (which always calls `tools.web_tools._get_backend()` first and
preserves the full 7-provider walk), but direct callers of
`agent.web_search_registry.get_active_*_provider()` previously diverged
from the legacy order and could return `None` for users with credentials
but no explicit `web.backend` config key.

Changes
-------
1. `_LEGACY_PREFERENCE` was shipped as a 4-tuple
   `("brave-free", "firecrawl", "searxng", "ddgs")` while the PR
   description and the legacy `_get_backend()` candidate order both
   call for the 7-tuple
   `(firecrawl, parallel, tavily, exa, searxng, brave-free, ddgs)`.
   Replaced with the 7-tuple. Verified empirically: with TAVILY+EXA keys
   and no config, `get_active_search_provider()` now returns tavily
   (was None); with EXA+PARALLEL it returns parallel (was None); with
   BRAVE+FIRECRAWL it returns firecrawl (was brave-free).

2. `agent/web_search_registry.py` — module docstring, `_resolve` step-3
   docstring, and inline comment all listed the old 4-tuple and claimed
   "brave-free first because it was the shipped default". The legacy
   default is `"firecrawl"`. Rewritten to match the new ordering and
   reference `tools.web_tools._get_backend()` as the source of truth.

3. `agent/web_search_registry.py` — `get_active_crawl_provider`
   docstring said "only Tavily implements it among built-in providers".
   Firecrawl also advertises `supports_crawl=True` after the previous
   commit. Updated to "Tavily and Firecrawl".

4. `plugins/web/tavily/provider.py` — module docstring said "Tavily is
   the only built-in backend that natively crawls". Updated.

5. `agent/web_search_provider.py` — ABC docstring mentioned only
   `search` / `extract` capabilities. Added `crawl` for accuracy.

6. `plugins/web/{firecrawl,parallel,exa}/provider.py` — dead plugin-level
   cache globals (`_firecrawl_client`, `_parallel_client`,
   `_async_parallel_client`, `_exa_client`) were declared but never read
   (all reads/writes go through `_wt.*` per the `extracting-inline-
   helpers-to-plugins` recipe). Removed the dead declarations; the
   reset-for-tests helpers in firecrawl + parallel now clear the
   canonical `_wt._<name>` slots, matching the pattern exa already used.

Tests
-----
218/218 web-targeted tests still pass (no test changes needed). 4910/4910
in `tests/tools/` still green.
Brecht-H added a commit to Brecht-H/hermes-agent that referenced this pull request May 16, 2026
…ne (#1)

* feat(web): extend ABC with supports_crawl and async-extract semantics

Two ABC additions to cover the surface area of the remaining four
providers (exa, parallel, tavily, firecrawl) which were untouched by the
initial spike:

1. supports_crawl() + crawl() — Tavily natively crawls a seed URL via
   its /crawl endpoint. Exposing supports_crawl=True lets the crawl
   tool's dispatcher route to Tavily when configured, falling back to
   the auxiliary-model summarization path otherwise. Firecrawl could
   add this in a follow-up (the SDK supports it; we just don't surface
   it as a tool today).

2. Async-or-sync extract() — Parallel's SDK is natively async
   (AsyncParallel.beta.extract); Exa and Tavily are sync; Firecrawl is
   sync but called inside asyncio.to_thread() with a 60s timeout. The
   ABC docstring now permits either shape: implementations declare
   their own sync/async signature and the dispatcher uses
   inspect.iscoroutinefunction to detect and await.

Also adds get_active_crawl_provider() to web_search_registry mirroring
the search/extract resolvers, with web.crawl_backend as the explicit
override config key.

No behavior change on its own — these are scaffolds for the four
remaining provider migrations.

* feat(web): exa plugin — first multi-capability migration (search + extract)

Migrates Exa from the inline `_exa_search()` / `_exa_extract()` helpers in
tools/web_tools.py to a bundled plugin at plugins/web/exa/.

This is the first plugin in this PR to advertise supports_extract=True,
exercising the multi-capability ABC path that the initial three migrations
(brave_free, ddgs, searxng — all search-only) did not cover.

Both Exa methods are sync — the SDK is sync-only. The web_extract_tool
dispatcher in tools/web_tools.py will continue to call them inline until
Task "dispatch-extract-all" cuts it over to the registry.

Behaviour preserved bit-for-bit aside from the ABC method-name change:
  - is_configured()  -> is_available()
  - provider_name()  -> name (property)
  - "exa" stays as the registered name
  - Module-level `_exa_client` cache + lazy `from exa_py import Exa`
    preserved at the new location.
  - Errors (ValueError for missing API key, ImportError for missing SDK,
    generic Exception) caught and surfaced as {"success": False, "error": ...}
    instead of raising.

Adds "exa" to _WEB_PLUGIN_SKIPLIST in hermes_cli/tools_config.py so the
hardcoded TOOL_CATEGORIES["web"] row and the plugin-injected row don't
duplicate during the spike. The skip-list goes away in the cleanup phase
along with the hardcoded row.

The legacy inline `_exa_search` / `_exa_extract` / `_get_exa_client` /
`_exa_client` in tools/web_tools.py are NOT deleted yet — the dispatcher
still references them. They go away in the next dispatcher-cutover commit.

E2E verified:
  - Plugin discovers + registers
  - .supports_search/.supports_extract/.supports_crawl = (True, True, False)
  - .get_setup_schema() returns the picker row shape
  - resolve(): explicit exa + EXA_API_KEY -> exa; without key -> exa (registered
    but unavailable, dispatcher surfaces "EXA_API_KEY not set" error)

* feat(web): parallel plugin — first async-extract plugin

Migrates Parallel.ai from inline `_parallel_search()` / `_parallel_extract()`
in tools/web_tools.py to a bundled plugin at plugins/web/parallel/.

First plugin in the codebase to expose an async :meth:`extract`:

  - search() is sync — Parallel.beta.search
  - extract() is **async def** — AsyncParallel.beta.extract

The ABC's docstring on supports_extract() already permits sync-or-async;
this commit is the first to exercise the async path. The web_extract_tool
dispatcher (next commit) detects coroutines via
inspect.iscoroutinefunction and awaits accordingly.

Behavior preserved:
  - PARALLEL_API_KEY required (raises ValueError if missing → surfaced
    as {"success": False, "error": "..."} instead)
  - PARALLEL_SEARCH_MODE env var honored (agentic|fast|one-shot, default
    agentic), validated via _resolve_search_mode()
  - Limit capped at 20 server-side via min(limit, 20)
  - Per-URL failure mode preserved: response.errors[] each become a
    result dict with an "error" field rather than raising
  - Module-level _parallel_client / _async_parallel_client caches kept
    (mirrors legacy singleton pattern)

Adds "parallel" to _WEB_PLUGIN_SKIPLIST in hermes_cli/tools_config.py so
the picker doesn't double-list.

The legacy inline _parallel_search, _parallel_extract, _get_parallel_client,
_get_async_parallel_client in tools/web_tools.py are NOT deleted yet — the
dispatcher still calls them. They go away when the dispatcher cuts over.

E2E verified:
  - inspect.iscoroutinefunction(p.search) -> False
  - inspect.iscoroutinefunction(p.extract) -> True
  - extract() returns a coroutine (not a list)
  - 5 providers register correctly (brave-free, ddgs, exa, parallel, searxng)

* feat(web): tavily plugin — first three-capability plugin (search + extract + crawl)

Migrates Tavily from inline _tavily_request() / _normalize_tavily_*
helpers in tools/web_tools.py to a bundled plugin at plugins/web/tavily/.

First plugin in the codebase to advertise supports_crawl=True. Tavily is
unique among built-in backends in offering a native /crawl endpoint that
walks linked pages from a seed URL with optional natural-language
instructions and depth ("basic" or "advanced").

Capabilities:
  - supports_search()  -> True (Tavily /search)
  - supports_extract() -> True (Tavily /extract)
  - supports_crawl()   -> True (Tavily /crawl)
  All sync (httpx.post under the hood).

The crawl method accepts forward-compat kwargs (instructions, depth,
limit) and is gated against unsafe URLs/policy by the dispatcher in
web_crawl_tool — exactly as before.

Behavior preserved:
  - TAVILY_API_KEY required (ValueError → typed error response)
  - TAVILY_BASE_URL env override honored
  - /crawl requires both body auth AND Bearer header — preserved
  - failed_results[] and failed_urls[] response keys mapped to per-URL
    items with error fields rather than raising
  - max_results capped at 20 server-side

Adds "tavily" to _WEB_PLUGIN_SKIPLIST.

The legacy inline _tavily_request / _normalize_tavily_search_results /
_normalize_tavily_documents / _TAVILY_BASE_URL in tools/web_tools.py are
NOT deleted yet — search/extract dispatch and the entire web_crawl_tool
function still reference them. They go away when those dispatchers are
cut over to the registry.

E2E verified:
  - Tavily registers with all 3 capabilities
  - Provider list now: brave-free, ddgs, exa, parallel, searxng, tavily

* feat(web): firecrawl plugin — largest migration (search + async extract + dual auth)

Migrates Firecrawl from inline code in tools/web_tools.py to a bundled
plugin at plugins/web/firecrawl/. By line count this is the largest of
the seven provider migrations: the firecrawl path captured most of the
file's vendor-specific complexity.

What moved into the plugin (all previously in tools/web_tools.py):

  Lazy Firecrawl SDK proxy
    - _load_firecrawl_cls() — caches the imported SDK class
    - _FirecrawlProxy + Firecrawl singleton — defers ~200ms of SDK
      imports until first construction or isinstance check.

  Client construction (dual auth)
    - _get_direct_firecrawl_config()  — direct FIRECRAWL_API_KEY/URL path
    - _get_firecrawl_gateway_url()    — managed Nous tool-gateway URL
    - _is_tool_gateway_ready()        — gateway URL + Nous token check
    - _has_direct_firecrawl_config()  — direct config present?
    - _get_firecrawl_client()         — combined client construction
                                        honoring web.use_gateway
    - check_firecrawl_api_key()       — top-level "is firecrawl usable"
    - _firecrawl_backend_help_suffix() — managed-gateway help string
    - _raise_web_backend_configuration_error() — typed misconfig error

  Response shape normalization (vendor-specific)
    - _to_plain_object(), _normalize_result_list() — SDK→dict helpers
    - _extract_web_search_results() — handles SDK/direct/gateway shapes
    - _extract_scrape_payload()     — nested-data unwrap for scrape

  Per-URL extract loop
    - 60s asyncio.wait_for timeout per URL
    - Pre-scrape website-policy gate
    - Post-scrape redirect-aware SSRF re-check
    - Format-aware content selection (markdown / html / auto)
    - Per-URL errors returned as {"error": str} entries, no raises

Extract is declared `async def` — each URL is scraped in
asyncio.to_thread(...). This is the second async-extract plugin after
parallel.

The plugin re-exports `Firecrawl` (the lazy proxy) and
`check_firecrawl_api_key()` so existing tests doing
`patch("tools.web_tools.Firecrawl")` or
`monkeypatch.setattr(web_tools, "check_firecrawl_api_key", ...)` keep
working — tools/web_tools.py re-exports both names in the next
dispatcher-cutover commit.

Note: web_crawl_tool still has its own Firecrawl crawl path inline
(separate from extract); the Firecrawl SDK supports /crawl but we don't
expose supports_crawl=True on this plugin yet. Tavily handles crawl
today. Adding Firecrawl crawl is a clean follow-up.

Adds "firecrawl" to _WEB_PLUGIN_SKIPLIST.

E2E verified:
  - All 7 providers register: brave-free, ddgs, exa, firecrawl,
    parallel, searxng, tavily
  - inspect.iscoroutinefunction(firecrawl.extract) -> True
  - Firecrawl proxy is a callable lazy proxy at module level
  - check_firecrawl_api_key reflects FIRECRAWL_API_KEY presence

* refactor(web): dispatch all three tools through web_search_registry

Cuts over web_search_tool, web_extract_tool, and web_crawl_tool in
tools/web_tools.py to dispatch through agent.web_search_registry
instead of the legacy hardcoded if-elif backend chains.

Per-tool changes:

  web_search_tool (sync)
    Replace 5 backend branches (parallel, exa, registry-3-providers,
    tavily, firecrawl-fallthrough) with a single registry path:
      1. _get_search_backend() resolves the configured name
      2. _wsp_get_provider(name) for explicit-config-wins semantics
      3. get_active_search_provider() fallback for typo / unknown name
      4. provider.search(query, limit) — sync for all 7 providers

  web_extract_tool (async)
    Replace 4 backend branches (parallel-async, exa-sync, tavily-sync,
    search-only-error, firecrawl-perurl-loop) with:
      1. Same provider resolution as search.
      2. When configured backend IS registered but doesn't support
         extract (search-only providers like brave-free), surface a
         typed "search-only" error matching the legacy text — tests
         assert that wording.
      3. inspect.iscoroutinefunction(provider.extract) detects sync vs
         async: parallel + firecrawl are async; exa + tavily are sync.
         Sync extracts run in asyncio.to_thread() so we don't block.

  web_crawl_tool (async)
    Replace tavily-specific branch + search-only-error block with:
      1. _wsp_get_provider(backend) — explicit config first
      2. Search-only typed error when the configured name doesn't
         support crawl (matches legacy phrasing)
      3. get_active_crawl_provider() fallback otherwise
      4. provider.crawl(url, **kwargs) — async-or-sync dispatch as above
      5. Response post-processing (LLM summarization, trimming) stays
         unchanged — it's not provider-specific.
    When no plugin advertises supports_crawl, falls through to the
    existing Firecrawl-via-web-summarize path below (unchanged).

Test updates (2 tests in tests/tools/test_web_tools_config.py):
  - test_web_search_clamps_limit_before_backend_call:
      patch("tools.web_tools._parallel_search") -> patch the registry
      provider returned by agent.web_search_registry.get_provider
  - test_search_error_response_does_not_expose_diagnostics:
      patch("tools.web_tools._get_firecrawl_client") -> same pattern

Tests unchanged (still pass):
  - All TestXBackendWiring classes (test _get_backend / _is_backend_available
    config-resolution, independent of dispatch)
  - All TestXSearchOnlyErrors classes (test the search-only error path
    via web_extract_tool / web_crawl_tool — error text preserved)
  - 141 passing web tests total, 0 regressions.

Dead-code cleanup deferred to a follow-up commit so this diff stays
focused on the cutover. After this commit:
  - tools.web_tools._exa_search / _exa_extract / _parallel_search /
    _parallel_extract / _tavily_request / _normalize_tavily_* /
    _get_firecrawl_client / _extract_web_search_results /
    _extract_scrape_payload / _to_plain_object / _normalize_result_list
    are no longer called by the dispatchers, but still exist.
  - The config-resolution layer (_get_backend, _is_backend_available,
    _is_tool_gateway_ready, _has_direct_firecrawl_config) IS still in
    use and must stay.
  - The Firecrawl proxy and check_firecrawl_api_key are still imported
    by integration tests and patched by unit tests — must stay (or be
    re-exported from the plugin).

* fix(web): preserve firecrawl crawl + website-policy gate after migration

Two regressions discovered by running the full tests/tools/ suite after
the dispatcher cutover, both fixed in this commit:

1. web_crawl_tool incorrectly errored "search-only" for firecrawl
---------------------------------------------------------------------
The cutover treated any provider with supports_crawl()==False as a
search-only backend and returned the typed search-only error. But
firecrawl can crawl via the legacy multi-page-extract path inside
web_crawl_tool — it just doesn't expose supports_crawl on the plugin
(adding native firecrawl crawl is a clean follow-up).

Fix: only emit the search-only error when the provider supports
NEITHER crawl NOR extract (brave-free / ddgs / searxng). When the
provider supports extract but not crawl (firecrawl), fall through to
the legacy firecrawl-via-extract path below.

2. firecrawl plugin's check_website_access wasn't patchable
---------------------------------------------------------------------
The plugin imported `from tools.website_policy import check_website_access`
INSIDE the extract() function body, so monkeypatching the name on
plugins.web.firecrawl.provider had no effect — the inner import re-bound
the name on every call.

Fix: hoist the import to module level. Cheap (website_policy itself
has no heavy deps) and makes the standard
monkeypatch.setattr(firecrawl_provider, "check_website_access", ...)
pattern work.

Test updates (tests/tools/test_website_policy.py — 4 tests):
  - test_web_extract_short_circuits_blocked_url
  - test_web_extract_blocks_redirected_final_url
    Both: patch the gate at plugins.web.firecrawl.provider (where it
    runs after migration) and force the firecrawl plugin to be the
    active extract provider via FIRECRAWL_API_KEY.
  - test_web_crawl_short_circuits_blocked_url
  - test_web_crawl_blocks_redirected_final_url
    Both: unchanged — the dispatcher-level gate at tools.web_tools.py
    line 1651 still uses the imported `check_website_access` name and
    the firecrawl-fallthrough path is exercised as before.

Verified: 22/22 tests/tools/test_website_policy.py pass.

* refactor(web): delete inline vendor helpers, re-export from plugins

Removes ~580 lines of dead code from tools/web_tools.py that were
superseded by the plugin migration but kept around in the cutover commit
to keep the diff focused. Replaces them with thin re-export shims so
existing tests and external callers that reach for the legacy
``tools.web_tools.<name>`` paths continue to work transparently.

Deleted from tools/web_tools.py
--------------------------------
- Lazy Firecrawl SDK proxy (_load_firecrawl_cls, _FirecrawlProxy,
  _FIRECRAWL_CLS_CACHE, the Firecrawl singleton)
- Firecrawl client section (_get_direct_firecrawl_config,
  _get_firecrawl_gateway_url, _is_tool_gateway_ready,
  _has_direct_firecrawl_config, _raise_web_backend_configuration_error,
  _firecrawl_backend_help_suffix, _get_firecrawl_client)
- Parallel client section (_get_parallel_client,
  _get_async_parallel_client, _parallel_client, _async_parallel_client)
- Tavily client section (_TAVILY_BASE_URL, _tavily_request,
  _normalize_tavily_search_results, _normalize_tavily_documents)
- Generic SDK normalizers (_to_plain_object, _normalize_result_list,
  _extract_web_search_results, _extract_scrape_payload)
- Exa client section (_get_exa_client, _exa_client, _exa_search,
  _exa_extract)
- Parallel helpers (_parallel_search, _parallel_extract)
- Duplicate inline check_firecrawl_api_key

Net: tools/web_tools.py drops from 2227 → 1613 lines (-614 lines).

Re-exports added at top of tools/web_tools.py
---------------------------------------------
- From plugins.web.firecrawl.provider:
  Firecrawl, _FirecrawlProxy, _FIRECRAWL_CLS_CACHE, _load_firecrawl_cls,
  _get_direct_firecrawl_config, _get_firecrawl_gateway_url,
  _is_tool_gateway_ready, _has_direct_firecrawl_config,
  _firecrawl_backend_help_suffix, _raise_web_backend_configuration_error,
  _get_firecrawl_client, _to_plain_object, _normalize_result_list,
  _extract_web_search_results, _extract_scrape_payload,
  check_firecrawl_api_key
- From plugins.web.tavily.provider:
  _tavily_request, _normalize_tavily_search_results,
  _normalize_tavily_documents
- From plugins.web.parallel.provider:
  _get_parallel_client, _get_async_parallel_client
- From plugins.web.exa.provider:
  _get_exa_client

Plus retained module-level imports for backward-compat with tests:
- httpx (tests patch tools.web_tools.httpx for tavily request mocking)
- build_vendor_gateway_url, _read_nous_access_token,
  resolve_managed_tool_gateway, managed_nous_tools_enabled,
  prefers_gateway (tests patch tools.web_tools.<name>)

Plugin indirection pattern (key technique)
------------------------------------------
For functions inside the firecrawl/parallel/exa plugins to honor
unit-test patches that target ``tools.web_tools.<name>``, the plugin
implementations now do ``import tools.web_tools as _wt`` at call time
and read helper names through that module (``_wt._read_nous_access_token``,
``_wt.Firecrawl``, ``_wt.prefers_gateway``, etc.). This makes the
existing test patches transparently reach the plugin code without any
test changes.

The cached client globals (_firecrawl_client, _firecrawl_client_config,
_parallel_client, _async_parallel_client, _exa_client) also now live on
tools.web_tools so existing test setup_method handlers that reset
``tools.web_tools._<vendor>_client = None`` between cases keep working.
The plugins read/write the cache via getattr/setattr on the web_tools
module.

Verified
--------
- 173/173 targeted web tests pass:
  test_web_providers.py, test_web_providers_brave_free.py,
  test_web_providers_ddgs.py, test_web_providers_searxng.py,
  test_web_tools_config.py, test_web_tools_tavily.py,
  test_website_policy.py, test_config_null_guard.py
- Compile-clean (py_compile.compile passes)
- All inline implementations now exist in exactly one place
  (plugins.web.<vendor>.provider)

Follow-up clean-up
------------------
- Drop _WEB_PLUGIN_SKIPLIST + hardcoded TOOL_CATEGORIES["web"] rows
  (next commit)
- Delete tools/web_providers/ directory entirely
- Add tests/plugins/web/ coverage
- Full tests/tools/ + tests/gateway/ regression sweep before promoting PR

* refactor(tools): drop hardcoded web picker rows + skiplist; plugins are sole source

Removes the seven hardcoded TOOL_CATEGORIES["web"] provider rows that
duplicated the plugin-registered providers, and deletes the
_WEB_PLUGIN_SKIPLIST that existed to prevent duplicate picker rows
during the migration. The Web Search & Extract category now derives its
provider rows entirely from agent.web_search_registry via
_plugin_web_search_providers(), matching how Spotify, Google Meet, and
the image_gen plugins are surfaced.

Removed (deduplicated against plugin schemas):
  - Firecrawl Cloud         → plugins.web.firecrawl
  - Exa                     → plugins.web.exa
  - Parallel                → plugins.web.parallel
  - Tavily                  → plugins.web.tavily
  - SearXNG                 → plugins.web.searxng
  - Brave Search (Free Tier) → plugins.web.brave_free
  - DuckDuckGo (ddgs)       → plugins.web.ddgs (post_setup hook preserved)

Retained in TOOL_CATEGORIES["web"]:
  - Nous Subscription   — requires requires_nous_auth +
                          managed_nous_feature + override_env_vars
                          to drive the managed-gateway UX. Not a
                          provider — a different *setup flow* for the
                          firecrawl backend.
  - Firecrawl Self-Hosted — points firecrawl at a private Docker URL
                            via FIRECRAWL_API_URL only. Same reason:
                            UX setup-flow row, not a provider.

These two rows describe alternative auth/billing paths for the
firecrawl backend; they intentionally share web_backend="firecrawl"
with the plugin row but light up different env-var prompts.

Plugin schema extensions
------------------------
- ddgs plugin's get_setup_schema() now emits `post_setup: "ddgs"` so
  selection still triggers the pip-install hook in _run_post_setup().
- _plugin_web_search_providers() passes `post_setup` through verbatim
  when present in the schema (other future plugins like camofox / a
  hypothetical playwright-web plugin can opt in the same way).
- Picker rows now carry both `web_backend` (legacy field consumed by
  setup + selection helpers) and `web_search_plugin_name`
  (informational marker), so behavior is identical between hardcoded
  and plugin-registered rows.

Net diff
--------
- hermes_cli/tools_config.py: -141/+50 lines (~91 lines net)
- plugins/web/ddgs/provider.py: +7/-4 (post_setup field + badge polish)

Verified
--------
- Compile-clean for both files
- Picker shows: 2 hardcoded rows (Nous Subscription, Firecrawl
  Self-Hosted) + 7 plugin rows (alphabetically: Brave Search,
  DuckDuckGo, Exa, Firecrawl, Parallel, SearXNG, Tavily). DuckDuckGo
  row carries post_setup="ddgs" for first-time install.
- 173 web-specific tests still pass.

* refactor(web): delete legacy tools/web_providers/ directory + migrate ABC tests

Removes the legacy in-tree provider scaffolding that PR #25182 fully
replaced with the plugin architecture:

  tools/web_providers/__init__.py        (6 lines)
  tools/web_providers/base.py            (89 lines — old ABCs)
  tools/web_providers/ARCHITECTURE.md    (73 lines — old design doc)

These were the staging-ground ABCs and provider modules that the
plugin migration absorbed. All seven web providers now implement the
single :class:`agent.web_search_provider.WebSearchProvider` ABC and
live under ``plugins/web/<vendor>/``. Nothing else in the tree imports
``tools.web_providers`` — verified via grep before deletion.

Test migration (tests/tools/test_web_providers.py)
--------------------------------------------------
Rewrote ``TestWebProviderABCs`` to test the new unified ABC at
:mod:`agent.web_search_provider`:

  - test_cannot_instantiate_abc_directly — abstract ``name`` + ``is_available``
  - test_concrete_search_only_provider_works — exercise default
    ``supports_extract=False`` / ``supports_crawl=False`` flags
  - test_concrete_multi_capability_provider_works — exercise all three
    capabilities, async extract supported (declared sync here for
    simplicity; real plugins like parallel + firecrawl use async)
  - test_search_only_provider_skips_extract_and_crawl — verify
    ``supports_*()`` flags default to False so search-only providers
    don't have to implement extract() or crawl()

The 9 other tests in the file (per-capability backend selection,
DEFAULT_CONFIG merge, dispatcher routing) test public helpers in
``tools.web_tools`` that still exist and pass unchanged.

agent/web_search_provider.py docstring updated to reflect that the
legacy ABCs no longer exist; the response-shape contract is preserved
bit-for-bit so external consumers see no behavioral change.

Net diff
--------
- tools/web_providers/ removed (-168 lines)
- tests/tools/test_web_providers.py rewritten ABC section (+78/-30 net,
  same coverage, new API)
- agent/web_search_provider.py docstring (-3/+5 lines)

Verified
--------
- 173/173 targeted web tests pass
- 12/12 ABC contract tests pass with the new interface
- No remaining grep hits for ``tools.web_providers`` outside of
  intentional historical references in plugin docstrings.

* test(plugins): tests/plugins/web/ — coverage for the 7-plugin migration

Adds 44 focused tests under tests/plugins/web/ covering the surface that
the PR #25182 web-provider migration introduced. Complements the
existing tests/tools/ coverage which is dispatcher-centric; this file is
plugin-centric and tests each plugin + the registry directly.

Test classes (44 tests, ~1.1s on 4 workers)
-------------------------------------------

TestBundledPluginsRegister (16 tests)
  - All seven plugins present in the registry after
    _ensure_plugins_discovered()
  - Per-plugin parametrized capability-flag assertions
    (brave-free / ddgs / searxng: search-only;
     exa / parallel / firecrawl: search + extract;
     tavily: search + extract + crawl)
  - Every plugin exposes name + display_name properties
  - Every plugin returns a picker-compatible get_setup_schema() dict

TestIsAvailable (7 tests)
  - Each premium plugin reports is_available()==False when its env var is
    absent and True once set (brave-free / searxng / tavily / exa /
    parallel)
  - firecrawl recognizes either FIRECRAWL_API_KEY or FIRECRAWL_API_URL
    as a "configured" signal
  - ddgs is the always-on fallback and must not raise from is_available()

TestRegistryResolution (4 tests)
  - Option B semantics validated end-to-end:
    1. Explicit configured provider wins even when is_available()==False
       (dispatcher surfaces typed credential errors, no silent switch)
    2. Unknown/typo name falls back to first available legacy-preference
       provider
    3. Asking for extract via a search-only backend falls back to an
       extract-capable available provider (capability-incompatible
       branch in _resolve())
    4. No config + no credentials → None (or ddgs if installed)

TestAsyncExtractDispatch (4 tests)
  - parallel + firecrawl extract() are coroutine functions (async path
    in dispatcher uses await)
  - exa + tavily extract() are sync (dispatcher wraps in
    asyncio.to_thread)

TestErrorResponseShapes (7 tests)
  - Plugins return typed error dicts (success=False + "error" key) when
    credentials are missing, never raise
  - async extract() returns list of per-URL error dicts
  - tavily crawl() returns {"results": [{"error": ...}]} on missing
    credentials

Design notes
------------
- All tests use real imports of plugin modules — no mocking of provider
  classes themselves — so they catch drift in the ABC, registry, and
  glue layer simultaneously. Per the hermes-agent-dev skill's E2E
  testing guidance.
- The autouse _isolate_env fixture clears every web-provider env var
  before each test so is_available() reflects the test's setup.
- Resolution tests use the lower-level _resolve() directly rather than
  rebuilding the HERMES_HOME config dance — same observable behavior,
  no sys.modules.pop side-effects that would break the ABC isinstance
  check inside ctx.register_web_search_provider().

* feat(web): firecrawl plugin natively supports crawl; delete legacy inline path

The web-provider migration originally left firecrawl crawl as the only
provider-specific code remaining inline in tools/web_tools.py (~250
lines of Firecrawl-specific crawl orchestration that didn't fit the
plugin's existing surface). This commit closes that gap.

What this adds
--------------
1. plugins/web/firecrawl/provider.py: implement async ``crawl(url, **kwargs)``
   - Accepts the same kwargs as the dispatcher passes to any crawl
     provider (``instructions``, ``depth``, ``limit``); Firecrawl's
     /crawl endpoint ignores ``instructions`` and ``depth`` so we log
     and drop with a clear info message.
   - Wraps the sync SDK ``crawl()`` call in asyncio.to_thread so the
     gateway event loop isn't blocked on a multi-page crawl.
   - Preserves the response-shape normalization across pydantic /
     typed-object / dict variants that the legacy inline code did.
   - Preserves per-page website-policy re-check (catches blocked
     redirects after the SDK returns).
   - Returns the same {"results": [...]} shape so the dispatcher's
     shared LLM-summarization post-processing path works unchanged.
   - Sets supports_crawl() to True so the dispatcher routes through
     the plugin instead of the legacy fallthrough.

2. tools/web_tools.py: delete the entire legacy firecrawl crawl block
   that used to run after "No registered provider supports crawl" —
   ~270 lines including:
   - check_firecrawl_api_key gate + typed error
   - inline SSRF + website-policy seed-URL gate (dispatcher already
     does this)
   - Firecrawl client setup with crawl_params
   - 100+ lines of pydantic/dict/typed-object normalization
   - Per-page LLM-processing loop (kept in the dispatcher's shared
     post-processing path; that's where it always belonged)
   - trimming + base64 image cleanup (still done in the dispatcher's
     shared path)

   Replaced with a single typed-error branch when no crawl-capable
   provider is available: "web_crawl has no available backend. Set
   FIRECRAWL_API_KEY (or FIRECRAWL_API_URL for self-hosted), or set
   TAVILY_API_KEY for Tavily."

Test updates
------------
- tests/tools/test_website_policy.py:
  - test_web_crawl_short_circuits_blocked_url: dispatcher seed-URL
    gate still runs on web_tools.check_website_access (no change to
    that patch), but the firecrawl client lockdown moved to the
    plugin module — patch firecrawl_provider._get_firecrawl_client
    instead of web_tools._get_firecrawl_client. The dispatcher
    short-circuits before the plugin runs, so the test still passes.
  - test_web_crawl_blocks_redirected_final_url: patch the per-page
    policy gate at plugins.web.firecrawl.provider.check_website_access
    (where it now runs) AND on web_tools (where the seed-URL gate
    still runs). Patch firecrawl_provider._get_firecrawl_client for
    the FakeCrawlClient injection. Both checks flow through the same
    fake_check function.
- tests/plugins/web/test_web_search_provider_plugins.py:
  - Update parametrized capability-flag spec: firecrawl supports_crawl
    is now True.
  - Add test_firecrawl_crawl_returns_error_dict_when_unconfigured —
    verifies inspect.iscoroutinefunction(p.crawl) is True and that
    the async crawl returns a per-page error dict (not a raise) when
    FIRECRAWL_API_KEY is missing.

Verified
--------
- 218/218 web tests pass (was 173, +44 plugin tests + 1 new firecrawl
  crawl test from this commit = 218 with the test deduplication).
- Compile-clean (py_compile passes on both files).
- Provider capabilities matrix confirmed end-to-end:
    name        search  extract  crawl   async-extract?  async-crawl?
    firecrawl   True    True     True    True            True
    tavily      True    True     True    False           False
  Both crawl-capable providers exercise the dispatcher's
  inspect.iscoroutinefunction async-or-sync detection.

Net diff
--------
- tools/web_tools.py: -254 lines (legacy inline crawl gone)
- plugins/web/firecrawl/provider.py: +185 lines (crawl method)
- test_website_policy.py: +14/-9 lines (patch locations)
- test_web_search_provider_plugins.py: +22/-1 lines (capability flag
  + new firecrawl crawl test)
- Total: -32 net LoC; tools/web_tools.py is now 1509 lines (was 1763
  before this commit, 2227 before the migration started).

* fix(web): align _LEGACY_PREFERENCE with legacy 7-provider order + doc cleanup

Self-review of the plugin migration surfaced one warning and a handful of
doc/dead-code cleanups. None affect production behaviour through the main
dispatcher (which always calls `tools.web_tools._get_backend()` first and
preserves the full 7-provider walk), but direct callers of
`agent.web_search_registry.get_active_*_provider()` previously diverged
from the legacy order and could return `None` for users with credentials
but no explicit `web.backend` config key.

Changes
-------
1. `_LEGACY_PREFERENCE` was shipped as a 4-tuple
   `("brave-free", "firecrawl", "searxng", "ddgs")` while the PR
   description and the legacy `_get_backend()` candidate order both
   call for the 7-tuple
   `(firecrawl, parallel, tavily, exa, searxng, brave-free, ddgs)`.
   Replaced with the 7-tuple. Verified empirically: with TAVILY+EXA keys
   and no config, `get_active_search_provider()` now returns tavily
   (was None); with EXA+PARALLEL it returns parallel (was None); with
   BRAVE+FIRECRAWL it returns firecrawl (was brave-free).

2. `agent/web_search_registry.py` — module docstring, `_resolve` step-3
   docstring, and inline comment all listed the old 4-tuple and claimed
   "brave-free first because it was the shipped default". The legacy
   default is `"firecrawl"`. Rewritten to match the new ordering and
   reference `tools.web_tools._get_backend()` as the source of truth.

3. `agent/web_search_registry.py` — `get_active_crawl_provider`
   docstring said "only Tavily implements it among built-in providers".
   Firecrawl also advertises `supports_crawl=True` after the previous
   commit. Updated to "Tavily and Firecrawl".

4. `plugins/web/tavily/provider.py` — module docstring said "Tavily is
   the only built-in backend that natively crawls". Updated.

5. `agent/web_search_provider.py` — ABC docstring mentioned only
   `search` / `extract` capabilities. Added `crawl` for accuracy.

6. `plugins/web/{firecrawl,parallel,exa}/provider.py` — dead plugin-level
   cache globals (`_firecrawl_client`, `_parallel_client`,
   `_async_parallel_client`, `_exa_client`) were declared but never read
   (all reads/writes go through `_wt.*` per the `extracting-inline-
   helpers-to-plugins` recipe). Removed the dead declarations; the
   reset-for-tests helpers in firecrawl + parallel now clear the
   canonical `_wt._<name>` slots, matching the pattern exa already used.

Tests
-----
218/218 web-targeted tests still pass (no test changes needed). 4910/4910
in `tests/tools/` still green.

* fix(web): preserve top-level error envelope on unconfigured systems

Surfaced by local E2E behavior-parity testing of PR vs origin/main: the
plugin-migrated dispatchers were quietly changing the error envelope
shape returned to function-calling models on unconfigured systems.

Two findings, both from per-result error wrapping bleeding into the
pre-flight configuration error path:

1. **search**: ``firecrawl.search()`` caught the
   ``ValueError("Web tools are not configured...")`` from
   ``_get_firecrawl_client()`` and returned it as
   ``{"success": False, "error": ...}``, losing the legacy
   ``{"error": "Error searching web: ..."}`` envelope that
   ``tool_error()`` emits on main. Models that special-case the
   ``error`` key still detect the failure, but the prefix is part of
   the legacy contract some users rely on.

2. **crawl**: ``firecrawl.crawl()`` caught the same pre-flight
   ``ValueError`` and wrapped it as a per-page error inside
   ``results[0]``. Main short-circuits on ``check_firecrawl_api_key()``
   BEFORE dispatching, so its unconfigured response is
   ``{"success": False, "error": "web_crawl requires Firecrawl..."}``
   at the top level. The PR's per-page burying hid the failure inside
   ``results[]`` where models that check ``result.get("error")`` would
   miss it.

Fix:
- ``plugins/web/firecrawl/provider.py``: pull
  ``_get_firecrawl_client()`` outside the broad ``try`` in
  ``search()``. Pre-flight ``ValueError`` / ``ImportError`` propagate
  to the dispatcher's top-level exception handler. In-flight SDK
  errors still get wrapped as ``{"success": False, ...}``.
- ``tools/web_tools.py``: mirror main's upstream availability gate in
  ``web_crawl_tool``. When the resolved crawl provider is
  ``is_available()==False``, short-circuit BEFORE dispatching with the
  same top-level error shape main emits.
- ``tests/tools/test_web_providers.py``: 2 regression tests
  (``TestUnconfiguredErrorEnvelopeParity``) lock in the behavior so
  future plugin work can't undo this.

Verified via local subprocess-based parity test (14/14 scenarios match
origin/main shape exactly) and full 210/210 web test suite green.

* fix(honcho): respect HOME-anchored default profile fallback

* fix(tests): exercise profile-mode HERMES_HOME for honcho fallback

The cherry-picked tests from #6173 set HERMES_HOME outside Path.home()/.hermes,
which forces get_default_hermes_root() down its Docker branch and returns
HERMES_HOME directly — so _get_default_hermes_home() never resolves to the
~/.hermes directory the tests were trying to assert about.

Rewire both tests to use the real profile layout (HERMES_HOME pointing at
~/.hermes/profiles/<name>) so _get_default_hermes_home() resolves back to
~/.hermes and the default-profile fallback is actually exercised.

* fix(clipboard): reject non-png clipboard images when png normalization fails

* fix(clipboard): only read PNG signature bytes, not entire file

Tighten _is_png_file() to read just the 8-byte PNG magic via path.open()
+ read(8), instead of slurping the entire image into memory only to check
the prefix.

* feat(goals): /subgoal — user-added criteria appended to active /goal (#25449)

* feat(goals): /subgoal — user-added criteria appended to active /goal

Layers a /subgoal command on top of the existing freeform Ralph judge
loop. The user can append extra criteria mid-loop; the judge factors
them into its done/continue verdict and the continuation prompt
surfaces them to the agent. No new tool, no agent self-judging — the
existing judge model just sees a richer prompt.

Forms:
  /subgoal                  show current subgoals
  /subgoal <text>           append a criterion
  /subgoal remove <n>       drop subgoal n (1-based)
  /subgoal clear            wipe all subgoals

How it integrates:

- GoalState gains `subgoals: List[str]` (default []), backwards-compat
  for existing state_meta rows.
- judge_goal accepts an optional subgoals kwarg; non-empty switches to
  JUDGE_USER_PROMPT_WITH_SUBGOALS_TEMPLATE which lists them as
  numbered criteria and asks 'is the goal AND every additional
  criterion satisfied?'
- next_continuation_prompt picks CONTINUATION_PROMPT_WITH_SUBGOALS_TEMPLATE
  when non-empty so the agent sees what to target.
- /subgoal is allowed mid-run on the gateway since it only touches the
  state the judge reads at turn boundary — no race with the running
  turn.
- Status line shows '... , N subgoals' when present.

Surface:
- hermes_cli/goals.py — field, prompt blocks, manager methods, judge weave
- hermes_cli/commands.py — /subgoal CommandDef
- cli.py — _handle_subgoal_command
- gateway/run.py — _handle_subgoal_command + mid-run dispatch
- tests/hermes_cli/test_goals.py — 15 new tests (backcompat, mutation,
  persistence, prompt template selection, judge-prompt content via mock,
  status-line rendering)

77 goal-related tests passing across goals + cli + gateway + tui.

* fix(goals): slash commands don't preempt the goal-continuation hook

Two findings from live-testing /subgoal:

1. Slash commands queued while the agent is running landed in
   _pending_input (same queue as real user messages). The goal hook's
   'is a real user message pending?' check returned True and silently
   skipped — but the slash command consumes its queue slot via
   process_command() which never re-fires the goal hook, so the loop
   stalls indefinitely. Now the hook peeks the queue and only defers
   when a non-slash payload is present.

2. The with-subgoals judge prompt was too soft — opus 4.7 said 'done,
   implying all requirements met' without verifying. Tightened to
   demand specific per-criterion evidence (file contents, output line,
   command result) and explicitly reject phrases like 'implying it was
   done.'

Live verified: /subgoal injected mid-loop now correctly forces the
judge to refuse done until the new criterion is met. Agent gets the
continuation prompt with subgoals listed, updates the script, judge
confirms done with specific evidence cited.

* fix(cli): harden skin yaml parsing for invalid section types

* fix(tests): correct skin engine test API call

The salvaged regression test called skin.get_spinner_list() which
doesn't exist on SkinConfig. Replace with direct dict access on
skin.spinner — same intent (verify default empty spinner is preserved
when user override is invalid).

* fix: simplify ACP approval bridging

Previously ACP dangerous-command approvals mixed an invalid ACP
payload shape with partial Hermes option mapping, and the callback
plumbing was shared across worker threads. This commit uses ACP
tool-call updates, preserves Hermes once/session/always semantics,
and scopes approval callbacks to the current worker thread.

- Build permission requests with `update_tool_call` and unique
  `perm-check-*` ids in `acp_adapter/permissions.py`
- Keep ACP option mapping explicit and fail closed on unknown outcomes
  or request failures
- Set approval callbacks inside the ACP executor worker and read them
  from thread-local state in `tools/terminal_tool.py`
- Replace duplicated ACP bridge coverage with focused tests in
  `tests/acp/test_permissions.py` and add a thread-local callback test

* chore(release): add AUTHOR_MAP entry for mrshu

Maps mr@shu.io to the mrshu GitHub handle so the release script
attributes the salvaged ACP approval bridging commit correctly.

* chore(release): add AUTHOR_MAP entries for 25-PR new-contributor batch

Pre-stages AUTHOR_MAP for 12 new contributors whose PRs are being salvaged
in the upcoming batch:

- 1RB        (#25462)
- ayushere   (#25342)
- domtriola  (#25424)
- ephron-ren (#25358)
- freqyfreqy (#25423)
- fu576      (#25369)
- kfa-ai     (#25398)
- magic524   (#25361)
- PaTTeeL    (#25359)
- pearjelly  (#25388)
- raymaylee  (#25394)
- Tianyu199509 (#25421)

* docs(user-guide): point tirith link to correct repo

* docs(lsp): replace "git worktree" with "git repository" in LSP docs

The word "worktree" (a git subcommand feature for parallel checkouts)
was used interchangeably with "repository" in the LSP docs, causing
confusion. LSP only requires a git-initialized directory, not an actual
worktree.

Fixes two instances: section "When LSP runs" and the troubleshooting
"Editing a file outside any git repo" heading.

* fix(agent): add Xiaomi MiMo to reasoning_content echo-back providers

Xiaomi MiMo emits reasoning via OpenAI's reasoning_content field and
requires reasoning_content on every assistant tool-call message when
replaying history. Without echo-back, subsequent API calls fail with
HTTP 400 — same shape as DeepSeek and Kimi/Moonshot thinking modes.

Adds _needs_mimo_tool_reasoning() detection (provider == 'xiaomi',
'mimo' in model, or xiaomimimo.com base url) and wires it into the
_needs_thinking_reasoning_pad() check.

Salvage of #25358 by @ephron-ren (manually re-applied — original branch
was severely stale against current main).

* fix(discord): handle forwarded messages via message_snapshots

Discord introduced message_snapshots for forwarded messages — text and
attachments live inside snap.content / snap.attachments rather than on
the parent message. _handle_message wasn't reading them, so forwards
showed up empty.

Defensively extracts snapshot text (when raw_content is empty) and
appends snapshot attachments to the working all_attachments list used
for type detection and media routing. hasattr/getattr guards keep this
safe on older discord.py installs without the field.

Salvage of #25462 by @1RB (manually re-applied — original branch was
stale against current main).

* fix(auxiliary): skip providers without credentials immediately

When the auxiliary client fallback chain reaches a provider that has no
credentials configured (no API key, no pool entry), the current code
just returns (None, None) which counts toward the per-call timeout
budget on the next attempt. Mark the provider unhealthy with a short
TTL so the chain advances quickly to the next viable option.

Closes #25384.

Salvage of #25395 by @AllynSheep.

* fix: gateway PID detection fails on Windows (two issues)

- _read_process_cmdline: /proc and 'ps' are unavailable on Windows,
  so process cmdline was always empty. Add psutil fallback (already
  a hard dependency used by _pid_exists in the same module).

- _record_looks_like_gateway: argv paths use backslashes on Windows
  but patterns use forward slashes/dots, so the fallback record check
  always failed. Normalize backslashes to forward slashes before
  matching.

Together these caused get_running_pid() to return None on Windows
even when the gateway process is alive, making the dashboard report
gateway as 'stopped' despite it functioning normally.

* feat(whatsapp): surface quoted reply metadata

* fix: show context compaction status

* fix(gateway): make Feishu ws connect override sync to preserve context manager

The Feishu adapter wrapped lark-oapi's Connect() callable to inject
ping_interval/ping_timeout overrides, but made the wrapper async. The
underlying library uses Connect() as an async context manager (async
with Connect(...) as ws:), which requires the call itself to be sync
and return an AsyncContextManager — making it async meant the wrapper
was awaited eagerly and ws never bound.

Restoring the sync wrapper preserves the protocol while still injecting
the overrides.

Salvage of #25388 by @pearjelly (manually re-applied — original branch
was severely stale against current main).

* fix: do not inherit api_mode when delegating across providers

Cross-provider delegation (e.g. MiniMax parent → DeepSeek child) must not
inherit the parent's api_mode, because each provider uses a different API
surface: MiniMax uses 'anthropic_messages' while DeepSeek uses
'chat_completions'. Inheriting the wrong mode causes 404 errors.

When the effective provider differs from the parent's provider, derive
api_mode from the target provider's defaults instead (None triggers
re-derivation).

Refs: Bug #20558, PR #20563

* fix(gateway): keep QQBot reconnect loop alive

* fix(auxiliary): forward custom_providers to compression model context-length detection

When auxiliary.compression.provider is "auto", the compression model
reuses the main model's provider and base_url.  The main model's
context_length was correctly picking up custom_providers per-model
overrides (via _custom_providers stored during __init__), but the
auxiliary compression model's context-length detection path in
_check_compression_model_feasibility was not passing custom_providers,
causing it to skip step 0b and fall through to models.dev.

This meant that for providers like NVIDIA NIM where the user has a
per-model context_length in custom_providers (e.g. 196608 for
minimax-m2.7), the auxiliary model would use the models.dev value
(204800) instead of the user-configured one — a subtle discrepancy
that could lead to silent compression issues when the auxiliary model
doesn't actually support the detected context length.

Fix: pass self._custom_providers (already stored as an instance attr
during __init__) to the get_model_context_length() call for the
auxiliary compression model.

* fix(background-review): silence memory provider teardown output leak

Background review fork redirected stdout/stderr around run_conversation()
so its iteration messages stay silent.  But the memory-provider teardown
(shutdown_memory_provider() and review_agent.close()) fired in the outer
finally block AFTER the redirect_stdout context exited — so provider
teardown prints (Honcho disconnect, Hindsight sync, etc.) leaked into
the parent terminal at end of every turn.

Moves the teardown inside the redirect_stdout scope on the success path
(and nulls review_agent so the finally safety-net skips double-shutdown).
The finally block is rewritten as an exception-path safety net that
re-opens a devnull redirect, since the original 'with' context has
already exited by the time finally runs.

Salvage of #25342 by @ayushere (manually re-applied + merged conflict
with current main's set_thread_tool_whitelist wiring).

* feat: add NovitaAI as LLM provider

Add NovitaAI as a first-class provider with dedicated model selection
flow, live pricing, and authoritative context length resolution.

- Register provider in PROVIDER_REGISTRY, HERMES_OVERLAYS, and all
  alias/label maps (ID: novita, aliases: novita-ai, novitaai)
- Add dedicated _model_flow_novita() with 3-tier model list fallback:
  Novita API → models.dev → static curated list
- Fetch live pricing from /v1/models with correct unit conversion
  (input_token_price_per_m is 0.0001 USD per Mtok)
- Add Novita-specific context length resolution (step 4b) in
  get_model_context_length(), prioritized over models.dev/OpenRouter
- Register api.novita.ai in _URL_TO_PROVIDER to prevent early return
  from the custom-endpoint code path
- Add models.dev mapping (novita → novita-ai)
- Add default auxiliary model (deepseek/deepseek-v3-0324)
- Add NOVITA_API_KEY to test isolation (conftest.py)
- Update docs: providers page, env vars reference, CLI reference,
  .env.example, README, and landing page

* docs: update NovitaAI description to "90+ models, pay-per-use"

* test(novita): cache pricing, add provider test coverage, AUTHOR_MAP entry

Follow-up to Alex-wuhu's NovitaAI provider commit. Adds:

- _pricing_cache hit/write in _fetch_novita_pricing (was missing — every
  pricing fetch was re-hitting the network), mirroring the
  fetch_ai_gateway_pricing pattern. force_refresh now also propagates
  from get_pricing_for_provider.
- TestNovitaProvider in tests/hermes_cli/test_api_key_providers.py
  covering profile load, alias resolution, registry auto-registration,
  model list parity between main.py and models.py, _URL_TO_PROVIDER,
  _PROVIDER_PREFIXES, context_size in _CONTEXT_LENGTH_KEYS, pricing
  unit conversion, and pricing cache behavior.
- AUTHOR_MAP entry for yanglongwei06@gmail.com → @Alex-yang00.

* docs: update NovitaAI provider positioning (#25532)

* fix(install): preserve pip entry point when re-running on symlinked install

setup_path() writes the user-facing hermes shim with `cat >`, which
follows existing symlinks. Older installs created
`$command_link_dir/hermes` as a symlink to `$HERMES_BIN`
(`venv/bin/hermes`), so re-running install.sh stomped the pip entry
point with a bash shim that exec'd itself in an infinite loop.

`rm -f` the link target before writing so the shim lands at
`$command_link_dir/hermes` and the venv entry point is left intact.

Adds a regression test that reproduces the symlink-stomp end-to-end
(creates the symlink, drives the real shim-write block from setup_path,
asserts the venv pip script body survives and the shim is now a regular
file). Both new assertions fail on origin/main and pass with the fix.

Closes #21454.

* feat(discord): render clarify choices as buttons

Brings Discord to parity with Telegram on the clarify tool's interactive
UX. Overrides BasePlatformAdapter.send_clarify on DiscordAdapter to attach
a button view when choices are present.

  - ClarifyChoiceView: one discord.ui.Button per choice (max 24, Discord's
    25-component view cap leaves one slot for Other) plus a final
    'Other (type answer)' button.
  - Numeric click -> tools.clarify_gateway.resolve_gateway_clarify(
    clarify_id, choice_text) using the canonical choice text from the
    gateway entry (falls back to the button label if the entry vanished).
  - Other click -> tools.clarify_gateway.mark_awaiting_text(clarify_id) so
    the gateway's text-intercept captures the next user message in this
    session as the response.
  - Auth via the shared _component_check_auth helper (same OR-semantics as
    ExecApprovalView / SlashConfirmView / UpdatePromptView / ModelPickerView).
  - Open-ended (no choices) path renders the prompt as a plain embed and
    relies on the existing text-intercept resolution.
  - Single-use: first valid click disables every button and updates the
    embed footer with who answered and what they chose.

No changes to BasePlatformAdapter.send_clarify or the gateway's
clarify_callback wiring -- the existing scaffolding already drives all
adapters; Discord just inherits the default text fallback today and gains
buttons by virtue of this override.

Test conftest extended: _FakeEmbed gains add_field() / set_footer() stubs
so tests can construct embedded views without monkey-patching per-test.

Original PR: #19249 by @LeonSGP43. This is a reshape of the contributor's
work onto current main's clarify infrastructure (clarify_id + entry-based
resolution shared with Telegram, instead of a parallel on_answer-closure
mechanism). The button view structure and UX shape are preserved.

Tests: 14 new tests in tests/gateway/test_discord_clarify_buttons.py.
391/391 existing Discord gateway tests still pass.

Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>

* fix(cli): allow rotating broken OpenRouter / AI Gateway key in `hermes model` flow (#25750)

Before: when `OPENROUTER_API_KEY` (or `AI_GATEWAY_API_KEY`) was already
set in ~/.hermes/.env, `hermes model openrouter` / `hermes model
ai-gateway` skipped the API-key prompt entirely and jumped straight to
the model picker. Users with a broken / expired / wrong key had no way
to replace it without editing ~/.hermes/.env by hand or re-running
`hermes setup` from scratch.

Both flows now route through the existing `_prompt_api_key()` helper,
which surfaces [K]eep / [R]eplace / [C]lear when a key is already
configured — the same UX the generic API-key providers (z.ai, MiniMax,
Gemini, etc.) and the Daytona setup already use.

* fix(install.ps1): pin uv sync to venv\, verify baseline imports on Windows (#25755)

* fix(cli): allow rotating broken OpenRouter / AI Gateway key in `hermes model` flow

Before: when `OPENROUTER_API_KEY` (or `AI_GATEWAY_API_KEY`) was already
set in ~/.hermes/.env, `hermes model openrouter` / `hermes model
ai-gateway` skipped the API-key prompt entirely and jumped straight to
the model picker. Users with a broken / expired / wrong key had no way
to replace it without editing ~/.hermes/.env by hand or re-running
`hermes setup` from scratch.

Both flows now route through the existing `_prompt_api_key()` helper,
which surfaces [K]eep / [R]eplace / [C]lear when a key is already
configured — the same UX the generic API-key providers (z.ai, MiniMax,
Gemini, etc.) and the Daytona setup already use.

* fix(install.ps1): pin uv sync target to venv\, verify baseline imports

Two related Windows-installer bugs that produce a broken venv with
`ModuleNotFoundError: No module named 'dotenv'` on first `hermes` run.

## Bug 1: uv sync ignores VIRTUAL_ENV, syncs into .venv\ instead of venv\

`Install-Dependencies` creates the venv at `venv\` via `uv venv venv`,
sets `$env:VIRTUAL_ENV = "$InstallDir\venv"`, then runs
`uv sync --extra all --locked`. Modern uv (>=0.5) ignores `VIRTUAL_ENV`
for the `sync` subcommand and uses the project default `.venv\`
instead. Result: deps land in `$InstallDir\.venv\`, `venv\` stays
empty except for the python.exe stub from the earlier `uv venv` call,
`hermes.exe` ends up wired to the wrong site-packages.

The bash installer (`scripts/install.sh`) already worked around this in
`install_deps()` line 1127 by passing `UV_PROJECT_ENVIRONMENT` — that
flag tells uv exactly where to put the project env regardless of
`VIRTUAL_ENV`. Port the same fix to PowerShell.

## Bug 2: no post-install verification

If the sync still misdirects for any other reason (uv version drift,
filesystem quirk, user re-run scenarios), the installer reports success
and the user only finds out by running `hermes` and getting an
unhelpful traceback. Add a baseline-import probe that runs the venv's
own python against the four packages every `hermes` invocation needs
(`dotenv`, `openai`, `rich`, `prompt_toolkit`). On failure, throw
with a recovery command tailored to whether a sibling `.venv\` exists.

User report (Windows 11, Python 3.13.5, Hermes v0.13.0): manual repro
steps were exactly this — `uv sync` landed in `.venv\`, recovered by
junctioning `venv\` → `.venv\` to bridge the path mismatch.

* fix(telegram): escape dynamic markdown in callback flows

Use MarkdownV2 formatting for Telegram callback follow-ups and interactive prompts where dynamic names or user text can break legacy Markdown parsing. Add regression coverage for reload-mcp, model picker, approval callbacks, and update prompts.

* fix(telegram): restore model-switch success path + author map

The cherry-picked PR over-indented the edit_message_text block for
the mm: (model selected → switch) success path so the confirmation
edit lived inside the preceding 'except Exception as exc' branch and
only fired when the callback raised. Dedent the try/except back to
12-space indent so it runs after the callback succeeds, restoring
the original flow that removes the inline buttons and shows the
'Switched to ...' confirmation.

Add a regression test (test_model_selected_edits_message_on_success)
that asserts edit_message_text is awaited and the result text is
routed through format_message (MARKDOWN_V2 + backtick survival).

Add phuongvm to scripts/release.py AUTHOR_MAP.

* fix(memory): skip OpenViking upload symlinks

* fix(codex-runtime): retire wedged sessions + post-tool watchdog + OAuth refresh classify (#25769)

Mirrors openclaw beta.8's app-server resilience fixes so a stuck codex
subprocess can't burn the full turn deadline and so users get a
`codex login` pointer instead of raw RPC errors when their token expires.

- TurnResult.should_retire signals the caller to drop+respawn codex.
- Deadline-hit path and dead-subprocess detection set should_retire so
  the next turn doesn't ride a CPU-spinning or auth-broken process.
- Post-tool watchdog (post_tool_quiet_timeout=90s): if a tool item
  completes and codex goes silent past the threshold without further
  output or turn/completed, fast-fail instead of waiting the full 600s.
  Resets on any non-tool activity so normal think-after-tool flows are
  not affected.
- <turn_aborted> and <turn_aborted/> in agent text are treated as
  terminal — some codex builds tear down a turn that way without
  emitting turn/completed.
- _classify_oauth_failure() inspects RPC error message + stderr tail
  for invalid_grant / token refresh / 401 / etc. and rewrites
  user-facing errors to 'run codex login'. Conservative: generic
  failures still surface verbatim. Fires at turn/start failure,
  turn/completed failure, and dead-subprocess paths.
- thread/start cross-fill: tolerate thread.id, thread.sessionId,
  top-level sessionId/threadId so future codex schema drift doesn't
  KeyError us at handshake.
- run_agent.py: when run_turn returns should_retire=True OR raises,
  close + null self._codex_session so the next turn respawns.

Tests: +30 cases across session + integration suites.
  tests/agent/transports/test_codex_app_server_session.py 50/50 pass
  tests/run_agent/test_codex_app_server_integration.py 27/27 pass
  Broader codex scope (transports + cli runtime/migration) 376/376 pass

* chore(release): add AUTHOR_MAP entries for second new-contributor batch

Pre-stages AUTHOR_MAP for 7 new contributors in the upcoming batch:

- HxT9          (#25760)
- evgyur        (#25651)
- AsoTora       (#25624)
- oxngon        (#25603)
- yifengingit   (#25589)
- vanthinh6886  (#25562)
- Arkmusn       (#25559)

EthanGuo-coder, wesleysimplicio, and zccyman are already in the map.

* fix: read approvals.timeout from config in CLI approval callback

The _approval_callback method in HermesCLI hardcoded timeout=60
instead of reading the approvals.timeout config value. This meant
the config setting was silently ignored for CLI interactive prompts.

Other approval paths (callbacks.py, tools/approval.py) already read
the config correctly — only cli.py was missed.

* fix: use AUTOINCREMENT id for message ordering instead of timestamp

On WSL2 (and similar environments), time.time() is not strictly monotonic
due to NTP sync or host clock adjustments. When clock regression occurs
during a multi-tool flush, later-inserted rows get earlier timestamps,
causing ORDER BY timestamp, id to sort them before rows that were written
first. This breaks the tool_calls/tool_response adjacency invariant and
triggers HTTP 400 from the API.

Use ORDER BY id instead, since id (INTEGER PRIMARY KEY AUTOINCREMENT)
always reflects true insertion order regardless of system clock behavior.

* docs: clarify media impact on session context

* fix: stop retrying initial MCP auth failures

* fix(gateway): enable text-intercept for multi-choice clarify fallback (#25567)

* fix: restrict .env file permissions to 0600

Set file mode 0600 on ~/.hermes/.env after creation in the installer and
after every write via memory_setup._write_env_vars(). This ensures only
the file owner can read/write API keys and tokens, matching standard
practice for credential files (.netrc, .aws/credentials, .ssh/config).

Fixes #25477

* fix(gateway): forward image attachments to background agent tasks

When the gateway spawned a background agent (e.g. for delegation), media
URLs and types from the originating message weren't forwarded — the bg
agent saw the prompt but no attached images. Vision-enabled tasks
effectively lost their inputs.

Forwards media_urls/media_types through the bg-task spawn path and
runs the same vision-enrichment step the main flow uses, so the bg
agent gets image descriptions inlined into its prompt.

Closes #25614.

Salvage of #25603 by @oxngon (manually re-applied — original branch
was severely stale against current main).

* fix(terminal): prevent safety filter false positives on keywords inside quoted strings

The _foreground_background_guidance() function matched background-wrapper
keywords (nohup/disown/setsid) anywhere in the command text, including
inside quoted strings, Python -c code, commit messages, and PR body text.

Two-layer fix:
1. Strip single-quoted, double-quoted, and backtick-quoted content before
   pattern matching via _strip_quotes() helper.
2. Tighten the regex to only match keywords at command-start positions
   (after ^, ;, &, &&, ||, or $() — not mid-argument.

Both layers are needed: quote stripping handles the common case of keywords
in string literals, and the position-aware regex handles unquoted cases
like 'export FOO=setsid' (word boundary match, wrong position).

Fixes #20064

* chore(release): map oswaldb22 noreply email for AUTHOR_MAP

Co-Authored-By: Oswald <oswaldb22@users.noreply.github.com>

* test(toolsets): lock web search into default platform coverage

Adds regression tests pinning web search into the WhatsApp and api-server
default platform-coverage toolsets. Pure test additions, no runtime change.

Salvage of the test-addition commit from #25692 by @wesleysimplicio.
(The AUTHOR_MAP fixup commit from the same PR landed separately as
529ec85c7.)

* fix(update): refresh lazy-installed backends on hermes update (#25766)

Pyproject's [all] extra was slimmed down in May 2026 — ~20 optional
backends moved to tools/lazy_deps.py and only install on first use.
hermes update runs uv pip install -e .[all] which doesn't touch any of
them, so pin bumps in LAZY_DEPS (CVE response, transitive fixes) were
silently ignored on already-activated backends.

Two changes:

1. _is_satisfied() now parses the spec and checks the installed version
   against the constraint via packaging.specifiers. Previously it
   returned True the moment the package name was importable, which made
   ensure() a name-presence gate rather than a version-pin gate.

2. New active_features() / refresh_active_features() pair: lists every
   feature with at least one of its packages currently installed, then
   re-runs ensure() on each. Refresh is invoked at the end of
   _cmd_update_impl, right after the [all] install completes. Cold
   backends (never activated) stay quiet — no churn for them.

Output during update is one summary block:
  → Refreshing 4 active lazy backend(s)...
    ↑ 1 refreshed: provider.anthropic
    ✓ 3 already current
or
    ⚠ memory.honcho failed to refresh: <pip stderr>

Failures never raise out of update — backends keep their previously-
installed version and we tell the user to rerun once upstream is fixed.
security.allow_lazy_installs=false is honored: features get marked
"skipped" with the reason shown.

Tests: 18 new unit tests covering version-aware satisfaction (exact pin,
range, extras blocks, missing package, malformed spec), active feature
discovery, and refresh status reporting. All 61 lazy_deps tests pass.

* fix(agent/gemini-cloudcode): seed delta defaults for reasoning-only stream chunks

_make_stream_chunk built delta_kwargs with only `role`, so a reasoning-only
chunk produced a SimpleNamespace without a `.content` attribute. Downstream
consumers that read `delta.content` then raised AttributeError on Gemini 2.5
Flash, where the thinking delta arrives before any content delta.

Seed `content`, `tool_calls`, …
teknium1 pushed a commit that referenced this pull request May 17, 2026
…template

Foundation commit for the browser-provider plugin migration (#25214).
Mirrors the architecture established by PR #25182 (web providers):

- agent/browser_provider.py — BrowserProvider ABC. Preserves the legacy
  CloudBrowserProvider lifecycle contract bit-for-bit (create_session,
  close_session, emergency_cleanup, session metadata shape) so the
  dispatcher in tools/browser_tool.py becomes a pure registry lookup.
  Renames is_configured() → is_available() for parity with WebSearchProvider.

- agent/browser_registry.py — selection registry with the same
  three-rule resolution as web_search_registry:
    1. Explicit config wins (returns even if is_available() == False so
       the dispatcher surfaces a precise credentials error)
    2. Single-eligible shortcut
    3. Legacy preference walk: browser-use → browserbase, filtered by
       availability. Firecrawl is intentionally NOT in the legacy walk
       (matches pre-migration behaviour — Firecrawl was only reachable
       via explicit browser.cloud_provider: firecrawl).

- hermes_cli/plugins.py — adds ctx.register_browser_provider() facade,
  one-liner mirror of register_web_search_provider().

No plugins registered yet; no dispatcher cutover yet. The next commits
move browserbase/browser-use/firecrawl into plugins/browser/<vendor>/
and switch tools/browser_tool.py over to the registry.
teknium1 pushed a commit that referenced this pull request May 17, 2026
…picker

Drops the three hardcoded browser-provider rows (Browserbase, Browser Use,
Firecrawl) from TOOL_CATEGORIES['browser']['providers'] and replaces them
with runtime injection from agent.browser_registry — mirroring the
_plugin_web_search_providers() pattern PR #25182 established for the
Web Search and Extract category.

Adds _plugin_browser_providers() helper in hermes_cli/tools_config.py
that walks list_providers() and builds a TOOL_CATEGORIES-shape dict per
provider via get_setup_schema(). The new visible_providers() hook calls
it for cat['name'] == 'Browser Automation'.

The three remaining hardcoded rows are non-provider UX setup-flow rows:
  - 'Nous Subscription (Browser Use cloud)' — managed Browser Use billed
    via Nous subscription; uses the browser-use plugin as the underlying
    backend but has distinct setup UX (requires_nous_auth gates it).
  - 'Local Browser' — headless Chromium, no CloudBrowserProvider.
  - 'Camofox' — anti-detection local Firefox; _is_camofox_mode()
    short-circuits the cloud-provider dispatch path entirely.

Verified the picker output matches pre-migration order/content:
  Local Browser, Camofox, Browser Use, Browserbase, Firecrawl
(with 'Nous Subscription' surfaced only when the user is Nous-authed,
unchanged from main).
teknium1 pushed a commit that referenced this pull request May 17, 2026
Mirrors tests/plugins/web/test_web_search_provider_plugins.py from PR #25182.
31 tests across 5 classes:

  TestBundledPluginsRegister (8 tests)
    - Three plugins register (browserbase, browser-use, firecrawl)
    - Each plugin's name + display_name accessible
    - get_setup_schema() returns picker-shaped dict with post_setup hook
    - All three lifecycle methods (create_session, close_session,
      emergency_cleanup) overridden on every plugin

  TestIsAvailable (4 tests)
    - browserbase needs BOTH BROWSERBASE_API_KEY and BROWSERBASE_PROJECT_ID
    - browserbase: api_key alone or project_id alone insufficient
    - browser-use satisfied by BROWSER_USE_API_KEY
    - firecrawl satisfied by FIRECRAWL_API_KEY

  TestRegistryResolution (8 tests) — most valuable, locks down
                                     pre-migration semantics:
    - _resolve(None) with no creds returns None (local mode)
    - _resolve('local') short-circuits to None
    - _resolve('browserbase') returns provider even when unavailable
      (so dispatcher surfaces typed credentials error)
    - _resolve('firecrawl') same: explicit-config wins
    - _resolve('unknown') falls through to auto-detect
    - Legacy walk picks browser-use over browserbase
    - browserbase-only configuration: browserbase wins
    - **Regression**: firecrawl is NEVER auto-selected even when
      single-eligible (preserves pre-migration gate; FIRECRAWL_API_KEY
      shared with web firecrawl must not silently route to paid cloud
      browser)

  TestLegacyAbcAliases (6 tests)
    - is_configured() delegates to is_available() for all three plugins
    - provider_name() returns display_name for all three plugins

  TestPickerIntegration (3 tests)
    - _plugin_browser_providers() exposes all three plugins as rows
    - Each row carries post_setup='agent_browser'
    - browser_plugin_name marker matches browser_provider

All tests use real imports — no mocking of provider classes — so the
suite catches drift in the ABC, registry, picker injection, and plugin
glue layer simultaneously.

31/31 passing.
jcmcneal added a commit to jcmcneal/hermes-agent that referenced this pull request May 20, 2026
After upstream PR NousResearch#25182 migrated all web providers to a plugin-registry
architecture (agent/web_search_provider + agent/web_search_registry),
Ollama Cloud needs to follow the same pattern rather than adding inline
dispatch code to the monolithic tools/web_tools.py.

New files:
- plugins/web/ollama/plugin.yaml  — plugin manifest (kind: backend)
- plugins/web/ollama/__init__.py   — register() entry point
- plugins/web/ollama/provider.py   — OllamaWebSearchProvider (search + extract)

The provider subclasses WebSearchProvider and implements:
- search() via POST /api/web_search
- extract() via POST /api/web_fetch (one URL at a time)
- is_available() checks OLLAMA_API_KEY
- get_setup_schema() for the hermes tools picker

Also wires ollama into:
- _LEGACY_PREFERENCE in agent/web_search_registry.py
- _get_backend(), _is_backend_available(), _web_requires_env(),
  check_web_api_key(), and __main__ in tools/web_tools.py
AlexFoxD pushed a commit to AlexFoxD/hermes-agent that referenced this pull request May 21, 2026
…registries

Both web_search_registry._resolve() and image_gen_registry.get_active_provider()
walked their registered providers and returned the first one matching the
capability flag — without checking whether that provider was actually
usable. On a fresh install with no credentials at all, this meant
get_active_search_provider() returned `brave-free` (legacy preference
order) even though BRAVE_SEARCH_API_KEY was unset, leading the
dispatcher to surface a "BRAVE_SEARCH_API_KEY is not set" error for a
provider the user never chose. Same bug shape in image_gen for FAL.

Resolution semantics now match tools.web_tools._get_backend():

  1. Explicit config name wins, ignoring is_available() — the dispatcher
     surfaces a precise "X_API_KEY is not set" error rather than silently
     switching backends. Matches user expectation: "I configured X, tell
     me what's wrong with X."
  2. Fallback (no explicit config) walks the legacy preference order
     filtered by is_available() — pick the highest-priority backend the
     user actually has credentials for.

is_available() is wrapped in a try/except so a buggy provider doesn't
brick resolution.

E2E verified:
  - No creds + no config: get_active_search_provider() -> None
  - Explicit brave-free + no key: get_active_search_provider() -> brave-free
    (and .is_available() correctly reports False)

This fix was identified during the spike (NousResearch#25182 finding NousResearch#1) and is
fold-in to the same PR rather than a follow-up.
AlexFoxD pushed a commit to AlexFoxD/hermes-agent that referenced this pull request May 21, 2026
… ABC tests

Removes the legacy in-tree provider scaffolding that PR NousResearch#25182 fully
replaced with the plugin architecture:

  tools/web_providers/__init__.py        (6 lines)
  tools/web_providers/base.py            (89 lines — old ABCs)
  tools/web_providers/ARCHITECTURE.md    (73 lines — old design doc)

These were the staging-ground ABCs and provider modules that the
plugin migration absorbed. All seven web providers now implement the
single :class:`agent.web_search_provider.WebSearchProvider` ABC and
live under ``plugins/web/<vendor>/``. Nothing else in the tree imports
``tools.web_providers`` — verified via grep before deletion.

Test migration (tests/tools/test_web_providers.py)
--------------------------------------------------
Rewrote ``TestWebProviderABCs`` to test the new unified ABC at
:mod:`agent.web_search_provider`:

  - test_cannot_instantiate_abc_directly — abstract ``name`` + ``is_available``
  - test_concrete_search_only_provider_works — exercise default
    ``supports_extract=False`` / ``supports_crawl=False`` flags
  - test_concrete_multi_capability_provider_works — exercise all three
    capabilities, async extract supported (declared sync here for
    simplicity; real plugins like parallel + firecrawl use async)
  - test_search_only_provider_skips_extract_and_crawl — verify
    ``supports_*()`` flags default to False so search-only providers
    don't have to implement extract() or crawl()

The 9 other tests in the file (per-capability backend selection,
DEFAULT_CONFIG merge, dispatcher routing) test public helpers in
``tools.web_tools`` that still exist and pass unchanged.

agent/web_search_provider.py docstring updated to reflect that the
legacy ABCs no longer exist; the response-shape contract is preserved
bit-for-bit so external consumers see no behavioral change.

Net diff
--------
- tools/web_providers/ removed (-168 lines)
- tests/tools/test_web_providers.py rewritten ABC section (+78/-30 net,
  same coverage, new API)
- agent/web_search_provider.py docstring (-3/+5 lines)

Verified
--------
- 173/173 targeted web tests pass
- 12/12 ABC contract tests pass with the new interface
- No remaining grep hits for ``tools.web_providers`` outside of
  intentional historical references in plugin docstrings.
AlexFoxD pushed a commit to AlexFoxD/hermes-agent that referenced this pull request May 21, 2026
Adds 44 focused tests under tests/plugins/web/ covering the surface that
the PR NousResearch#25182 web-provider migration introduced. Complements the
existing tests/tools/ coverage which is dispatcher-centric; this file is
plugin-centric and tests each plugin + the registry directly.

Test classes (44 tests, ~1.1s on 4 workers)
-------------------------------------------

TestBundledPluginsRegister (16 tests)
  - All seven plugins present in the registry after
    _ensure_plugins_discovered()
  - Per-plugin parametrized capability-flag assertions
    (brave-free / ddgs / searxng: search-only;
     exa / parallel / firecrawl: search + extract;
     tavily: search + extract + crawl)
  - Every plugin exposes name + display_name properties
  - Every plugin returns a picker-compatible get_setup_schema() dict

TestIsAvailable (7 tests)
  - Each premium plugin reports is_available()==False when its env var is
    absent and True once set (brave-free / searxng / tavily / exa /
    parallel)
  - firecrawl recognizes either FIRECRAWL_API_KEY or FIRECRAWL_API_URL
    as a "configured" signal
  - ddgs is the always-on fallback and must not raise from is_available()

TestRegistryResolution (4 tests)
  - Option B semantics validated end-to-end:
    1. Explicit configured provider wins even when is_available()==False
       (dispatcher surfaces typed credential errors, no silent switch)
    2. Unknown/typo name falls back to first available legacy-preference
       provider
    3. Asking for extract via a search-only backend falls back to an
       extract-capable available provider (capability-incompatible
       branch in _resolve())
    4. No config + no credentials → None (or ddgs if installed)

TestAsyncExtractDispatch (4 tests)
  - parallel + firecrawl extract() are coroutine functions (async path
    in dispatcher uses await)
  - exa + tavily extract() are sync (dispatcher wraps in
    asyncio.to_thread)

TestErrorResponseShapes (7 tests)
  - Plugins return typed error dicts (success=False + "error" key) when
    credentials are missing, never raise
  - async extract() returns list of per-URL error dicts
  - tavily crawl() returns {"results": [{"error": ...}]} on missing
    credentials

Design notes
------------
- All tests use real imports of plugin modules — no mocking of provider
  classes themselves — so they catch drift in the ABC, registry, and
  glue layer simultaneously. Per the hermes-agent-dev skill's E2E
  testing guidance.
- The autouse _isolate_env fixture clears every web-provider env var
  before each test so is_available() reflects the test's setup.
- Resolution tests use the lower-level _resolve() directly rather than
  rebuilding the HERMES_HOME config dance — same observable behavior,
  no sys.modules.pop side-effects that would break the ABC isinstance
  check inside ctx.register_web_search_provider().
teknium1 pushed a commit that referenced this pull request May 22, 2026
Mirrors the architecture established by the web (#25182), browser
(#25214), and video_gen (#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from #15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after #25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes #26241
Gpapas pushed a commit to Gpapas/hermes-agent that referenced this pull request May 23, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
Mucky010 pushed a commit to Mucky010/hermes-agent that referenced this pull request May 24, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
exosyphon pushed a commit to exosyphon/hermes-agent that referenced this pull request May 24, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
mathias3 pushed a commit to mathias3/hermes-agent that referenced this pull request May 28, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
Bryce-huang pushed a commit to wbkunlun/hermes-agent that referenced this pull request May 29, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241

#AI commit#
mosaiq-systems pushed a commit to mosaiq-systems/hermes-agent that referenced this pull request May 29, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…registries

Both web_search_registry._resolve() and image_gen_registry.get_active_provider()
walked their registered providers and returned the first one matching the
capability flag — without checking whether that provider was actually
usable. On a fresh install with no credentials at all, this meant
get_active_search_provider() returned `brave-free` (legacy preference
order) even though BRAVE_SEARCH_API_KEY was unset, leading the
dispatcher to surface a "BRAVE_SEARCH_API_KEY is not set" error for a
provider the user never chose. Same bug shape in image_gen for FAL.

Resolution semantics now match tools.web_tools._get_backend():

  1. Explicit config name wins, ignoring is_available() — the dispatcher
     surfaces a precise "X_API_KEY is not set" error rather than silently
     switching backends. Matches user expectation: "I configured X, tell
     me what's wrong with X."
  2. Fallback (no explicit config) walks the legacy preference order
     filtered by is_available() — pick the highest-priority backend the
     user actually has credentials for.

is_available() is wrapped in a try/except so a buggy provider doesn't
brick resolution.

E2E verified:
  - No creds + no config: get_active_search_provider() -> None
  - Explicit brave-free + no key: get_active_search_provider() -> brave-free
    (and .is_available() correctly reports False)

This fix was identified during the spike (NousResearch#25182 finding NousResearch#1) and is
fold-in to the same PR rather than a follow-up.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
… ABC tests

Removes the legacy in-tree provider scaffolding that PR NousResearch#25182 fully
replaced with the plugin architecture:

  tools/web_providers/__init__.py        (6 lines)
  tools/web_providers/base.py            (89 lines — old ABCs)
  tools/web_providers/ARCHITECTURE.md    (73 lines — old design doc)

These were the staging-ground ABCs and provider modules that the
plugin migration absorbed. All seven web providers now implement the
single :class:`agent.web_search_provider.WebSearchProvider` ABC and
live under ``plugins/web/<vendor>/``. Nothing else in the tree imports
``tools.web_providers`` — verified via grep before deletion.

Test migration (tests/tools/test_web_providers.py)
--------------------------------------------------
Rewrote ``TestWebProviderABCs`` to test the new unified ABC at
:mod:`agent.web_search_provider`:

  - test_cannot_instantiate_abc_directly — abstract ``name`` + ``is_available``
  - test_concrete_search_only_provider_works — exercise default
    ``supports_extract=False`` / ``supports_crawl=False`` flags
  - test_concrete_multi_capability_provider_works — exercise all three
    capabilities, async extract supported (declared sync here for
    simplicity; real plugins like parallel + firecrawl use async)
  - test_search_only_provider_skips_extract_and_crawl — verify
    ``supports_*()`` flags default to False so search-only providers
    don't have to implement extract() or crawl()

The 9 other tests in the file (per-capability backend selection,
DEFAULT_CONFIG merge, dispatcher routing) test public helpers in
``tools.web_tools`` that still exist and pass unchanged.

agent/web_search_provider.py docstring updated to reflect that the
legacy ABCs no longer exist; the response-shape contract is preserved
bit-for-bit so external consumers see no behavioral change.

Net diff
--------
- tools/web_providers/ removed (-168 lines)
- tests/tools/test_web_providers.py rewritten ABC section (+78/-30 net,
  same coverage, new API)
- agent/web_search_provider.py docstring (-3/+5 lines)

Verified
--------
- 173/173 targeted web tests pass
- 12/12 ABC contract tests pass with the new interface
- No remaining grep hits for ``tools.web_providers`` outside of
  intentional historical references in plugin docstrings.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
Adds 44 focused tests under tests/plugins/web/ covering the surface that
the PR NousResearch#25182 web-provider migration introduced. Complements the
existing tests/tools/ coverage which is dispatcher-centric; this file is
plugin-centric and tests each plugin + the registry directly.

Test classes (44 tests, ~1.1s on 4 workers)
-------------------------------------------

TestBundledPluginsRegister (16 tests)
  - All seven plugins present in the registry after
    _ensure_plugins_discovered()
  - Per-plugin parametrized capability-flag assertions
    (brave-free / ddgs / searxng: search-only;
     exa / parallel / firecrawl: search + extract;
     tavily: search + extract + crawl)
  - Every plugin exposes name + display_name properties
  - Every plugin returns a picker-compatible get_setup_schema() dict

TestIsAvailable (7 tests)
  - Each premium plugin reports is_available()==False when its env var is
    absent and True once set (brave-free / searxng / tavily / exa /
    parallel)
  - firecrawl recognizes either FIRECRAWL_API_KEY or FIRECRAWL_API_URL
    as a "configured" signal
  - ddgs is the always-on fallback and must not raise from is_available()

TestRegistryResolution (4 tests)
  - Option B semantics validated end-to-end:
    1. Explicit configured provider wins even when is_available()==False
       (dispatcher surfaces typed credential errors, no silent switch)
    2. Unknown/typo name falls back to first available legacy-preference
       provider
    3. Asking for extract via a search-only backend falls back to an
       extract-capable available provider (capability-incompatible
       branch in _resolve())
    4. No config + no credentials → None (or ddgs if installed)

TestAsyncExtractDispatch (4 tests)
  - parallel + firecrawl extract() are coroutine functions (async path
    in dispatcher uses await)
  - exa + tavily extract() are sync (dispatcher wraps in
    asyncio.to_thread)

TestErrorResponseShapes (7 tests)
  - Plugins return typed error dicts (success=False + "error" key) when
    credentials are missing, never raise
  - async extract() returns list of per-URL error dicts
  - tavily crawl() returns {"results": [{"error": ...}]} on missing
    credentials

Design notes
------------
- All tests use real imports of plugin modules — no mocking of provider
  classes themselves — so they catch drift in the ABC, registry, and
  glue layer simultaneously. Per the hermes-agent-dev skill's E2E
  testing guidance.
- The autouse _isolate_env fixture clears every web-provider env var
  before each test so is_available() reflects the test's setup.
- Resolution tests use the lower-level _resolve() directly rather than
  rebuilding the HERMES_HOME config dance — same observable behavior,
  no sys.modules.pop side-effects that would break the ABC isinstance
  check inside ctx.register_web_search_provider().
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…template

Foundation commit for the browser-provider plugin migration (NousResearch#25214).
Mirrors the architecture established by PR NousResearch#25182 (web providers):

- agent/browser_provider.py — BrowserProvider ABC. Preserves the legacy
  CloudBrowserProvider lifecycle contract bit-for-bit (create_session,
  close_session, emergency_cleanup, session metadata shape) so the
  dispatcher in tools/browser_tool.py becomes a pure registry lookup.
  Renames is_configured() → is_available() for parity with WebSearchProvider.

- agent/browser_registry.py — selection registry with the same
  three-rule resolution as web_search_registry:
    1. Explicit config wins (returns even if is_available() == False so
       the dispatcher surfaces a precise credentials error)
    2. Single-eligible shortcut
    3. Legacy preference walk: browser-use → browserbase, filtered by
       availability. Firecrawl is intentionally NOT in the legacy walk
       (matches pre-migration behaviour — Firecrawl was only reachable
       via explicit browser.cloud_provider: firecrawl).

- hermes_cli/plugins.py — adds ctx.register_browser_provider() facade,
  one-liner mirror of register_web_search_provider().

No plugins registered yet; no dispatcher cutover yet. The next commits
move browserbase/browser-use/firecrawl into plugins/browser/<vendor>/
and switch tools/browser_tool.py over to the registry.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…picker

Drops the three hardcoded browser-provider rows (Browserbase, Browser Use,
Firecrawl) from TOOL_CATEGORIES['browser']['providers'] and replaces them
with runtime injection from agent.browser_registry — mirroring the
_plugin_web_search_providers() pattern PR NousResearch#25182 established for the
Web Search and Extract category.

Adds _plugin_browser_providers() helper in hermes_cli/tools_config.py
that walks list_providers() and builds a TOOL_CATEGORIES-shape dict per
provider via get_setup_schema(). The new visible_providers() hook calls
it for cat['name'] == 'Browser Automation'.

The three remaining hardcoded rows are non-provider UX setup-flow rows:
  - 'Nous Subscription (Browser Use cloud)' — managed Browser Use billed
    via Nous subscription; uses the browser-use plugin as the underlying
    backend but has distinct setup UX (requires_nous_auth gates it).
  - 'Local Browser' — headless Chromium, no CloudBrowserProvider.
  - 'Camofox' — anti-detection local Firefox; _is_camofox_mode()
    short-circuits the cloud-provider dispatch path entirely.

Verified the picker output matches pre-migration order/content:
  Local Browser, Camofox, Browser Use, Browserbase, Firecrawl
(with 'Nous Subscription' surfaced only when the user is Nous-authed,
unchanged from main).
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
Mirrors tests/plugins/web/test_web_search_provider_plugins.py from PR NousResearch#25182.
31 tests across 5 classes:

  TestBundledPluginsRegister (8 tests)
    - Three plugins register (browserbase, browser-use, firecrawl)
    - Each plugin's name + display_name accessible
    - get_setup_schema() returns picker-shaped dict with post_setup hook
    - All three lifecycle methods (create_session, close_session,
      emergency_cleanup) overridden on every plugin

  TestIsAvailable (4 tests)
    - browserbase needs BOTH BROWSERBASE_API_KEY and BROWSERBASE_PROJECT_ID
    - browserbase: api_key alone or project_id alone insufficient
    - browser-use satisfied by BROWSER_USE_API_KEY
    - firecrawl satisfied by FIRECRAWL_API_KEY

  TestRegistryResolution (8 tests) — most valuable, locks down
                                     pre-migration semantics:
    - _resolve(None) with no creds returns None (local mode)
    - _resolve('local') short-circuits to None
    - _resolve('browserbase') returns provider even when unavailable
      (so dispatcher surfaces typed credentials error)
    - _resolve('firecrawl') same: explicit-config wins
    - _resolve('unknown') falls through to auto-detect
    - Legacy walk picks browser-use over browserbase
    - browserbase-only configuration: browserbase wins
    - **Regression**: firecrawl is NEVER auto-selected even when
      single-eligible (preserves pre-migration gate; FIRECRAWL_API_KEY
      shared with web firecrawl must not silently route to paid cloud
      browser)

  TestLegacyAbcAliases (6 tests)
    - is_configured() delegates to is_available() for all three plugins
    - provider_name() returns display_name for all three plugins

  TestPickerIntegration (3 tests)
    - _plugin_browser_providers() exposes all three plugins as rows
    - Each row carries post_setup='agent_browser'
    - browser_plugin_name marker matches browser_provider

All tests use real imports — no mocking of provider classes — so the
suite catches drift in the ABC, registry, picker injection, and plugin
glue layer simultaneously.

31/31 passing.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
Mirrors the architecture established by the web (NousResearch#25182), browser
(NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations:

* `tools/fal_common.py` — stateless atoms shared by both FAL-backed
  plugins (image_gen + video_gen). Holds the lazy `fal_client` import
  helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`,
  `_extract_http_status`. Stateful pieces (`fal_client` module global,
  `_managed_fal_client*` cache, `_submit_fal_request`,
  `_resolve_managed_fal_gateway`, `_get_managed_fal_client`)
  intentionally stay on `tools.image_generation_tool` so the existing
  `monkeypatch.setattr(image_tool, ...)` patch sites keep working
  unchanged.

* `plugins/video_gen/fal/__init__.py` — drops its inline
  `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`.

* `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin.
  `FalImageGenProvider` is a thin registration adapter that resolves
  the legacy module via `import tools.image_generation_tool as _it`
  and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at
  call time. The 18-model catalog, `_build_fal_payload`, managed-
  gateway selection, and Clarity Upscaler chaining all remain in
  `tools.image_generation_tool` as the single source of truth —
  the plugin is a registration adapter, not a parallel implementation.

* `tools/image_generation_tool.py::_dispatch_to_plugin_provider` —
  drops the `configured == "fal"` skip. Setting `image_gen.provider:
  fal` now routes through the registry like any other provider; the
  plugin re-enters this module's pipeline so behavior is identical.
  Unset `image_gen.provider` still falls through to the in-tree
  pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696).

* `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from
  `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by
  `_plugin_image_gen_providers` like every other backend) and the
  `getattr(provider, "name") == "fal"` skip that protected against
  duplication with the hardcoded row. The "Nous Subscription" row
  stays as a setup-flow entry — same shape browser kept "Nous
  Subscription (Browser Use cloud)" after NousResearch#25214.

* `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering
  the ABC surface, call-time indirection (verifying
  `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes
  effect through the plugin), response-shape stamping, exception
  handling, and registry wiring.

* `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess
  harness mirroring `tests/plugins/browser/check_parity_vs_main.py`.
  Pins one path to origin/main, one to the worktree; runs six
  scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds,
  explicit-fal-with-model, typo provider, managed-gateway-only) and
  diffs the reduced shape `{dispatch_kind, provider_name, model}`
  per scenario. The only acceptable diff is "legacy_fal → plugin
  (fal)" for explicit-FAL paths — every other delta is flagged as
  a regression.

* `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins`
  — flips the previous `test_fal_skipped_to_avoid_duplicate` to
  match the new shape (FAL is a plugin now, no dedup needed).

Verified: 195/195 tests across
`tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py`
pass on this branch with no test patches modified outside the picker
test that asserted the old skip behaviour.

Fixes NousResearch#26241
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 P3 Low — cosmetic, nice to have tool/web Web search and extraction type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants