Repository navigation
feat(agent): add opt-in keyless parallel search mcp - #354
winlp4ever merged 6 commits into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
winlp4ever
left a comment
winlp4ever
left a comment
There was a problem hiding this comment.
Hey George, thanks a lot for this, and for opening the issue first. A free search option with no key to set up is something I've wanted for self-hosters, so I'm happy you picked it up.
Some context that changes the shape a bit. We're moving the agent fully into the browser, and the server-side agent (backend/topix/agents/, WebSearchHandler, the YAML configs) is getting deleted soon. The parts of your PR that hook into the browser engine are in the right place: the keyless mode in resolve.ts, web-search.ts and the settings pane. Could you drop the server agent changes? That's the handler branch, the config.py validator bypass and the server agent paragraph in the README.
The bigger thing I'd like to talk through is where the call to Parallel happens. Right now every keyless search goes browser → our /ai/search → Parallel. On the hosted instance all our users would reach Parallel from one IP and share one anonymous allowance, and the endpoint becomes something anyone can call without signing in. Local boards are also meant to work with zero calls to our backend. Do you know if search.parallel.ai/mcp sends CORS headers? If it does, I'd prefer the browser to call it directly. Desktop can go direct in any case through Tauri's plugin-http, like BYOK search already does. /ai/search would then be a fallback, and we can require sign-in for it on hosted.
On the backend side I think we can drop the mcp SDK. Streamable HTTP is JSON-RPC over POST, so a small httpx call does the job in one round trip instead of about five, and it's easy to port to TS for the direct path.
Smaller point: Parallel is special-cased in five spots (resolveService, ai_search, meter_search, validate_engine, the handler). With search_parallel in _SEARCH_FNS and one shared set of keyless engines, the next keyless provider would be a one-line change.
I left a few inline notes. Thanks again, I'd love to get this merged.
| x_provider_key: Annotated[str | None, Header()] = None, | ||
| ) -> None: | ||
| """Guard keyless search by IP; retain existing metering for other engines.""" | ||
| if body.engine == "parallel": |
There was a problem hiding this comment.
This branch skips auth and quota, so the only guard left is the per-IP window. _client_ip takes the first X-Forwarded-For hop, which the caller controls, so a script that rotates that header never hits the 120/min cap. The BYOK relay uses the same helper, but there you need a key to get in. Here nothing is needed, and since all the traffic leaves from our server, one abuser can burn Parallel's anonymous limit for every user.
If we keep the relay, I'd require a signed-in user on this branch (self-hosters still get it), or at least a much lower cap.
| if not body.query.strip() or body.max_results < 1: | ||
| raise HTTPException(status.HTTP_400_BAD_REQUEST, "Query and positive max_results are required") | ||
| try: | ||
| output = await search_parallel(body.query, max_results=body.max_results, session_id=x_run_id) |
There was a problem hiding this comment.
x_run_id ends up as the MCP session_id. That's our internal run id (it also keys airun:{user}:{run} for metering), so Parallel could group all the searches of a user's run. Since the point of this option is that it's anonymous, I'd leave session_id out or send a random one per request.
| if retry_after: | ||
| response.headers["Retry-After"] = retry_after | ||
| raise HTTPException(code if code == 429 else 502, f"Parallel search failed (HTTP {code})") from error | ||
| except httpx.TimeoutException as error: |
There was a problem hiding this comment.
Only HTTPStatusError and TimeoutException get mapped here. A ConnectError, an McpError from the session, the RuntimeError raised on isError, or a ValidationError / JSONDecodeError on an unexpected payload all fall through to with_standard_response as a 500, with the raw upstream message in details. Could you catch the rest as a 502 with a generic message?
| url: str | ||
| title: str | None = None | ||
| publish_date: str | None = None | ||
| excerpts: list[str] |
There was a problem hiding this comment.
If one source comes back with excerpts: null or without the field (a PDF with no extracted text, for example), model_validate fails for the whole payload and we lose the other results too. A default of [] that also accepts None would keep the good ones.
| arguments["session_id"] = session_id | ||
| # A dedicated client has no auth, cookies, API keys or shared-client defaults. | ||
| try: | ||
| async with httpx.AsyncClient( |
There was a problem hiding this comment.
Each search builds a fresh client and MCP session: TLS handshake, initialize, notifications/initialized, tools/call, then the DELETE. That's around five round trips per query, and they all count against the anonymous limit. A module-level client with the same _anonymous_request hook keeps credentials out just as well. A plain JSON-RPC POST (see my main comment) would also let us drop the mcp dependency.
| event_hooks={"request": [_anonymous_request]}, | ||
| ) as client: | ||
| async with streamable_http_client(MCP_URL, http_client=client) as (read, write, _): | ||
| async with ClientSession(read, write, read_timeout_seconds=timedelta(seconds=60)) as session: |
There was a problem hiding this comment.
The session timeout is fixed at 60s while the HTTP client uses timeout or 30s. A caller asking for a longer timeout won't get it, and depending on which one fires first we return a 500 (McpError) or a 504 (TimeoutException). I'd derive both from the same value.
| wrapper: RunContextWrapper[Context], | ||
| query: str, | ||
| ) -> WebSearchOutput: | ||
| if search_engine == WebSearchOption.PARALLEL: |
There was a problem hiding this comment.
This file and the config.py validator are on the server agent path, which we're removing soon, so I'd drop these changes. The enable_summary change above also changes behaviour for every engine on that path, so I'd keep it out of this PR in any case.
| kind: ServiceKind, | ||
| ctx: ResolveContext, | ||
| ): ServiceResolution => { | ||
| if (kind === "search" && ctx.searchEngine === "parallel") { |
There was a problem hiding this comment.
This makes Parallel count as available in every case, including desktop with no server URL set, and offline. Settings show it as ready, the agent gets web_search, and each call fails with a network error. The other engines resolve to off there, so the tool is hidden. Could we only return keyless when the relay (or, with a direct path, the network) is reachable?
| const { provider, llm, searchEngine, search, codeKey, parseKey } = get() | ||
| try { | ||
| if (hasAnyKey(get())) { | ||
| if (hasAnyKey(get()) || searchEngine === "parallel") { |
There was a problem hiding this comment.
Small one: clear() wipes localStorage but leaves searchEngine set to parallel in memory. The current session keeps using Parallel, then after a reload it falls back to perplexity, which is off for signed-out users, and search disappears with no explanation. Resetting searchEngine in clear() would line the two up.
|
Thanks @winlp4ever, really helpful review. Yes on CORS: I'll drop the server agent bits (handler, config.py validator, README server paragraph) and rework toward browser/desktop → Parallel direct, with Pushing that up shortly. |
…arch Responds to review on vcmf#354. - Revert the server-agent changes: `WebSearchHandler`'s parallel branch (and the `enable_summary` change it carried for every engine), the `config.py` validator bypass and the `WebSearchOption.PARALLEL` member. That tree is being deleted; keyless search belongs to the browser engine. - Replace the MCP SDK session (initialize + notifications + tools/call + DELETE, ~5 round trips) with one plain JSON-RPC POST over httpx, on a reused module-level client whose request hook strips credential headers. Drops the `mcp` dependency, so pyproject and uv.lock match main again. - Register `search_parallel` in `_SEARCH_FNS` and gate on one shared `KEYLESS_SEARCH_ENGINES` set instead of per-callsite `== "parallel"`. - The relay is now a fallback, so require a signed-in user for it on a hosted deploy: a forgeable `X-Forwarded-For` window was its only guard. Self-hosted keeps it open behind a tighter per-IP cap. - Never send the internal run id as an upstream session handle; use a throwaway request id. - Map ConnectError / tool `isError` / ValidationError / JSONDecodeError to 502 with a generic message rather than a 500 carrying the raw upstream reply, and derive the HTTP timeout from the caller's single `timeout` value. - Default a missing or null `excerpts` to `[]` so one text-less source does not invalidate the whole payload. Co-authored-by: George Pickett <georgeatparallel@users.noreply.github.com>
…ktop Responds to review on vcmf#354. `search.parallel.ai/mcp` sends `Access-Control-Allow-Origin: *` and allows the MCP request headers, so the client can reach it cross-origin. The browser now calls it directly and desktop goes through plugin-http like BYOK search, which means a local board searches the web with no call to a Dim0 backend at all. `/ai/search` stays only as the fallback when the direct call cannot get through. - Add `services/parallel-search.ts`: the same one-POST JSON-RPC exchange the backend relay speaks, with a throwaway request id and no credential headers. - Resolve `keyless` through a shared `KEYLESS_SEARCH_ENGINES` set, and only while the network is reachable — offline it resolves `off` and `web_search` is hidden, like every other unreachable engine, instead of advertising a tool whose every call would fail. - Reset `searchEngine` in `clear()` so the in-memory selection and localStorage agree instead of diverging until the next reload. - Update the settings copy, README and ADR-AGENT-003 to the new shape, and cover the direct path, the relay fallback, offline resolution and `clear()` in unit and e2e tests. Co-authored-by: George Pickett <georgeatparallel@users.noreply.github.com>
|
Pushed e2aea28 with the review rework:
Happy to tweak further if anything still looks off. |
A refusal from Parallel (a rate limit, a tool error) means the shared anonymous allowance is already spent, so replaying the same query through `/ai/search` would only spend it again from our IP. Distinguish that from a transport failure — the case the relay fallback exists for — and let the refusal surface to the agent instead. Co-authored-by: George Pickett <georgeatparallel@users.noreply.github.com>
|
One more small commit on top (bf706fc): if Parallel itself refuses a request (e.g. rate limit), the client no longer falls back to the |
|
Thanks a lot George, the rework is really nice. Calling Parallel from the client is the direction I wanted, and you also took care of the refusal case so it doesn't get relayed twice. I ran the backend tests, the frontend tests and There are two small things I'd like fixed before merging.
Apart from that the PR is in good shape. Once these two are in I'll try to merge it tomorrow or the day after. |
Co-authored-by: George Pickett <georgeatparallel@users.noreply.github.com>
Co-authored-by: George Pickett <georgeatparallel@users.noreply.github.com>
|
Thanks @winlp4ever, good catches on both. Pushed 3bd87ef:
Backend unit tests, |
|
Thanks @winlp4ever for the careful review and for merging! |
What & why
Adds optional, free keyless Parallel Search MCP to native web search and the browser agent. Select Settings → Web search → Parallel (keyless). The browser and desktop app call Parallel directly;
/ai/searchis only a fallback (signed-in users on hosted). Existing defaults and saved keys stay intact. Anonymous rate limits apply.I work at Parallel. For context: Artificial Analysis's Search API benchmarks.
Related issue
Closes #353.
How was it tested?
Clean installation, live native search and
/ai/search, anonymous headers and User-Agent, errors, timeouts and cleanup. Browser selection, persistence and agent tool-result flow tested with a scripted model. No live LLM or database-backed full-stack run.Checklist
maintype(scope): message)