diff --git a/tests/tools/test_web_result_cache.py b/tests/tools/test_web_result_cache.py
index 516b31e482..0f28132a07 100644
--- a/tests/tools/test_web_result_cache.py
+++ b/tests/tools/test_web_result_cache.py
@@ -147,26 +147,9 @@ def test_single_flight_coalesces_concurrent_identical_queries():
# ── extract cache ────────────────────────────────────────────────────────
-def _put_and_get(url="https://example.com/a", content="hello world",
- fmt=None, **kw):
- extract_cache_put(url, content, title="T", format=fmt, **kw)
- return extract_cache_get(url, format=fmt)
-
-
-def test_extract_cache_roundtrip(monkeypatch, _isolated_cache):
- # _store_full_text writes to the real hermes dir; redirect it here.
- stored = {}
-
- def fake_store(url, content):
- p = _isolated_cache / "page.md"
- p.write_text(content, encoding="utf-8")
- stored["path"] = str(p)
- return str(p)
-
- import tools.web_tools as wt
- monkeypatch.setattr(wt, "_store_full_text", fake_store)
-
- hit = _put_and_get()
+def test_extract_cache_roundtrip(_isolated_cache):
+ extract_cache_put("https://example.com/a", "hello world", title="T")
+ hit = extract_cache_get("https://example.com/a")
assert hit is not None
assert hit["content"] == "hello world"
assert hit["title"] == "T"
@@ -174,31 +157,40 @@ def test_extract_cache_roundtrip(monkeypatch, _isolated_cache):
def test_extract_cache_expired_entry_is_miss(monkeypatch, _isolated_cache):
- import tools.web_tools as wt
- p = _isolated_cache / "page.md"
- p.write_text("x", encoding="utf-8")
- monkeypatch.setattr(wt, "_store_full_text", lambda u, c: str(p))
extract_cache_put("https://e.com", "x")
monkeypatch.setattr(wrc, "ttl_seconds", lambda: 0.0)
assert extract_cache_get("https://e.com") is None
-def test_extract_cache_format_participates_in_key(monkeypatch, _isolated_cache):
- import tools.web_tools as wt
- p = _isolated_cache / "page.md"
- p.write_text("md", encoding="utf-8")
- monkeypatch.setattr(wt, "_store_full_text", lambda u, c: str(p))
- extract_cache_put("https://e.com", "md", format="markdown")
+def test_extract_cache_format_participates_in_key(_isolated_cache):
+ extract_cache_put("https://e.com", "md content", format="markdown")
assert extract_cache_get("https://e.com", format="html") is None
assert extract_cache_get("https://e.com", format="markdown") is not None
-def test_extract_cache_oversized_page_not_indexed(monkeypatch, _isolated_cache):
+def test_extract_cache_formats_do_not_overwrite_each_other(_isolated_cache):
+ """Regression (#94618 review finding 3): html and markdown copies of one
+ URL must be stored independently — the original implementation shared a
+ URL-keyed backing file, so the later write clobbered the earlier one."""
+ extract_cache_put("https://e.com/page", "# MARKDOWN VERSION", format="markdown")
+ extract_cache_put("https://e.com/page", "HTML VERSION
", format="html")
+ md = extract_cache_get("https://e.com/page", format="markdown")
+ html = extract_cache_get("https://e.com/page", format="html")
+ assert md is not None and md["content"] == "# MARKDOWN VERSION"
+ assert html is not None and html["content"] == "HTML VERSION
"
+
+
+def test_extract_cache_provider_participates_in_key(_isolated_cache):
+ """Switching extract backends within the TTL must not serve the old
+ backend's rendering (#94618 review, additional risk 3)."""
+ extract_cache_put("https://e.com/p", "firecrawl version", provider="firecrawl")
+ assert extract_cache_get("https://e.com/p", provider="tavily") is None
+ hit = extract_cache_get("https://e.com/p", provider="firecrawl")
+ assert hit is not None and hit["content"] == "firecrawl version"
+
+
+def test_extract_cache_oversized_page_not_indexed(_isolated_cache):
import tools.web_tools as wt
- monkeypatch.setattr(
- wt, "_store_full_text",
- lambda u, c: str(_isolated_cache / "should-not-happen.md"),
- )
big = "x" * (wt.MAX_STORED_TEXT_CHARS + 1)
extract_cache_put("https://big.com", big)
assert extract_cache_get("https://big.com") is None
@@ -239,10 +231,6 @@ def test_extract_cache_corrupt_index_is_empty(_isolated_cache):
def test_extract_cache_disabled_by_config(monkeypatch, _isolated_cache):
- import tools.web_tools as wt
- p = _isolated_cache / "page.md"
- p.write_text("x", encoding="utf-8")
- monkeypatch.setattr(wt, "_store_full_text", lambda u, c: str(p))
extract_cache_put("https://e.com", "x")
monkeypatch.setattr(wrc, "_web_config", lambda: {"cache_enabled": False})
assert extract_cache_get("https://e.com") is None
diff --git a/tools/web_result_cache.py b/tools/web_result_cache.py
index 8163f51a18..9799e34612 100644
--- a/tools/web_result_cache.py
+++ b/tools/web_result_cache.py
@@ -29,6 +29,7 @@ Disable with ``web.cache_enabled: false``; both TTLs come from
import hashlib
import json
import logging
+import os
import re
import threading
import time
@@ -152,9 +153,16 @@ class SearchMemo:
with self._store_lock:
lock = self._key_locks.get(key)
if lock is None:
- # Bound the lock table alongside the store.
+ # Bound the lock table alongside the store — but never evict
+ # a HELD lock: dropping one lets a concurrent identical
+ # request mint a fresh lock and issue a duplicate paid call
+ # (review finding on #94618). locked() under _store_lock is
+ # a safe snapshot because flight locks are only ever
+ # acquired by callers that already hold a reference.
if len(self._key_locks) > 256:
- self._key_locks.clear()
+ self._key_locks = {
+ k: v for k, v in self._key_locks.items() if v.locked()
+ }
lock = threading.Lock()
self._key_locks[key] = lock
return lock
@@ -227,26 +235,57 @@ def _save_index(index: dict) -> None:
reverse=True,
)[:_INDEX_MAX_ENTRIES]
index = dict(newest)
- tmp = path.with_suffix(".tmp")
+ # Per-process tmp name: CLI, gateway, cron, and subagent processes
+ # all write this index; a shared fixed tmp filename would let two
+ # concurrent writers truncate each other mid-write. os.replace is
+ # atomic per writer, so the worst cross-process outcome is one
+ # writer's entry winning — a lost cache insert, never a torn file.
+ tmp = path.with_suffix(f".tmp.{os.getpid()}")
tmp.write_text(json.dumps(index), encoding="utf-8")
tmp.replace(path)
except Exception as exc: # noqa: BLE001
logger.debug("Failed to save web extract cache index: %s", exc)
-def _url_digest(url: str, format: Optional[str]) -> str:
- # format participates in the key: an html extract is not a markdown one.
- raw = f"{url}\n{format or 'markdown'}"
+def _url_digest(url: str, format: Optional[str], provider: str = "") -> str:
+ # format AND provider participate in the key: an html extract is not a
+ # markdown one, and one backend's rendering of a page is not another's.
+ raw = f"{url}\n{format or 'markdown'}\n{provider or ''}"
return hashlib.sha256(raw.encode("utf-8")).hexdigest()[:16]
-def extract_cache_get(url: str, format: Optional[str] = None) -> Optional[dict]:
+def _entry_file_path(url: str, format: Optional[str], provider: str) -> Optional[Path]:
+ """Dedicated cache file per (url, format, provider) entry.
+
+ Deliberately NOT the truncate-store file from ``_store_full_text`` — that
+ filename keys on URL alone, so html/markdown (or two providers') copies
+ of one URL would overwrite each other (review finding on #94618). The
+ truncate-store file keeps its role for read_file paging; these files
+ exist only for cache reuse and carry the full key in their name.
+ """
+ d = _cache_dir()
+ if d is None:
+ return None
+ try:
+ from urllib.parse import urlparse
+ host = (urlparse(url).hostname or "page").replace(":", "_")
+ slug = re.sub(r"[^A-Za-z0-9._-]", "-", host)[:60].strip("-") or "page"
+ except Exception: # noqa: BLE001
+ slug = "page"
+ return d / f"{slug}-{_url_digest(url, format, provider)}.cache.md"
+
+
+def extract_cache_get(
+ url: str,
+ format: Optional[str] = None,
+ provider: str = "",
+) -> Optional[dict]:
"""Return {'url','title','content'} for a fresh cached page, else None."""
if not cache_enabled():
return None
with _index_lock:
index = _load_index()
- entry = index.get(_url_digest(url, format))
+ entry = index.get(_url_digest(url, format, provider))
if not entry:
return None
if (time.time() - float(entry.get("fetched_at", 0))) >= ttl_seconds():
@@ -276,28 +315,32 @@ def extract_cache_put(
content: str,
title: str = "",
format: Optional[str] = None,
+ provider: str = "",
) -> None:
"""Store one successful extraction's full clean text for TTL reuse.
- Pages larger than the truncate-store ceiling are NOT indexed for reuse:
- the stored copy would be incomplete, and serving it back as if whole
- would silently lose the tail. (The capped file is still written by the
- truncate-store path for read_file paging — we just don't index it.)
+ Writes a dedicated per-(url, format, provider) cache file (see
+ ``_entry_file_path``) — never the URL-keyed truncate-store file, which
+ different formats/providers would overwrite. Pages larger than the
+ truncate-store ceiling are not cached: serving a capped copy back as if
+ whole would silently lose the tail.
"""
if not cache_enabled() or not content:
return
try:
- from tools.web_tools import MAX_STORED_TEXT_CHARS, _store_full_text
+ from tools.web_tools import MAX_STORED_TEXT_CHARS
if len(content) > MAX_STORED_TEXT_CHARS:
return
- file_path = _store_full_text(url, content)
- if not file_path:
+ file_path = _entry_file_path(url, format, provider)
+ if file_path is None:
return
+ from tools.spill_safety import write_text_exclusive
+ write_text_exclusive(file_path, content, private=False, overwrite=True)
with _index_lock:
index = _load_index()
- index[_url_digest(url, format)] = {
+ index[_url_digest(url, format, provider)] = {
"url": url,
- "file": file_path,
+ "file": str(file_path),
"title": title or "",
"fetched_at": time.time(),
}
diff --git a/tools/web_tools.py b/tools/web_tools.py
index dd09b88d8c..0d9aca14f3 100644
--- a/tools/web_tools.py
+++ b/tools/web_tools.py
@@ -1160,22 +1160,136 @@ async def web_extract_tool(
if not safe_urls:
results = []
else:
+ backend = _get_extract_backend()
+
+ # All seven providers (brave-free, ddgs, searxng, exa, parallel,
+ # tavily, firecrawl) now live as plugins. The dispatcher is a
+ # registry lookup + delegation. Some providers' extract() is
+ # async (parallel, firecrawl), others sync (exa, tavily) — we
+ # detect coroutine functions and await; sync functions run
+ # inline (the policy gate, SSRF re-check, etc. live inside the
+ # provider itself for the firecrawl per-URL loop).
+ _ensure_web_plugins_loaded()
+ from agent.web_search_registry import (
+ get_active_extract_provider,
+ get_provider as _wsp_get_provider,
+ _disabled_web_plugin_for,
+ )
+
+ provider = _wsp_get_provider(backend) if backend else None
+ if provider is None or not provider.supports_extract():
+ # When the configured name IS registered but doesn't support
+ # extract (search-only providers like brave-free / ddgs /
+ # searxng), surface that as a typed "search-only" error
+ # rather than silently switching backends. When the name
+ # isn't registered at all (typo / uninstalled plugin), fall
+ # through to the active-provider walk.
+ if provider is not None and not provider.supports_extract():
+ return json.dumps(
+ {
+ "success": False,
+ "error": (
+ f"{provider.display_name} is a search-only "
+ "backend and cannot extract URL content. "
+ "Set web.extract_backend to firecrawl, "
+ "tavily, exa, or parallel."
+ ),
+ },
+ ensure_ascii=False,
+ )
+ from tools.tool_backend_helpers import (
+ selection_error,
+ selection_exists,
+ )
+
+ if backend and selection_exists("web"):
+ # Strict selection: a stored-but-unregistered backend
+ # errors by name instead of silently switching to
+ # whatever the availability walk finds.
+ disabled_key = _disabled_web_plugin_for(capability="extract")
+ if disabled_key:
+ _vendor = disabled_key.split("/", 1)[-1]
+ error_text = (
+ f"web.extract_backend is set to '{_vendor}', but "
+ f"its plugin ('{disabled_key}') is disabled in "
+ f"config. Re-enable it with `hermes plugins "
+ f"enable {disabled_key}` (or remove it from "
+ "plugins.disabled)."
+ )
+ else:
+ error_text = selection_error(
+ "web",
+ f"'{backend}'",
+ "no registered web extract provider has that name",
+ )
+ return json.dumps(
+ {"success": False, "error": error_text},
+ ensure_ascii=False,
+ )
+ provider = get_active_extract_provider()
+ if provider is None:
+ # If the configured backend is a bundled web plugin the
+ # user explicitly disabled, the backend is set correctly
+ # and the real fix is to re-enable the plugin — say so
+ # instead of telling them to set web.extract_backend
+ # (which they already did). #40190 follow-up.
+ disabled_key = _disabled_web_plugin_for(capability="extract")
+ if disabled_key:
+ _vendor = disabled_key.split("/", 1)[-1]
+ return json.dumps(
+ {
+ "success": False,
+ "error": (
+ f"web.extract_backend is set to '{_vendor}', "
+ f"but its plugin ('{disabled_key}') is disabled "
+ "in config. Re-enable it with "
+ f"`hermes plugins enable {disabled_key}` "
+ "(or remove it from plugins.disabled)."
+ ),
+ },
+ ensure_ascii=False,
+ )
+ return json.dumps(
+ {
+ "success": False,
+ "error": (
+ "No web extract provider configured. "
+ "Set web.extract_backend to firecrawl, "
+ "tavily, exa, or parallel."
+ ),
+ },
+ ensure_ascii=False,
+ )
+
+
# ── Extract cache (tools/web_result_cache.py) ─────────────────
- # Disk-backed via the existing cache/web full-text store: a URL
- # extracted within the TTL is served from disk instead of
- # re-scraped. Sits AFTER the secret-URL and SSRF gates so hits
- # skip only the vendor call, never a safety check. Cached
- # entries re-run the normal truncate pipeline below, so a
- # different caller char_limit still works off one scrape.
+ # Disk-backed via cache/web: a URL extracted within the TTL is
+ # served from disk instead of re-scraped. Deliberately placed
+ # AFTER the secret-URL gate, SSRF gate, provider resolution, and
+ # strict-selection validation, and gated per-URL on the website
+ # blocklist policy — a hit skips only the vendor call, never a
+ # control. Policy-blocked URLs are treated as cache misses so
+ # dispatch handles them exactly as it would without a cache.
+ # Keys include the provider and format, so switching backends or
+ # formats within the TTL never serves the other's content.
from tools.web_result_cache import (
extract_cache_get as _extract_cache_get,
extract_cache_put as _extract_cache_put,
)
+ from tools.website_policy import check_website_access as _check_site
cached_results: Dict[int, Dict[str, Any]] = {}
fetch_urls: List[str] = []
fetch_positions: List[int] = []
for position, url in enumerate(safe_urls):
- hit = _extract_cache_get(url, format=format)
+ hit = None
+ try:
+ _policy_block = _check_site(url)
+ except Exception: # noqa: BLE001 — policy errors fail open like dispatch
+ _policy_block = None
+ if _policy_block is None:
+ hit = _extract_cache_get(
+ url, format=format, provider=provider.name
+ )
if hit is not None:
cached_results[position] = hit
else:
@@ -1185,107 +1299,6 @@ async def web_extract_tool(
if not fetch_urls:
results = [cached_results[i] for i in range(len(safe_urls))]
else:
- backend = _get_extract_backend()
-
- # All seven providers (brave-free, ddgs, searxng, exa, parallel,
- # tavily, firecrawl) now live as plugins. The dispatcher is a
- # registry lookup + delegation. Some providers' extract() is
- # async (parallel, firecrawl), others sync (exa, tavily) — we
- # detect coroutine functions and await; sync functions run
- # inline (the policy gate, SSRF re-check, etc. live inside the
- # provider itself for the firecrawl per-URL loop).
- _ensure_web_plugins_loaded()
- from agent.web_search_registry import (
- get_active_extract_provider,
- get_provider as _wsp_get_provider,
- _disabled_web_plugin_for,
- )
-
- provider = _wsp_get_provider(backend) if backend else None
- if provider is None or not provider.supports_extract():
- # When the configured name IS registered but doesn't support
- # extract (search-only providers like brave-free / ddgs /
- # searxng), surface that as a typed "search-only" error
- # rather than silently switching backends. When the name
- # isn't registered at all (typo / uninstalled plugin), fall
- # through to the active-provider walk.
- if provider is not None and not provider.supports_extract():
- return json.dumps(
- {
- "success": False,
- "error": (
- f"{provider.display_name} is a search-only "
- "backend and cannot extract URL content. "
- "Set web.extract_backend to firecrawl, "
- "tavily, exa, or parallel."
- ),
- },
- ensure_ascii=False,
- )
- from tools.tool_backend_helpers import (
- selection_error,
- selection_exists,
- )
-
- if backend and selection_exists("web"):
- # Strict selection: a stored-but-unregistered backend
- # errors by name instead of silently switching to
- # whatever the availability walk finds.
- disabled_key = _disabled_web_plugin_for(capability="extract")
- if disabled_key:
- _vendor = disabled_key.split("/", 1)[-1]
- error_text = (
- f"web.extract_backend is set to '{_vendor}', but "
- f"its plugin ('{disabled_key}') is disabled in "
- f"config. Re-enable it with `hermes plugins "
- f"enable {disabled_key}` (or remove it from "
- "plugins.disabled)."
- )
- else:
- error_text = selection_error(
- "web",
- f"'{backend}'",
- "no registered web extract provider has that name",
- )
- return json.dumps(
- {"success": False, "error": error_text},
- ensure_ascii=False,
- )
- provider = get_active_extract_provider()
- if provider is None:
- # If the configured backend is a bundled web plugin the
- # user explicitly disabled, the backend is set correctly
- # and the real fix is to re-enable the plugin — say so
- # instead of telling them to set web.extract_backend
- # (which they already did). #40190 follow-up.
- disabled_key = _disabled_web_plugin_for(capability="extract")
- if disabled_key:
- _vendor = disabled_key.split("/", 1)[-1]
- return json.dumps(
- {
- "success": False,
- "error": (
- f"web.extract_backend is set to '{_vendor}', "
- f"but its plugin ('{disabled_key}') is disabled "
- "in config. Re-enable it with "
- f"`hermes plugins enable {disabled_key}` "
- "(or remove it from plugins.disabled)."
- ),
- },
- ensure_ascii=False,
- )
- return json.dumps(
- {
- "success": False,
- "error": (
- "No web extract provider configured. "
- "Set web.extract_backend to firecrawl, "
- "tavily, exa, or parallel."
- ),
- },
- ensure_ascii=False,
- )
-
logger.info(
"Web extract via %s: %d URL(s)", provider.name, len(fetch_urls)
)
@@ -1293,6 +1306,7 @@ async def web_extract_tool(
# Async-or-sync dispatch: parallel + firecrawl have async
# extract(); exa + tavily are sync.
import inspect
+ _extract_rescued = False
try:
if inspect.iscoroutinefunction(provider.extract):
results = await provider.extract(fetch_urls, format=format)
@@ -1304,6 +1318,7 @@ async def web_extract_tool(
)
except Exception as exc: # noqa: BLE001 — candidate for rescue
if _rescue_eligible(provider):
+ _extract_rescued = True
failed = [
{"url": u, "title": "", "content": "", "error": str(exc)}
for u in fetch_urls
@@ -1322,27 +1337,34 @@ async def web_extract_tool(
and all(r.get("error") for r in results)
and _rescue_eligible(provider)
):
+ _extract_rescued = True
results = await asyncio.to_thread(
_rescue_extract, provider.name, fetch_urls, results
)
# Cache each successful fetch's full clean text for TTL reuse
# (best-effort; oversized pages are skipped by the cache).
- for fetched_pos, fetched in enumerate(results):
- if fetched_pos >= len(fetch_urls):
- break
- if fetched.get("error"):
- continue
- _content = (
- fetched.get("raw_content", "") or fetched.get("content", "")
- )
- if _content:
- _extract_cache_put(
- fetch_urls[fetched_pos],
- _content,
- title=fetched.get("title", ""),
- format=format,
+ # NEVER cache a rescue-served batch: it came from a ring
+ # vendor, not the chosen backend, and caching it would make
+ # the one-shot rescue sticky for a whole TTL — the next call
+ # must attempt the chosen backend again.
+ if not _extract_rescued:
+ for fetched_pos, fetched in enumerate(results):
+ if fetched_pos >= len(fetch_urls):
+ break
+ if fetched.get("error"):
+ continue
+ _content = (
+ fetched.get("raw_content", "") or fetched.get("content", "")
)
+ if _content:
+ _extract_cache_put(
+ fetch_urls[fetched_pos],
+ _content,
+ title=fetched.get("title", ""),
+ format=format,
+ provider=provider.name,
+ )
# Merge fetched results back with cache hits, restoring the
# safe_urls order the downstream reconstruction expects.
diff --git a/website/docs/user-guide/features/web-search.md b/website/docs/user-guide/features/web-search.md
index daf4465e6e..ceda417248 100644
--- a/website/docs/user-guide/features/web-search.md
+++ b/website/docs/user-guide/features/web-search.md
@@ -69,11 +69,11 @@ Repeat web calls within a short window are served from cache instead of the paid
| Call | Cache | Scope |
|------|-------|-------|
| `web_search` — same query (case/whitespace-insensitive), same provider | In-memory memo | Per process |
-| `web_extract` — same URL, same format | Full text stored under `~/.hermes/cache/web/` | Shared across CLI, gateway, cron, and subagent processes |
+| `web_extract` — same URL, same format, same provider | Full text stored under `~/.hermes/cache/web/` | Shared across CLI, gateway, cron, and subagent processes |
Concurrent identical searches (a parallel subagent fan-out firing the same query at once) are **coalesced into a single backend request** — the first caller pays; the rest share the response. Requested search limits are bucketed up to 10/20/50/100 so near-identical requests (`limit=5` vs `limit=8`) share one entry, with each caller receiving its requested count.
-Only successful responses are cached. Failures always retry the backend, and responses served by the one-shot keyless rescue are never cached (the next call attempts your chosen backend again). Cached extracts re-run the normal truncation pipeline, so a different `char_limit` on the second call works off the same stored scrape.
+Only successful responses are cached. Failures always retry the backend, responses served by the one-shot keyless rescue are never cached (the next call attempts your chosen backend again), and URLs matched by your `security.website_blocklist` are never served from cache. Cached extracts re-run the normal truncation pipeline, so a different `char_limit` on the second call works off the same stored scrape.
```yaml
# ~/.hermes/config.yaml