feat(web-search): SearXNG-backed web_search tool (slice 1 of MCP/A2A gateway ADR) - #1009
Conversation
…A2A gateway ADR) Ships the first real slice of the MCP Gateway / A2A Gateway / web-search request: a KV-credential-configured web_search() client for a self-hosted SearXNG(-compatible) metasearch instance's JSON API, reusing the existing SSRF-safe transport boundary (ModelClient._validate_provider/_open_provider) rather than adding a new HTTP dependency. 100% statement/branch/docstring coverage on the new module; full suite (3327 tests) still passes. The MCP gateway, A2A gateway, and Camoufox-browsing pieces of the original request are recorded as a full-vision design in ADR 0123 (DDD Bounded Context / Context Map / Ubiquitous Language per product-goal-directive.md §5) but deliberately not built here: quarantine-sandbox-runtime (the requested isolation backend) is verified still stub-only (develop branch has no HTTP/CLI entrypoint, real work lives on unmerged Draft PR stack #1-#13, blocked on .github#1590), and there is no concrete Strix/Noema caller yet to justify building protocol-gateway plumbing against. The ADR also documents that Camoufox session isolation already ships today via Wardnet for one narrow use (privacy_policy_analysis.py), a fact worth surfacing before committing to a different isolation backend for the same problem. Research: SearXNG (AGPL-3.0) implemented; Whoogle (MIT) evaluated and rejected as archived/defunct since 2024; YaCy (GPL-2.0-or-later) documented as the next self-hosted engine; Brave Search API documented as the commercial fallback. Camoufox confirmed MPL-2.0, Playwright-compatible, no official MCP server (the camofox-mcp this repo already pins is third-party). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| headers=headers, | ||
| method="GET", | ||
| ) | ||
| with client._open_provider(request, destination, timeout=timeout) as response: |
There was a problem hiding this comment.
🟨 HTTP errors bypass response limits
A SearXNG endpoint can return an oversized HTTP error body. _open_provider buffers it without a limit, allowing remote memory exhaustion.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed real, but not fixed in this PR — it's out of scope for this diff. The unbounded read is inside ModelClient._open_provider itself (orchestrator.py): on any HTTP status >= 400 it does a plain body = response.read() with no size bound before raising HTTPError, for every caller of _open_provider across the codebase, not something specific to web_search.py. This PR's diff never touches orchestrator.py, and _open_provider has no way for a caller like web_search() to intervene — the unbounded read already happened by the time control returns.
Fixing it properly means bounding that read inside _open_provider itself (mirroring the MAX_DISCOVERY_RESPONSE_BYTES bounded-read-then-check pattern model_discovery.py already uses), which is shared transport infrastructure several other currently-open PRs are also editing. Doing that here would be scope creep beyond this PR's stated "slice 1, web-search tool only" contract and adds avoidable merge-conflict risk. Leaving this thread open/unresolved for a follow-up PR against _open_provider directly rather than fixing it unilaterally in this one.
Generated by Claude Code
seonghobae
added
area: api
ChatGPT Codex Connector
…2a-foundation Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
Rebased onto current
|
…the parser
Three unresolved Devin review findings on this PR:
- _searxng_origins() discarded SEARXNG_URL's path component when building
request_base_url, so an instance reverse-proxied under a prefix (e.g.
https://host/searxng) was queried at the host root (https://host/search)
instead of its own /search endpoint. Preserve parsed.path.
- web_search()'s timeout parameter had no validation: a bool, non-positive,
NaN, or infinite value reached socket setup and failed inconsistently
instead of a stable ValueError. Validate it the same way
orchestrator._validate_provider_probe_timeout() already does (reject
bool/non-numeric, require a finite value in a bounded range).
- _parse_results(), an untrusted-JSON parsing seam (SearXNG's response
body), had no fuzz coverage, violating this repo's "new parsing seams
get a fuzz/targets.py target" convention (AGENTS.md/CLAUDE.md). Added
exercise_web_search_results() plus shaped and arbitrary-JSON Hypothesis
properties.
The fourth finding on this PR ("HTTP errors bypass response limits" --
ModelClient._open_provider does an unbounded response.read() on any >=400
status) is real but lives entirely in shared orchestrator.py transport
code this PR's diff never touches, not in web_search.py; replied on that
thread rather than fixing it here to avoid scope creep into a file several
other concurrently open PRs are also editing.
Verification:
- RED: confirmed the subpath bug manually against unfixed _searxng_origins()
(returned the host root, dropping /searxng).
- GREEN: tests/test_web_search.py (33 tests, up from 21) and
tests/fuzz/test_fuzz_properties.py (21 tests, up from 19) all pass.
- ruff clean on all four touched files.
- interrogate on web_search.py + fuzz/targets.py: 100%.
- py_compile clean; git diff --check clean.
- tests/test_conventions.py and the broader web_search/fuzz-keyword slice
of the full suite (65 tests) pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
Triage: fixed three of four unresolved Devin findingsNo merge conflict against Fixed (
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
An integer timeout too large for float() (e.g. 10**400) made math.isfinite(timeout) raise OverflowError instead of the ValueError the function promises, per Devin Review on this PR. The bounds check against finite MIN_TIMEOUT_SECONDS/MAX_TIMEOUT_SECONDS already implies finiteness (NaN and +/-infinity fail every comparison) and, unlike math.isfinite(), Python's int/float comparison never raises OverflowError on an arbitrarily large int -- so dropping the now-redundant isfinite() call both fixes the bug and removes a branch instead of adding one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Fixed the last open finding on this PR's own diff at Fix: dropped That leaves the one remaining finding on this PR — "HTTP errors bypass response limits" / unbounded error-body buffering — confirmed real but out of scope for this diff, as already noted: the unbounded |
| # A bounds check against finite MIN/MAX already excludes NaN and | ||
| # +/-infinity (every comparison with them is False), and unlike | ||
| # math.isfinite() it never raises OverflowError on an int too large | ||
| # to convert to a C double. | ||
| or not MIN_TIMEOUT_SECONDS <= timeout <= MAX_TIMEOUT_SECONDS |
There was a problem hiding this comment.
📝 Info: Large timeout validation remains total
The bounds comparison rejects NaN, infinities, and arbitrarily large integers without float conversion. Valid finite timeouts retain the same accepted range.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
The product owner asked for contextual-orchestrator to become an MCP
Gateway and an A2A Gateway so internal agents (Strix, Noema) can search
the web on another agent's behalf, decide when external search is needed,
and verify claims — a "Perplexity-search-like emulation" for Strix, a "Web
Search Tool" for Noema — backed by SearXNG plus other self-hostable
metasearch engines, with Camoufox for browsing, isolated via
ContextualWisdomLab/quarantine-sandbox-runtime.That request bundles four separable pieces. This PR ships the one piece that
can run today for real, and records the rest as an honest, reconstructable
design rather than faking it:
contextual_orchestrator/web_search.py—web_search(query, ...) -> list[WebSearchResult], backed by a self-hostedSearXNG(-compatible) instance's
GET /search?format=jsonAPI.KV-credential-configured (
SEARXNG_URL, optionalSEARXNG_TOKEN), samepattern as every other provider secret in this repo. Reuses
ModelClient._validate_provider/_open_providerfor its SSRF-safe HTTPboundary — the same primitive
privacy_policy_analysis.crawl_policy_documentalready uses for Wardnet — rather than adding a new HTTP dependency or a
second hand-rolled egress check.
only, in
docs/adr/0123-web-search-mcp-a2a-gateway-foundation.md. Notbuilt here.
Why not build the rest now
quarantine-sandbox-runtimeisn't ready. Verified live(
gh api repos/ContextualWisdomLab/quarantine-sandbox-runtime/pulls):developis a stub; real work is an unmerged Draft PR stack (#1→#6→#9→#10→#13,all open); the crate has no HTTP/CLI entrypoint or container backend, only
a Rust library with a newly-added
CommandExecutionBackendcontract notyet wired to anything real; real container isolation is externally blocked
on
ContextualWisdomLab/.github#1590(no LSM-capable CI runner).quarantine-sandbox-runtime.compose.camoufox-wardnet.yaml+privacy_policy_analysis._render_policy_document_with_camoufoxis a real,reviewed, tested MCP client that renders JS-heavy pages through a
DNS-pinned Wardnet egress proxy, for one purpose (privacy-policy ZDR
discovery). The ADR records this and recommends extending that boundary
when Camoufox browsing is built for search fact-checking, rather than
silently picking
quarantine-sandbox-runtimeper the letter of theoriginal request when a working answer to the actual problem (network
isolation for a browser) already exists and is deployed.
MCP-resolving harness (unlike OpenCode Review, which resolves
opencode.jsonc's MCP servers directly) — that's why this request is"add it to contextual-orchestrator," not duplicate work. But building
protocol-gateway plumbing before a real caller is wired up to use it would
be exactly the speculative abstraction this repo's own conventions reject.
Duplicate-work check
gh pr list --repo ContextualWisdomLab/contextual-orchestrator --state open --search "mcp OR a2a OR search OR searxng OR camoufox"and a repo-wide grepfor
mcp,a2a,searxng,web_search,perplexityfound no open PR andno existing product code for this capability. The only
web_searchhits areserver.py's honesty gate, which rejects a caller-supplied OpenAI-styleweb_search_optionschat-completions parameter — unrelated, and unaffectedby this change (this tool is a separate, explicitly-invoked function, never
an implicit chat-completions parameter).
Research (full detail:
docs/library_research.md)Google closed its scraping workaround in 2024.
self-hosted engine (own crawled P2P index, JSON/XML API); not implemented
for lack of a deployment to test against.
fork, no official MCP server (the
camofox-mcpthis repo already pinsby digest is a third-party wrapper).
ADR 0123 (DDD per
docs/product-goal-directive.md§5)Records Subdomain classification (core/supporting/generic), Bounded
Contexts (Web Search / Provider Routing / Privacy Policy Analysis / future
MCP Gateway / A2A Gateway / Camoufox Browsing), a Context Map (Anti-Corruption
Layer to SearXNG, minimal Shared Kernel to
ModelClient's transportprimitives, Conformist relationships to the external MCP and A2A specs),
Ubiquitous Language (disambiguating "MCP Gateway" from the existing
per-model "tool calling" capability detection in
chat_capability.py), andthe Value-Object/Domain-Service/Aggregate/Invariant mapping for
web_search()— plus an explicit "why no Aggregate/Repository/Domain Eventyet" note (nothing persists a search; premature to model one).
Test plan
pytest tests/test_web_search.py— 21 tests, all pass; no realnetwork calls (mocked
ModelClient._open_provider/_resolve_addresses,matching
test_privacy_policy_analysis.py's existing convention).coverage run --branch --source=contextual_orchestrator.web_search -m pytest tests/test_web_search.py→ 100% statement, 100% branch.
interrogate contextual_orchestrator/web_search.py→ 100%.pytest tests -q→ 3327 passed, 2 skipped, 0 failed.interrogate .(whole repo) → 100% docstring coverage maintained.py_compileon the new module and test file.🤖 Generated with Claude Code