mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
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).
This commit is contained in:
kshitijk4poor
Teknium
parent
e8cee87e85
commit
21e3a863bb
4 changed files with 243 additions and 275 deletions
|
|
@ -96,7 +96,10 @@ class TestBundledPluginsRegister:
|
|||
("exa", True, True, False),
|
||||
("parallel", True, True, False),
|
||||
("tavily", True, True, True),
|
||||
("firecrawl", True, True, False),
|
||||
# firecrawl: search + extract + crawl. Crawl was originally
|
||||
# disabled in the migration (fell through to a legacy inline
|
||||
# path); the follow-up commit enabled it natively.
|
||||
("firecrawl", True, True, True),
|
||||
],
|
||||
)
|
||||
def test_capability_flags_match_spec(
|
||||
|
|
@ -451,3 +454,22 @@ class TestErrorResponseShapes:
|
|||
assert isinstance(result["results"], list)
|
||||
if result["results"]:
|
||||
assert "error" in result["results"][0]
|
||||
|
||||
def test_firecrawl_crawl_returns_error_dict_when_unconfigured(self) -> None:
|
||||
"""firecrawl crawl is async (wraps SDK in to_thread); error must be
|
||||
surfaced via the per-page result shape, not raised."""
|
||||
_ensure_plugins_loaded()
|
||||
from agent.web_search_registry import get_provider
|
||||
|
||||
p = get_provider("firecrawl")
|
||||
assert p is not None
|
||||
assert inspect.iscoroutinefunction(p.crawl)
|
||||
result = asyncio.run(p.crawl("https://example.com"))
|
||||
assert isinstance(result, dict)
|
||||
assert "results" in result
|
||||
assert isinstance(result["results"], list)
|
||||
# Without FIRECRAWL_API_KEY, the plugin's _get_firecrawl_client()
|
||||
# raises ValueError which is caught and returned as a per-page error.
|
||||
assert len(result["results"]) >= 1
|
||||
assert "error" in result["results"][0]
|
||||
assert result["results"][0]["url"] == "https://example.com"
|
||||
|
|
|
|||
|
|
@ -454,6 +454,9 @@ async def test_web_crawl_short_circuits_blocked_url(monkeypatch):
|
|||
monkeypatch.setenv("FIRECRAWL_API_KEY", "fake-key")
|
||||
# Allow test URLs past SSRF check so website policy is what gets tested
|
||||
monkeypatch.setattr(web_tools, "is_safe_url", lambda url: True)
|
||||
# The dispatcher-level (seed-URL) policy gate still lives on web_tools.
|
||||
# No per-page gate runs in this test because the dispatcher returns
|
||||
# immediately when the seed is blocked, before delegating to the plugin.
|
||||
monkeypatch.setattr(
|
||||
web_tools,
|
||||
"check_website_access",
|
||||
|
|
@ -464,10 +467,13 @@ async def test_web_crawl_short_circuits_blocked_url(monkeypatch):
|
|||
"message": "Blocked by website policy",
|
||||
},
|
||||
)
|
||||
# If the dispatcher ever reaches the firecrawl plugin's crawl(), the test
|
||||
# fails — pin the plugin module's client lookup so we'd notice.
|
||||
from plugins.web.firecrawl import provider as firecrawl_provider
|
||||
monkeypatch.setattr(
|
||||
web_tools,
|
||||
firecrawl_provider,
|
||||
"_get_firecrawl_client",
|
||||
lambda: pytest.fail("firecrawl should not run for blocked crawl URL"),
|
||||
lambda: pytest.fail("firecrawl plugin should not run for blocked crawl URL"),
|
||||
)
|
||||
monkeypatch.setattr("tools.interrupt.is_interrupted", lambda: False)
|
||||
|
||||
|
|
@ -480,13 +486,17 @@ async def test_web_crawl_short_circuits_blocked_url(monkeypatch):
|
|||
@pytest.mark.asyncio
|
||||
async def test_web_crawl_blocks_redirected_final_url(monkeypatch):
|
||||
from tools import web_tools
|
||||
from plugins.web.firecrawl import provider as firecrawl_provider
|
||||
|
||||
# web_crawl_tool checks for Firecrawl env before website policy
|
||||
# Force the firecrawl plugin to be the active crawl provider.
|
||||
monkeypatch.setenv("FIRECRAWL_API_KEY", "fake-key")
|
||||
# Allow test URLs past SSRF check so website policy is what gets tested
|
||||
monkeypatch.setattr(web_tools, "is_safe_url", lambda url: True)
|
||||
|
||||
def fake_check(url):
|
||||
# Dispatcher seed-URL gate (web_tools.check_website_access call)
|
||||
# and plugin per-page gate (firecrawl_provider.check_website_access
|
||||
# call) both flow through this single fake_check.
|
||||
if url == "https://allowed.test":
|
||||
return None
|
||||
if url == "https://blocked.test/final":
|
||||
|
|
@ -512,8 +522,13 @@ async def test_web_crawl_blocks_redirected_final_url(monkeypatch):
|
|||
]
|
||||
}
|
||||
|
||||
# After PR #25182 follow-up: per-page policy gate lives in
|
||||
# plugins.web.firecrawl.provider.crawl(). Patch the gate + client at
|
||||
# the plugin location. The dispatcher-level (seed) gate also reads
|
||||
# web_tools.check_website_access — patch both.
|
||||
monkeypatch.setattr(web_tools, "check_website_access", fake_check)
|
||||
monkeypatch.setattr(web_tools, "_get_firecrawl_client", lambda: FakeCrawlClient())
|
||||
monkeypatch.setattr(firecrawl_provider, "check_website_access", fake_check)
|
||||
monkeypatch.setattr(firecrawl_provider, "_get_firecrawl_client", lambda: FakeCrawlClient())
|
||||
monkeypatch.setattr("tools.interrupt.is_interrupted", lambda: False)
|
||||
|
||||
result = json.loads(await web_tools.web_crawl_tool("https://allowed.test", use_llm_processing=False))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue