refactor(web): all web providers as image_gen-style plugins#25182
Closed
kshitijk4poor wants to merge 24 commits into
Closed
refactor(web): all web providers as image_gen-style plugins#25182kshitijk4poor wants to merge 24 commits into
kshitijk4poor wants to merge 24 commits into
Conversation
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.
…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().
…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.
3 tasks
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.
3 tasks
7 tasks
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates every web provider (search + extract + crawl) out of
tools/web_tools.pyinto the bundled-plugin architecture, matching theimage_gentemplate. All seven providers now live underplugins/web/<vendor>/, registering throughagent.web_search_registryinstead of hardcodedif backend == ...chains in the dispatcher.This unblocks user-added web providers (drop a directory under
~/.hermes/plugins/web/and they appear inhermes toolsautomatically — same UX as image_gen / spotify / google_meet plugins).Scope — what moved
brave-freeBRAVE_SEARCH_API_KEYddgsddgs)searxngSEARXNG_URLexaEXA_API_KEYparallelPARALLEL_API_KEYtavilyTAVILY_API_KEYfirecrawlFIRECRAWL_API_KEY/FIRECRAWL_API_URL) or Nous Tool GatewayEach plugin is
plugins/web/<vendor>/{plugin.yaml,__init__.py,provider.py}.What changes for users
Nothing. The setup wizard,
hermes toolspicker, 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:
TOOL_CATEGORIES["web"]rows inhermes_cli/tools_config.pytools/web_tools.py(_get_firecrawl_client,_tavily_request,_parallel_search, etc. — ~600 LoC)if backend == "exa": ... elif backend == "tavily": ...chains in all three dispatcherstools/web_providers/base.pyAfter:
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()plugins/web/<vendor>/provider.pyinspect.iscoroutinefunctionasync/sync detectionagent/web_search_provider.pywithsupports_search/extract/crawlcapability flagsRegistry resolution semantics (Option B — conservative smart fallback)
_resolve()inagent/web_search_registry.pyfollows three rules in order:web.<capability>_backend(orweb.backend) names a registered + capable provider, return it — even ifis_available()is False. The dispatcher then surfaces a precise"X_API_KEY is not set"error instead of silently switching backends._LEGACY_PREFERENCE(firecrawl → parallel → tavily → exa → searxng → brave-free → ddgs) and pick the first one that's capable AND available. Matches the historictools.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
parallelandfirecrawldeclareasync def extract();exaandtavilyare sync. The dispatcher detects viainspect.iscoroutinefunction(provider.extract)and eitherawaits the result or runs the sync call inasyncio.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_firecrawl_client,_parallel_client,_async_parallel_client,_exa_client) live ontools.web_toolsso unit tests that reset them between cases keep workingThe plugin implementations use
import tools.web_tools as _wtindirection to read helpers like_wt._read_nous_access_token,_wt.Firecrawl,_wt.prefers_gatewayso test patches that target the legacy paths reach the plugin code transparently.Diff stat
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.pyweb 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.pytests/tools/test_web_tools_config.py(backend selection, client config, error responses)tests/tools/test_web_tools_tavily.pytests/tools/test_website_policy.py(per-URL gate + redirect re-check)tests/tools/test_config_null_guard.pytests/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 validTestIsAvailable(7) — every plugin'sis_available()correctly reflects env-var presenceTestRegistryResolution(4) — explicit-config-wins-when-unavailable, typo-falls-back, capability-incompatible-falls-back, no-config behaviorTestAsyncExtractDispatch(4) —inspect.iscoroutinefunctioncorrectly 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_backendoverride 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:web_search_toolon an unconfigured system. Main returns{"error": "Error searching web: Web tools are not configured..."}(viatool_error(), nosuccesskey). The migrated dispatcher was returning{"success": False, "error": "Web tools are not configured..."}becausefirecrawl.search()was catchingValueErrorfrom_get_firecrawl_client()itself instead of letting it propagate. Fix: pull_get_firecrawl_client()out of the broadtryin the plugin'ssearch(). Pre-flight errors propagate to the dispatcher's top-levelexcepthandler, matching main's legacy envelope. In-flight SDK errors still get wrapped per-result.web_crawl_toolon an unconfigured system. Main short-circuits oncheck_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'scrawl()except ValueErrorwas wrapping the pre-flight error as a per-page failure insideresults[]. Models that checkresult.get("error")would miss the failure. Fix: mirror main's upstream availability gate in the dispatcher — when the resolved crawl provider isis_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::TestUnconfiguredErrorEnvelopeParitylock 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_PREFERENCEorder with the legacytools.web_tools._get_backend()candidate tuple so direct callers ofagent.web_search_registry.get_active_*_provider()(no_get_backend()interpose) don't returnNonefor users who have credentials but no explicitweb.backendconfig key. Verified empirically: TAVILY+EXA → tavily; EXA+PARALLEL → parallel; BRAVE+FIRECRAWL → firecrawl, all matchingmain. 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)
tools/browser_providers/so third-party browser backends drop in identically. Filed as Mirror web-provider plugin migration for browser providers #25214.Commits
24 focused commits on
kshitijk4poor/hermes-agent:spike/web-providers-plugin: