feat: proxy Tavily web search through API server#1576
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a proxied web-search: new API POST route using TAVILY_API_KEY and Tavily SDK, moves Tavily usage to the API, updates agent web-search to call the API (with apiUrl and optional authToken), propagates authToken from desktop agent, and exposes TAVILY_API_KEY in env and CI workflows. Changes
Sequence DiagramsequenceDiagram
participant Desktop as Desktop Agent
participant Manager as Agent Manager
participant API as API Server
participant Tavily as Tavily Service
Desktop->>Manager: runAgent(options, authToken?)
Manager->>API: POST /api/chat/tools/web-search<br/>Body: { query, maxResults }<br/>Header: Authorization: Bearer authToken
API->>API: authenticate user + rate limit + validate/clamp input
API->>Tavily: TavilyClient.search(query, maxResults) using TAVILY_API_KEY
Tavily-->>API: results
API->>API: sanitize results (title, url, content)
API-->>Manager: JSON results
Manager-->>Desktop: deliver results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
force-pushed
the
mastra-tavily
branch
from
5a4da77 to
bfe45c5
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
force-pushed
the
mastra-tavily
branch
from
bfe45c5 to
b2a635f
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/agent/src/tools/web-search.ts (1)
47-53:resultSchema.parse()on a successful response is correct, but consider guarding proxy errors before Zod parsing.Line 49 includes
await response.text()in the thrown error, which surfaces full API server error bodies (e.g., rate-limit messages, stack traces in non-production) back to the agent. Since the proxy is internal, this may be acceptable, but it can also leak sensitive operational detail.More importantly on line 53:
resultSchema.parse(await response.json())throws aZodErrorif the API ever changes its response shape. Wrapping this parse with a clearer error message improves debuggability:🛠️ Suggested hardening
- if (!response.ok) { - throw new Error( - `Web search proxy returned ${response.status}: ${await response.text()}`, - ); - } - - return resultSchema.parse(await response.json()); + if (!response.ok) { + // Avoid leaking full server error bodies; log on the agent side if needed + throw new Error(`Web search proxy error: HTTP ${response.status}`); + } + + const json = await response.json(); + const parsed = resultSchema.safeParse(json); + if (!parsed.success) { + throw new Error("Web search proxy returned an unexpected response shape"); + } + return parsed.data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent/src/tools/web-search.ts` around lines 47 - 53, The current code reads the response body multiple ways and calls resultSchema.parse(await response.json()) directly, which can leak raw proxy bodies and will throw an uncaught ZodError if the shape changes; fix by first reading the response body once (e.g., const body = await response.text() or await response.json() depending on status), on non-ok responses throw an error with sanitized/limited body (do not include full raw stack traces), and on ok responses parse the already-read JSON inside a try/catch around resultSchema.parse to catch ZodError and rethrow a clearer error that includes the status and validation error details (reference resultSchema.parse, response.ok, and the response body variable) so parsing failures surface with context instead of raw ZodError or full proxy body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/api/tools/web-search/route.ts`:
- Around line 5-12: The POST handler (function POST) currently allows
authenticated users to call the Tavily proxy endpoint without limits; add
per-user rate limiting using the existing Upstash KV/Redis store to prevent
quota exhaustion. In POST, after obtaining session via auth.api.getSession and
before performing the external API call, look up/increment a user-specific
counter keyed by session.user.id (or session.user.email) in Upstash, enforce a
max requests-per-window (or per-session budget), and return a 429 Response when
the limit is exceeded; ensure counters use TTLs for sliding/rolling windows and
decrement or reset appropriately. Integrate the rate-limit check as an early
guard in POST and log rate-limit hits; reuse any existing Upstash client/utils
in the repo or create a small helper (e.g., ensureRateLimit(userId)) to
encapsulate the logic.
- Around line 34-48: The code instantiates tavily({ apiKey: env.TAVILY_API_KEY
}) inside the request handler and returns raw error.message to clients; move the
Tavily axios client to module scope as a lazy singleton (create a module-level
variable and initialize it on first use, referenced by the handler that calls
client.search) to avoid recreating the client per request, and change the catch
block so it logs the full error detail server-side (e.g., console.error or your
request logger with the caught error object from the catch) while returning a
generic Response.json({ error: "Search service unavailable" }, { status: 502 })
to the caller instead of error.message; keep references to tavily,
client.search, env.TAVILY_API_KEY and Response.json when applying the changes.
In `@apps/api/src/env.ts`:
- Line 49: TAVILY_API_KEY is currently required in env.ts which will crash
deployments without it; change the zod schema entry for TAVILY_API_KEY to be
optional (use z.string().optional()) and then update the web-search route
(route.ts) to check env.TAVILY_API_KEY at runtime and return a 503 (or otherwise
degrade gracefully) when the key is missing instead of relying on it being
present at startup.
---
Nitpick comments:
In `@packages/agent/src/tools/web-search.ts`:
- Around line 47-53: The current code reads the response body multiple ways and
calls resultSchema.parse(await response.json()) directly, which can leak raw
proxy bodies and will throw an uncaught ZodError if the shape changes; fix by
first reading the response body once (e.g., const body = await response.text()
or await response.json() depending on status), on non-ok responses throw an
error with sanitized/limited body (do not include full raw stack traces), and on
ok responses parse the already-read JSON inside a try/catch around
resultSchema.parse to catch ZodError and rethrow a clearer error that includes
the status and validation error details (reference resultSchema.parse,
response.ok, and the response body variable) so parsing failures surface with
context instead of raw ZodError or full proxy body.
| try { | ||
| const client = tavily({ apiKey: env.TAVILY_API_KEY }); | ||
| const response = await client.search(body.query, { maxResults }); | ||
|
|
||
| return Response.json({ | ||
| results: response.results.map((r) => ({ | ||
| title: r.title, | ||
| url: r.url, | ||
| content: r.content, | ||
| })), | ||
| }); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : "Search failed"; | ||
| return Response.json({ error: message }, { status: 502 }); | ||
| } |
There was a problem hiding this comment.
Tavily client instantiated on every request; raw Tavily error text forwarded to caller.
Two separate concerns here:
-
tavily({ apiKey: ... })creates a new configured axios client on every request. Promote it to module scope as a lazy singleton. -
Line 47 returns
error.messageverbatim in the 502 body. Tavily error messages can include quota-exhaustion details or API-key validation text, which need not be exposed to the agent. Return a generic message and log the detail server-side.
♻️ Proposed refactor
+// Module-level singleton — initialised once per cold start
+let _client: ReturnType<typeof tavily> | null = null;
+function getClient() {
+ if (!_client) _client = tavily({ apiKey: env.TAVILY_API_KEY! });
+ return _client;
+}
+
export async function POST(request: Request): Promise<Response> {
// ... auth / body parsing ...
try {
- const client = tavily({ apiKey: env.TAVILY_API_KEY });
- const response = await client.search(body.query, { maxResults });
+ const response = await getClient().search(body.query, { maxResults });
return Response.json({ results: response.results.map(...) });
} catch (error) {
- const message = error instanceof Error ? error.message : "Search failed";
- return Response.json({ error: message }, { status: 502 });
+ console.error("[web-search] Tavily error:", error);
+ return Response.json({ error: "Search failed" }, { status: 502 });
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/api/tools/web-search/route.ts` around lines 34 - 48, The
code instantiates tavily({ apiKey: env.TAVILY_API_KEY }) inside the request
handler and returns raw error.message to clients; move the Tavily axios client
to module scope as a lazy singleton (create a module-level variable and
initialize it on first use, referenced by the handler that calls client.search)
to avoid recreating the client per request, and change the catch block so it
logs the full error detail server-side (e.g., console.error or your request
logger with the caught error object from the catch) while returning a generic
Response.json({ error: "Search service unavailable" }, { status: 502 }) to the
caller instead of error.message; keep references to tavily, client.search,
env.TAVILY_API_KEY and Response.json when applying the changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/api/src/app/api/chat/tools/web-search/route.ts (2)
34-36: Tavily client is re-instantiated on every request — move to module level.
tavily({ apiKey: env.TAVILY_API_KEY })is called inside the handler on every POST. Sinceenv.TAVILY_API_KEYis static for the lifetime of the process, the client can be a module-level singleton.♻️ Proposed refactor
import { auth } from "@superset/auth/server"; import { tavily } from "@tavily/core"; import { env } from "@/env"; +const tavilyClient = tavily({ apiKey: env.TAVILY_API_KEY }); export async function POST(request: Request): Promise<Response> { // ... try { - const client = tavily({ apiKey: env.TAVILY_API_KEY }); - const response = await client.search(body.query, { maxResults }); + const response = await tavilyClient.search(body.query, { maxResults });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/chat/tools/web-search/route.ts` around lines 34 - 36, The Tavily client is being recreated on every request inside the POST handler; move the instantiation to module scope so a single singleton is reused. Create a module-level constant (e.g., tavilyClient = tavily({ apiKey: env.TAVILY_API_KEY })) and replace the in-handler tavily({ ... }) call with tavilyClient in the function that currently does const client = tavily(...); ensure any error handling still uses the same client variable name (client or tavilyClient) and that env.TAVILY_API_KEY is read once at module load.
5-49: No rate limiting on the endpoint — authenticated users can generate unbounded Tavily costs.Any valid session can call
POST /api/tools/web-searchin a tight loop. There is no per-user or global rate cap. Given that Tavily is a paid API, this is a direct cost-amplification risk.Consider adding a per-user rate limit (e.g., via an existing middleware or
upstash/ratelimitbacked by theKV_REST_API_URL/KV_REST_API_TOKENalready present in the env) before the Tavily call. At a minimum, log the user ID and query so abuse can be detected via API server logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/chat/tools/web-search/route.ts` around lines 5 - 49, Add per-user rate limiting to the POST handler in route.ts (the exported async function POST) before calling tavily({ apiKey: env.TAVILY_API_KEY }) to prevent unbounded paid API usage; implement this using your existing upstash/ratelimit helper or middleware (backed by KV_REST_API_URL/KV_REST_API_TOKEN) and check/consume a token for session.user.id (or another stable user identifier), returning a 429 when the quota is exceeded. Also emit a server log entry including the session user id and the sanitized query string prior to invoking client.search so abuse can be audited; keep logging minimal (user id + query) and ensure any sensitive data is redacted. Ensure the rate-check happens after session validation and before the try/catch around the tavily call so rejected requests never reach the Tavily client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/api/chat/tools/web-search/route.ts`:
- Around line 45-48: The current catch block in
apps/api/src/app/api/chat/tools/web-search/route.ts returns error.message (from
`@tavily/core`) to clients which may leak internal details; update the catch in
the async handler (the try/catch that returns Response.json) to always send a
generic error body (e.g., { error: "Search failed" } or similar) and a 502
status, while logging the full error (the caught error or error.message and
stack) to the server logger or console for debugging; keep the error instanceof
Error check only for logging purposes and do not forward the raw error.message
to the Response.json payload.
- Around line 1-49: Update the env schema so TAVILY_API_KEY is optional (make
env.TAVILY_API_KEY optional in your env validation) and in the POST handler
(function POST in route.ts) guard usage by checking env.TAVILY_API_KEY before
calling tavily(); if the key is missing return an appropriate error response
(e.g., 501/400 with a clear message like "TAVILY_API_KEY not configured")
instead of constructing the client, and only call tavily({ apiKey:
env.TAVILY_API_KEY }) and client.search(...) when the key is present.
- Around line 28-32: The code uses Number.isFinite on rawMax which allows floats
(e.g., 3.7) to pass, so change the validation/coercion for maxResults to ensure
an integer between 1 and 10: either use Number.isInteger(rawMax) in the
conditional or explicitly coerce with Math.trunc/Math.floor/Math.round (e.g.,
const sanitized = Math.trunc(Number(rawMax)); const maxResults =
Number.isFinite(sanitized) ? Math.min(Math.max(sanitized, 1), 10) : 5), and make
sure the integer maxResults is what you pass to client.search().
---
Nitpick comments:
In `@apps/api/src/app/api/chat/tools/web-search/route.ts`:
- Around line 34-36: The Tavily client is being recreated on every request
inside the POST handler; move the instantiation to module scope so a single
singleton is reused. Create a module-level constant (e.g., tavilyClient =
tavily({ apiKey: env.TAVILY_API_KEY })) and replace the in-handler tavily({ ...
}) call with tavilyClient in the function that currently does const client =
tavily(...); ensure any error handling still uses the same client variable name
(client or tavilyClient) and that env.TAVILY_API_KEY is read once at module
load.
- Around line 5-49: Add per-user rate limiting to the POST handler in route.ts
(the exported async function POST) before calling tavily({ apiKey:
env.TAVILY_API_KEY }) to prevent unbounded paid API usage; implement this using
your existing upstash/ratelimit helper or middleware (backed by
KV_REST_API_URL/KV_REST_API_TOKEN) and check/consume a token for session.user.id
(or another stable user identifier), returning a 429 when the quota is exceeded.
Also emit a server log entry including the session user id and the sanitized
query string prior to invoking client.search so abuse can be audited; keep
logging minimal (user id + query) and ensure any sensitive data is redacted.
Ensure the rate-check happens after session validation and before the try/catch
around the tavily call so rejected requests never reach the Tavily client.
| const rawMax = body.maxResults; | ||
| const maxResults = | ||
| typeof rawMax === "number" && Number.isFinite(rawMax) | ||
| ? Math.min(Math.max(rawMax, 1), 10) | ||
| : 5; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@tavily/core search() maxResults parameter type integer number typescript
💡 Result:
I couldn’t find any public docs/source for @tavily/core’s search() signature via web search right now (all queries returned no results), so I can’t verify the exact exported TypeScript type for maxResults.
That said, in TypeScript there is no distinct integer type—integers are represented as number. So SDKs virtually always type an “integer” option like this as:
maxResults?: number
and rely on runtime validation (or the API) to enforce “must be an integer”.
Practical guidance:
await tavily.search("query", { maxResults: 10 }); // pass an integer numberIf you want to enforce integer-ness yourself:
const maxResults = 10;
if (!Number.isInteger(maxResults)) throw new Error("maxResults must be an integer");🏁 Script executed:
# Check for package.json files
fd -t f 'package.json' | head -5
# Look for `@tavily/core` in lock files
rg '@tavily/core' --type json -A 3 2>/dev/null | head -30Repository: superset-sh/superset
Length of output: 395
🏁 Script executed:
# Search for uses of tavily client.search
rg 'client\.search' --type ts --type tsx -B 2 -A 3
# Look for TypeScript type definitions related to tavily
fd -e 'd.ts' | xargs rg 'maxResults|Tavily' 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for uses of tavily client.search properly
rg 'client\.search' -A 3 -B 2
# Also check for type definitions in node_modules
find . -path ./node_modules -prune -o -name '*tavily*' -type f -print | head -20Repository: superset-sh/superset
Length of output: 576
🌐 Web query:
Tavily API maxResults parameter specification documentation
💡 Result:
In the official Tavily Search API, the parameter is named max_results (snake_case), not maxResults.
- Parameter:
max_results - Type: integer
- Default:
5 - Meaning: “The maximum number of search results to return.”
- Allowed range:
0 <= x <= 20[1] - Tavily’s docs also note
max_resultsis one of the parameters that must be set manually when usingauto_parameters, because it affects response size. [1]
If you’re seeing maxResults (camelCase), that’s typically from a third-party wrapper/integration, not Tavily’s native REST API (example: Wanaku’s Tavily tool exposes maxResults:int). [2]
Sources:
[1] Tavily Docs – Search endpoint API reference (parameter max_results, default/range, auto_parameters note)
[2] Wanaku MCP Router docs – Tavily tool parameter maxResults
Number.isFinite accepts floats — maxResults may forward a non-integer to Tavily.
Number.isFinite(3.7) returns true, so a float value like 3.7 passes validation and is sent to client.search(). The Tavily API expects max_results to be an integer.
🛠️ Proposed fix
const rawMax = body.maxResults;
const maxResults =
- typeof rawMax === "number" && Number.isFinite(rawMax)
+ Number.isInteger(rawMax)
? Math.min(Math.max(rawMax, 1), 10)
: 5;📝 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.
| const rawMax = body.maxResults; | |
| const maxResults = | |
| typeof rawMax === "number" && Number.isFinite(rawMax) | |
| ? Math.min(Math.max(rawMax, 1), 10) | |
| : 5; | |
| const rawMax = body.maxResults; | |
| const maxResults = | |
| Number.isInteger(rawMax) | |
| ? Math.min(Math.max(rawMax, 1), 10) | |
| : 5; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/api/chat/tools/web-search/route.ts` around lines 28 - 32,
The code uses Number.isFinite on rawMax which allows floats (e.g., 3.7) to pass,
so change the validation/coercion for maxResults to ensure an integer between 1
and 10: either use Number.isInteger(rawMax) in the conditional or explicitly
coerce with Math.trunc/Math.floor/Math.round (e.g., const sanitized =
Math.trunc(Number(rawMax)); const maxResults = Number.isFinite(sanitized) ?
Math.min(Math.max(sanitized, 1), 10) : 5), and make sure the integer maxResults
is what you pass to client.search().
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : "Search failed"; | ||
| return Response.json({ error: message }, { status: 502 }); | ||
| } |
There was a problem hiding this comment.
Tavily error messages forwarded verbatim to the client may leak internal details.
error.message from @tavily/core can include API-key status hints, rate-limit details, or other implementation information. Returning it in the 502 response exposes these to the caller.
🛡️ Proposed fix
} catch (error) {
- const message = error instanceof Error ? error.message : "Search failed";
- return Response.json({ error: message }, { status: 502 });
+ console.error("[web-search] Tavily error:", error);
+ return Response.json({ error: "Search failed" }, { status: 502 });
}📝 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.
| } catch (error) { | |
| const message = error instanceof Error ? error.message : "Search failed"; | |
| return Response.json({ error: message }, { status: 502 }); | |
| } | |
| } catch (error) { | |
| console.error("[web-search] Tavily error:", error); | |
| return Response.json({ error: "Search failed" }, { status: 502 }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/api/chat/tools/web-search/route.ts` around lines 45 - 48,
The current catch block in apps/api/src/app/api/chat/tools/web-search/route.ts
returns error.message (from `@tavily/core`) to clients which may leak internal
details; update the catch in the async handler (the try/catch that returns
Response.json) to always send a generic error body (e.g., { error: "Search
failed" } or similar) and a 502 status, while logging the full error (the caught
error or error.message and stack) to the server logger or console for debugging;
keep the error instanceof Error check only for logging purposes and do not
forward the raw error.message to the Response.json payload.
b2a635f to
fac3a4c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/agent/src/tools/web-search.ts (1)
39-55: No timeout on the proxyfetch— agent can hang indefinitely.The
fetchto the proxy has noAbortController/ signal timeout. If the API server is slow or unreachable, the tool will stall until the host runtime enforces its own deadline.♻️ Proposed fix
+ const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), 30_000); const response = await fetch(`${apiUrl}/api/chat/tools/web-search`, { method: "POST", headers: { ... }, body: JSON.stringify({ ... }), + signal: controller.signal, }); + clearTimeout(timer);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent/src/tools/web-search.ts` around lines 39 - 55, The fetch to the proxy (the call that uses apiUrl, authToken and input.query and assigns to response) needs an AbortController-based timeout so the agent doesn't hang: create an AbortController, start a timer (configurable timeout) that calls controller.abort(), pass controller.signal into the fetch options, and clear the timer after the fetch completes; ensure abort errors are handled/propagated consistently with the existing response.ok error path. Use the AbortController/signal around the existing fetch/response logic so the request is canceled on timeout and resources are cleaned up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/package.json`:
- Line 33: The package.json dependency for "@tavily/core" specifies a
non-existent version "^0.7.1"; update the dependency entry for "@tavily/core" in
package.json to a published version (e.g., "^0.5.11") so npm install can resolve
it, then run npm install and verify the lockfile updates and CI passes.
---
Duplicate comments:
In `@apps/api/src/app/api/chat/tools/web-search/route.ts`:
- Around line 5-12: Add per-user sliding-window rate limiting inside the POST
handler (before any external Tavily call) by using Upstash KV to track request
timestamps/counts for session.user.id returned by auth.api.getSession; implement
an atomic increment-and-expire or a sliding-window algorithm (store recent
request timestamps or a token-bucket counter) keyed by the user ID, reject
requests with a 429 Response when the quota for the window is exceeded, and
ensure the check occurs in the POST function immediately after verifying
session?.user to prevent any authenticated user from calling Tavily unboundedly.
- Around line 28-32: The current clamp uses Number.isFinite on rawMax which
allows floats (e.g., 3.7) to pass; update the computation of maxResults so it
always becomes an integer before clamping: validate body.maxResults (rawMax) as
a finite number, coerce to an integer (e.g., Math.trunc or Math.floor on rawMax)
and then clamp between 1 and 10, falling back to 5 if invalid; change the
expression that defines maxResults (using rawMax and maxResults identifiers)
accordingly.
- Around line 41-55: Move the tavily({ apiKey: env.TAVILY_API_KEY })
instantiation out of the request handler and implement a module-level lazy
singleton (e.g., a top-level let tavilyClient and an init/get function used by
the handler) so you don't recreate an axios client per request; then in the
catch block for the handler (where you currently check error instanceof Error
and call Response.json) log the full error server-side (using the existing
logger) but return a generic message like "Search service unavailable" with
status 502 to the caller, avoiding forwarding error.message from `@tavily/core`.
---
Nitpick comments:
In `@packages/agent/src/tools/web-search.ts`:
- Around line 39-55: The fetch to the proxy (the call that uses apiUrl,
authToken and input.query and assigns to response) needs an
AbortController-based timeout so the agent doesn't hang: create an
AbortController, start a timer (configurable timeout) that calls
controller.abort(), pass controller.signal into the fetch options, and clear the
timer after the fetch completes; ensure abort errors are handled/propagated
consistently with the existing response.ok error path. Use the
AbortController/signal around the existing fetch/response logic so the request
is canceled on timeout and resources are cleaned up.
| "@superset/shared": "workspace:*", | ||
| "@superset/trpc": "workspace:*", | ||
| "@t3-oss/env-nextjs": "^0.13.8", | ||
| "@tavily/core": "^0.7.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@tavily/core npm package latest version
💡 Result:
The latest published version of the @tavily/core npm package is 0.5.11. [1]
Update @tavily/core to a valid published version — 0.7.1 does not exist.
The latest published version of @tavily/core is 0.5.11. Specifying ^0.7.1 will cause npm install to fail due to an unresolvable dependency. Update the version constraint to ^0.5.11 or lower.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/package.json` at line 33, The package.json dependency for
"@tavily/core" specifies a non-existent version "^0.7.1"; update the dependency
entry for "@tavily/core" in package.json to a published version (e.g.,
"^0.5.11") so npm install can resolve it, then run npm install and verify the
lockfile updates and CI passes.
| execute: async (input, context) => { | ||
| const apiUrl = context?.requestContext?.get("apiUrl"); | ||
| const authToken = context?.requestContext?.get("authToken"); | ||
|
|
||
| if (!apiUrl || !authToken) { | ||
| throw new Error( | ||
| "TAVILY_API_KEY environment variable is not set. Web search is unavailable.", | ||
| "Web search requires apiUrl and authToken in request context.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing local-dev fallback — breaks stated test plan requirement.
The test plan specifies: "Local dev without the proxy (TAVILY_API_KEY in env) should continue to work as a fallback." But the code throws unconditionally when apiUrl or authToken are absent, and @tavily/core was removed from packages/agent/package.json, so there is no remaining code path for a direct Tavily call.
Any invocation of web_search without the proxy context (e.g., CLI, server-side agent, unit tests) will hard-fail.
Either restore the fallback path (and re-add @tavily/core to the agent package), or update the test plan to reflect that TAVILY_API_KEY alone no longer suffices.
🛠️ Proposed fix — proxy-first with direct Tavily fallback
Re-add "@tavily/core": "^0.7.1" to packages/agent/package.json, then:
+import { tavily } from "@tavily/core";
import { createTool } from "@mastra/core/tools";
import { z } from "zod"; execute: async (input, context) => {
const apiUrl = context?.requestContext?.get("apiUrl");
const authToken = context?.requestContext?.get("authToken");
- if (!apiUrl || !authToken) {
- throw new Error(
- "Web search requires apiUrl and authToken in request context.",
- );
- }
+ if (apiUrl && authToken) {
+ const response = await fetch(`${apiUrl}/api/chat/tools/web-search`, {
+ method: "POST",
+ headers: {
+ "Content-Type": "application/json",
+ Authorization: `Bearer ${authToken}`,
+ },
+ body: JSON.stringify({
+ query: input.query,
+ maxResults: input.maxResults,
+ }),
+ });
+
+ if (!response.ok) {
+ throw new Error(
+ `Web search proxy returned ${response.status}: ${await response.text()}`,
+ );
+ }
+
+ return resultSchema.parse(await response.json());
+ }
+
+ // Fallback: direct Tavily call for local dev / server-side use
+ const tavilyApiKey = process.env.TAVILY_API_KEY;
+ if (!tavilyApiKey) {
+ throw new Error(
+ "Web search requires either (apiUrl + authToken) or TAVILY_API_KEY in the environment.",
+ );
+ }
+ const client = tavily({ apiKey: tavilyApiKey });
+ const tvResponse = await client.search(input.query, {
+ maxResults: input.maxResults,
+ });
+ return {
+ results: tvResponse.results.map((r) => ({
+ title: r.title,
+ url: r.url,
+ content: r.content,
+ })),
+ };
-
- const response = await fetch(`${apiUrl}/api/chat/tools/web-search`, {
- ...
- });
-
- if (!response.ok) { ... }
-
- return resultSchema.parse(await response.json());
},
saddlepaddle
force-pushed
the
mastra-tavily
branch
from
fac3a4c to
3a07810
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/agent/src/tools/web-search.ts (1)
39-49: Add a request timeout to the proxyfetch.Without an
AbortControllersignal, a hung or slow API server will block the tool indefinitely, stalling the agent and holding the request thread.♻️ Proposed fix — add timeout via AbortController
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10_000); + const response = await fetch(`${apiUrl}/api/chat/tools/web-search`, { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${authToken}`, }, body: JSON.stringify({ query: input.query, maxResults: input.maxResults, }), + signal: controller.signal, }); + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent/src/tools/web-search.ts` around lines 39 - 49, The fetch call in web-search.ts that posts to `${apiUrl}/api/chat/tools/web-search` needs an AbortController-based timeout to avoid hanging; create an AbortController, start a timer (e.g., using setTimeout with a configurable timeout value) that calls controller.abort(), pass controller.signal into the fetch options, and clear the timer once the response is received; update the logic around the fetch (the block constructing `response`) to handle the abort error case (detect DOMException/name=='AbortError' or similar) and surface a clear timeout error instead of letting the request hang.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent/src/tools/web-search.ts`:
- Around line 29-31: The variables apiUrl and authToken (from
RequestContext.get) are typed as unknown; add explicit narrowing before using
them in execute: use type guards (e.g., check typeof apiUrl === "string" and
typeof authToken === "string") or assert them (e.g., const apiUrlStr = apiUrl as
string) and throw or return an error if they are not strings, then use the
narrowed apiUrlStr/authTokenStr in the template literal and Authorization
header; reference the execute function and the RequestContext.get("apiUrl") /
RequestContext.get("authToken") calls when making the change.
---
Duplicate comments:
In `@apps/api/src/env.ts`:
- Line 49: The previous validation issue is resolved by making TAVILY_API_KEY
optional in the env schema; no code change needed—ensure TAVILY_API_KEY remains
z.string().optional() in the env schema and that route.ts continues to guard for
its absence and return a 503 where appropriate so the graceful-degradation
pattern stays intact.
In `@packages/agent/src/tools/web-search.ts`:
- Around line 33-37: The WebSearch execution currently throws when apiUrl or
authToken are missing; restore the local-dev fallback so calls outside the
desktop proxy work by using the Tavily direct client when TAVILY_API_KEY is
present: add back the "@tavily/core" dependency, import the Tavily client in
packages/agent/src/tools/web-search.ts, and in the execute function replace the
unconditional throw that checks apiUrl/authToken with logic that uses proxy
(apiUrl+authToken) if provided, otherwise constructs/uses the Tavily client
using process.env.TAVILY_API_KEY and proceeds; ensure the code still throws only
if neither proxy credentials nor TAVILY_API_KEY are available.
---
Nitpick comments:
In `@packages/agent/src/tools/web-search.ts`:
- Around line 39-49: The fetch call in web-search.ts that posts to
`${apiUrl}/api/chat/tools/web-search` needs an AbortController-based timeout to
avoid hanging; create an AbortController, start a timer (e.g., using setTimeout
with a configurable timeout value) that calls controller.abort(), pass
controller.signal into the fetch options, and clear the timer once the response
is received; update the logic around the fetch (the block constructing
`response`) to handle the abort error case (detect
DOMException/name=='AbortError' or similar) and surface a clear timeout error
instead of letting the request hang.
| execute: async (input, context) => { | ||
| const apiUrl = context?.requestContext?.get("apiUrl"); | ||
| const authToken = context?.requestContext?.get("authToken"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for RequestContext type definition in the repository
rg -n "RequestContext" --type ts -A 5 -B 1 | head -100Repository: superset-sh/superset
Length of output: 2938
🏁 Script executed:
#!/bin/bash
# Also look specifically for interface or type definitions of RequestContext
rg -n "interface RequestContext|type RequestContext" --type ts -A 10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the web-search.ts file to see the actual usage on lines 29-43
cat -n packages/agent/src/tools/web-search.ts | sed -n '25,45p'Repository: superset-sh/superset
Length of output: 827
🌐 Web query:
Mastra RequestContext type definition Map string unknown documentation
💡 Result:
In Mastra, RequestContext is a class (not a type alias) that wraps an internal registry Map and exposes set/get/has/delete/clear/keys/values/entries/forEach, plus toJSON() and an all getter.
From @mastra/core’s published type defs (dist/request-context/index.d.ts), you can see the Map<string, unknown> surface type in forEach (the callback’s 3rd arg is the backing map):
forEach(..., map: Map<string, unknown>) => void[1]
The same file defines the class and its constructor shape (iterable of [string, unknown] when untyped, or tuples derived from Values when typed) and also includes the reserved keys MASTRA_RESOURCE_ID_KEY / MASTRA_THREAD_ID_KEY [1].
Also, RequestContext.all is documented as a way to access the whole object (useful for destructuring) [2].
Sources: UNPKG type definition [1], Mastra release notes mentioning RequestContext.all [2].
Add type narrowing or assertions for apiUrl and authToken to comply with type-safety guidelines.
RequestContext.get() returns unknown type (Map<string, unknown>). The falsy check on line 33 (if (!apiUrl || !authToken)) does not narrow unknown to string, leaving both variables typed as unknown when used in the template literal (line 39) and Authorization header (line 43). Add explicit type narrowing via type assertions (as string) or type guards to satisfy the coding guideline requiring type-safe TypeScript without any/unknown without proper narrowing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent/src/tools/web-search.ts` around lines 29 - 31, The variables
apiUrl and authToken (from RequestContext.get) are typed as unknown; add
explicit narrowing before using them in execute: use type guards (e.g., check
typeof apiUrl === "string" and typeof authToken === "string") or assert them
(e.g., const apiUrlStr = apiUrl as string) and throw or return an error if they
are not strings, then use the narrowed apiUrlStr/authTokenStr in the template
literal and Authorization header; reference the execute function and the
RequestContext.get("apiUrl") / RequestContext.get("authToken") calls when making
the change.
Keep TAVILY_API_KEY server-side by routing desktop agent web search requests through an authenticated API endpoint instead of requiring the key in the Electron app bundle.
saddlepaddle
force-pushed
the
mastra-tavily
branch
from
3a07810 to
b8634ad
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/deploy-preview.yml (1)
210-210: LGTM — verifyTAVILY_API_KEYsecret is provisioned in thepreviewenvironment.The propagation is correctly scoped to
deploy-apionly. One operational note: ifsecrets.TAVILY_API_KEYis not yet added to the GitHubpreviewenvironment, Actions will silently resolve it to an empty string. The API'senv.tsmarks the key as optional, so the fallback to direct Tavily calls will kick in rather than failing hard — but web search will be silently non-functional in preview deployments until the secret is added.Also applies to: 257-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-preview.yml at line 210, The workflow exposes TAVILY_API_KEY into the deploy-api job but the preview environment may not have the secret provisioned; ensure the GitHub secret named TAVILY_API_KEY is added to the preview environment so ${{ secrets.TAVILY_API_KEY }} is not empty at runtime—verify in GitHub Settings > Environments (preview) and add the secret, and confirm env.ts (which reads TAVILY_API_KEY) will receive a value so web search via Tavily is functional in preview deployments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/agent/src/tools/web-search.ts`:
- Around line 29-37: The execute handler reads
context?.requestContext?.get("apiUrl") and get("authToken") which return unknown
and are used as strings; narrow them to string (e.g., validate typeof apiUrl ===
"string" and typeof authToken === "string" after extraction) before using in the
template literal and Authorization header in execute, and instead of
unconditionally throwing when missing, implement a proxy-first with local-dev
fallback: if requestContext lacks apiUrl/authToken, attempt to load a fallback
API key (TAVILY_API_KEY) via importing the fallback helper from `@tavily/core` (or
process.env) and construct a default apiUrl/authToken accordingly so non-desktop
invocations and local tests do not hard-fail; ensure error is only thrown if
both requestContext and fallback are absent.
---
Nitpick comments:
In @.github/workflows/deploy-preview.yml:
- Line 210: The workflow exposes TAVILY_API_KEY into the deploy-api job but the
preview environment may not have the secret provisioned; ensure the GitHub
secret named TAVILY_API_KEY is added to the preview environment so ${{
secrets.TAVILY_API_KEY }} is not empty at runtime—verify in GitHub Settings >
Environments (preview) and add the secret, and confirm env.ts (which reads
TAVILY_API_KEY) will receive a value so web search via Tavily is functional in
preview deployments.
1 participant
Summary
POST /api/tools/web-searchendpoint to API server that proxies Tavily requests, keepingTAVILY_API_KEYserver-sideweb_searchtool to route through the proxy whenapiUrl+authTokenare available (desktop app), with direct Tavily fallback for local dev / server-side useapiUrlandauthTokenfrom the desktop'sStreamWatcher→runAgent→ MastraRequestContextTest plan
TAVILY_API_KEYset, hitPOST /api/tools/web-searchwith a valid session cookie — should return Tavily resultsweb_searchshould proxy through the API server (check API server logs)TAVILY_API_KEYin env) should still work as fallbackSummary by CodeRabbit
New Features
Chores