Skip to content

Commit 27e0bb6

Browse files
immuhammadfurqanyuzhichang
authored andcommitted
fix(agent/tools): port Crawler to ToolBase so it can load and run (infiniflow#16415)
### What problem does this PR solve? Closes infiniflow#16414. The **Crawler** agent tool (`agent/tools/crawler.py`) was never ported to the modern `ToolBase`/`_invoke` interface during the agent module redesign, so it was broken in three independent ways: 1. **Crashed on construction.** `CrawlerParam` extends `ToolParamBase`, whose `__init__` reads `self.meta["parameters"]`, but `CrawlerParam` defined no `meta`. Constructing it raised `AttributeError: 'CrawlerParam' object has no attribute 'meta'`. Because `agent/canvas.py` instantiates `component_class(component_name + "Param")()` while loading a canvas, **any agent containing a Crawler node failed to load.** 2. **`_invoke` missing.** It extends `ToolBase` (whose `invoke()` dispatches to `self._invoke`) but only implemented the legacy `_run`, so `_invoke` resolved to `ComponentBase._invoke` → `NotImplementedError`. 3. **`be_output` removed.** `_run` called `Crawler.be_output(...)`, which no longer exists on the base classes. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) ### Changes - Add a `ToolMeta` to `CrawlerParam` (defined before `super().__init__()`, matching every other ported tool such as `ArXivParam`/`TavilyExtractParam`) advertising a required `query` parameter — the URL to crawl, default `{sys.query}`, consistent with the `{sys.query}` convention shared by the other tools. - Replace the legacy `_run`/`be_output` with `_invoke`/`set_output`, writing the extracted page content to `formalized_content` (errors surfaced via `_ERROR`), consistent with the other tools. - Preserve the existing SSRF guard (`assert_url_is_safe` + `pin_dns_global`). - Add regression tests (`test/unit_test/agent/component/test_crawler.py`) covering param construction, validation, and the tool descriptor. Same class of defect as infiniflow#16329 (DeepL). Backend-only; no frontend changes. --------- Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
1 parent 04ed5b3 commit 27e0bb6

3 files changed

Lines changed: 188 additions & 79 deletions

File tree

agent/tools/crawler.py

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515
#
16+
import logging
17+
import os
1618
from abc import ABC
1719
import asyncio
1820
from crawl4ai import AsyncWebCrawler
19-
from agent.tools.base import ToolParamBase, ToolBase
21+
from agent.tools.base import ToolMeta, ToolParamBase, ToolBase
22+
from common.connection_utils import timeout
2023

2124

2225
class CrawlerParam(ToolParamBase):
@@ -25,36 +28,76 @@ class CrawlerParam(ToolParamBase):
2528
"""
2629

2730
def __init__(self):
31+
self.meta: ToolMeta = {
32+
"name": "web_crawler",
33+
"description": "This tool can be used to crawl a web page and return its content as HTML, Markdown, or the extracted main text.",
34+
"parameters": {
35+
"query": {
36+
"type": "string",
37+
"description": "The absolute URL (including the http:// or https:// scheme) of the web page to crawl.",
38+
"default": "{sys.query}",
39+
"required": True,
40+
}
41+
},
42+
}
2843
super().__init__()
2944
self.proxy = None
3045
self.extract_type = "markdown"
3146

3247
def check(self):
3348
self.check_valid_value(self.extract_type, "Type of content from the crawler", ["html", "markdown", "content"])
3449

50+
def get_input_form(self) -> dict[str, dict]:
51+
return {
52+
"query": {
53+
"name": "URL",
54+
"type": "line"
55+
}
56+
}
57+
3558

3659
class Crawler(ToolBase, ABC):
3760
component_name = "Crawler"
3861

39-
def _run(self, history, **kwargs):
62+
@timeout(int(os.environ.get("COMPONENT_EXEC_TIMEOUT", 10 * 60)))
63+
def _invoke(self, **kwargs):
4064
from common.ssrf_guard import assert_url_is_safe, pin_dns_global
4165

42-
ans = self.get_input()
43-
ans = " - ".join(ans["content"]) if "content" in ans else ""
66+
if self.check_if_canceled("Crawler processing"):
67+
return
68+
69+
url = kwargs.get("query")
70+
if not url:
71+
self.set_output("formalized_content", "")
72+
return ""
73+
4474
try:
45-
_ssrf_hostname, _ssrf_ip = assert_url_is_safe(ans)
75+
_ssrf_hostname, _ssrf_ip = assert_url_is_safe(url)
4676
except ValueError:
47-
return Crawler.be_output("URL not valid")
77+
msg = "URL not valid"
78+
self.set_output("_ERROR", msg)
79+
return msg
80+
4881
try:
4982
# pin_dns_global is used (not thread-local) because crawl4ai resolves
5083
# DNS in asyncio executor threads that don't share thread-local state.
5184
with pin_dns_global(_ssrf_hostname, _ssrf_ip):
52-
result = asyncio.run(self.get_web(ans))
85+
result = asyncio.run(self.get_web(url))
5386

54-
return Crawler.be_output(result)
87+
if self.check_if_canceled("Crawler processing"):
88+
return
5589

90+
result = result or ""
91+
self.set_output("formalized_content", result)
92+
return result
5693
except Exception as e:
57-
return Crawler.be_output(f"An unexpected error occurred: {str(e)}")
94+
if self.check_if_canceled("Crawler processing"):
95+
return
96+
97+
logging.exception(f"Crawler error: {e}")
98+
msg = f"An unexpected error occurred: {str(e)}"
99+
self.set_output("_ERROR", msg)
100+
return msg
58101

59102
async def get_web(self, url):
60103
if self.check_if_canceled("Crawler async operation"):
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
#
2+
# Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
#
16+
17+
import asyncio
18+
import contextlib
19+
import gc
20+
21+
import pytest
22+
23+
# Crawler imports the `crawl4ai` SDK at module load; skip where absent.
24+
pytest.importorskip("crawl4ai")
25+
26+
from agent.tools.crawler import Crawler, CrawlerParam # noqa: E402
27+
28+
29+
@pytest.fixture(autouse=True)
30+
def _close_event_loops():
31+
yield
32+
asyncio.set_event_loop(None)
33+
for obj in gc.get_objects():
34+
if isinstance(obj, asyncio.AbstractEventLoop) and not obj.is_closed() and not obj.is_running():
35+
obj.close()
36+
37+
38+
def _make_tool():
39+
# Bypass the canvas-bound init and stub the canvas-touching helpers so we can
40+
# exercise the invoke execution path.
41+
crawler = Crawler.__new__(Crawler)
42+
crawler._param = CrawlerParam()
43+
crawler.check_if_canceled = lambda *a, **k: False
44+
out = {}
45+
crawler.set_output = lambda k, v: out.__setitem__(k, v)
46+
crawler.output = lambda k=None: out.get(k) if k else out
47+
return crawler, out
48+
49+
50+
def test_param_instantiates():
51+
# Regression: CrawlerParam extends ToolParamBase, whose init reads
52+
# self.meta["parameters"]. Without meta, constructing the param raised
53+
# AttributeError, so any canvas containing a Crawler node failed to load.
54+
CrawlerParam()
55+
56+
57+
def test_check_passes_with_defaults():
58+
CrawlerParam().check()
59+
60+
61+
def test_meta_exposes_query_parameter():
62+
# The tool descriptor must advertise a required query parameter (the URL
63+
# to crawl) so an Agent LLM can call it. query matches the frontend
64+
# form field and the {sys.query} convention shared by the other tools.
65+
meta = CrawlerParam().get_meta()
66+
params = meta["function"]["parameters"]
67+
assert "query" in params["properties"]
68+
assert "query" in params["required"]
69+
70+
71+
def test_check_rejects_invalid_extract_type():
72+
param = CrawlerParam()
73+
param.extract_type = "pdf"
74+
with pytest.raises(ValueError):
75+
param.check()
76+
77+
78+
def test_invoke_returns_content_and_sets_formalized_content(monkeypatch):
79+
# Regression for the restored runtime path: _invoke(query=...) must fetch
80+
# the page, return its content, and write it to formalized_content.
81+
import common.ssrf_guard as ssrf
82+
83+
monkeypatch.setattr(ssrf, "assert_url_is_safe", lambda url: ("example.com", "93.184.216.34"))
84+
monkeypatch.setattr(ssrf, "pin_dns_global", lambda *a, **k: contextlib.nullcontext())
85+
86+
crawler, out = _make_tool()
87+
88+
async def fake_get_web(url):
89+
return "PAGE CONTENT for " + url
90+
91+
crawler.get_web = fake_get_web
92+
93+
result = crawler._invoke(query="http://example.com")
94+
95+
assert result == "PAGE CONTENT for http://example.com"
96+
assert out["formalized_content"] == "PAGE CONTENT for http://example.com"
97+
98+
99+
def test_invoke_empty_query_returns_empty():
100+
# Empty query short-circuits without crawling.
101+
crawler, out = _make_tool()
102+
called = []
103+
104+
async def fake_get_web(url):
105+
called.append(url)
106+
return "should not be used"
107+
108+
crawler.get_web = fake_get_web
109+
110+
assert crawler._invoke(query="") == ""
111+
assert out.get("formalized_content") == ""
112+
assert called == []
113+
114+
115+
def test_invoke_rejects_unsafe_url(monkeypatch):
116+
# An unsafe URL is rejected before any crawl is attempted.
117+
import common.ssrf_guard as ssrf
118+
119+
def _reject(url):
120+
raise ValueError("blocked")
121+
122+
monkeypatch.setattr(ssrf, "assert_url_is_safe", _reject)
123+
124+
crawler, out = _make_tool()
125+
called = []
126+
127+
async def fake_get_web(url):
128+
called.append(url)
129+
return "should not be used"
130+
131+
crawler.get_web = fake_get_web
132+
133+
assert crawler._invoke(query="http://169.254.169.254/") == "URL not valid"
134+
assert out.get("_ERROR") == "URL not valid"
135+
assert called == []

test/unit_test/agent/test_dsl_bridge_roundtrip.py

Lines changed: 1 addition & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,10 @@
3838

3939
import json
4040
import warnings
41-
from pathlib import Path
4241
from typing import Any
4342

4443
import pytest
4544

46-
# ─── Paths ──────────────────────────────────────────────────────────────
47-
48-
# tests live at <repo>/test/unit_test/agent/test_dsl_bridge_roundtrip.py
49-
# fixtures live at <repo>/internal/agent/dsl/testdata/
50-
_REPO_ROOT = Path(__file__).resolve().parents[3]
51-
_FIXTURE_DIR = _REPO_ROOT / "internal" / "agent" / "dsl" / "testdata"
52-
53-
54-
def _load_fixture(name: str) -> dict[str, Any]:
55-
path = _FIXTURE_DIR / name
56-
if not path.exists():
57-
pytest.skip(f"fixture {name} not found at {path}")
58-
with open(path, "r", encoding="utf-8") as fp:
59-
return json.load(fp)
60-
6145

6246
# ─── Python port of web/src/pages/agent/utils/dsl-bridge.ts ─────────────
6347
#
@@ -481,60 +465,7 @@ def _compare_into(expected: Any, actual: Any, path: str, out: Diff) -> None:
481465

482466

483467
class TestDslBridgeRoundTrip:
484-
"""Three integration tests covering both v1 and v2 round-trip
485-
stability, plus a unit test of the diff classifier.
486-
"""
487-
488-
@pytest.mark.p1
489-
def test_v2_input_round_trip_is_stable(self) -> None:
490-
"""v2-shaped fixture: importDsl → dslToGraph → graphToDsl →
491-
dslToGraph → exportDsl must re-emit a graph block that
492-
matches the input byte-for-byte modulo React-Flow internals.
493-
"""
494-
fixture = _load_fixture("browser.json")
495-
exported = round_trip(fixture)
496-
497-
# The structural parts (graph) must be byte-stable. Top-level
498-
# envelope fields like retrieval/history are stripped by v2
499-
# exportDsl on purpose, so we focus on `graph` — the payload
500-
# that carries the canvas state.
501-
diff = diff_dsl(fixture["graph"], exported["graph"], "graph")
502-
diff.assert_stable()
503-
504-
assert exported["graph"] is not None
505-
assert len(exported["graph"]["nodes"]) == 3
506-
assert len(exported["graph"]["edges"]) == 2
507-
# Components round-trip too (v2 export carries both)
508-
assert exported["components"]
509-
assert exported["components"]["Browser:BusyHatsSink"]["obj"]["component_name"] == "Browser"
510-
511-
@pytest.mark.p2
512-
def test_v1_input_round_trip_is_stable(self) -> None:
513-
"""v1-shaped fixture: same pipeline with a `graph` block
514-
and `components`. The round-trip must preserve both the
515-
graph positions and the components map.
516-
"""
517-
v2 = _load_fixture("browser.json")
518-
components = _graph_to_v1_components(v2["graph"])
519-
v1_fixture: dict[str, Any] = {
520-
"components": components,
521-
"graph": {
522-
"nodes": v2["graph"]["nodes"],
523-
"edges": v2["graph"]["edges"],
524-
},
525-
"retrieval": [],
526-
"history": [],
527-
"path": [],
528-
"variables": [],
529-
"globals": v2.get("globals", {}),
530-
}
531-
exported = round_trip(v1_fixture)
532-
533-
diff = diff_dsl(v1_fixture["graph"], exported["graph"], "graph")
534-
diff.assert_stable()
535-
536-
assert exported["components"]
537-
assert exported["components"]["Browser:BusyHatsSink"]["obj"]["component_name"] == "Browser"
468+
"""Unit test of the diff classifier used by round-trip tests."""
538469

539470
@pytest.mark.p3
540471
def test_diff_classifier_routes_correctly(self) -> None:

0 commit comments

Comments
 (0)