Skip to content

[codex] provider docs, SearXNG integration, and repo hardening - #2

Merged
Coldaine merged 6 commits into
feat/multi-provider-capabilitiesfrom
feat/provider-docs
Apr 12, 2026
Merged

[codex] provider docs, SearXNG integration, and repo hardening#2
Coldaine merged 6 commits into
feat/multi-provider-capabilitiesfrom
feat/provider-docs

Conversation

@Coldaine

@Coldaine Coldaine commented Apr 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

This stacks feat/provider-docs on top of feat/multi-provider-capabilities and packages the follow-on work into a reviewable branch.

What Changed

  • add authoritative provider documentation under docs/providers/
  • add docs/CAPABILITY_MATRIX.md and a SearXNG adoption plan under docs/plans/
  • add SearXNG provider support and explicit endpoint-based configuration
  • add shared runtime seams for provider metadata, HTTP behavior, and execution backend boundaries
  • add CI, issue templates, PR template, and doc coverage tests
  • clean up stale duplicate files and move optional SearXNG Docker assets under docker/searxng/

Why

  • make provider overlap and current coverage explicit in docs
  • keep provider-specific detail out of high-level docs and centralize comparison truth in the matrix
  • prepare the CLI-first codebase for future remote execution without coupling to localhost or local containers
  • harden runtime behavior around provider validation, request handling, and documentation drift

User / Developer Impact

  • coldsearch is now the primary CLI name, with usearch retained as a compatibility alias
  • SearXNG is available as a provider with explicit base URL configuration
  • provider docs and matrix now exist as source-of-truth references for capability support
  • repo governance now requires docs/tests to stay aligned with provider and runtime changes

Validation

  • npm test
  • npm run typecheck

Notes

  • This PR targets feat/multi-provider-capabilities intentionally because it is a stacked follow-on branch, not a direct PR to main.
  • The repo still contains optional SearXNG Docker assets, but local-container assumptions remain out of scope for icarus-laptop.

Summary

This PR delivers comprehensive provider documentation, SearXNG integration, and repository hardening for ColdSearch. It adds authoritative provider reference materials and runtime abstractions for future remote execution, tightens HTTP/request handling and agent safety, and updates CLI/package identity to coldsearch while retaining usearch as a compatibility alias.

Major Changes

  • Project rebranding:

    • CLI renamed from usearchcoldsearch (compat alias usearch retained).
    • npm package renamed usearchcoldsearch.
    • Config path canonicalized to ~/.config/coldsearch/config.toml with fallback to legacy ~/.config/usearch/config.toml.
    • New src/app.ts exports APP_NAME/APP_VERSION/APP_USER_AGENT and dynamic version string formatting.
  • Provider documentation & registry:

    • Added authoritative docs under docs/providers/ for Tavily, Firecrawl, Exa, Brave, Serper, Jina, SearXNG.
    • Added docs/CAPABILITY_MATRIX.md, docs/providers/README.md, and docs/plans/searxng.md.
    • New src/providers.ts registry with ProviderMetadata, list/get/create utilities, providerSupportsCapability validation, and ProviderName type.
  • SearXNG integration:

    • New adapter src/adapters/searxng.ts (capability: search).
    • Requires providers.searxng.options.baseUrl or env SEARXNG_BASE_URL; no localhost assumptions.
    • Optional Docker assets moved to docker/searxng/ (compose + settings).
  • Runtime seams & execution backend:

    • New ExecutionBackend interface and LocalExecutionBackend in src/execution/backend.ts to decouple CLI from FanoutEngine and prepare for remote backends.
    • FanoutEngine now validates provider existence and capability before selection.
  • Shared HTTP/request behavior:

    • Added src/http.ts with fetchWithPolicy, fetchJson, fetchText, RequestPolicy and HTTPRequestError (timeouts, retries, labeled errors, safe body capture).
  • Adapter and types refactor:

    • Added CapabilityName, AdapterCallOptions, CrawlCallOptions in src/types.ts.
    • All adapters updated to accept options param: search(query, apiKey, options?), extract(url, apiKey, options?), crawl(...options?).
    • Adapters moved to use fetchJson/fetchText and tightened capability typings.
    • Adapters registry moved to provider registry usage (createRegisteredAdapter), and adapters index re-exports SearXNGAdapter.
  • Agent refactor:

    • Agent tool protocol moved from regex/plain-text tool calls to strict JSON payloads ({ type: "tool", tool, args } and { type: "final", answer }) with parseAgentPayload.
    • Agent execution backend switched to ExecutionBackend (default LocalExecutionBackend).
    • URL fetch validation tightened (hostname/IP blocking, DNS checks); fetches use fetchWithPolicy and APP_USER_AGENT.
    • Agent now uses validateFetchUrl + fetchValidatedBody to avoid localhost/container coupling and to cap response bodies.
  • CLI and runtime changes:

    • CLI routes search/extract/crawl through LocalExecutionBackend.
    • Version output uses dynamic formatVersionString().
    • Removed legacy single-provider run path and earlier local-container assumptions in code.
    • Config resolution behavior changed: resolveConfigPath prefers provided path, otherwise uses default only if exists, else falls back to legacy path; resolveKeyRef/env: indirection removed and getNextKey exported removed.
  • CI, templates, and governance:

    • Added .github/workflows/ci.yml (Node 20, typecheck, test).
    • Issue templates: provider-addition and hardening forms; PR template with capability/docs/test checklist.
    • .gitattributes enforces LF line endings.
    • Added issue/PR templates and doc-coverage tests.
  • Tests:

    • Added tests: agent payload parsing, CLI version sync, providers docs coverage, runtime seam checks (including SearXNG baseUrl enforcement).
  • Documentation consolidation:

    • Moved architecture/progress/plan material into docs/ and removed several legacy root docs (PLAN.md, TASK.md, root PROGRESS.md, root architecture.md, legacy SKILL/TASK files).
    • Updated README, SKILL, and various docs to reflect coldsearch naming, SearXNG examples, and new architecture framing.

Key Behavioral Changes

  • Config resolution prefers new default path, falls back to legacy path if missing.
  • FanoutEngine enforces provider capability support before selecting providers.
  • Key selection adapted to support keyless providers (getNextKeyOrEmpty).
  • HTTP requests across adapters use centralized timeout/retry policy and produce HTTPRequestError with URL/status/body on failure.
  • Agent interactions require structured JSON payloads rather than ad-hoc text parsing.
  • Provider adapters receive per-provider options via AdapterCallOptions (providerOptions).

Not In Scope

  • Implementing a remote execution backend (only seams/abstractions added).
  • Local-container assumptions for icarus-laptop remain out of scope.

Example config snippet

[capabilities.search]
providers = ["searxng", "tavily", "exa", "brave", "serper"]

[providers.searxng]
[providers.searxng.keyPool]
keys = []
strategy = "round-robin"
[providers.searxng.options]
baseUrl = "https://search.example.internal" # or set SEARXNG_BASE_URL env var

Add detailed documentation for all 6 providers:
- Tavily (7 tools)
- Firecrawl (8 tools)
- Exa (6 tools)
- Brave (7 tools)
- Serper (11 tools)
- Jina (6 tools)

Also add CAPABILITY_MATRIX.md for cross-provider comparison.

Total: 45+ tools documented across the full API surface.

This documents the gap between current implementation (3 generic tools)
and the full provider API surface that needs to be implemented.
@coderabbitai

coderabbitai Bot commented Apr 12, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Renames the project to ColdSearch, centralizes HTTP with timeout/retry policies, adds a provider registry and SearXNG adapter, introduces an ExecutionBackend (LocalExecutionBackend) atop FanoutEngine, refactors adapters to use shared HTTP helpers, and updates docs, CI, templates, and tests.

Changes

Cohort / File(s) Summary
Project identity & entrypoints
package.json, src/app.ts, src/index.ts
Rename package to coldsearch, add APP_* constants and formatVersionString, expose coldsearch CLI while retaining usearch alias, re-export new modules.
Central HTTP layer
src/http.ts
New request policy utilities: fetchWithPolicy, fetchJson, fetchText, HTTPRequestError, and RequestPolicy (timeouts, retries, backoff, abort handling).
Provider registry & types
src/providers.ts, src/types.ts
Add provider registry, ProviderMetadata, utilities (list/get/create), introduce CapabilityName, AdapterCallOptions, CrawlCallOptions, and update SearchAdapter signatures and capability typing.
Execution backend & engine integration
src/execution/backend.ts, src/engine/fanout.ts
Add ExecutionBackend interface and LocalExecutionBackend wrapping FanoutEngine; FanoutEngine validates provider capability support and forwards providerOptions to adapters.
Adapter refactor & SearXNG adapter
src/adapters/... (brave.ts,exa.ts,serper.ts,tavily.ts,jina.ts,firecrawl.ts,searxng.ts), src/adapters/index.ts
Refactor adapters to use shared fetchJson/fetchText, tighten capability typings, extend method signatures to accept options, add SearXNGAdapter, and delegate adapter creation to provider registry.
Agent & LLM tool parsing
src/agent/agent.ts, src/agent/tools.ts, src/agent/llm.ts
Switch agent tool protocol to JSON parseAgentPayload; agent now accepts an executionBackend (defaults to LocalExecutionBackend); LLM clients use fetchJson and include APP_USER_AGENT; added stricter URL fetch validation for agent tool calls.
HTTP/Network hardening & helpers usage
src/http.ts, src/resolvers/bws.ts, various adapters
Adapters and LLM/agents replaced ad-hoc fetch/error handling with centralized fetch helpers; Bitwarden resolver uses APP_USER_AGENT and guards concurrent init.
Config & key management
src/config.ts, src/engine/keypool.ts
Remove resolveKeyRef/getNextKey exports and env-indirection; add config-path probing with legacy fallback and export LEGACY_CONFIG_PATH; keypool JSDoc clarifies process-local rotation.
CLI & runtime behavior
src/cli.ts, src/app.ts, tests test/cli-version.test.mjs, test/runtime-seams.test.mjs
CLI routes to LocalExecutionBackend, uses dynamic version string and APP_NAME/compat alias, removed legacy single-provider path; tests added for CLI version and runtime seam checks (provider capability, SearXNG baseUrl).
Docs, governance & provider pages
README.md, SKILL.md, docs/*, docs/providers/*, docs/CAPABILITY_MATRIX.md, docs/plans/searxng.md
Extensive documentation updates: rename to ColdSearch, add provider docs, capability matrix, SearXNG plan, architecture/progress updates, and provider governance rules.
CI, templates, examples & Docker
.gitattributes, .github/workflows/ci.yml, .github/ISSUE_TEMPLATE/*, .github/pull_request_template.md, config.example.toml, docker/searxng/*
Add LF enforcement, Node 20 CI (install, typecheck, test), issue/PR templates, update example config to include SearXNG, add Docker Compose and SearXNG settings.
Tests for parsing & docs consistency
test/agent-payloads.test.mjs, test/providers-docs.test.mjs
Add unit tests for parseAgentPayload behavior and provider docs/README/CAPABILITY_MATRIX consistency.
Removed legacy planning files
PLAN.md, TASK.md, files (4)/*
Delete older plan/progress/skill/arch files in favor of consolidated docs/ artifacts.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as CLI
    participant Backend as LocalExecutionBackend
    participant Engine as FanoutEngine
    participant Registry as Provider\ Registry
    participant Adapter as SearchAdapter
    participant HTTP as fetchJson / fetchText
    participant API as Provider\ API

    User->>CLI: coldsearch search "query"
    CLI->>Backend: search("query", options)
    Backend->>Engine: engine.search("query", options)
    Engine->>Registry: getProvidersForCapability("search")
    Registry-->>Engine: ["searxng","tavily",...]
    Engine->>Engine: validate provider capabilities
    Engine->>Adapter: adapter.search(query, apiKey, {providerOptions})
    Adapter->>HTTP: fetchJson(url, init, {label, timeout, retries})
    HTTP->>API: HTTP request (timeout/retry)
    API-->>HTTP: JSON response
    HTTP-->>Adapter: parsed JSON
    Adapter-->>Engine: NormalizedResult[]
    Engine-->>Backend: {results, providersUsed, errors}
    Backend-->>CLI: {results, providersUsed, errors}
    CLI-->>User: display results
Loading
sequenceDiagram
    participant LLM as LLM\ API
    participant Agent as Agent
    participant Parser as parseAgentPayload()
    participant Backend as LocalExecutionBackend
    participant Tools as Tool\ Executor

    LLM->>Agent: model response (text)
    Agent->>Parser: parseAgentPayload(text)
    alt Valid JSON payload
        Parser-->>Agent: AgentPayload (tool | final)
        alt Tool Call
            Agent->>Backend: execute tool via backend
            Backend-->>Agent: tool results
            Agent->>LLM: continue with tool results
        else Final Answer
            Agent-->>Agent: return final answer
        end
    else Invalid JSON
        Parser-->>Agent: null
        Agent->>LLM: request corrected JSON
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐰 A hop for the code, a tidy refactor leap,

ColdSearch wakes where old usearch used to sleep.
Fetches now honor timeouts, retries, and scope,
Registry, backend, adapters—each in its hope.
Docs, tests, and docker—small carrots for the heap.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically summarizes the main changes: adding provider documentation, SearXNG integration support, and repository hardening measures.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/provider-docs

Comment @coderabbitai help to get the list of available commands and usage tips.

@Coldaine
Coldaine marked this pull request as ready for review April 12, 2026 17:11
Copilot AI review requested due to automatic review settings April 12, 2026 17:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR formalizes provider documentation and governance, introduces a SearXNG provider (endpoint-configured), and hardens the runtime with shared seams (provider registry, HTTP request policy, execution backend boundary) while migrating the CLI branding to coldsearch with usearch as an alias.

Changes:

  • Added authoritative provider docs (docs/providers/*), a required capability matrix, and a SearXNG adoption plan with doc-coverage tests.
  • Implemented SearXNG adapter + provider registry, plus a shared HTTP request layer and a local execution backend seam.
  • Hardened repo automation: CI workflow, PR/issue templates, and real tests (runtime seams, CLI version, agent payload parsing).

Reviewed changes

Copilot reviewed 61 out of 63 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/runtime-seams.test.mjs Adds regression tests for provider capability validation and SearXNG baseUrl requirement.
test/providers-docs.test.mjs Ensures provider registry ↔ docs/matrix/index stay in sync.
test/cli-version.test.mjs Verifies --version output matches package metadata.
test/agent-payloads.test.mjs Tests structured JSON payload parsing for agent tool/final outputs.
TASK.md Removes legacy Phase 1 task doc (now superseded by docs structure).
src/types.ts Introduces CapabilityName and adapter call option plumbing.
src/resolvers/bws.ts Uses shared user-agent and makes init() concurrency-safe with a promise latch.
src/providers.ts Adds provider registry + metadata + capability checks + adapter factory.
src/index.ts Updates package exports to include app/providers/http/execution backend.
src/http.ts Adds shared fetch policy with timeouts/retries and JSON/text helpers.
src/execution/backend.ts Introduces execution backend interface and local backend implementation.
src/engine/keypool.ts Corrects thread-safety claims and adds getNextKeyOrEmpty() for keyless providers.
src/engine/fanout.ts Validates provider capability support + passes provider options into adapter calls.
src/config.ts Adds coldsearch config path + legacy fallback + improves config-not-found messaging.
src/cli.ts Migrates CLI to coldsearch naming, uses backend seam, updates --version and help text.
src/app.ts Centralizes app name/version/user-agent and config dir names.
src/agent/tools.ts Replaces regex tool parsing with structured JSON payload parsing.
src/agent/llm.ts Routes LLM calls through shared HTTP helpers and normalizes token usage fields.
src/agent/agent.ts Switches agent to backend seam + structured payload loop + shared fetch policy.
src/adapters/tavily.ts Replaces SDK usage with direct HTTP calls via shared request layer.
src/adapters/serper.ts Uses shared fetchJson and normalizes position-based scoring more robustly.
src/adapters/searxng.ts Adds SearXNG adapter with explicit baseUrl configuration and normalized results.
src/adapters/jina.ts Moves extraction to shared fetchText and updates adapter signature.
src/adapters/index.ts Delegates adapter creation/listing to provider registry.
src/adapters/firecrawl.ts Moves to shared fetchJson; improves crawl polling error handling.
src/adapters/exa.ts Moves to shared fetchJson for search/extract calls.
src/adapters/brave.ts Moves to shared fetchJson for search calls.
SKILL.md Updates skill to coldsearch naming and config path.
README.md Rewrites README around ColdSearch direction + doc sources of truth + SearXNG config.
PROGRESS.md Removes root-level redirect stub (progress now lives under docs).
PLAN.md Removes legacy implementation plan (replaced by docs/plans).
package.json Renames package to coldsearch, adds coldsearch bin, and enables real tests/typecheck.
package-lock.json Updates lock metadata + bin entries to include coldsearch.
files (4)/SKILL.md Removes stale duplicate doc copy.
files (4)/PROGRESS.md Removes stale duplicate doc copy.
files (4)/config.example.toml Removes stale duplicate doc copy.
files (4)/CLAUDE.md Removes stale duplicate doc copy.
files (4)/architecture.md Removes stale duplicate doc copy.
files (4).zip Adds archived bundle of removed duplicates.
docs/providers/tavily.md Adds detailed Tavily provider reference page.
docs/providers/serper.md Adds detailed Serper provider reference page.
docs/providers/searxng.md Adds detailed SearXNG provider reference page.
docs/providers/README.md Adds provider docs index + update rule tying registry/matrix/plans together.
docs/providers/jina.md Adds detailed Jina provider reference page.
docs/providers/firecrawl.md Adds detailed Firecrawl provider reference page.
docs/providers/exa.md Adds detailed Exa provider reference page.
docs/providers/brave.md Adds detailed Brave provider reference page.
docs/PROGRESS.md Updates progress to current “state/deferrals/priorities” framing.
docs/plans/searxng.md Adds explicit SearXNG adoption plan and constraints.
docs/NORTH_STAR.md Updates “North Star” to reflect ColdSearch + hybrid execution direction.
docs/KEY_MANAGEMENT.md Updates config path references to coldsearch.
docs/CAPABILITY_MATRIX.md Adds authoritative provider/capability comparison matrix.
docs/BWS_INTEGRATION.md Updates branding references to ColdSearch.
docs/architecture.md Refactors architecture doc around seams/layers/backends and governance docs.
docker/searxng/settings.yml Adds optional SearXNG config for operator-managed deployment.
docker/searxng/docker-compose.yml Adds optional Docker compose for SearXNG.
config.example.toml Updates example config to include SearXNG + coldsearch paths and examples.
architecture.md Removes root-level redirect stub (architecture now under docs).
.github/workflows/ci.yml Adds CI pipeline for typecheck + tests.
.github/pull_request_template.md Adds PR checklist tied to docs/matrix/plans/tests.
.github/ISSUE_TEMPLATE/provider-addition.yml Adds issue template for provider adoption work.
.github/ISSUE_TEMPLATE/hardening.yml Adds issue template for runtime hardening work.
.gitattributes Normalizes Markdown line endings to LF.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/adapters/searxng.ts Outdated
Comment on lines +33 to +36
const searchUrl = new URL("/search", baseUrl);
searchUrl.searchParams.set("q", query);
searchUrl.searchParams.set("format", "json");

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new URL("/search", baseUrl) will drop any path component from baseUrl (e.g., https://host/searxng/ becomes https://host/search). If SearXNG is served under a subpath, requests will go to the wrong endpoint. Consider constructing the URL relative to the configured base (e.g., using new URL("search", ensureTrailingSlash(baseUrl))) so both root-hosted and subpath-hosted deployments work.

Copilot uses AI. Check for mistakes.
services:
searxng:
container_name: searxng
image: searxng/searxng:latest

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compose file pins the SearXNG image to :latest, which makes deployments non-reproducible and can introduce breaking changes unexpectedly. Pin to a specific tag/digest (and document the expected version) so the optional Docker assets remain stable over time.

Suggested change
image: searxng/searxng:latest
# Pin to a specific SearXNG release so deployments remain reproducible.
image: searxng/searxng:2024.6.21

Copilot uses AI. Check for mistakes.
Comment thread src/engine/fanout.ts
Comment on lines 253 to +260
query: string
): Promise<ProviderResult> {
try {
const apiKey = await keyPoolManager.getNextKey(provider);
const apiKey = await keyPoolManager.getNextKeyOrEmpty(provider);
const adapter = createAdapter(provider);
const results = await adapter.search(query, apiKey);
const results = await adapter.search(query, apiKey, {
providerOptions: this.config.providers[provider]?.options,
});

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNextKeyOrEmpty() returns an empty string when a provider has no keys configured. That’s needed for truly keyless providers (e.g., SearXNG), but it also means key-required providers with an empty/misconfigured key pool will fail later with a provider 401/403 instead of a clear configuration error. Consider failing fast when keyPool.keys is empty for providers that require credentials (e.g., via a requiresApiKey/keyless flag in the provider registry) so users get an actionable config error.

Copilot uses AI. Check for mistakes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 068b9a0fb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/http.ts
retryDelayMs?: number;
}

const DEFAULT_TIMEOUT_MS = 10000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Increase default HTTP timeout for long-running API calls

DEFAULT_TIMEOUT_MS is set to 10s for every fetchWithPolicy call, and most callers (including ClaudeClient.complete and OpenAIClient.complete) do not override it, so normal LLM completions that take longer than 10 seconds will fail with timeout errors even when the upstream API is healthy. This introduces a broad runtime regression for agent mode and any slower provider responses; the default should be higher or specialized per call site.

Useful? React with 👍 / 👎.

Comment thread src/adapters/searxng.ts Outdated
);
}

const searchUrl = new URL("/search", baseUrl);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve configured SearXNG base path when building URL

Using new URL("/search", baseUrl) always resets the path to the domain root, so a configured base URL like https://host/searxng is silently rewritten to https://host/search. In deployments where SearXNG is served behind a subpath (common with reverse proxies), requests will hit the wrong endpoint and fail; build the search URL relative to the configured path instead of forcing an absolute /search.

Useful? React with 👍 / 👎.

coderabbitai[bot]
coderabbitai Bot previously requested changes Apr 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
SKILL.md (1)

1-30: ⚠️ Potential issue | 🟠 Major

The guide still defaults to legacy usearch commands after the rename.

These updated lines introduce coldsearch, but most command examples and requirement/troubleshooting references remain usearch/old path. Please standardize examples on coldsearch and keep a single explicit alias note for usearch.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@SKILL.md` around lines 1 - 30, The docs were partially updated to
"coldsearch" but still use legacy "usearch" command examples and references;
update all examples, troubleshooting, requirements, and command snippets to
consistently use "coldsearch" (including commands like "coldsearch --agent",
"coldsearch extract", "coldsearch crawl") and add a single explicit note that
"usearch" is an alias for "coldsearch" if desired; ensure the SKILL title and
all CLI examples, install/usage lines, and any path or troubleshooting
references are changed to the new name so there are no mixed mentions of
"usearch".
src/adapters/firecrawl.ts (1)

132-155: ⚠️ Potential issue | 🟠 Major

Don't collapse permanent poll failures into a timeout.

The blanket catch { continue; } hides non-transient failures during crawl polling, so a bad token, invalid job ID, or schema regression can burn the full polling window and surface only as "Firecrawl crawl timed out". That makes real outages much harder to diagnose.

💡 Proposed fix
-      } catch {
-        continue;
+      } catch (error) {
+        if (
+          error instanceof HTTPRequestError &&
+          error.status !== undefined &&
+          error.status < 500 &&
+          error.status !== 429
+        ) {
+          throw error;
+        }
+        continue;
       }
 
+      if (statusData.success === false || statusData.error) {
+        throw new Error(`Firecrawl crawl failed: ${statusData.error || "Unknown error"}`);
+      }
+
       if (statusData.status === "completed" && statusData.data) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapters/firecrawl.ts` around lines 132 - 155, The try/catch around
fetchJson in the poll loop (where fetchJson is called with
`${this.baseUrl}/crawl/${jobId}` using apiKey and assigning statusData) swallows
all errors and treats them as transient; change this so non-transient errors
(e.g., HTTP 4xx like 401/403, 404, or schema/validation errors returned in the
response) are surfaced immediately instead of continuing the loop: inspect the
thrown error or response inside the catch from fetchJson, and rethrow or throw a
new Error including the status/code/message for permanent failures (auth/invalid
job/schema), while only continuing on true network/timeouts/retryable errors;
ensure the thrown errors include context (jobId, apiKey masked if needed) and
keep transient handling (continue) for network timeouts only.
src/adapters/tavily.ts (1)

62-68: ⚠️ Potential issue | 🟡 Minor

Clamp fallback search scores to a valid floor.

Line 67 can generate negative fallback scores when result index is high, which can skew downstream ranking logic.

💡 Proposed fix
-      score: result.score ?? (1 - index * 0.1),
+      score: result.score ?? Math.max(0.1, 1 - index * 0.1),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapters/tavily.ts` around lines 62 - 68, The fallback score calculation
in the mapping over response.results (inside the Tavily adapter) can produce
negative values for high indexes; update the score assignment (the property
'score' in the map return) to clamp the fallback to a valid floor (e.g., use
Math.max(0, 1 - index * 0.1) or a clamp to [0,1]) so that scores never go below
0 while keeping existing result.score when present; adjust in the function that
maps response.results in src/adapters/tavily.ts where 'title', 'url', 'snippet',
'score', and 'source: this.name' are set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitattributes:
- Around line 1-2: The .gitattributes currently only enforces LF for Markdown;
update the .gitattributes file to broaden coverage by adding common
TypeScript/Node text patterns (e.g., *.ts, *.tsx, *.js, *.jsx, *.json, *.yml,
*.yaml, *.md, package.json, *.env, Dockerfile, *.html, *.css, *.scss) and set
them to text eol=lf so line endings are normalized across platforms; modify the
existing entries (the lines shown in .gitattributes) to include these patterns
and ensure binary files remain unmodified by not marking known binaries as text.

In @.github/pull_request_template.md:
- Line 1: The template currently starts with "## Summary" which violates MD041;
change the first line to an H1 by replacing "## Summary" with "# Summary" (or
otherwise ensure the very first line is a top-level heading) so the file's
initial heading uses a single leading '#' to satisfy MD041.

In `@docker/searxng/settings.yml`:
- Around line 12-13: The placeholder secret_key in docker/searxng/settings.yml
(the line secret_key: "replace-me-for-production") should be made explicitly
dangerous: add a clear warning comment immediately above that line stating this
value must be replaced for production, must be a securely generated long random
secret (recommend >=32-64 bytes/base64), must not be checked into source
control, and suggest using an environment variable or secrets manager; update
documentation comment to show how to supply a real secret (reference secret_key
and settings.yml) and leave limiter unchanged.

In `@docs/BWS_INTEGRATION.md`:
- Line 5: The doc currently mixes the command/name "ColdSearch" with
instructions using "usearch"; update all command examples, configuration keys,
and CLI invocations to use "coldsearch" as the primary name (e.g., replace
occurrences of the `usearch` command, config keys, and paths with `coldsearch`)
and add a single note near the top stating that `usearch` is supported as a
compatibility alias for legacy users; ensure references like CLI examples,
environment variable names, and file paths consistently use `coldsearch` while
preserving a short sentence that `usearch` can be used as an alias.

In `@docs/KEY_MANAGEMENT.md`:
- Line 10: Normalize all references in KEY_MANAGEMENT.md to a single canonical
path and command (replace mixed uses of "~/.config/usearch/..." and "usearch"
with the chosen primary path "~/.config/coldsearch/config.toml" and primary
command if applicable), and add one short sentence describing alias/fallback
behavior (e.g., that prior "~/.config/usearch" locations or "usearch" CLI will
be recognized as aliases or migrated) so readers aren’t left with split
instructions; update every occurrence of the old path/command in the document
and include a single "alias/fallback" note near the top or the config section to
explain migration behavior.

In `@docs/providers/firecrawl.md`:
- Around line 264-297: Update the Rate Limits and Pricing (Credits) sections to
reflect the current official Firecrawl values: change Standard concurrent
requests to 50, Growth to 100, Scale to 150, and verify pricing tiers against
the official pricing page; add billing frequency notes (monthly vs yearly
discounts) and adjust any credit amounts if they differ on the official pricing
page. Add source links to the three official references
(https://docs.firecrawl.dev/rate-limits, https://www.firecrawl.dev/pricing,
https://docs.firecrawl.dev/billing) next to the Rate Limits and Pricing headings
and include an "as of [YYYY-MM-DD]" timestamp to indicate when the data was
verified; also remove or flag the P95 latency (3.4s) claim unless you can cite
the docs—if uncited, change it to "P95 latency: see source" with the link.
Ensure you update the headings "Rate Limits", "Pricing (Credits)" and the
“Notes” section (where P95 latency appears) accordingly.

In `@docs/providers/serper.md`:
- Around line 25-253: The markdown has repeated MD058/MD031 spacing violations
around tables and fenced code blocks (e.g., sections documenting
serper_image_search, serper_news_search, serper_video_search,
serper_maps_search, serper_places_search, serper_scholar_search,
serper_patents_search, serper_autocomplete, serper_get_reviews and the initial
parameter table); fix by ensuring a blank line exists before and after every
table and before and after every fenced code block (triple-backtick blocks), and
normalize the example blocks to include the language tag (```json) followed by a
blank line then the JSON and a closing blank line before ``` so all tables and
examples conform to the suggested pattern and resolve MD058/MD031.
- Around line 276-303: Update the "## Rate Limits" and "## Pricing" sections to
correct the incorrect free-tier claim and clarify tiered limits and prices:
remove the "300 queries/second" attribution from the Free tier (it belongs to
the Ultimate plan), state that the Free tier (2,500 queries) has no specified
rate limit, and change the pricing line to show correct per-tier starting prices
(e.g., Starter $1.00/1k, Ultimate $0.30/1k) with explicit mention which price
applies to which tier; add an "as of" date (e.g., "as of April 2026") and
include citations to the official serper.dev pages for rate limits and pricing
in the doc near these headings so readers can verify current values.

In `@docs/providers/tavily.md`:
- Around line 26-172: The markdown is failing lint rules MD058/MD031 because
several tables and fenced code blocks (e.g., the parameter tables under headings
like `tavily_web_search`, `tavily_news_search`, `tavily_extract`, `tavily_map`,
`tavily_crawl`, `tavily_answer`, `tavily_research` and the JSON examples) are
not surrounded by blank lines; go through the file and ensure there is an empty
line before and after every table and every fenced code block, ensure code
fences use proper triple backticks with a language (e.g., ```json) and remove
stray/incorrect fencing, and apply the same spacing fixes to all examples and
tables shown in the diff.

In `@src/adapters/jina.ts`:
- Around line 18-20: The search method in src/adapters/jina.ts currently
declares a return type of Promise<never[]> which misleadingly suggests an
empty-array result; update the async function signature for search(_query:
string, _apiKey: string) to return Promise<never> to accurately represent that
it always throws (or alternatively change it to Promise<NormalizedResult[]> if
you prefer to preserve the interface contract), ensuring the function name
search is the target of the change.

In `@src/adapters/searxng.ts`:
- Around line 22-35: The code uses new URL("/search", baseUrl) which discards
any configured pathname (e.g., /searxng/); instead construct the search URL
relative to the provided base so the base path is preserved: parse
configuredBaseUrl/baseUrl into a URL object (referencing configuredBaseUrl and
baseUrl), then create the search URL with a relative segment (e.g., "search" or
"./search") so it appends to the existing pathname (referencing searchUrl and
the new URL creation) before setting searchParams; update the searchUrl creation
accordingly to preserve reverse-proxied pathnames.

In `@src/adapters/tavily.ts`:
- Around line 116-117: The assignment to limit (const limit = options?.limit ??
10) doesn't validate non-integer or non-positive values before it's used for URL
slicing (where urls.slice(0, limit) is called); update the logic that computes
limit to coerce and validate options.limit into a positive integer (e.g.,
parse/Number it, fallback to 10 if NaN/undefined, Math.floor or Number.isInteger
check, and clamp to a minimum of 1) and then use that sanitized limit where
urls.slice(0, limit) (or similar slicing) is performed to avoid
empty/negative/partial slices and inconsistent crawl behavior.

In `@src/agent/agent.ts`:
- Line 2: SearchAgent is hard-wired to LocalExecutionBackend; change it to
depend on the ExecutionBackend interface instead by adding an
ExecutionBackend-typed field to SearchAgent and accepting an implementation via
the constructor (or a setter) so callers can inject LocalExecutionBackend or a
remote/test double; remove direct instantiation of LocalExecutionBackend inside
SearchAgent (references where new LocalExecutionBackend() appears) and replace
uses with the injected this.executionBackend; ensure imports reference the
ExecutionBackend type and update any places in SearchAgent (including the block
around the other instantiation occurrences) that construct or assume a concrete
backend to use the injected backend instead.
- Around line 179-186: The fetchContent method currently calls fetchWithPolicy
on model-supplied URLs without validation; parse the input URL in fetchContent
(use the URL constructor) and reject any non-http/https schemes, then reject
hostnames that are localhost/loopback (::1, 127.0.0.0/8), RFC1918 private ranges
(10/8, 172.16/12, 192.168/16), link-local (169.254/16), fc00::/7, or known cloud
metadata endpoints (e.g., 169.254.169.254 and common provider hostnames); if the
hostname is an IP string use net.isIP and range checks, otherwise resolve the
hostname (dns.lookup) and verify the resolved IPs are public before calling
fetchWithPolicy; return an error or throw when validation fails so
fetchWithPolicy is never called with internal/private targets.

In `@src/agent/llm.ts`:
- Around line 136-138: The expression assigning totalTokens in the llm module
has ambiguous grouping due to mixing ?? and + (see totalTokens and
data.usage.total_tokens); explicitly parenthesize the fallback calculation so
the intent is clear—wrap the entire fallback sum in parentheses (e.g., (
(data.usage.prompt_tokens ?? 0) + (data.usage.completion_tokens ?? 0) )) as the
right-hand operand of the outer ?? to ensure clear operator precedence and
readability.

In `@src/agent/tools.ts`:
- Around line 138-142: The current branch that returns a final answer should
reject answers that are only whitespace: replace the existing check
(payload.type === "final" && typeof payload.answer === "string") with one that
trims and verifies non-empty content (e.g., typeof payload.answer === "string"
&& payload.answer.trim().length > 0) and, if the trimmed string is empty, reject
that payload (throw an error or return an appropriate non-final error/handled
response) instead of returning a blank final; update the code in the
payload.type === "final" handling to use payload.answer.trim() for both
validation and the returned answer.

In `@src/config.ts`:
- Around line 24-39: The resolveConfigPath function inefficiently calls
readFileSync just to test existence; change it to use fs.existsSync or
fs.accessSync to probe DEFAULT_CONFIG_PATH (instead of reading its contents) so
you avoid allocating the file contents before loadConfig re-reads it. Update
resolveConfigPath to return DEFAULT_CONFIG_PATH when the probe succeeds, fall
back to LEGACY_CONFIG_PATH on ENOENT, and let loadConfig continue to read the
file as before; reference resolveConfigPath, loadConfig, DEFAULT_CONFIG_PATH and
LEGACY_CONFIG_PATH when making the change.

In `@src/http.ts`:
- Around line 35-42: Update shouldRetryError to distinguish caller-initiated
aborts by checking the caller's AbortSignal (e.g., a passed-in signal or
options.signal) or exposing an externalAborted flag: only return true for
AbortError when the caller signal is not aborted (meaning it was an internal
timeout); if the caller signal is aborted, return false and ensure the request
logic's catch block (the code that currently maps errors to a timeout message)
immediately rethrows or throws a distinct AbortError for caller cancellation
instead of allowing a retry. Reference: shouldRetryError, HTTPRequestError, and
the catch logic that converts errors to timeout messages.

---

Outside diff comments:
In `@SKILL.md`:
- Around line 1-30: The docs were partially updated to "coldsearch" but still
use legacy "usearch" command examples and references; update all examples,
troubleshooting, requirements, and command snippets to consistently use
"coldsearch" (including commands like "coldsearch --agent", "coldsearch
extract", "coldsearch crawl") and add a single explicit note that "usearch" is
an alias for "coldsearch" if desired; ensure the SKILL title and all CLI
examples, install/usage lines, and any path or troubleshooting references are
changed to the new name so there are no mixed mentions of "usearch".

In `@src/adapters/firecrawl.ts`:
- Around line 132-155: The try/catch around fetchJson in the poll loop (where
fetchJson is called with `${this.baseUrl}/crawl/${jobId}` using apiKey and
assigning statusData) swallows all errors and treats them as transient; change
this so non-transient errors (e.g., HTTP 4xx like 401/403, 404, or
schema/validation errors returned in the response) are surfaced immediately
instead of continuing the loop: inspect the thrown error or response inside the
catch from fetchJson, and rethrow or throw a new Error including the
status/code/message for permanent failures (auth/invalid job/schema), while only
continuing on true network/timeouts/retryable errors; ensure the thrown errors
include context (jobId, apiKey masked if needed) and keep transient handling
(continue) for network timeouts only.

In `@src/adapters/tavily.ts`:
- Around line 62-68: The fallback score calculation in the mapping over
response.results (inside the Tavily adapter) can produce negative values for
high indexes; update the score assignment (the property 'score' in the map
return) to clamp the fallback to a valid floor (e.g., use Math.max(0, 1 - index
* 0.1) or a clamp to [0,1]) so that scores never go below 0 while keeping
existing result.score when present; adjust in the function that maps
response.results in src/adapters/tavily.ts where 'title', 'url', 'snippet',
'score', and 'source: this.name' are set.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f3693498-5e40-4ebd-9e53-627a559fac9b

📥 Commits

Reviewing files that changed from the base of the PR and between 991dce7 and 068b9a0.

⛔ Files ignored due to path filters (2)
  • files (4).zip is excluded by !**/*.zip
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (61)
  • .gitattributes
  • .github/ISSUE_TEMPLATE/hardening.yml
  • .github/ISSUE_TEMPLATE/provider-addition.yml
  • .github/pull_request_template.md
  • .github/workflows/ci.yml
  • PLAN.md
  • PROGRESS.md
  • README.md
  • SKILL.md
  • TASK.md
  • architecture.md
  • config.example.toml
  • docker/searxng/docker-compose.yml
  • docker/searxng/settings.yml
  • docs/BWS_INTEGRATION.md
  • docs/CAPABILITY_MATRIX.md
  • docs/KEY_MANAGEMENT.md
  • docs/NORTH_STAR.md
  • docs/PROGRESS.md
  • docs/architecture.md
  • docs/plans/searxng.md
  • docs/providers/README.md
  • docs/providers/brave.md
  • docs/providers/exa.md
  • docs/providers/firecrawl.md
  • docs/providers/jina.md
  • docs/providers/searxng.md
  • docs/providers/serper.md
  • docs/providers/tavily.md
  • files (4)/CLAUDE.md
  • files (4)/PROGRESS.md
  • files (4)/SKILL.md
  • files (4)/architecture.md
  • files (4)/config.example.toml
  • package.json
  • src/adapters/brave.ts
  • src/adapters/exa.ts
  • src/adapters/firecrawl.ts
  • src/adapters/index.ts
  • src/adapters/jina.ts
  • src/adapters/searxng.ts
  • src/adapters/serper.ts
  • src/adapters/tavily.ts
  • src/agent/agent.ts
  • src/agent/llm.ts
  • src/agent/tools.ts
  • src/app.ts
  • src/cli.ts
  • src/config.ts
  • src/engine/fanout.ts
  • src/engine/keypool.ts
  • src/execution/backend.ts
  • src/http.ts
  • src/index.ts
  • src/providers.ts
  • src/resolvers/bws.ts
  • src/types.ts
  • test/agent-payloads.test.mjs
  • test/cli-version.test.mjs
  • test/providers-docs.test.mjs
  • test/runtime-seams.test.mjs
💤 Files with no reviewable changes (9)
  • architecture.md
  • PROGRESS.md
  • files (4)/PROGRESS.md
  • files (4)/config.example.toml
  • files (4)/CLAUDE.md
  • files (4)/SKILL.md
  • PLAN.md
  • files (4)/architecture.md
  • TASK.md

Comment thread .gitattributes
Comment thread .github/pull_request_template.md Outdated
Comment thread docker/searxng/settings.yml
Comment thread docs/BWS_INTEGRATION.md
Comment thread docs/KEY_MANAGEMENT.md
Comment thread src/agent/agent.ts Outdated
Comment thread src/agent/llm.ts Outdated
Comment thread src/agent/tools.ts
Comment thread src/config.ts
Comment thread src/http.ts Outdated
Tighten request and agent safety checks, close adapter edge cases, and align the renamed CLI docs so the branch can merge cleanly after review.
coderabbitai[bot]
coderabbitai Bot previously requested changes Apr 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/agent/agent.ts (1)

256-272: ⚠️ Potential issue | 🟠 Major

Cap the fetched body before calling response.text().

Line 268 and Line 272 fully buffer the response body. A model-directed fetch to a very large HTML page or a binary blob can exhaust memory before the truncation in src/agent/tools.ts ever runs, so this path should reject unsupported content types early and enforce a byte limit while streaming.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/agent.ts` around lines 256 - 272, The fetch path currently calls
response.text() which fully buffers bodies and can OOM; update the logic in the
block around fetchWithPolicy/parsedUrl so you first validate/whitelist
contentType (use the existing contentType variable) and immediately reject
unsupported types (non text/html) to avoid buffering binaries, and for text/html
stream response.body with a reader and TextDecoder enforcing a MAX_BODY_BYTES
limit (e.g. MAX_BODY_BYTES constant) while accumulating bytes; once limit or EOF
reached decode the accumulated chunk to a string and pass that string to
this.extractTextFromHTML(html) instead of calling response.text(), and ensure
you abort/close the reader when the limit is exceeded.
src/adapters/firecrawl.ts (1)

83-85: ⚠️ Potential issue | 🟡 Minor

Validate options.limit before sending it to Firecrawl.

CrawlCallOptions.limit can still be 0, negative, or fractional here, and this path forwards it verbatim to POST /crawl. Please normalize it the same way src/adapters/tavily.ts now does so malformed caller input does not turn into provider errors.

♻️ Proposed fix
-    const limit = options?.limit ?? 10;
+    const rawLimit = options?.limit;
+    const limit =
+      typeof rawLimit === "number" && Number.isFinite(rawLimit)
+        ? Math.max(1, Math.floor(rawLimit))
+        : 10;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapters/firecrawl.ts` around lines 83 - 85, The code currently forwards
CrawlCallOptions.limit verbatim to POST /crawl (you create the variables
normalizedUrl and limit); validate and normalize limit before sending it: coerce
options?.limit to a finite number, convert to an integer (e.g. Math.floor), and
clamp to a minimum of 1, falling back to the default 10 when options?.limit is
missing or invalid, so negative, zero, or fractional inputs do not get forwarded
to the provider.
src/adapters/tavily.ts (1)

146-168: ⚠️ Potential issue | 🟡 Minor

Deduplicate after seeding the original URL.

If Tavily already returns the original URL within the first limit - 1 hits, this branch skips unshift() and extract only sees limit - 1 URLs. Build the candidate list with the seed URL first, then de-duplicate and slice once.

♻️ Proposed fix
-    const urls = (searchResponse.results || [])
-      .map(r => r.url)
-      .filter((u): u is string => !!u)
-      .slice(0, limit - 1); // Leave room for original URL
-
-    // Add the original URL if not present
     const normalizedUrl = url.trim();
-    if (!urls.includes(normalizedUrl)) {
-      urls.unshift(normalizedUrl);
-    }
+    const urls = [
+      normalizedUrl,
+      ...(searchResponse.results || [])
+        .map((r) => r.url)
+        .filter((u): u is string => !!u),
+    ];
+    const uniqueUrls = [...new Set(urls)].slice(0, limit);
@@
-          urls: urls.slice(0, limit),
+          urls: uniqueUrls,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapters/tavily.ts` around lines 146 - 168, The code seeds the candidate
URLs after taking a slice of search results, so if the original URL is already
present you end up with only limit-1 URLs passed to fetchJson; change the logic
in the tavily adapter so you first create a candidate array starting with
normalizedUrl, then append searchResponse.results.map(r => r.url), dedupe that
array (keeping first occurrence of each URL) and finally slice(0, limit) before
calling fetchJson (TavilyExtractResponse, this.baseUrl, apiKey); update the urls
variable used in the POST body to this deduplicated-and-sliced list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitattributes:
- Around line 1-15: Update the .gitattributes file to also include optional
patterns suggested in the review: add entries for *.sh, *.mjs, *.cjs and
.gitignore with "text eol=lf" so shell scripts, modern JS module files and the
Git config file get consistent LF endings; keep all existing entries (e.g.,
package.json, *.json) intact and follow the same "text eol=lf" format used in
the current .gitattributes.

In `@docker/searxng/settings.yml`:
- Around line 47-49: The comment says "Disable generic engines..." but the
wikipedia engine entry has disabled: false, which enables it; update the
wikipedia engine block by either setting disabled: true for the "wikipedia"
entry or change the preceding comment to accurately state that the listed
engines are enabled by default, ensuring the "wikipedia" name and "disabled" key
are consistent with the intended behavior.

In `@docs/providers/firecrawl.md`:
- Around line 38-46: Several fenced code blocks under the "**Example:**"
headings in docs/providers/firecrawl.md are missing blank lines before and/or
after the ```json blocks (violating markdownlint MD031); edit the file to insert
a single blank line immediately before the opening ```json and a single blank
line immediately after the closing ``` for each example block (the occurrences
near the Example heading shown and the ranges mentioned: 66-76, 97-104, 123-132,
150-156, 158-171, 188-198, 217-223, 240-247) so every fenced block is separated
from surrounding text.
- Around line 264-276: Add a single blank line immediately after each
second-level heading in this section (e.g., after "## Rate Limits" and after "##
Pricing (Credits)") so the heading is separated from the following paragraph,
and repeat the same fix for the other heading occurrences noted ("## Rate
Limits" instances at lines referenced in the review). Ensure there is exactly
one empty line between the heading and the next content to satisfy markdownlint
MD022.

In `@docs/providers/serper.md`:
- Around line 331-332: Replace the promotional bullet "Most comprehensive search
API for Google data" with a neutral, verifiable description that cites supported
endpoints/features (e.g., "Provides API access to Google search results and
related endpoints such as web, images, and knowledge graph") and keep the
pricing line factual by changing "Official pricing is credit-pack based rather
than a monthly subscription model" only if needed to a more neutral phrasing
like "Official pricing uses credit packs rather than a monthly subscription";
update the two bullets so they are factual and verifiable instead of using
superlatives.
- Around line 280-292: Update the "Free Tier" column entries in the Serper
provider tables: replace the header/value "Free Tier" / "✅ 2,500" with "Shared
account quota (2,500 total)" and change each individual cell that currently
shows "✅ 2,500" (e.g., for serper_web_search, serper_image_search,
serper_news_search, serper_video_search, serper_shopping_search,
serper_maps_search, serper_places_search, serper_scholar_search,
serper_patents_search, serper_autocomplete, serper_get_reviews) to that new
phrasing; additionally add a short note under each table stating "All endpoints
share a single account-level pool of 2,500 free credits" (also apply the same
edits to the second table block referenced).

In `@docs/providers/tavily.md`:
- Around line 17-205: The docs page currently presents Tavily’s full API surface
(tavily_map, tavily_answer, tavily_research, tavily_crawl) as available tools
but the repository adapter (functions search, extract, crawl in the Tavily
adapter) only implements search, extract and a heuristic crawl; update the docs
to explicitly state which endpoints are implemented by the ColdSearch adapter
vs. which are upstream-only: add a short note under the Available Tools header
that marks tavily_map, tavily_answer, tavily_research and native /crawl as
"upstream API — not wired into ColdSearch" and mark search, extract and crawl as
"implemented by ColdSearch adapter (src/adapters/tavily.ts — functions: search,
extract, crawl)"; ensure the capability matrix reflects the same distinction
(e.g., separate columns "Upstream API" vs "Adapter implemented") so readers
won’t be misled.

In `@src/agent/agent.ts`:
- Around line 238-241: The current branch treats any parsed payload as
acceptable and can return tool-control JSON when
parseAgentPayload(finalResponse.content) yields a non-"final" payload (e.g.,
type "tool"); change the logic so that if finalPayload is missing or
finalPayload.type !== "final" you do NOT return finalResponse.content but
instead trigger a final-only retry or synthesize a reply from the collected
context: detect this condition around
parseAgentPayload/finalPayload/finalResponse, discard non-final payloads, then
either invoke the existing final-only retry path (or call a helper that issues
one last LLM prompt constrained to produce {"type":"final",...}) and use its
answer, or build a synthesized final answer from conversation context and tool
outputs before returning to the user.
- Around line 255-256: The hostname validation in validateFetchUrl currently
checks DNS answers but returns the original URL, allowing a TOCTOU DNS rebinding
when fetchWithPolicy later calls native fetch(); fix by having validateFetchUrl
return (or attach) the resolved safe IP and use that IP when initiating the HTTP
connection instead of the original hostname (or implement a pinned connection
that reuses the resolved DNS result); update callers: modify validateFetchUrl to
provide a { url, resolvedIp } result and change fetchWithPolicy (and the
lower-level connector used by http.ts -> fetch()) to connect to the resolvedIp
(or to accept a pinned DNS result) so the connection uses the validated address
rather than doing a fresh DNS lookup.

In `@src/config.ts`:
- Around line 46-50: The error message always points users to the default config
location even when they passed a custom path; update the ENOENT handling so it
tailors guidance based on the input config path: inspect the variable named path
(and the DEFAULT_CONFIG_DIR_NAME constant) and if path is the default/no
explicit --config use the existing “Create one at
~/.config/${DEFAULT_CONFIG_DIR_NAME}/config.toml…” text, otherwise produce a
message that says the specified config path was not found and suggests verifying
the --config value or creating the file at that path; keep the thrown Error text
concise and include the missing path in both cases.

In `@src/http.ts`:
- Around line 33-35: The sleep helper blocks cancellation; change sleep(ms:
number) to accept an optional AbortSignal (e.g., sleep(ms: number, signal?:
AbortSignal): Promise<void>) and implement it so it returns immediately rejected
if signal.aborted, sets a timeout, and registers a signal.onabort listener that
clears the timeout and rejects with an AbortError (and always removes the
listener on completion) so the delay is aborted instantly; update all callers
(places that call sleep, including where init.signal is present around lines
100-116) to pass init.signal and handle the rejected AbortError appropriately.
- Around line 65-116: The function fetchWithPolicy currently retries every
request method; change it to only apply automatic retries to safe/idempotent
methods (e.g., GET, HEAD, PUT, DELETE, OPTIONS) or when the caller explicitly
opts-in for non-idempotent retries via a new flag (e.g.,
policy.allowNonIdempotentRetries). Concretely, inside fetchWithPolicy (use the
existing requestUrl, init, policy, retries variables), compute the effective
method (default to "GET" when init.method is undefined), and gate the retry
logic (the checks that call shouldRetryStatus and shouldRetryError and the retry
loop behavior) so retries occur only if the method is idempotent OR
policy.allowNonIdempotentRetries === true; otherwise set retries to 0 or skip
retry branches so POST/patch-like methods won’t be retried by default. Ensure
the new behavior is honored both for status-based retries and error-based
retries and that existing abort/signal handling remains unchanged.

In `@test/runtime-seams.test.mjs`:
- Line 62: The test's assertion using assert.match(requestedUrl, ...) is too
weak and can miss missing query params; update the test around
assert.match/requestedUrl to parse the URL (e.g., new URL(requestedUrl)) and
explicitly assert that searchParams.has('q') is true and that
searchParams.get('format') === 'json' (and any other expected params), or
alternatively tighten the regex to require q= and format=json in the query
string so the seam ensures both parameters are present.

---

Outside diff comments:
In `@src/adapters/firecrawl.ts`:
- Around line 83-85: The code currently forwards CrawlCallOptions.limit verbatim
to POST /crawl (you create the variables normalizedUrl and limit); validate and
normalize limit before sending it: coerce options?.limit to a finite number,
convert to an integer (e.g. Math.floor), and clamp to a minimum of 1, falling
back to the default 10 when options?.limit is missing or invalid, so negative,
zero, or fractional inputs do not get forwarded to the provider.

In `@src/adapters/tavily.ts`:
- Around line 146-168: The code seeds the candidate URLs after taking a slice of
search results, so if the original URL is already present you end up with only
limit-1 URLs passed to fetchJson; change the logic in the tavily adapter so you
first create a candidate array starting with normalizedUrl, then append
searchResponse.results.map(r => r.url), dedupe that array (keeping first
occurrence of each URL) and finally slice(0, limit) before calling fetchJson
(TavilyExtractResponse, this.baseUrl, apiKey); update the urls variable used in
the POST body to this deduplicated-and-sliced list.

In `@src/agent/agent.ts`:
- Around line 256-272: The fetch path currently calls response.text() which
fully buffers bodies and can OOM; update the logic in the block around
fetchWithPolicy/parsedUrl so you first validate/whitelist contentType (use the
existing contentType variable) and immediately reject unsupported types (non
text/html) to avoid buffering binaries, and for text/html stream response.body
with a reader and TextDecoder enforcing a MAX_BODY_BYTES limit (e.g.
MAX_BODY_BYTES constant) while accumulating bytes; once limit or EOF reached
decode the accumulated chunk to a string and pass that string to
this.extractTextFromHTML(html) instead of calling response.text(), and ensure
you abort/close the reader when the limit is exceeded.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f4caa66c-b29a-4e0b-a062-baaef2337f47

📥 Commits

Reviewing files that changed from the base of the PR and between 068b9a0 and ed690fe.

📒 Files selected for processing (20)
  • .gitattributes
  • .github/pull_request_template.md
  • SKILL.md
  • docker/searxng/settings.yml
  • docs/BWS_INTEGRATION.md
  • docs/KEY_MANAGEMENT.md
  • docs/providers/firecrawl.md
  • docs/providers/serper.md
  • docs/providers/tavily.md
  • src/adapters/firecrawl.ts
  • src/adapters/jina.ts
  • src/adapters/searxng.ts
  • src/adapters/tavily.ts
  • src/agent/agent.ts
  • src/agent/llm.ts
  • src/agent/tools.ts
  • src/config.ts
  • src/http.ts
  • test/agent-payloads.test.mjs
  • test/runtime-seams.test.mjs

Comment thread .gitattributes
Comment thread docker/searxng/settings.yml Outdated
Comment thread docs/providers/firecrawl.md
Comment thread docs/providers/firecrawl.md
Comment thread docs/providers/serper.md Outdated
Comment thread src/agent/agent.ts Outdated
Comment thread src/config.ts Outdated
Comment thread src/http.ts Outdated
Comment thread src/http.ts Outdated
Comment thread test/runtime-seams.test.mjs Outdated
Pin agent fetches to validated DNS results, cap response bodies, and tighten provider docs and request handling so the PR clears the remaining review findings.
coderabbitai[bot]
coderabbitai Bot previously requested changes Apr 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/providers/serper.md`:
- Around line 328-334: The line "Real-time Google results (not cached)" in the
Notes section should be sourced or rephrased: either add a citation to Serper's
official docs/FAQ/marketing page that explicitly claims real-time results and
include the URL in the docs/providers/serper.md Notes, or change the text to an
attribution like "Claims to provide real-time Google results (per Serper's
documentation)" so the statement is clearly presented as Serper's claim rather
than an asserted fact.
- Line 9: Update the provider overview sentence in docs/providers/serper.md to
list all documented tools: extend the existing list ("web, images, news, videos,
shopping, maps, scholar, and patents") to also include serper_autocomplete and
serper_get_reviews so the overview matches the Available Tools section; edit the
one-line overview string to append ", serper_autocomplete, and
serper_get_reviews" (or rephrase to include all tool names explicitly) ensuring
consistency with the documented tool names in the Available Tools section.
- Around line 317-327: Add a minimal JSON example to the "Response Structure"
section showing the shape of the returned object (include keys searchParameters,
knowledgeGraph, answerBox, organic, peopleAlsoAsk, relatedSearches) so consumers
can see field nesting and typical item keys (e.g., organic items with title,
link, snippet; relatedSearches with query). Place the example as a fenced
```json``` code block immediately after the bulleted list and use small
realistic placeholders (e.g., "q": "example query", "gl": "us") to illustrate
array and object shapes without exhaustive fields.
- Around line 180-187: Clarify the relationship between maps and places for
serper_places_search: update the serper_places_search documentation to
explicitly state whether both endpoints ("POST /places" and "POST /maps") are
accepted interchangeably or if one is the canonical endpoint to prefer; mention
that parameters are the same as serper_maps_search and, if both work, say they
are aliases that behave identically, or if one is primary, mark that endpoint as
canonical and indicate the other is deprecated/redirected. Reference the symbol
serper_places_search and the endpoints "POST /places" and "POST /maps" when
making the change so readers know which API routes the clarification applies to.

In `@docs/providers/tavily.md`:
- Line 39: The table row for the `include_raw_content` option uses "Include full
page content" as a description; change that phrase to "Include full-page
content" (hyphenate "full-page") so the compound adjective properly modifies
"content" in the docs (update the `include_raw_content` table entry /
description).

In `@src/adapters/firecrawl.ts`:
- Around line 121-122: The current polling loop in firecrawl.ts uses hardcoded
pollInterval (2000ms) and maxAttempts (30); change it to read configurable
values from the crawl options: add optional pollIntervalMs and maxPollAttempts
to CrawlCallOptions in types.ts, provide sensible defaults (e.g., 2000ms and 30)
where the crawl is invoked, and replace the hardcoded constants in the loop (the
variables referenced in the for loop and setTimeout) with the options values so
callers can override them for large crawls.

In `@src/agent/agent.ts`:
- Around line 333-404: fetchValidatedBody currently rejects any non-2xx response
but doesn't explicitly explain or report redirect (3xx) behavior; update
fetchValidatedBody to explicitly detect 3xx status codes and reject with a
clearer error message that includes the status code and the
response.headers.location (if present) and add a brief inline comment above the
statusCode check stating that redirects are intentionally not followed to
prevent redirect-based SSRF (note: Node's http/https.request does not
auto-follow redirects). Keep existing behavior for other non-2xx codes but
improve the error text to include statusCode and Location when available so
callers can distinguish redirects from other failures.
- Around line 82-93: The IPv6 check block (the if (version === 6) branch using
the address variable) misses IPv4-mapped IPv6 addresses like ::ffff:127.0.0.1
which bypass the blocked-IP logic; update this branch to explicitly detect
IPv4-mapped addresses (e.g. addresses that start with ::ffff: or the
::ffff:0:0/96 form), parse the mapped IPv4 portion and run the existing IPv4
private/loopback checks on it, or replace the manual prefix checks with a robust
parser such as ipaddr.js (call ipaddr.parse(address).toIPv4Address() when
mapped) so mapped IPv4s are treated the same as their IPv4 equivalents. Ensure
you modify the block that references version and address in agent.ts
accordingly.

In `@src/http.ts`:
- Around line 162-176: fetchJson currently attempts response.json()
unconditionally which can produce misleading parse errors for non-JSON
responses; modify fetchJson (the function that calls fetchWithPolicy and uses
policy.label) to first inspect response.headers.get('content-type') and verify
it contains "application/json" or "+json" before calling response.json(); if the
header is missing or non-JSON, throw a clear Error that includes policy.label
(or "Request") and the actual Content-Type (and optionally a short snippet of
the body) so callers get an explicit message rather than a JSON parse failure,
while keeping the existing try/catch for parsing errors.

In `@test/runtime-seams.test.mjs`:
- Around line 47-53: Add a failing-fetch test alongside the existing
global.fetch stub in runtime-seams.test.mjs: create a test that replaces
global.fetch (the same symbol used in the current stub) to either throw an error
or return a Response with a non-200 status and error body, call the code path
that invokes fetch (so requestedUrl is still set when appropriate), and assert
that the module under test handles the failure correctly (e.g., rejects, returns
a specific error value, or logs/marks an error). Ensure you restore the original
global.fetch after the test to avoid cross-test pollution.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 11377ab0-9298-4402-9918-bf618a7355c7

📥 Commits

Reviewing files that changed from the base of the PR and between ed690fe and 6a45ad7.

📒 Files selected for processing (11)
  • .gitattributes
  • docker/searxng/settings.yml
  • docs/providers/firecrawl.md
  • docs/providers/serper.md
  • docs/providers/tavily.md
  • src/adapters/firecrawl.ts
  • src/adapters/tavily.ts
  • src/agent/agent.ts
  • src/config.ts
  • src/http.ts
  • test/runtime-seams.test.mjs

Comment thread docs/providers/serper.md

## Overview

Serper.dev provides fast Google Search API access with structured SERP data. Supports all major Google verticals including web, images, news, videos, shopping, maps, scholar, and patents.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update overview to list all documented tools.

The overview mentions "web, images, news, videos, shopping, maps, scholar, and patents" but the Available Tools section documents 11 tools including serper_autocomplete and serper_get_reviews. Add these to the overview for completeness.

📝 Suggested addition
-Serper.dev provides fast Google Search API access with structured SERP data. Supports all major Google verticals including web, images, news, videos, shopping, maps, scholar, and patents.
+Serper.dev provides fast Google Search API access with structured SERP data. Supports all major Google verticals including web, images, news, videos, shopping, maps, scholar, patents, autocomplete, and reviews.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/serper.md` at line 9, Update the provider overview sentence in
docs/providers/serper.md to list all documented tools: extend the existing list
("web, images, news, videos, shopping, maps, scholar, and patents") to also
include serper_autocomplete and serper_get_reviews so the overview matches the
Available Tools section; edit the one-line overview string to append ",
serper_autocomplete, and serper_get_reviews" (or rephrase to include all tool
names explicitly) ensuring consistency with the documented tool names in the
Available Tools section.

Comment thread docs/providers/serper.md
Comment on lines +180 to +187
### 7. `serper_places_search`

Google Places search (alias for maps).

**Endpoint:** `POST /places` or `POST /maps`

**Parameters:** Same as `serper_maps_search`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clarify the places/maps endpoint relationship.

The documentation states serper_places_search is an "alias for maps" with endpoint "POST /places or POST /maps". Clarify whether both endpoints are accepted by the API, or if one is canonical, to help users choose the correct endpoint.

💡 Suggested clarification

If both endpoints work identically:

-**Endpoint:** `POST /places` or `POST /maps`
+**Endpoint:** `POST /places` (functionally equivalent to `/maps`)

Or, if one is the primary endpoint:

-Google Places search (alias for maps).
+Google Places search (uses the same endpoint as maps).

-**Endpoint:** `POST /places` or `POST /maps`
+**Endpoint:** `POST /maps`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/serper.md` around lines 180 - 187, Clarify the relationship
between maps and places for serper_places_search: update the
serper_places_search documentation to explicitly state whether both endpoints
("POST /places" and "POST /maps") are accepted interchangeably or if one is the
canonical endpoint to prefer; mention that parameters are the same as
serper_maps_search and, if both work, say they are aliases that behave
identically, or if one is primary, mark that endpoint as canonical and indicate
the other is deprecated/redirected. Reference the symbol serper_places_search
and the endpoints "POST /places" and "POST /maps" when making the change so
readers know which API routes the clarification applies to.

Comment thread docs/providers/serper.md
Comment on lines +317 to +327
## Response Structure

All endpoints return consistent JSON with:

- `searchParameters` - Query details
- `knowledgeGraph` - Knowledge panel (if available)
- `answerBox` - Direct answer (if available)
- `organic` - Main search results
- `peopleAlsoAsk` - Related questions
- `relatedSearches` - Related queries

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding a response example for clarity.

The Response Structure section lists field names but doesn't show their structure. Adding a minimal example response would help users understand the shape of the data.

💡 Example addition
 - `relatedSearches` - Related queries
+
+**Example Response:**
+
+```json
+{
+  "searchParameters": { "q": "example query", "gl": "us" },
+  "organic": [
+    { "title": "...", "link": "...", "snippet": "..." }
+  ],
+  "relatedSearches": [
+    { "query": "..." }
+  ]
+}
+```
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/serper.md` around lines 317 - 327, Add a minimal JSON example
to the "Response Structure" section showing the shape of the returned object
(include keys searchParameters, knowledgeGraph, answerBox, organic,
peopleAlsoAsk, relatedSearches) so consumers can see field nesting and typical
item keys (e.g., organic items with title, link, snippet; relatedSearches with
query). Place the example as a fenced ```json``` code block immediately after
the bulleted list and use small realistic placeholders (e.g., "q": "example
query", "gl": "us") to illustrate array and object shapes without exhaustive
fields.

Comment thread docs/providers/serper.md
Comment on lines +328 to +334
## Notes

- Real-time Google results (not cached)
- Returns Knowledge Graph, Answer Box, People Also Ask
- Supports all major Google verticals
- Provides API access to Google search results and related endpoints such as web, images, maps, and knowledge-graph-style responses.
- Official pricing uses credit packs rather than a monthly subscription.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Source the real-time results claim or add a reference.

Line 330 states "Real-time Google results (not cached)" which is an infrastructure claim about Serper's service. Consider adding a source citation (e.g., from Serper's official documentation or marketing materials) or rephrasing to indicate this is Serper's stated behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/serper.md` around lines 328 - 334, The line "Real-time Google
results (not cached)" in the Notes section should be sourced or rephrased:
either add a citation to Serper's official docs/FAQ/marketing page that
explicitly claims real-time results and include the URL in the
docs/providers/serper.md Notes, or change the text to an attribution like
"Claims to provide real-time Google results (per Serper's documentation)" so the
statement is clearly presented as Serper's claim rather than an asserted fact.

Comment thread docs/providers/tavily.md
| `include_domains` | string[] | No | Whitelist domains |
| `exclude_domains` | string[] | No | Blacklist domains |
| `include_answer` | boolean | No | Include AI-generated answer |
| `include_raw_content` | boolean | No | Include full page content |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Optional: Consider hyphenating "full-page" as a compound adjective.

Static analysis suggests "full-page content" when used as a compound adjective modifying a noun. This is a minor style consideration.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~39-~39: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e_raw_content` | boolean | No | Include full page content | Example: ```json { "q...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/tavily.md` at line 39, The table row for the
`include_raw_content` option uses "Include full page content" as a description;
change that phrase to "Include full-page content" (hyphenate "full-page") so the
compound adjective properly modifies "content" in the docs (update the
`include_raw_content` table entry / description).

Comment thread src/adapters/firecrawl.ts
Comment on lines 121 to 122
for (let attempt = 0; attempt < maxAttempts; attempt++) {
await new Promise((resolve) => setTimeout(resolve, pollInterval));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider making polling parameters configurable.

The polling interval (2s) and max attempts (30) are hardcoded, resulting in a fixed 60s maximum wait. For large crawls, this may be insufficient.

♻️ Optional: Accept polling configuration via options
 async crawl(
   url: string,
   apiKey: string,
   options?: CrawlCallOptions
 ): Promise<CrawlResult[]> {
+    const maxAttempts = options?.maxPollAttempts ?? 30;
+    const pollInterval = options?.pollIntervalMs ?? 2000;
     // ... existing code
-    const maxAttempts = 30;
-    const pollInterval = 2000;

This would require extending CrawlCallOptions in types.ts.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapters/firecrawl.ts` around lines 121 - 122, The current polling loop
in firecrawl.ts uses hardcoded pollInterval (2000ms) and maxAttempts (30);
change it to read configurable values from the crawl options: add optional
pollIntervalMs and maxPollAttempts to CrawlCallOptions in types.ts, provide
sensible defaults (e.g., 2000ms and 30) where the crawl is invoked, and replace
the hardcoded constants in the loop (the variables referenced in the for loop
and setTimeout) with the options values so callers can override them for large
crawls.

Comment thread src/agent/agent.ts
Comment on lines +82 to +93
if (version === 6) {
const normalized = address.toLowerCase();
return (
normalized === "::1" ||
normalized.startsWith("fc") ||
normalized.startsWith("fd") ||
normalized.startsWith("fe8") ||
normalized.startsWith("fe9") ||
normalized.startsWith("fea") ||
normalized.startsWith("feb")
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any IPv6 handling utilities or tests in the codebase
rg -n "ipv6|IPv6|isIP" --type ts -C 2

Repository: Coldaine/ColdSearch

Length of output: 1108


🏁 Script executed:

#!/bin/bash
# Read the complete isBlockedIpAddress function
cat -n src/agent/agent.ts | sed -n '64,100p'

Repository: Coldaine/ColdSearch

Length of output: 1238


🏁 Script executed:

#!/bin/bash
# Search for test files related to IP blocking
fd -e "\.test\.(ts|js)$|\.spec\.(ts|js)$" -x grep -l "isBlockedIpAddress\|IPv6\|blocked.*ip" {}

Repository: Coldaine/ColdSearch

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Search for IPv4-mapped IPv6 address handling (::ffff:)
rg "::ffff|mapped.*ipv|ipv.*mapped" --type ts -i

Repository: Coldaine/ColdSearch

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Find where isBlockedIpAddress is called
rg -n "isBlockedIpAddress" --type ts

Repository: Coldaine/ColdSearch

Length of output: 316


🏁 Script executed:

#!/bin/bash
# Check if IPv4-mapped IPv6 addresses would be caught by Node.js isIP
# Also look for any existing IPv6 parsing logic
node <<'EOF'
const { isIP } = require('net');

// Test various address forms
const addresses = [
  '::1',
  '::ffff:127.0.0.1',
  'fc00::1',
  'fd00::1',
  'fe80::1',
  '2001:db8::1'
];

addresses.forEach(addr => {
  console.log(`${addr}: isIP=${isIP(addr)}`);
});
EOF

Repository: Coldaine/ColdSearch

Length of output: 167


🏁 Script executed:

#!/bin/bash
# Look at the context where isBlockedIpAddress is called
cat -n src/agent/agent.ts | sed -n '300,320p'

Repository: Coldaine/ColdSearch

Length of output: 1041


🏁 Script executed:

#!/bin/bash
# Also check what resolvedAddresses contains and where it comes from
cat -n src/agent/agent.ts | sed -n '290,315p'

Repository: Coldaine/ColdSearch

Length of output: 1186


IPv4-mapped IPv6 addresses bypass the blocked IP check.

The current prefix checks (fc, fd, fe8-feb) will not match IPv4-mapped IPv6 addresses like ::ffff:127.0.0.1 or ::ffff:10.0.0.1, which represent non-public IPs and should be blocked. Consider parsing IPv6 addresses to detect and handle mapped addresses explicitly, or use a dedicated IPv6 parsing library to ensure all address forms are properly validated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/agent.ts` around lines 82 - 93, The IPv6 check block (the if
(version === 6) branch using the address variable) misses IPv4-mapped IPv6
addresses like ::ffff:127.0.0.1 which bypass the blocked-IP logic; update this
branch to explicitly detect IPv4-mapped addresses (e.g. addresses that start
with ::ffff: or the ::ffff:0:0/96 form), parse the mapped IPv4 portion and run
the existing IPv4 private/loopback checks on it, or replace the manual prefix
checks with a robust parser such as ipaddr.js (call
ipaddr.parse(address).toIPv4Address() when mapped) so mapped IPv4s are treated
the same as their IPv4 equivalents. Ensure you modify the block that references
version and address in agent.ts accordingly.

Comment thread src/agent/agent.ts
Comment on lines +333 to 404
private async fetchValidatedBody(target: ValidatedFetchTarget): Promise<{ contentType: string; body: string }> {
const client = target.url.protocol === "https:" ? https : http;

return await new Promise((resolve, reject) => {
const request = client.request(
{
protocol: target.url.protocol,
hostname: target.url.hostname,
port: target.url.port || undefined,
method: "GET",
path: `${target.url.pathname}${target.url.search}`,
headers: {
"User-Agent": `Mozilla/5.0 (compatible; ${APP_USER_AGENT})`,
Accept: "text/html, application/xhtml+xml, text/plain;q=0.9",
"Accept-Encoding": "identity",
},
servername: target.url.hostname,
lookup: (_hostname, _options, callback) => {
callback(null, target.resolvedAddress, target.family);
},
},
(response) => {
const statusCode = response.statusCode ?? 0;
if (statusCode < 200 || statusCode >= 300) {
response.resume();
reject(new Error(`Agent fetch failed with HTTP ${statusCode}`));
return;
}

const contentType = response.headers["content-type"] || "";
const normalizedContentType = Array.isArray(contentType) ? contentType[0] || "" : contentType;
if (
!normalizedContentType.includes("text/html") &&
!normalizedContentType.includes("application/xhtml+xml") &&
!normalizedContentType.startsWith("text/")
) {
response.resume();
reject(new Error(`Unsupported fetch content type: ${normalizedContentType || "unknown"}`));
return;
}

const chunks: Buffer[] = [];
let totalBytes = 0;

response.on("data", (chunk: Buffer) => {
totalBytes += chunk.length;
if (totalBytes > MAX_FETCH_BODY_BYTES) {
response.destroy(new Error(`Agent fetch exceeded ${MAX_FETCH_BODY_BYTES} bytes`));
return;
}

chunks.push(chunk);
});

response.on("end", () => {
resolve({
contentType: normalizedContentType,
body: Buffer.concat(chunks).toString("utf8"),
});
});

response.on("error", reject);
}
);

request.setTimeout(AGENT_FETCH_TIMEOUT_MS, () => {
request.destroy(new Error(`Agent fetch timed out after ${AGENT_FETCH_TIMEOUT_MS}ms`));
});
request.on("error", reject);
request.end();
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider handling redirect responses.

The fetchValidatedBody method rejects non-2xx responses but doesn't handle redirects (3xx). If the server returns a redirect to an internal/blocked host, the current code would reject it (which is safe), but the error message may be confusing. Additionally, HTTP/HTTPS libraries may follow redirects automatically in some configurations.

♻️ Explicitly disable redirects and add clearer messaging

The current approach is safe since it rejects 3xx, but consider adding a comment explaining this is intentional to prevent redirect-based SSRF.

         (response) => {
           const statusCode = response.statusCode ?? 0;
+          // Reject redirects to prevent redirect-based SSRF attacks
           if (statusCode < 200 || statusCode >= 300) {
             response.resume();
-            reject(new Error(`Agent fetch failed with HTTP ${statusCode}`));
+            reject(new Error(`Agent fetch failed with HTTP ${statusCode}${statusCode >= 300 && statusCode < 400 ? ' (redirects not followed for security)' : ''}`));
             return;
           }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/agent/agent.ts` around lines 333 - 404, fetchValidatedBody currently
rejects any non-2xx response but doesn't explicitly explain or report redirect
(3xx) behavior; update fetchValidatedBody to explicitly detect 3xx status codes
and reject with a clearer error message that includes the status code and the
response.headers.location (if present) and add a brief inline comment above the
statusCode check stating that redirects are intentionally not followed to
prevent redirect-based SSRF (note: Node's http/https.request does not
auto-follow redirects). Keep existing behavior for other non-2xx codes but
improve the error text to include statusCode and Location when available so
callers can distinguish redirects from other failures.

Comment thread src/http.ts
Comment on lines +162 to +176
export async function fetchJson<T>(
input: string | URL,
init: RequestInit = {},
policy: RequestPolicy = {}
): Promise<T> {
const response = await fetchWithPolicy(input, init, policy);

try {
return (await response.json()) as T;
} catch (error) {
throw new Error(
`${policy.label || "Request"} returned invalid JSON: ${(error as Error).message}`
);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding Content-Type validation before JSON parsing.

fetchJson parses the response as JSON without validating the Content-Type header. Servers may return non-JSON bodies (e.g., HTML error pages) with a 2xx status, leading to confusing parse errors.

♻️ Optional: Validate Content-Type
 export async function fetchJson<T>(
   input: string | URL,
   init: RequestInit = {},
   policy: RequestPolicy = {}
 ): Promise<T> {
   const response = await fetchWithPolicy(input, init, policy);
 
+  const contentType = response.headers.get("content-type") || "";
+  if (!contentType.includes("application/json")) {
+    throw new Error(
+      `${policy.label || "Request"} returned unexpected Content-Type: ${contentType}`
+    );
+  }
+
   try {
     return (await response.json()) as T;
   } catch (error) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function fetchJson<T>(
input: string | URL,
init: RequestInit = {},
policy: RequestPolicy = {}
): Promise<T> {
const response = await fetchWithPolicy(input, init, policy);
try {
return (await response.json()) as T;
} catch (error) {
throw new Error(
`${policy.label || "Request"} returned invalid JSON: ${(error as Error).message}`
);
}
}
export async function fetchJson<T>(
input: string | URL,
init: RequestInit = {},
policy: RequestPolicy = {}
): Promise<T> {
const response = await fetchWithPolicy(input, init, policy);
const contentType = response.headers.get("content-type") || "";
if (!contentType.includes("application/json")) {
throw new Error(
`${policy.label || "Request"} returned unexpected Content-Type: ${contentType}`
);
}
try {
return (await response.json()) as T;
} catch (error) {
throw new Error(
`${policy.label || "Request"} returned invalid JSON: ${(error as Error).message}`
);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/http.ts` around lines 162 - 176, fetchJson currently attempts
response.json() unconditionally which can produce misleading parse errors for
non-JSON responses; modify fetchJson (the function that calls fetchWithPolicy
and uses policy.label) to first inspect response.headers.get('content-type') and
verify it contains "application/json" or "+json" before calling response.json();
if the header is missing or non-JSON, throw a clear Error that includes
policy.label (or "Request") and the actual Content-Type (and optionally a short
snippet of the body) so callers get an explicit message rather than a JSON parse
failure, while keeping the existing try/catch for parsing errors.

Comment on lines +47 to +53
global.fetch = async (input) => {
requestedUrl = input.toString();
return new Response(JSON.stringify({ results: [] }), {
status: 200,
headers: { "content-type": "application/json" },
});
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding error case testing for the fetch stub.

The stub always returns a successful response. Consider adding a test case that verifies error handling when the SearXNG endpoint fails.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/runtime-seams.test.mjs` around lines 47 - 53, Add a failing-fetch test
alongside the existing global.fetch stub in runtime-seams.test.mjs: create a
test that replaces global.fetch (the same symbol used in the current stub) to
either throw an error or return a Response with a non-200 status and error body,
call the code path that invokes fetch (so requestedUrl is still set when
appropriate), and assert that the module under test handles the failure
correctly (e.g., rejects, returns a specific error value, or logs/marks an
error). Ensure you restore the original global.fetch after the test to avoid
cross-test pollution.

@Coldaine
Coldaine dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot] April 12, 2026 18:22

Superseded by follow-up commits and passing CodeRabbit status checks on newer PR heads.

@Coldaine
Coldaine merged commit 49a561c into feat/multi-provider-capabilities Apr 12, 2026
4 checks passed
@Coldaine
Coldaine deleted the feat/provider-docs branch April 12, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants