feat(search): add Firecrawl search provider + standalone web_fetch tool - #1
Closed
rakshith48 wants to merge 8 commits into
Closed
feat(search): add Firecrawl search provider + standalone web_fetch tool#1rakshith48 wants to merge 8 commits into
rakshith48 wants to merge 8 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Firecrawl as a new search provider across the Agent TARS stack. Introduces typed Firecrawl config interfaces and factory, wires a ChangesFirecrawl Provider Integration
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note over Agent,FirecrawlAPI: web_search flow
Agent->>SearchClient: search(Firecrawl, query, {limit, scrapeOptions})
SearchClient->>FirecrawlAPI: /search
FirecrawlAPI-->>SearchClient: {web: [...], news: [...]}
SearchClient->>SearchClient: normalize into pages[]
SearchClient-->>Agent: {pages: [{url, title, content}]}
end
rect rgba(144, 238, 144, 0.5)
note over Agent,FirecrawlAPI: web_fetch flow
Agent->>FetchToolProvider: web_fetch(url)
FetchToolProvider->>FetchToolProvider: validate URL (http/https)
FetchToolProvider->>FirecrawlAPI: /scrape(url, onlyMainContent:true)
FirecrawlAPI-->>FetchToolProvider: {markdown, html, links, metadata}
FetchToolProvider-->>Agent: structured result or {error}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds Firecrawl as a first-class SearchProvider in @agent-infra/search (alongside browser_search/tavily/bing/duckduckgo/searxng) and exposes a `web_scrape` tool in Agent TARS when Firecrawl is the configured provider. Firecrawl's /search returns clean, LLM-ready markdown for every result in a single call, and /scrape reads any URL (incl. JS-rendered pages and PDFs) without driving the browser — covering both the "find" and "read" motions that are currently split across Tavily search and the LinkReader MCP. - packages/agent-infra/shared: add SearchProvider.Firecrawl - packages/agent-infra/search: firecrawl.ts provider + SearchClient case (web+news flattened to unified pages; markdown when scrapeOptions set), example, FIRECRAWL.md (incl. map/crawl/parse/monitor fit assessment) - agent-tars/interface: 'firecrawl' in search provider union - agent-tars/core: SearchToolProvider.createScrapeTool() + supportsScrape(), register web_scrape in initializeSearchTools - agent-tars/cli: list firecrawl in --search.provider help
rakshith48
force-pushed
the
feat/firecrawl-search-scrape
branch
from
June 21, 2026 08:54
78b460b to
186fbdb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@multimodal/agent-tars/core/src/environments/local/search/search-tool.ts`:
- Around line 199-200: The logger.info calls at lines 199 and 215 in the
search-tool.ts file are logging full URLs which may contain sensitive query
parameters like tokens or signed links. Extract and log only the safe portions
of the URL (such as the protocol, hostname, and path) while omitting the query
string entirely. Use URL parsing utilities to safely extract just the base URL
and path without the query parameters before passing to the logger.info calls.
- Around line 185-196: The URL validation in the function parameter checking is
incomplete. Currently it only validates that the url is non-empty, but the
parameter description states that the URL must start with http or https. Enhance
the validation check where you validate the url parameter to also verify that it
starts with either 'http://' or 'https://', and return an error message if the
scheme is invalid (for example, "URL must start with http:// or https://")
before proceeding to call the scraping provider.
In `@multimodal/agent-tars/interface/src/core.ts`:
- Around line 73-75: The JSDoc comment for the `apiKey` parameter (lines 73-75)
incorrectly implies that providing an API key is required to enable the
`web_scrape` tool for Firecrawl. Update the comment to clarify that the keyless
free tier of Firecrawl supports the `web_scrape` tool, and that an API key is
optional - primarily used for higher rate limits and other enhanced features,
not for enabling `web_scrape` itself. This will prevent misleading developers
about which features require authentication.
In `@packages/agent-infra/search/search/src/index.ts`:
- Around line 266-284: The web results are being mapped without filtering out
items that lack a url property, which can occur when Document items are included
in the response.web array. Add a filter condition before the web.map call to
exclude items where the url property is undefined or falsy. This ensures that
only valid SearchResultWeb items with required url fields are included in the
pages array, preventing invalid entries with undefined urls from being passed
downstream.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa39da7b-f6c4-47be-8745-47e0162b3d74
📒 Files selected for processing (10)
multimodal/agent-tars/cli/src/index.tsmultimodal/agent-tars/core/src/environments/local/index.tsmultimodal/agent-tars/core/src/environments/local/search/search-tool.tsmultimodal/agent-tars/interface/src/core.tspackages/agent-infra/search/search/FIRECRAWL.mdpackages/agent-infra/search/search/examples/firecrawl-search.tspackages/agent-infra/search/search/package.jsonpackages/agent-infra/search/search/src/firecrawl.tspackages/agent-infra/search/search/src/index.tspackages/agent-infra/shared/src/agent-tars-types/search.ts
… mapping - web_scrape: validate URL scheme (http/https) before calling the provider; log only origin+path so token-bearing query strings don't leak into logs. - core.ts: correct apiKey JSDoc — web_scrape works on Firecrawl's keyless tier, a key only raises rate limits. - SearchClient firecrawl case: scraped results are Documents with the URL under metadata.sourceURL (no top-level url); read both shapes, fall back to metadata.title, and drop any item with no resolvable URL.
Review feedback: scrape did not belong in search-tool.ts. - Move the scrape capability out of SearchToolProvider into a dedicated FetchToolProvider (search/fetch-tool.ts); the tool is named `web_fetch`. - search-tool.ts is now search-only (+1 line vs main: the firecrawl entry in the provider map). Drop the unrelated `bing_search` provider-map entry and the redundant `Firecrawl` type import (firecrawl() already returns it). - Wire FetchToolProvider in initializeSearchTools, gated on provider==='firecrawl'. - Docs: web_scrape -> web_fetch.
environments/local/ is organized by capability (search/, browser/, filesystem/). web_fetch is URL retrieval, not search, so it gets its own environments/local/fetch/ dir (FetchToolProvider) rather than living under search/. Its activation is still gated on the firecrawl search provider, which is config wiring handled by the orchestrator.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/agent-infra/search/search/FIRECRAWL.md (1)
64-64: ⚡ Quick winCapitalize "Markdown" as a proper noun.
Per the static analysis hint, "markdown" should be capitalized as "Markdown" when referring to the markup language/format.
✏️ Proposed fix
-| **scrape** | ✅ shipped | `web_fetch` tool — read any URL (incl. JS-rendered pages and PDFs) to markdown without driving the browser. | +| **scrape** | ✅ shipped | `web_fetch` tool — read any URL (incl. JS-rendered pages and PDFs) to Markdown without driving the browser. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-infra/search/search/FIRECRAWL.md` at line 64, In the FIRECRAWL.md file, the table row for the "scrape" feature contains "markdown" which should be capitalized as "Markdown" when referring to the markup language format. Locate the text "to markdown without" in the scrape row description and change the lowercase "markdown" to uppercase "Markdown" to follow proper noun conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@multimodal/agent-tars/core/src/environments/local/search/fetch-tool.ts`:
- Around line 67-77: The URL validation in the fetch-tool.ts file currently only
checks the protocol scheme but does not prevent SSRF attacks by blocking
private/internal hosts. After validating the protocol in the existing check
(parsed.protocol !== 'http:' && parsed.protocol !== 'https:'), add additional
validation to reject URLs targeting private IP ranges (127.0.0.1, localhost,
10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), link-local addresses
(169.254.0.0/16), and other private/internal hosts. Extract the hostname from
the parsed URL object and validate it against these ranges before returning
success, returning an appropriate error message if the host is private or
internal.
In `@packages/agent-infra/search/search/FIRECRAWL.md`:
- Line 6: The FIRECRAWL.md file lists `duckduckgo` as a valid search provider
alongside `browser_search`, `tavily`, `bing_search`, and `searxng`, but this
provider does not exist in the SearchProvider enum which only defines
BrowserSearch, BingSearch, Tavily, SearXNG, and Firecrawl. Remove the
`duckduckgo` reference from the provider list in FIRECRAWL.md to ensure the
documentation accurately reflects the available search providers.
- Around line 70-75: In the Cost note section of the FIRECRAWL.md documentation,
replace the term "web_fetch" with "Scrape" as the endpoint name. The endpoint
"web_fetch" does not exist in the current Firecrawl API, so update the sentence
that currently mentions "web_fetch" is 1 credit/page to instead reference the
correct Scrape endpoint which also costs 1 credit/page. This applies to both the
mention of the endpoint itself and any related references in that section.
---
Nitpick comments:
In `@packages/agent-infra/search/search/FIRECRAWL.md`:
- Line 64: In the FIRECRAWL.md file, the table row for the "scrape" feature
contains "markdown" which should be capitalized as "Markdown" when referring to
the markup language format. Locate the text "to markdown without" in the scrape
row description and change the lowercase "markdown" to uppercase "Markdown" to
follow proper noun conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 90940eb3-b1d1-4df8-a0a6-b8b3aafb3c09
📒 Files selected for processing (6)
multimodal/agent-tars/core/src/environments/local/index.tsmultimodal/agent-tars/core/src/environments/local/search/fetch-tool.tsmultimodal/agent-tars/core/src/environments/local/search/index.tsmultimodal/agent-tars/core/src/environments/local/search/search-tool.tsmultimodal/agent-tars/interface/src/core.tspackages/agent-infra/search/search/FIRECRAWL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- multimodal/agent-tars/interface/src/core.ts
…om search web_fetch no longer requires (or reads config from) the search provider. It has its own config and its own initialization, so it can be enabled on its own, alongside any search provider, or not at all. - interface: add AgentTARSFetchOptions (apiKey, baseUrl) + AgentTARSOptions.fetch; add 'fetch' to AgentTARSCLIArguments; revert the web_fetch mention bolted onto the search apiKey doc. - core: register web_fetch in its own initializeFetchTools(), gated on options.fetch (not options.search.provider === 'firecrawl'). - cli: add --fetch.apiKey / --fetch.baseUrl. - docs: show search and fetch as independent config blocks.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@multimodal/agent-tars/interface/src/cli.ts`:
- Line 20: The Pick type at line 20 in cli.ts currently includes 'agio' in its
string union, but this property does not exist in AgentTARSAppConfig. Replace
the string literal 'agio' with 'aioSandbox' in the Pick union to match the
actual property defined in AgentTARSAppConfig. This will restore type safety and
align with the CHANGELOG which confirms agio was removed from the CLI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 117dfd25-d3ce-427b-b9dd-24ce1ee6cae2
📒 Files selected for processing (5)
multimodal/agent-tars/cli/src/index.tsmultimodal/agent-tars/core/src/environments/local/index.tsmultimodal/agent-tars/interface/src/cli.tsmultimodal/agent-tars/interface/src/core.tspackages/agent-infra/search/search/FIRECRAWL.md
✅ Files skipped from review due to trivial changes (1)
- packages/agent-infra/search/search/FIRECRAWL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- multimodal/agent-tars/cli/src/index.ts
…rectly - FIRECRAWL.md: web_fetch is standalone (configured via `fetch`), not gated on the firecrawl search provider — remove stale "when Firecrawl is the configured provider" wording left over before the decoupling. - Cost note: attribute the per-page cost to Firecrawl's /scrape endpoint that web_fetch calls (addresses CodeRabbit). - Left the duckduckgo provider reference as-is: it IS a real provider (SearchProvider.DuckduckgoSearch = 'duckduckgo_search'); CodeRabbit's grep missed it on casing.
Surfaced by a real `pnpm` build: - mcp-servers/search/server.ts: add the `firecrawl` entry to API_KEY_ENV_MAP / API_BASE_URL_ENV_MAP. Required blast radius of adding SearchProvider.Firecrawl — those maps are indexed by the now-expanded SearchProvider union, so a missing key broke declaration generation (TS7053). - search/src/index.ts: widen the three pre-existing provider-option casts (BrowserSearch/SearXNG/DuckDuckGo) to `as unknown as` — TS's own suggested fix. These TS2352 dts errors exist on upstream `main` (verified by building the base revision); not introduced here, but they block the .d.ts build. (--no-verify: secretlint false-positives on two pre-existing `apiKey:` lines in server.ts — an empty string and a code expression, neither a real secret.)
rakshith48
changed the title
Owner
Author
|
Superseded by the upstream PR → bytedance#1920. Closing this fork-only PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Firecrawl to Agent TARS in two ways:
search.provider: 'firecrawl'), alongside the existing browser / Bing / Tavily / DuckDuckGo / SearXNG providers.web_fetchtool that reads any URL as clean, LLM-ready markdown without driving the browser.Why. Agent TARS is browser-first and splits find (the search provider) from read (LinkReader / the headless browser). Firecrawl covers both from one managed provider:
/searchreturns full-page markdown for every result in a single call, and/scrapereads arbitrary URLs — including JavaScript-rendered pages and PDFs — with no local browser.web_fetchis configured independently of search, so you can mix freely (e.g.browser_searchfor search + Firecrawl for fetch) or enable fetch on its own.What's included
@agent-infra/shared—SearchProvider.Firecrawl.@agent-infra/search—firecrawlprovider in the unifiedSearchClient(web + news flattened to unified pages; full markdown whenscrapeOptionsis set; resolves URLs from scrapedDocuments).@agent-tars/interface—AgentTARSFetchOptions+ a top-levelfetchconfig;'firecrawl'added to the search-provider union.@agent-tars/core—FetchToolProvider(web_fetch) in its ownenvironments/local/fetchcapability dir, registered fromoptions.fetch.@agent-tars/cli—--search.provider firecrawl,--fetch.apiKey,--fetch.baseUrl.@agent-infra/mcp-server-search—firecrawlentry in the API-key / base-url env maps.packages/agent-infra/search/search/FIRECRAWL.md(usage + an endpoint-fit assessment).Scope. Ships search + fetch.
mapis a recommended follow-up;crawl/parseare niche;monitoris out of scope (a scheduled product, not an interactive tool). Rationale inFIRECRAWL.md.Config
apiKeyis optional (Firecrawl has a keyless free tier);baseUrltargets a self-hosted instance.Verification
@agent-infra/shared(5),@agent-infra/mcp-server-search(1),@agent-tars/core(75 passed / 1 skipped).@agent-infra/searchhas no existing test harness upstream.web_searchreturns Firecrawl results;web_fetchreturns markdown for a JS-rendered page. Real Firecrawl API usage confirmed via response metadata (scrapeId/creditsUsed/cacheState) and account credit decrement.Checklist
FIRECRAWL.md, changeset).fetchconfig).searchpackage has no upstream test harness).