feat(pi): optional multi-provider web search for the coding agent - #5951
Open
BillLeoutsakosvl346 wants to merge 6 commits into
Open
feat(pi): optional multi-provider web search for the coding agent#5951BillLeoutsakosvl346 wants to merge 6 commits into
BillLeoutsakosvl346 wants to merge 6 commits into
Conversation
Adds a search provider dropdown (Exa, Serper, Parallel, Firecrawl) to the Pi block, off by default. The selected provider's key comes from the block field or Workspace Settings → BYOK; a Sim-hosted key is never spent, so a missing key fails the run with a setup message instead of quietly billing Sim. Search is available in all three modes. Local Dev and Review Code register a host-side tool that goes through the existing provider tools, while Create PR has no host in the loop and gets a generated Pi extension in the sandbox. Both paths derive their requests from one normalizer and are held together by a parity test, since the sandbox copy cannot import Sim's code. Results are normalized to title, URL, snippet, and publication date, capped per field and per envelope, marked untrusted in the prompt, and limited to 20 searches per run so a tool loop cannot drain the workspace's quota. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryAdds optional, BYOK-only web search to every Pi Coding Agent mode.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking defects identified. The provider mappings, credential boundaries, permission checks, execution-mode integrations, bounded request behavior, normalization paths, and secret-redaction boundaries are internally consistent and covered by focused parity and backend tests.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/handlers/pi/pi-handler.ts | Resolves optional search configuration, enforces tool permissions before credential access, and dispatches mode-specific tool implementations. |
| apps/sim/executor/handlers/pi/search/normalize.ts | Defines bounded provider-neutral arguments, request mappings, result normalization, and serialized output limits. |
| apps/sim/executor/handlers/pi/search/tool.ts | Implements the host-side search adapter using existing registered tools, explicit user credentials, failure classification, and a per-run call budget. |
| apps/sim/executor/handlers/pi/search/extension-source.ts | Generates the Create PR sandbox extension with equivalent provider requests, normalization, timeout, response-size, and call-count controls. |
| apps/sim/executor/handlers/pi/cloud-backend.ts | Installs and activates the sandbox search extension when configured while extending credential redaction across surfaced outputs and errors. |
| apps/sim/executor/handlers/pi/cloud-review-backend.ts | Registers host-side web search for Review Code and adjusts its sealed capability prompt and tool allowlist. |
| apps/sim/executor/handlers/pi/keys.ts | Adds strict provider parsing and resolves search credentials exclusively from block input or workspace BYOK. |
| apps/sim/lib/core/security/redaction.ts | Extends sensitive-field recognition to prefixed API-key names and passphrases. |
| apps/sim/tools/serper/search.ts | Preserves the optional date supplied on Serper organic search results. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Block[Pi block inputs] --> Handler[Pi handler]
Handler --> Permission[Search permission preflight]
Permission --> Keys[Block key or workspace BYOK]
Keys --> Mode{Execution mode}
Mode -->|Local Dev| Host[Host-side web_search tool]
Mode -->|Review Code| Host
Mode -->|Create PR| Sandbox[Generated sandbox extension]
Host --> Tools[Existing provider ToolConfig]
Sandbox --> APIs[Provider API]
Tools --> Normalize[Normalized bounded results]
APIs --> Normalize
Normalize --> Agent[Pi agent]
Reviews (1): Last reviewed commit: "feat(pi): optional multi-provider web se..." | Re-trigger Greptile
`check:utils` bans `JSON.parse(JSON.stringify(...))`. The round-trip was normalizing the host body to its wire form, which buys nothing here: the bodies are plain JSON and `toEqual` already ignores undefined members. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Video demo: Screen.Recording.2026-07-26.at.12.48.51.PM.mp4 |
Create PR streams the whole Pi run through one Connect server-stream (`commands.run` -> envd `Process.Start`), held open for the full `PI_TIMEOUT_MS`. Mid-stream it could die with: [internal] protocol error: received unsupported compressed output That string is `@connectrpc/connect-web`, not Pi — Pi has no Connect dependency at all. connect's `compressedFlag` is `0b00000001` and gzip's magic first byte is `0x1f`; `0x1f & 0x01 === 1`, so a raw gzip body fed to the envelope reader trips this on byte one. It reads as "the server sent a compressed envelope" but really means "this was never a Connect envelope" — an HTTP-level gzip that was not transparently decompressed. e2b 2.30.0 pinned `@connectrpc/connect-web@2.0.0-rc.3` and drove envd through undici 7 with `allowH2: true`. e2b 2.36.1 moves to stable connect-web 2.1.2 and loads undici 8.8.0 when Node >= 22.19.0 — exactly our engine floor — so the failing path gets a different HTTP stack. The connect-web upgrade alone is not the fix: 2.0.0-rc.3 and 2.1.2 ship a byte-identical `connect-transport.js` (bar the copyright year), and connect-web still has no `acceptCompression` option by design. The undici 8 swap is the part that matters. `@e2b/code-interpreter@2.7.0` only asks for `e2b: ^2.28.0`, so the override pins the floor we actually need. Verified API-compatible: every method we call (`Sandbox.create`, `runCode`, `commands.run`, `files.read/write`, `kill`, `Template`, `defaultBuildLogger`, `waitForTimeout`) has an identical signature across the two versions, and we never touch `SandboxPaginator`, the one type that changed.
…ted scope Follow-ups from review of the web-search work. Each fix lands in both the host adapter (`normalize.ts`) and the Create PR sandbox copy (`extension-source.ts`), with the extension test asserting the two produce byte-identical envelopes. - `usableUrl` was the one provider-controlled field not whitespace-bounded: title/snippet/date all go through `collapseWhitespace`, `url` only trimmed. Up to 2048 chars of newlines and control characters could ride into the envelope. Dropped rather than collapsed — `url` must stay byte-exact to stay resolvable, so collapsing would emit a different, still-dead link, and a URL carrying raw whitespace is already malformed under RFC 3986. - `numResults: null` (or `''`, or `[]`) returned 1 result, not the documented default of 5: `Number(null)` is a finite 0, so the clamp floor won rather than the default. Only a real number or a non-blank numeric string now counts as the model having asked for a count. - Envelope truncation was silent. When results were dropped to fit the 50 KB ceiling the model read the short list as the complete answer. It now carries a message saying so, and the message is inside what gets measured so the note cannot push a truncated envelope back over the ceiling. - The budget is per *block execution*, not per workflow run: the counter lives in the tool spec and both adapters build a fresh one per execution, so a Pi block inside a Loop gets the full allowance every iteration. The constant, the agent-facing message, and the docs all claimed "per run". Renamed to `PI_SEARCH_MAX_CALLS_PER_EXECUTION` and corrected the wording rather than tightening the cap, since a shared ceiling would fail late iterations of a legitimate fan-out. - The Search API Key tooltip promised "switching providers clears this field". That clear is driven through the collaborative editor setter, so a workflow imported, forked, or updated via the API keeps the previous provider's key — exactly the case where sending it to a new vendor matters. Docs also gain a warning that Create PR hands both the model key and the search key to the agent as environment variables, which Pi copies into every bash child. That matters most for Settings > BYOK keys: those are workspace-scoped, only admins can manage them, and the API only ever returns them masked — yet anyone who can run a Pi block in Create PR mode can read the raw value.
The "you cannot add a provider without mirroring it" story rested on two mechanisms that did not hold. Verified by adding a fifth provider to `PI_SEARCH_PROVIDERS` and running the build: it produced only two errors, and every test still passed. - `normalizePiSearchRecords` assigns to `let built` inside its switch rather than returning, so unlike its two siblings a missing case was not a type error — it silently normalized the new provider to zero results. Added an explicit `never` check. - The sandbox copy's `normalizeRecords` used a trailing `else` for Firecrawl, so an unmirrored provider was silently normalized with Firecrawl's field names; `extractRecords` did the same with its `payload.data` tail. Both now test for `firecrawl` explicitly and throw otherwise. - `Record<PiSearchProvider, ...>` on the `TOOLS` and `payloads` fixtures looked like exhaustiveness guards but are inert: `apps/sim/tsconfig.json` excludes `**/*.test.ts`, and vitest transpiles without typechecking. Both suites drive their providers off `Object.keys(fixture)`, so a missing provider was skipped rather than failed. Each suite now asserts its fixture covers the registry. Re-running the same experiment now yields three compile errors plus two test failures naming the missing fixtures.
A fallback exists so a key has somewhere to go when the field is unavailable. The Search API Key field is unconditionally available: unlike the model key, whose visibility runs through `shouldRequireApiKeyForModel` and its `isHosted` branch, `getSearchApiKeyCondition` gates only on whether a provider is selected. So the fallback never had a configuration to cover. Removing it also closes an escalation. Workspace BYOK keys are admin-managed and the API only ever returns them masked, yet `resolvePiSearchKey` would resolve one for any member who could run the block — and in Create PR that key is handed to the sandbox as an environment variable, which Pi copies into every bash child. A member could read a credential the product deliberately never shows them. Requiring the key on the block keeps the sandbox exposure to a key its author already holds. Nothing depends on the fallback: it has never shipped. - `resolvePiSearchKey` is now synchronous and returns the key, since there is no lookup left to await. `byokProviderId` leaves the search registry and `PiSearchKeySource` / `PiSearchKeyResolution` are gone — with one source, `keySource` carried no information, and the logging rationale for it (a block field silently shadowing a stored key) no longer exists. - The field is now `required`. Safe alongside its condition: the serializer's required check returns early for fields that are not visible, so a Pi block with search off still validates. Pinned by a test. Docs and the block's tooltip, placeholder, and best practices updated. The Create PR key-exposure callout now explains the missing fallback rather than recommending the block field as a way around it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional internet search capability to the Pi Coding Agent block: a Search Provider dropdown (None, Exa, Serper, Parallel, Firecrawl), off by default, available in all three modes — Create PR, Review Code, and Local Dev.
web_searchtool that reuses the existingexa_search/serper_search/parallel_search/firecrawl_searchtools. Create PR has no host in the agent loop, so it gets a generated Pi extension written into the sandbox that makes its own boundedfetchcalls. Both paths derive their requests from the same normalizer insearch/normalize.ts, andsearch/parity.test.tsasserts the two produce byte-identical URLs, headers, and bodies per provider — the sandbox copy cannot import Sim's code, so that test is what keeps it from drifting.Two drive-by fixes it needed:
serper_searchwas dropping thedatefield on organic results, and Sim's redaction patterns did not match keys named*ApiKeyorpassphrase.Test plan
search/{normalize,tool,extension-source,parity}.test.ts— 62 tests covering argument clamping, per-provider normalization and fallback chains, envelope byte caps, key-source precedence, failure classification, the per-run budget, and host/sandbox request paritypi-handler,keys,local-backend,cloud-backend,cloud-review-backend,blocks/blocks/pi,redaction,tools/serper— 826 tests pass, including backward compatibility for Pi blocks saved before the fields existedtsc --noEmit,biome check, repolintandformatclean on the rebased baseReviewer notes
Two things worth a second opinion, both deliberate:
pi.mdxas a reason not to enable search when reviewing untrusted forks of private repos.Made with Cursor