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.
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.
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).
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().
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.
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.
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
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.
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).
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
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
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)
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)
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.
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 (#25182 finding #1) and is
fold-in to the same PR rather than a follow-up.
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 d8735963f), 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)
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.
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 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'].
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/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']
The interactive CLI /model picker was the third call-site duplicating
the inline config-slice + list_authenticated_providers pattern that
PR #23666 consolidated for the dashboard and TUI. Route it through
load_picker_context() + build_models_payload() too so all surfaces
that show authenticated providers share one substrate.
Side effect: cli.py now also benefits from the latent v12+ keyed
providers fix (custom_providers populated via
get_compatible_custom_providers, not cfg.get raw).
The aux-task switcher (hermes_cli/main.py) and gateway model
switcher (gateway/run.py) deliberately stay on the legacy path —
they use different config sections (auxiliary.<task>.*) and a
different config loader (_load_gateway_config) respectively, so
forcing them through ConfigContext would either overload its
semantics or grow the module past the clean refactor scope.
Three call-sites in the codebase each duplicated the same config-slice
+ list_authenticated_providers + post-processing pattern:
- hermes_cli/web_server.py /api/model/options
- tui_gateway/server.py model.options JSON-RPC
- tui_gateway/server.py model.save_key JSON-RPC
This consolidates them onto hermes_cli/inventory.py:
load_picker_context() -> ConfigContext
Replaces the 17-LOC config-slice (model.{default,name,provider,
base_url}, providers:, custom_providers:) every consumer did
inline.
ConfigContext.with_overrides(*, current_provider=, current_model=,
current_base_url=) -> ConfigContext
Truthy-only overlay for TUI agent-session state on top of disk
config. Empty getattr(agent, ...) attrs MUST NOT clobber disk.
build_models_payload(ctx, *, include_unconfigured, picker_hints,
canonical_order, max_models) -> dict
Single payload builder. Delegates curation to
list_authenticated_providers (does not call provider_model_ids
per row \u2014 that pulls non-agentic models). picker_hints +
canonical_order produce the TUI ModelPickerDialog shape;
defaults match the dashboard's existing /api/model/options
contract.
Two latent bugs fixed by consolidation:
1. The dashboard read cfg.get('custom_providers') directly, missing
the v12+ keyed providers: form. Now both surfaces go through
get_compatible_custom_providers().
2. The TUI's canonical-merge keyed on is_user_defined to decide order.
Section 3 of list_authenticated_providers sets is_user_defined=True
on rows from the providers: config dict even when the slug is
canonical \u2014 that silently demoted them to the picker tail.
_reorder_canonical now keys on slug membership instead.
Stats: +666 / -145 (net +521). Module 240 LOC; 18 behavior tests.
This PR replaces the rejected #23369 (which bundled the consolidation
with new scriptable CLI surfaces \u2014 hermes models list/status, hermes
providers list \u2014 and a JSON contract that have no external user
demand). Just the refactor; the CLI surface is deferred to a separate
PR gated on actual demand.
Refs #23359.
Follow-up on the salvaged feat commit:
- Keep the constructor / config / yaml-example default at 3 so existing
gateway and CLI users see no behavioural change. PR #13754 (which this
builds on) had lowered the default to 2 to chase pre-feature parity in
the system-prompt-present case, at the cost of quietly halving the
protected head for the gateway path (which strips the system prompt
before calling compress()). With the new "system prompt is implicit"
semantics, default 3 gives every caller a stable head shape.
- agent/context_engine.py: bring the ABC's protect_first_n docstring in
line with the new semantics so plugin context engines interpret the
config key the same way the built-in compressor does.
- tests: adjust the default-value test (3, not 2) and a stale comment;
per-test protect_first_n=2/3/1 values added in PR #13754 stay as-is
since those tests fix concrete head shapes.
The number of head messages preserved verbatim across context compactions
was previously hardcoded to 3 in AIAgent.__init__. Expose it as
`compression.protect_first_n` in config, matching the existing
`protect_last_n` pattern.
Motivation: users who rely on rolling compaction for long-running sessions
had the opening user/assistant exchange pinned as head forever, which
doesn't always match how they want the session framed after many
compactions. Lowering to 1 preserves the system prompt + first non-system
message; lowering to 0 preserves only the system prompt and lets the
entire first exchange age out naturally through the summary.
Semantics: `protect_first_n` counts non-system head messages protected
**in addition to** the system prompt, which is always implicitly protected
when present. Same meaning across both code paths:
protect_first_n=0 → system prompt only (or nothing if no system message)
protect_first_n=2 → system prompt + first 2 non-system messages (default)
This unifies the CLI path (which reads messages with the system prompt at
position 0) and the gateway path (where the gateway /compress handler
strips the system prompt before calling compress() — see
gateway/run.py L9150-9154 on the parent fork). Previously these two paths
disagreed:
CLI path: protect_first_n=1 → protect system prompt only
Gateway path: protect_first_n=1 → protect first USER turn forever
In practice on long-running gateway sessions the old semantics pinned
whatever stale aside happened to be the first user message, reinserting
it into every compaction summary indefinitely.
Default chosen as 2 (not 3) so that the effective protected head count
remains 3 messages in the common case — assuming a system prompt is
present, default protection becomes system + 2 non-system = 3 total,
matching the pre-feature behaviour where `protect_first_n` was hardcoded
to protect 3 messages total. Sessions without a system prompt will see a
small behaviour change (2 protected head messages instead of 3), but this
is the rare path and the new semantics make the system-prompt-present
case the well-defined one.
Changes:
- agent/context_compressor.py: redefine protect_first_n as the count of
non-system head messages protected beyond the implicit system-prompt
guarantee; both paths converge. Constructor default updated to 2.
- hermes_cli/config.py: add `compression.protect_first_n` default (2),
matching the new semantics. `show_config` label tweaked to
'Protect first: N non-system head messages' for clarity.
- run_agent.py: read protect_first_n from config; 0 is now valid (system
prompt is always implicitly protected).
- cli-config.yaml.example: document the new key and rationale.
- tests/agent/test_context_compressor.py: cover default, override, the
end-to-end `protect_first_n=0` and `protect_first_n=1` behaviour,
the no-system-prompt (gateway) path, and the new shared-semantics
regression test.
Fixes#13751
Tested on Ubuntu 24.04.
By default, once Hermes participates in a Discord thread (auto-created on
@mention or replied in once) it auto-responds to every subsequent message
in that thread without requiring further @mentions. That's the right default
for one-on-one conversations and isolated channel threads.
But it's a confirmed footgun in multi-bot threads. When a user invokes one
bot per turn — addressing Codex first, then Hermes — every other bot in the
thread also fires on every message, burning credits and spamming the channel.
Author has hit this personally in active multi-bot research-team threads.
Add a new `discord.thread_require_mention` config key (env:
`DISCORD_THREAD_REQUIRE_MENTION`), default `false` to preserve existing
behavior. When `true`, the in-thread mention shortcut is disabled and
threads are gated the same way channels are. Explicit @mentions still pass
through as expected.
Mirrors the existing helper shape (config.extra > env > default) and the
existing yaml→env bridge pattern used by `require_mention`.
Changes:
- gateway/platforms/discord.py: new `_discord_thread_require_mention()`
helper; in_bot_thread shortcut now AND's with `not _discord_thread_require_mention()`
- gateway/config.py: bridge `discord.thread_require_mention` from config.yaml
to `DISCORD_THREAD_REQUIRE_MENTION` env var (mirrors the existing
`require_mention` bridge two lines above)
- hermes_cli/config.py: add `thread_require_mention: False` default to
DEFAULT_CONFIG['discord']
- tests/gateway/test_discord_free_response.py: 4 new tests covering default
behaviour (in-thread shortcut still works), enabled behaviour (mention
required in threads), enabled+mentioned (mention still passes through),
and yaml-via-config.extra path. Also clears DISCORD_* env vars in the
`adapter` fixture so process-env state from the contributor's shell
doesn't leak into per-test behaviour.
- tests/gateway/test_config.py: 2 new tests covering the yaml→env bridge
(both the apply-from-yaml and env-precedence-over-yaml paths)
- website/docs/user-guide/messaging/discord.md: document the new env var
+ config key with multi-bot rationale; cross-link from `auto_thread`
section
Tested on Ubuntu 24.04.
Free-response channels are intended as lightweight chat surfaces — the bot
responds to every message without requiring an @mention. But the auto-thread
gate only checked DISCORD_NO_THREAD_CHANNELS, not DISCORD_FREE_RESPONSE_CHANNELS,
so every message in a free-response channel still spawned a brand-new thread.
That turns a chat channel into a thread-spawning machine: 1 thread per message.
The user-facing docs at website/docs/user-guide/messaging/discord.md already
describe the intended behavior ("Free-response channels also skip auto-threading
— the bot replies inline rather than spinning off a new thread per message"),
so this is a code-vs-docs gap, not a design change.
Fix: OR is_free_channel into skip_thread alongside the existing no_thread_channels
check. One-line production change.
Regression test added at tests/gateway/test_discord_free_response.py:
test_discord_free_response_channel_skips_auto_thread asserts that a message
in a free-response channel never calls _auto_create_thread. Reverting the
one-line fix causes the test to fail with 'Expected mock to not have been
awaited. Awaited 1 times.' — i.e. the test demonstrates the bug concretely.
Lets platform plugins own their YAML→env config bridge instead of forcing
core gateway/config.py to know every platform's schema.
The hook receives the full parsed config.yaml and the platform's own
sub-dict, may mutate os.environ (env > YAML precedence preserved via the
standard `not os.getenv(...)` guards), and may return a dict to merge
into PlatformConfig.extra. It runs during load_gateway_config() after
the existing generic shared-key loop and before _apply_env_overrides(),
mirroring the env_enablement_fn dispatch pattern (#21306, #21331).
Pure addition — no behavior change for existing platforms. Each of the
eight platforms with hardcoded YAML→env blocks today (discord, telegram,
whatsapp, slack, dingtalk, mattermost, matrix, feishu, ~252 LOC in
gateway/config.py) can migrate in independent follow-up PRs; the
hardcoded blocks remain functional in the meantime, and their
`not os.getenv(...)` guards make them no-ops for any env var the hook
already set.
Test coverage: 10 new tests in tests/gateway/test_platform_registry.py
covering field default, callable acceptance, env mutation, extras
merge, both signature args, exception swallowing, missing/non-dict
sections, and env > YAML precedence.
Refs #3823, #24356.
Closes#24836.
Followup to PR #24182 — caught when scanning OpenClaw for recent codex
fixes we hadn't considered. OpenClaw learned the hard way (#80815) that
migrating plugins which codex itself reports as unavailable produces
config that fails at activation time.
Our /codex-runtime codex_app_server enable path queries codex's
plugin/list and migrates everything where installed=true. We were
trusting codex's installation state and ignoring its availability
field. So a plugin that's installed=true but availability=UNAVAILABLE
(broken local install) or REQUIRES_AUTH (OAuth expired or never
completed) would get an [plugins."<n>@openai-curated"] entry in
~/.codex/config.toml — and the user's first codex turn after enabling
the runtime would fail because codex refuses to activate it.
Fix: filter on availability in _query_codex_plugins(). Only emit
plugins where availability is empty (older codex versions without the
field — preserve backward compat) or explicitly AVAILABLE.
Tests:
test_plugin_discovery_skips_unavailable_plugins — verifies 4 cases:
- good-plugin (installed=True, availability=AVAILABLE) → migrated
- broken-plugin (installed=True, availability=UNAVAILABLE) → skipped
- auth-pending (installed=True, availability=REQUIRES_AUTH) → skipped
- legacy-plugin (installed=True, no availability field) → migrated
(older codex versions; preserve backward compat)
Docs:
Added bullet to 'What's NOT migrated' list in the docs page calling
out the availability filter and why.
Other OpenClaw codex PRs I reviewed but did NOT apply (with reasoning):
- #81591 (load Codex for selectable models): we resolve runtime
per-call already, no startup-time gating to fix
- #81510 (cron compatibility): we documented cron as untested; their
fix is for OpenClaw-specific cron orchestration shape
- #81223 (rotate incompatible context-engine threads): we don't
have a Lossless context engine equivalent
- #80688 (constrain sandbox): we don't have an outer-sandbox concept
- #80616 (release on turn_aborted): we already handle status=
interrupted in turn/completed correctly
- #80278 (expose activeModel in plugin SDK): not our surface
- #80792 (default destructive_actions on): we don't expose that knob
56 codex-runtime migration tests still green (+1 new).
The Analytics page and the token/cost surfaces on the Models page show
local debug estimates only. They count input+output (and a bar viz adds
cache_read+reasoning, missing cache_write entirely) from successful
main-agent responses that returned a usable usage block.
Excluded silently:
- All auxiliary calls — context compression, title generation, vision,
session search, web extract, smart approvals, MCP routing, plugin LLM
access (13 production call sites bypass update_token_counts)
- Provider-side retries, fallback attempts
- Any call whose usage block didn't come back
- cache_write_tokens (column exists in sessions table but not returned
by /api/analytics/models)
Real-world impact: a user on Kimi K2.6 saw 150K local vs 27M on the
OpenRouter side over the same window. Precise-looking numbers next to
provider billing create false confidence and support load.
This change adds dashboard.show_token_analytics (default False) to gate:
- The Analytics nav item (hidden from sidebar when off)
- The Analytics page (renders an explanation card instead of charts)
- Token bars, totals, cost figures, avg/api_calls on the Models page
The Models page keeps capability metadata (context window, vision,
tools, reasoning), the use-as-main/aux menu, sessions count, and
last-used timestamps when the flag is off.
Set dashboard.show_token_analytics: true in config.yaml to opt back in
to the local debug estimate. Fixing the underlying accounting (issue
#23270) is a separate, larger workstream.
Refs: #23270, #21705
Both addresses route to the same GitHub account (@simpolism / snav). Adding
the mappings here keeps release notes from showing two separate contributors
for what is one person's work, and unblocks subsequent PRs from this account
that would otherwise each need their own scripts/release.py noise.
- test_background_review_does_not_narrow_toolset_schema: review fork must
NOT pass enabled_toolsets to AIAgent (full parent schema = matching
Anthropic cache key on the 'tools' field).
- test_background_review_installs_thread_local_whitelist: the runtime
whitelist that replaces schema-level narrowing must contain memory +
skills tools and exclude terminal / send_message / delegate_task /
web_search / execute_code.
- test_review_fork_inherits_parent_cached_system_prompt: new test for
PR #17276's first root cause — the fork's _cached_system_prompt must
equal the parent's byte-for-byte.
- test_review_fork_pins_session_start_and_session_id: defensive belt-and-
suspenders for the cached-prompt inheritance.
Inverted the original test_background_review_agent_uses_restricted_toolsets
(which asserted the schema-level narrowing) — that narrowing was the
direct cause of #25322's cache miss, and the runtime whitelist replaces
its safety claim without breaking cache parity.
Refs #25322, #15204, PR #17276.
Belt-and-suspenders complement to the cached-system-prompt inheritance:
pin session_start and session_id to the parent's so any code path that
re-renders parts of the system prompt (compression, plugin hooks)
still produces byte-identical output. The cached-prompt assignment
already short-circuits the normal rebuild path, but these pins
guarantee parity even if a future code path bypasses the cache.
Idea from simpolism's reference PR #25427 for #25322.
Co-Authored-By: simpolism <32201324+simpolism@users.noreply.github.com>
Background review fork is supposed to hit Anthropic's prefix cache on the
parent's messages_snapshot, but currently doesn't (cache_read=0 on every
fork). Two root causes, fixed in this commit:
1. System prompt is rebuilt at fork time. _cached_system_prompt starts as
None, so run_conversation calls _build_system_prompt, which embeds a
minute-precision "Conversation started: ..." timestamp. Reviews fire
10+ turns after session start, so the minute differs from main's,
producing a 1-character diff that invalidates the byte-exact cache key.
Fix: inherit the parent's _cached_system_prompt directly (same idea as
#17089, which was self-closed for only fixing this half).
2. Tools schema was narrowed via enabled_toolsets=["memory","skills"] for
safety. Anthropic's cache key includes `tools`, which sits before
`system` in the cache hierarchy, so even byte-identical `system` won't
hit when `tools` differs from main's full set.
Fix: drop the schema-level restriction so `tools` matches main, and
deny non-whitelisted tools at runtime via the existing
get_pre_tool_call_block_message gate (hermes_cli/plugins.py:1085,
already called at all three dispatch sites). Install/clear a thread-
local whitelist (added in the previous commit) on the daemon thread.
Append a soft constraint to the review prompt so the model knows.
Real E2E on Sonnet 4.5 (12-tool task + auto-triggered review):
- Per review-call cost: $0.331 → $0.035 (~89% reduction)
- End-to-end per run: $0.848 → $0.629 (~26% reduction)
- Review fork cache_create / cache_read: 88,385 / 0 → 1,234 / 94,404
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds set_thread_tool_whitelist / clear_thread_tool_whitelist to
hermes_cli/plugins.py. When set on the current thread, restricts which
tools can pass through get_pre_tool_call_block_message; non-whitelisted
tools are blocked with a configurable deny message.
Mirrors the per-thread approval-callback pattern already used by
set_approval_callback (tools/terminal_tool.py:190). Used by
_spawn_background_review to deny non-memory/non-skill tools at runtime
while inheriting the parent agent's full tools schema for prefix-cache
parity (see follow-up commit).
Tests cover allow / deny / clear / cross-thread isolation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes#25028.
The lazy-install hooks added in #25014 installed packages correctly but
failed to rebind module-level globals after install:
- Slack: missing aiohttp rebind → NameError on file uploads
- Feishu: none of the ~25 lark_oapi symbols rebound → TypeError on
adapter instantiation
- Matrix: mautrix.types enums stayed as stubs → mismatched values at
runtime
Introduces tools.lazy_deps.ensure_and_bind() — a DRY helper that
combines ensure() + importer-callable + globals().update(). This
eliminates the error-prone pattern of manually listing every global
that needs updating after lazy-install. Each platform adapter now
defines a single _import() function returning all bindings.
Also fixes: pyproject.toml [slack] extra was missing aiohttp (needed
by slack-bolt's async path).
Follow-up on @pty819's t2a_v2 endpoint fix:
- Default model: speech-02 -> speech-02-hd (bare 'speech-02' is not in the
supported enum; t2a_v2 rejects it with 400). Official enum: speech-01-hd,
speech-01-turbo, speech-02-hd, speech-02-turbo, speech-2.6-hd/turbo,
speech-2.8-hd/turbo.
- Default voice: female-shaonv -> English_expressive_narrator. The
legacy speech-01-series short ID doesn't resolve cleanly on the
speech-02+ models that are now the default.
- Default base URL: api.minimaxi.com -> api.minimax.io (matches the
canonical host in the published docs; api-uw.minimax.io is the
reduced-latency alt).
- Add GroupId support via tts.minimax.group_id config or MINIMAX_GROUP_ID
env var. Some MiniMax accounts scope TTS requests by group; without it,
requests 401. Only appended when not already in the user's base_url.
Tests rewritten to cover both the default t2a_v2 path (hex-encoded audio
in JSON, nested voice_setting/audio_setting) and the legacy
text_to_speech path (raw audio bytes, flat payload). Adds coverage for
GroupId config/env wiring and error surfacing.
Also adds AUTHOR_MAP entry for pty819's GitHub-noreply email.
The MiniMax TTS defaults were outdated:
- DEFAULT_MINIMAX_MODEL was 'speech-01' but MiniMax now uses 'speech-02'
- DEFAULT_MINIMAX_BASE_URL was 'https://api.minimax.chat/v1/text_to_speech'
which no longer works; the correct endpoint is
'https://api.minimaxi.com/v1/t2a_v2'
Users who configured tts.provider: minimax were getting model-not-supported
errors because the hardcoded defaults did not match available API permissions.
Slack platform-blocks native slash commands inside thread replies ("/queue
is not supported in threads. Sorry!") and there is no app-side setting to
re-enable them. As a workaround, rewrite a leading '!' to '/' for any known
gateway command before downstream processing — so '!queue', '!stop',
'!model gpt-5.4' etc. work inside Slack threads (and anywhere else).
Only the first token is checked against is_gateway_known_command(), so
casual messages like '!nice work' pass through to the agent unchanged.
Downstream pipeline (MessageType.COMMAND tagging, gateway dispatcher,
thread reply routing) is unchanged.
Adds 6 tests covering rewrite, args preservation, thread routing,
casual-message passthrough, '@bot' suffix, and plain '/' still-works.
`hermes tools` -> "All Platforms" took ~14s to render the checklist
because building the toolset labels called `get_nous_auth_status()` ~31x
transitively (`_toolset_has_keys` -> `_visible_providers` ->
`get_nous_subscription_features` -> `managed_nous_tools_enabled`).
Each call did a synchronous OAuth refresh POST to
portal.nousresearch.com (~350ms even on the failure path), so one menu
paint burned >13s of HTTP and 31 single-use Nous refresh tokens.
Secondary hot spot: every `get_env_value()` re-read and re-sanitised
the entire .env file. 116 reads with O(lines x known-keys) scanning
added ~300ms of CPU per render.
Fix is two process-level caches, both mtime-keyed so login/logout/edit
invalidate naturally:
* `hermes_cli/auth.py`: memoise `get_nous_auth_status()` for 15s keyed
on auth.json mtime. Splits `_compute_nous_auth_status()` as the
uncached impl. Adds `invalidate_nous_auth_status_cache()`.
* `hermes_cli/config.py`: memoise `load_env()` keyed on .env
(path, mtime, size). Adds `invalidate_env_cache()`, wired into
`save_env_value`, `remove_env_value`, and the sanitize-on-load
writer so writers don't return stale dicts on same-second writes.
Before/after on Teknium's box (real HERMES_HOME, no Nous login):
* "All Platforms" cold path: ~13,874ms -> ~691ms label-build
* Warm re-open within the same process: ~122ms -> ~17ms
Side benefit: stops burning a Nous refresh token on every menu paint,
which was risking the portal's reuse-detection revocation logic.
`_reconfigure_provider()` handled `image_gen_plugin_name` in both
branches (no-env-vars early return and post-env-vars) but never mirrored
the same handling for `video_gen_plugin_name`. The first-time
`_configure_provider()` path correctly routes to
`_select_plugin_video_gen_provider()`; reconfigure forgot to.
Repro:
1. Enable video_gen in `hermes tools` → Configure for All Platforms.
2. Go back into `hermes tools` → Reconfigure tool → Video Generation.
3. Pick xAI (with XAI_API_KEY already set).
4. Hit Enter at the "keep current key?" prompt.
Expected: `video_gen.provider: xai` written to config.yaml.
Actual: function returns silently; no `video_gen:` block ever written;
`video_generate` tool fails with "No video generation backend is
configured."
Fix: add the missing `video_gen_plugin_name` branch in both code paths
of `_reconfigure_provider()`, mirroring the existing
`image_gen_plugin_name` handling and the first-time configure logic.
Tests: `tests/hermes_cli/test_video_gen_picker.py` covers both branches
(env-vars-set keep-current and no-env-vars paths).
AGENTS.md and CONTRIBUTING.md both now state:
1. No new memory providers in the repo. The set under plugins/memory/
(honcho, mem0, supermemory, byterover, hindsight, holographic,
openviking, retaindb) is closed. New backends ship as standalone
plugin repos that users install into ~/.hermes/plugins/ via the
same MemoryProvider ABC, discovery path, and hermes memory setup
integration. PRs adding a new plugins/memory/<name>/ directory get
closed with a pointer to publish as their own repo.
2. Skill authoring standards (hardline) — applies to all new or
modernized skills (bundled, optional, contributed):
- description <= 60 chars, one sentence, ends with period, no
marketing words, no name repetition (verification snippet
included)
- tools referenced in SKILL.md prose must be native Hermes tools
or MCP servers the skill expects — no grep/cat/sed/find etc.
when search_files/read_file/patch already cover them
- platforms: gating audited against actual POSIX-only primitives
- author credits the human contributor first, not 'Hermes Agent'
- SKILL.md uses modern section order with line targets
- scripts/references/templates layout for non-trivial logic
- tests at tests/skills/test_<skill>_skill.py, stdlib + mock only
- .env.example edits isolated to a delimited block
CONTRIBUTING.md includes a good/bad description example and a
'don't say / say' table mapping shell utilities to native tools.
AGENTS.md points the agent at references/new-skill-pr-salvage.md
for the full salvage checklist.
Salvages the closed PR #2010 (Mibayy's EVM multi-chain skill) and folds the
existing optional-skills/blockchain/base/ skill into it, so we ship one
unified EVM skill instead of two overlapping ones.
Pulled in from base/:
- 8 missing Base-specific tokens (AERO, DEGEN, TOSHI, BRETT, WELL,
cbETH, cbBTC, wstETH, rETH) added to KNOWN_TOKENS['base'] —
base/ had 11, evm/ only had 3 (USDC/DAI/WETH).
- L1 data-fee pitfall note for rollups (Base, Arbitrum, Optimism, zkSync).
- Batch-size chunking in rpc_batch (Base RPC caps batches at 10 calls
per JSON-RPC request; adding more known tokens tripped that limit
and broke 'wallet --chain base' with a 'list index out of range'
error). Ported the chunking pattern from base/_rpc_batch_chunk.
Latent bugs found and fixed while smoke-testing the merge:
- cmd_multichain and cmd_allowance both iterated KNOWN_TOKENS[chain]
with 'for contract, (symbol, _name) in known.items()' — but the dict
shape is {symbol: contract_str}, not {addr: (sym, name)}. This raised
'too many values to unpack (expected 2)' on every non-zero balance.
Now iterates as 'for symbol, contract in known.items()'.
- Input validation: added is_valid_address / is_valid_txhash /
require_address / require_txhash helpers and wired them into
cmd_wallet, cmd_tx, cmd_token, cmd_activity, cmd_allowance,
cmd_decode, cmd_contract, cmd_multichain. Fails fast with exit 2
on malformed input instead of burning an RPC round-trip on garbage.
Documentation:
- SKILL.md now flags that this skill supersedes optional-skills/blockchain/base.
- Pitfalls expanded for ENS (single-endpoint dependency on
ensideas.com), tx decoding (single-endpoint dependency on
4byte.directory), and rollup L1 fees.
- Regenerated website/docs/user-guide/skills/optional/blockchain/
blockchain-evm.md and removed the old blockchain-base.md page;
catalog updated.
Removed:
- optional-skills/blockchain/base/SKILL.md
- optional-skills/blockchain/base/scripts/base_client.py
- website/docs/user-guide/skills/optional/blockchain/blockchain-base.md
Smoke-tested live against Base mainnet: stats, price, token, wallet
(vitalik.eth — 3.12 ETH + 13.88 USDC + 4.23 DAI + 0.06 WETH on Base)
and allowance (ethereum, 7 unlimited approvals to Uniswap/Permit2).
Original PR #2010 author: Mibayy.
Original base/ skill author: youssefea.
