feat: improve Composio tool description with service categories and u… #106
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces Composio connector integration with new API routes for OAuth authorization, connector discovery, and disconnection. Adds composio tool modules for session management and tool retrieval. Integrates Composio tools with MCP server and updates package dependencies to support the functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Route<br/>(authorize)
participant Auth as getApiKeyAccountId
participant Composio as Composio Client
participant CallbackUtil as getCallbackUrl
Client->>API: POST /api/connectors/authorize<br/>(x-api-key, connector)
API->>Auth: Validate API key
Auth-->>API: Return accountId
API->>CallbackUtil: getCallbackUrl({destination: "connectors"})
CallbackUtil-->>API: Return callback URL
API->>Composio: authorizeConnector(accountId, connector, callbackUrl)
Composio->>Composio: Create session & initiate OAuth
Composio-->>API: Return redirectUrl
API-->>Client: 200 {connector, redirectUrl, success: true}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @app/api/connectors/authorize/route.ts:
- Line 33: The handler reads callback_url from the request body and passes it
directly to authorizeConnector, creating an open-redirect risk; add a domain
validation helper (e.g., isAllowedCallbackUrl) that parses the URL safely and
checks hostname against an allowlist of trusted hosts (including subdomains and
localhost), and in the route handler validate callback_url after parsing and
before calling authorizeConnector — if invalid return a 400 via
NextResponse.json with an "Invalid callback_url domain" error; update any tests
or types to reflect that callback_url may be rejected.
- Around line 31-47: The endpoint is doing manual presence checks on
request.json() fields (account_id, connector, callback_url) which is verbose and
doesn't enforce types; replace the manual checks with the project's validate()
call using a Zod schema that declares account_id: string, connector: string
(with any constraints if needed), and optional callback_url: string; call
validate(request, schema) at the top of the handler, destructure the validated
result instead of request.json(), and remove the manual NextResponse.json 400
branches so the validate helper handles type/required errors consistently.
In @app/api/connectors/disconnect/route.ts:
- Around line 26-35: Replace the manual JSON parse/required-check with a
Zod-backed validate call: create a Zod schema (e.g., const DisconnectBodySchema
= z.object({ connected_account_id: z.string().min(1) })) and use the shared
validate(request, DisconnectBodySchema) helper inside the POST handler to parse
and validate the body; then read connected_account_id from the validated result
instead of using the manual if (!connected_account_id) check and return the
proper NextResponse on validation failure using the validate helper’s error
handling semantics.
In @app/api/connectors/route.ts:
- Around line 29-37: Replace the manual query param check for account_id with
Zod-based validation: define an AccountQuerySchema (e.g., z.object({ account_id:
z.string().min(1) })) and call the existing validate function inside the GET
handler to parse request.nextUrl/searchParams instead of using
request.nextUrl.searchParams.get("account_id"); assign the validated value to
accountId from the validate result and remove the manual if (!accountId) branch
so errors are handled by validate. Ensure you import z and validate, name the
schema AccountQuerySchema, and use the validated account_id where the code
currently references accountId.
In @lib/composio/getCallbackUrl.ts:
- Around line 37-39: The URL construction in getCallbackUrl uses options.roomId
raw, which can break the path or query if it contains characters like ../ ? #;
validate or encode options.roomId before building path: either enforce a safe
pattern (e.g., reject/normalize values not matching allowed chars) or apply
encoding (use encodeURIComponent on the roomId) when creating the path variable
so const path = options.roomId ? `/chat/${encodedRoomId}` : "/chat"; thereby
preventing path traversal and query corruption.
In @lib/composio/toolRouter/getTools.ts:
- Around line 28-35: The current getComposioTools function unsafely casts
session.tools() to a Record, which is actually a Tools class instance; change it
to return the actual Tools instance (remove the cast) or convert it safely by
using Tools methods—e.g., return the Tools object and update callers (like
registerComposioTools.ts) to use tools.get(toolName) or tools.list() instead of
bracket indexing, or perform runtime validation by mapping tools.list() into a
plain Record before returning; update getComposioTools to expose the correct
type and adjust all usages that rely on tools[args.tool_name].
In @lib/mcp/tools/composio/registerComposioTools.ts:
- Around line 23-28: The description for the zod schema field tool_name in
registerComposioTools.ts is incomplete: it lists only COMPOSIO_SEARCH_TOOLS,
COMPOSIO_MANAGE_CONNECTIONS, COMPOSIO_GET_TOOL_SCHEMAS, and
COMPOSIO_MULTI_EXECUTE_TOOL while getTools.ts exposes two additional meta-tools
(COMPOSIO_REMOTE_BASH_TOOL and COMPOSIO_REMOTE_WORKBENCH); update the
.describe(...) text for tool_name to include those missing values (or explicitly
note why they are excluded) so the schema documentation matches getTools.ts
entries and avoid confusion for callers validating tool_name.
🧹 Nitpick comments (6)
lib/composio/connectors/getConnectors.ts (1)
19-29: Consider adding error handling for external API calls.The function makes two external API calls (
composio.create()andsession.toolkits()) that could fail due to network issues, invalid credentials, or API errors. Consider wrapping in try/catch or documenting that callers are responsible for error handling.♻️ Optional: Add error handling
export async function getConnectors(userId: string): Promise<ConnectorInfo[]> { + if (!userId) { + throw new Error("userId is required"); + } + const composio = getComposioClient(); const session = await composio.create(userId); const toolkits = await session.toolkits();lib/composio/toolRouter/createSession.ts (1)
14-28: LGTM with minor suggestion.The function is well-documented and correctly uses the callback URL helper for consistent OAuth redirects. Consider adding an explicit return type annotation for better maintainability and IDE support.
♻️ Optional: Add explicit return type
-export async function createToolRouterSession(userId: string, roomId?: string) { +export async function createToolRouterSession(userId: string, roomId?: string): Promise<ReturnType<Awaited<ReturnType<typeof getComposioClient>>["create"]>> {Alternatively, if Composio exports a session type, use that directly for clarity.
lib/composio/connectors/disconnectConnector.ts (2)
12-14: Consider adding input validation forconnectedAccountId.The function accepts any string, including empty strings, which would result in an invalid API URL. Adding a guard clause would provide clearer error messages.
♻️ Suggested validation
export async function disconnectConnector( connectedAccountId: string, ): Promise<{ success: boolean }> { + if (!connectedAccountId?.trim()) { + throw new Error("connectedAccountId is required"); + } + const apiKey = getComposioApiKey();
18-18: Consider extracting the Composio API base URL to a configuration constant.Hardcoding the full URL makes it harder to switch environments (e.g., staging vs production) or update the API version consistently across all endpoints.
lib/composio/getCallbackUrl.ts (1)
18-21: Consider using environment variables for base URLs.Hardcoded URLs reduce deployment flexibility. If the frontend domain changes or for staging environments, code changes would be required.
Suggested approach
function getFrontendBaseUrl(): string { - const isProd = process.env.VERCEL_ENV === "production"; - return isProd ? "https://chat.recoupable.com" : "http://localhost:3001"; + return process.env.FRONTEND_BASE_URL || + (process.env.VERCEL_ENV === "production" + ? "https://chat.recoupable.com" + : "http://localhost:3001"); }lib/mcp/tools/composio/registerComposioTools.ts (1)
90-103: New session created on every tool invocation.
getComposioToolscreates a new session viacreateToolRouterSessionon each call. If this involves network overhead, consider caching sessions per user (with appropriate TTL) for better performance under frequent use.This is worth verifying - if session creation is lightweight or the SDK handles caching internally, this may be fine as-is.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
app/api/connectors/authorize/route.tsapp/api/connectors/disconnect/route.tsapp/api/connectors/route.tslib/composio/connectors/authorizeConnector.tslib/composio/connectors/disconnectConnector.tslib/composio/connectors/getConnectors.tslib/composio/connectors/index.tslib/composio/getCallbackUrl.tslib/composio/toolRouter/createSession.tslib/composio/toolRouter/getTools.tslib/composio/toolRouter/index.tslib/mcp/tools/composio/index.tslib/mcp/tools/composio/registerComposioTools.tslib/mcp/tools/index.tspackage.json
🧰 Additional context used
📓 Path-based instructions (1)
app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
app/api/**/*.{ts,tsx}: All API routes should have JSDoc comments
All API endpoints should use a validate function for input parsing with Zod schema validation
Files:
app/api/connectors/authorize/route.tsapp/api/connectors/disconnect/route.tsapp/api/connectors/route.ts
🧬 Code graph analysis (9)
lib/composio/toolRouter/createSession.ts (3)
lib/composio/toolRouter/index.ts (1)
createToolRouterSession(1-1)lib/composio/client.ts (1)
getComposioClient(7-12)lib/composio/getCallbackUrl.ts (1)
getCallbackUrl(30-40)
app/api/connectors/authorize/route.ts (3)
lib/networking/getCorsHeaders.ts (1)
getCorsHeaders(6-12)lib/composio/connectors/authorizeConnector.ts (1)
authorizeConnector(23-45)lib/composio/connectors/index.ts (1)
authorizeConnector(2-2)
lib/composio/connectors/getConnectors.ts (1)
lib/composio/client.ts (1)
getComposioClient(7-12)
lib/composio/toolRouter/getTools.ts (1)
lib/composio/toolRouter/createSession.ts (1)
createToolRouterSession(14-29)
lib/mcp/tools/composio/index.ts (1)
lib/mcp/tools/composio/registerComposioTools.ts (1)
registerComposioTools(83-111)
lib/composio/connectors/authorizeConnector.ts (2)
lib/composio/client.ts (1)
getComposioClient(7-12)lib/composio/getCallbackUrl.ts (1)
getCallbackUrl(30-40)
app/api/connectors/route.ts (4)
app/api/connectors/authorize/route.ts (1)
OPTIONS(9-14)app/api/connectors/disconnect/route.ts (1)
OPTIONS(9-14)lib/networking/getCorsHeaders.ts (1)
getCorsHeaders(6-12)lib/composio/connectors/getConnectors.ts (1)
getConnectors(19-30)
lib/composio/connectors/disconnectConnector.ts (2)
lib/composio/connectors/index.ts (1)
disconnectConnector(3-3)lib/composio/getComposioApiKey.ts (1)
getComposioApiKey(6-14)
lib/mcp/tools/index.ts (1)
lib/mcp/tools/composio/index.ts (1)
registerAllComposioTools(12-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (9)
lib/composio/toolRouter/index.ts (1)
1-2: LGTM!Clean barrel exports with proper type-only export syntax for
ComposioTool.lib/composio/connectors/getConnectors.ts (1)
6-11: LGTM!Well-defined interface with appropriate optional field for
connectedAccountId.lib/mcp/tools/index.ts (2)
17-17: LGTM!Clean import following the existing pattern.
28-28: LGTM!Composio tools registration is correctly integrated and placed in alphabetical order with other tool registrations, maintaining consistency with the existing codebase structure.
package.json (1)
21-22: Verify Composio 0.3.4 API compatibility with the new code.This is a minor version bump from
0.2.xto0.3.x. Under semver0.xconventions, minor version bumps can introduce breaking changes. The code uses standard Composio APIs (new Composio(),composio.create(), session management) which appear compatible across versions, but ensure the updated package has been tested or check the official Composio 0.3.4 release notes to confirm no breaking changes affect the new API surfaces.lib/mcp/tools/composio/index.ts (1)
1-14: LGTM!Clean wrapper implementation with good JSDoc documentation. The design allows for easy extension when additional Composio tools need to be registered in the future.
lib/composio/connectors/index.ts (1)
1-3: LGTM!Clean barrel exports providing a centralized public API for the connectors module.
lib/composio/connectors/authorizeConnector.ts (1)
23-45: LGTM!The authorization flow is well-structured with clear documentation. The function properly handles the optional
customCallbackUrlparameter with a sensible default. Error handling is appropriately delegated to the caller.lib/mcp/tools/composio/registerComposioTools.ts (1)
83-110: LGTM on the overall structure.The meta-tool pattern with Zod validation, comprehensive description, and proper error handling is well-implemented. The approach of wrapping Composio tools provides a clean integration with the MCP server.
|
|
||
| try { | ||
| const body = await request.json(); | ||
| const { account_id, connector, callback_url } = body; |
There was a problem hiding this comment.
Validate callback_url to prevent open redirect.
The callback_url parameter is passed directly to authorizeConnector without validation. An attacker could supply a malicious URL to redirect users after OAuth. Consider validating against an allowlist of trusted domains.
Suggested validation approach
// Add domain validation helper
function isAllowedCallbackUrl(url: string): boolean {
try {
const parsed = new URL(url);
const allowedHosts = ['chat.recoupable.com', 'localhost'];
return allowedHosts.some(host => parsed.hostname === host || parsed.hostname.endsWith(`.${host}`));
} catch {
return false;
}
}
// In the handler, after parsing:
if (callback_url && !isAllowedCallbackUrl(callback_url)) {
return NextResponse.json(
{ error: "Invalid callback_url domain" },
{ status: 400, headers },
);
}🤖 Prompt for AI Agents
In @app/api/connectors/authorize/route.ts at line 33, The handler reads
callback_url from the request body and passes it directly to authorizeConnector,
creating an open-redirect risk; add a domain validation helper (e.g.,
isAllowedCallbackUrl) that parses the URL safely and checks hostname against an
allowlist of trusted hosts (including subdomains and localhost), and in the
route handler validate callback_url after parsing and before calling
authorizeConnector — if invalid return a 400 via NextResponse.json with an
"Invalid callback_url domain" error; update any tests or types to reflect that
callback_url may be rejected.
| try { | ||
| const body = await request.json(); | ||
| const { connected_account_id } = body; | ||
|
|
||
| if (!connected_account_id) { | ||
| return NextResponse.json( | ||
| { error: "connected_account_id is required" }, | ||
| { status: 400, headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing Zod schema validation per coding guidelines.
As per the coding guidelines, all API endpoints should use a validate function for input parsing with Zod schema validation. The current manual validation should be replaced with a Zod schema.
♻️ Suggested implementation with Zod
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
import { disconnectConnector } from "@/lib/composio/connectors/disconnectConnector";
+import { z } from "zod";
+
+const disconnectRequestSchema = z.object({
+ connected_account_id: z.string().min(1, "connected_account_id is required"),
+});Then update the POST handler:
try {
const body = await request.json();
- const { connected_account_id } = body;
-
- if (!connected_account_id) {
- return NextResponse.json(
- { error: "connected_account_id is required" },
- { status: 400, headers },
- );
- }
+ const parseResult = disconnectRequestSchema.safeParse(body);
+
+ if (!parseResult.success) {
+ return NextResponse.json(
+ { error: parseResult.error.issues[0]?.message ?? "Invalid request body" },
+ { status: 400, headers },
+ );
+ }
+
+ const { connected_account_id } = parseResult.data;Based on coding guidelines, all API endpoints should use a validate function for input parsing with Zod schema validation.
📝 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.
| try { | |
| const body = await request.json(); | |
| const { connected_account_id } = body; | |
| if (!connected_account_id) { | |
| return NextResponse.json( | |
| { error: "connected_account_id is required" }, | |
| { status: 400, headers }, | |
| ); | |
| } | |
| try { | |
| const body = await request.json(); | |
| const parseResult = disconnectRequestSchema.safeParse(body); | |
| if (!parseResult.success) { | |
| return NextResponse.json( | |
| { error: parseResult.error.issues[0]?.message ?? "Invalid request body" }, | |
| { status: 400, headers }, | |
| ); | |
| } | |
| const { connected_account_id } = parseResult.data; |
🤖 Prompt for AI Agents
In @app/api/connectors/disconnect/route.ts around lines 26 - 35, Replace the
manual JSON parse/required-check with a Zod-backed validate call: create a Zod
schema (e.g., const DisconnectBodySchema = z.object({ connected_account_id:
z.string().min(1) })) and use the shared validate(request, DisconnectBodySchema)
helper inside the POST handler to parse and validate the body; then read
connected_account_id from the validated result instead of using the manual if
(!connected_account_id) check and return the proper NextResponse on validation
failure using the validate helper’s error handling semantics.
| try { | ||
| const accountId = request.nextUrl.searchParams.get("account_id"); | ||
|
|
||
| if (!accountId) { | ||
| return NextResponse.json( | ||
| { error: "account_id query parameter is required" }, | ||
| { status: 400, headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing Zod schema validation per coding guidelines.
As per the coding guidelines, all API endpoints should use a validate function for input parsing with Zod schema validation. The current manual query parameter validation should be replaced with a Zod schema.
♻️ Suggested implementation with Zod
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
import { getConnectors } from "@/lib/composio/connectors";
+import { z } from "zod";
+
+const getConnectorsQuerySchema = z.object({
+ account_id: z.string().min(1, "account_id query parameter is required"),
+});Then update the GET handler:
try {
- const accountId = request.nextUrl.searchParams.get("account_id");
-
- if (!accountId) {
- return NextResponse.json(
- { error: "account_id query parameter is required" },
- { status: 400, headers },
- );
- }
+ const queryParams = {
+ account_id: request.nextUrl.searchParams.get("account_id"),
+ };
+ const parseResult = getConnectorsQuerySchema.safeParse(queryParams);
+
+ if (!parseResult.success) {
+ return NextResponse.json(
+ { error: parseResult.error.issues[0]?.message ?? "Invalid query parameters" },
+ { status: 400, headers },
+ );
+ }
+
+ const { account_id: accountId } = parseResult.data;Based on coding guidelines, all API endpoints should use a validate function for input parsing with Zod schema validation.
📝 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.
| try { | |
| const accountId = request.nextUrl.searchParams.get("account_id"); | |
| if (!accountId) { | |
| return NextResponse.json( | |
| { error: "account_id query parameter is required" }, | |
| { status: 400, headers }, | |
| ); | |
| } | |
| try { | |
| const queryParams = { | |
| account_id: request.nextUrl.searchParams.get("account_id"), | |
| }; | |
| const parseResult = getConnectorsQuerySchema.safeParse(queryParams); | |
| if (!parseResult.success) { | |
| return NextResponse.json( | |
| { error: parseResult.error.issues[0]?.message ?? "Invalid query parameters" }, | |
| { status: 400, headers }, | |
| ); | |
| } | |
| const { account_id: accountId } = parseResult.data; |
🤖 Prompt for AI Agents
In @app/api/connectors/route.ts around lines 29 - 37, Replace the manual query
param check for account_id with Zod-based validation: define an
AccountQuerySchema (e.g., z.object({ account_id: z.string().min(1) })) and call
the existing validate function inside the GET handler to parse
request.nextUrl/searchParams instead of using
request.nextUrl.searchParams.get("account_id"); assign the validated value to
accountId from the validate result and remove the manual if (!accountId) branch
so errors are handled by validate. Ensure you import z and validate, name the
schema AccountQuerySchema, and use the validated account_id where the code
currently references accountId.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| if (!account_id) { | ||
| return NextResponse.json( | ||
| { error: "account_id is required" }, | ||
| { status: 400, headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
API keys
- actual: account_id
- required: x-api-key header validation
| if (!connected_account_id) { | ||
| return NextResponse.json( | ||
| { error: "connected_account_id is required" }, | ||
| { status: 400, headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
API keys
- actual: account_id
- required: x-api-key header validation
NOTE: please update CLAUDE.md file to note this rule to never use account_id.
| * | ||
| * Body: { connected_account_id: string } | ||
| */ | ||
| export async function POST(request: NextRequest): Promise<NextResponse> { |
There was a problem hiding this comment.
KISS principle
- actual:
POST app/api/connectors/disconnect/route.ts - required:
DELETE app/api/connectors/route.ts
app/api/connectors/route.ts
Outdated
| if (!accountId) { | ||
| return NextResponse.json( | ||
| { error: "account_id query parameter is required" }, | ||
| { status: 400, headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
API keys
- actual: account_id
- required: x-api-key header validation
NOTE: please update CLAUDE.md file to note this rule to never use account_id.
lib/composio/getCallbackUrl.ts
Outdated
| /** | ||
| * Get the base URL for the frontend based on environment. | ||
| */ | ||
| function getFrontendBaseUrl(): string { | ||
| const isProd = process.env.VERCEL_ENV === "production"; | ||
| return isProd ? "https://chat.recoupable.com" : "http://localhost:3001"; | ||
| } |
There was a problem hiding this comment.
Single Responsibility Principle
- actual: getFrontendBaseUrl defined in a file names
getCallbackUrl - required: new lib file for getFrontendBaseUrl.ts
There was a problem hiding this comment.
KISS principle
- actual: wrapper tool around
getComposioToolscall. - required: pass raw results from
lib/composio/toolRouter/getTools.tsin the response of this file. - note: update getTools to limit results to the subset you want. Once we're ready to add all the tools, it's a simpler update.
- Use x-api-key auth instead of account_id in request bodies/params - Add Zod validation for disconnect connector endpoint - Move DELETE to /api/connectors (KISS - RESTful design) - Extract getFrontendBaseUrl to own file (SRP) - Simplify registerComposioTools to return raw tools - Filter Composio tools to allowed subset in getTools.ts - Update CLAUDE.md with auth guidelines
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/composio/getFrontendBaseUrl.ts`:
- Around line 9-12: getFrontendBaseUrl currently returns localhost for any
non-production VERCEL_ENV causing OAuth redirects to fail in Vercel preview
builds; update getFrontendBaseUrl to use VERCEL_URL when present (prepend
"https://" because VERCEL_URL lacks a protocol) for preview and other
non-production deployments, falling back to "http://localhost:3001" only if
VERCEL_URL is undefined; reference the getFrontendBaseUrl function and replace
the isProd check/return logic accordingly.
In `@lib/mcp/tools/composio/registerComposioTools.ts`:
- Around line 10-19: The composio tool currently accepts account_id from LLM
input (composioToolSchema) which allows forging; change it to not accept
account_id from tool input by removing account_id from composioToolSchema and
instead obtain the authenticated principal's account_id in the MCP request
handling code and pass that server-side into getComposioTools (or, if you prefer
a minimal change, validate that the parsed account_id equals the authenticated
principal before calling getComposioTools). Update any call sites that construct
tool context to use the server-extracted account_id and remove/ignore the
incoming account_id field so OAuth token access is always tied to the
authenticated principal.
♻️ Duplicate comments (3)
app/api/connectors/route.ts (1)
31-39: Missing Zod schema validation for GET query parameters.As per coding guidelines, all API endpoints should use a validate function for input parsing with Zod schema validation. The
account_idis derived from API key auth, but the pattern should still use a dedicated validator for any query parameters if they're added in the future.Currently this is acceptable since the only "input" is the API key header which is validated by
getApiKeyAccountId. However, if query parameters are added later, create avalidateGetConnectorsQuery.tsfile.lib/composio/getCallbackUrl.ts (1)
31-33: EncoderoomIdto prevent URL corruption.If
roomIdcontains special characters (e.g.,../,?,#), it could corrupt the URL path or enable path traversal. ApplyencodeURIComponentfor safety.♻️ Suggested fix
// Chat destination - const path = options.roomId ? `/chat/${options.roomId}` : "/chat"; + const path = options.roomId ? `/chat/${encodeURIComponent(options.roomId)}` : "/chat"; return `${baseUrl}${path}?connected=true`;lib/composio/toolRouter/getTools.ts (1)
41-49: Avoid castingsession.tools()to a Record.Line 42 assumes a plain object, but the Composio SDK typically returns a Tools class; bracket indexing can fail and yield an empty allowlist. Prefer SDK methods (e.g.,
get/list) and build a Record explicitly.🔧 Suggested direction (adjust to actual SDK method names)
- const allTools = (await session.tools()) as Record<string, ComposioTool>; - - // Filter to only allowed tools - const filteredTools: Record<string, ComposioTool> = {}; - for (const toolName of ALLOWED_TOOLS) { - if (allTools[toolName]) { - filteredTools[toolName] = allTools[toolName]; - } - } + const tools = await session.tools(); + + // Filter to only allowed tools using SDK APIs + const filteredTools: Record<string, ComposioTool> = {}; + for (const toolName of ALLOWED_TOOLS) { + const tool = await tools.get(toolName); + if (tool) { + filteredTools[toolName] = tool as ComposioTool; + } + }Composio TypeScript SDK session.tools() return type and supported methods (get/list)
🧹 Nitpick comments (3)
lib/composio/connectors/validateDisconnectConnectorBody.ts (1)
11-16: JSDoc references incorrect endpoint.The comment states "POST /api/connectors/disconnect" but this validator is used by the DELETE handler at
/api/connectors. Consider updating for accuracy./** - * Validates request body for POST /api/connectors/disconnect. + * Validates request body for DELETE /api/connectors. * * `@param` body - The request body * `@returns` A NextResponse with an error if validation fails, or the validated body if validation passes. */app/api/connectors/route.ts (1)
75-75: Consider handling JSON parse errors explicitly.
request.json()throws aSyntaxErrorif the body is not valid JSON. While the generic catch block handles this, it would return a 500 status. Consider catching JSON parse errors separately to return 400.♻️ Suggested improvement
try { const accountIdOrError = await getApiKeyAccountId(request); if (accountIdOrError instanceof NextResponse) { return accountIdOrError; } - const body = await request.json(); + let body: unknown; + try { + body = await request.json(); + } catch { + return NextResponse.json( + { error: "Invalid JSON body" }, + { status: 400, headers }, + ); + } const validated = validateDisconnectConnectorBody(body);lib/composio/getCallbackUrl.ts (1)
10-15: Consider exporting types for consumer use.
CallbackDestinationandCallbackOptionsare defined but not exported. If other modules need to reference these types (e.g., for type-safe function calls), consider exporting them.-type CallbackDestination = "chat" | "connectors"; +export type CallbackDestination = "chat" | "connectors"; -interface CallbackOptions { +export interface CallbackOptions { destination: CallbackDestination; roomId?: string; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CLAUDE.mdapp/api/connectors/authorize/route.tsapp/api/connectors/route.tslib/composio/connectors/validateDisconnectConnectorBody.tslib/composio/getCallbackUrl.tslib/composio/getFrontendBaseUrl.tslib/composio/toolRouter/getTools.tslib/mcp/tools/composio/registerComposioTools.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/connectors/authorize/route.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Apply Single Responsibility Principle (SRP) - one exported function per file
Apply DRY principle (Don't Repeat Yourself) - extract shared logic into reusable utilities
Files:
lib/composio/getFrontendBaseUrl.tslib/composio/connectors/validateDisconnectConnectorBody.tslib/composio/toolRouter/getTools.tslib/composio/getCallbackUrl.tsapp/api/connectors/route.tslib/mcp/tools/composio/registerComposioTools.ts
!(lib/supabase)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER import
@/lib/supabase/serverClientoutside oflib/supabase/directory
Files:
lib/composio/getFrontendBaseUrl.tslib/composio/connectors/validateDisconnectConnectorBody.tsCLAUDE.mdlib/composio/toolRouter/getTools.tslib/composio/getCallbackUrl.tsapp/api/connectors/route.tslib/mcp/tools/composio/registerComposioTools.ts
app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
app/api/**/*.{ts,tsx}: All API routes should have JSDoc comments
All API endpoints should use a validate function for input parsing with Zod for schema validation
Files:
app/api/connectors/route.ts
🧠 Learnings (7)
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/validation/**/*.{ts,tsx} : Create `validate<EndpointName>Body.ts` files for POST/PUT request body validation
Applied to files:
lib/composio/connectors/validateDisconnectConnectorBody.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to app/api/**/*.{ts,tsx} : All API endpoints should use a validate function for input parsing with Zod for schema validation
Applied to files:
CLAUDE.mdapp/api/connectors/route.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to app/api/**/*.{ts,tsx} : All API routes should have JSDoc comments
Applied to files:
CLAUDE.mdapp/api/connectors/route.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Apply Single Responsibility Principle (SRP) - one exported function per file
Applied to files:
lib/composio/getCallbackUrl.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Apply DRY principle (Don't Repeat Yourself) - extract shared logic into reusable utilities
Applied to files:
lib/composio/getCallbackUrl.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/supabase/**/*.{ts,tsx} : All Supabase database calls must be in `lib/supabase/[table_name]/[function].ts`
Applied to files:
lib/composio/getCallbackUrl.ts
📚 Learning: 2026-01-14T22:12:03.883Z
Learnt from: CR
Repo: Recoupable-com/Recoup-API PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T22:12:03.883Z
Learning: Applies to lib/validation/**/*.{ts,tsx} : Create `validate<EndpointName>Query.ts` files for GET query parameter validation
Applied to files:
app/api/connectors/route.ts
🧬 Code graph analysis (3)
lib/composio/connectors/validateDisconnectConnectorBody.ts (1)
lib/networking/getCorsHeaders.ts (1)
getCorsHeaders(6-12)
lib/composio/toolRouter/getTools.ts (2)
lib/composio/toolRouter/index.ts (3)
ComposioTool(2-2)getComposioTools(2-2)createToolRouterSession(1-1)lib/composio/toolRouter/createSession.ts (1)
createToolRouterSession(14-29)
lib/composio/getCallbackUrl.ts (1)
lib/composio/getFrontendBaseUrl.ts (1)
getFrontendBaseUrl(9-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (6)
lib/composio/connectors/validateDisconnectConnectorBody.ts (1)
1-34: LGTM!The validation module follows the established pattern with Zod schema, exported type inference, and proper error handling with CORS headers. This aligns with the coding guidelines for input validation. Based on learnings, this correctly implements the
validate<EndpointName>Body.tspattern.app/api/connectors/route.ts (1)
9-98: LGTM on overall structure!The API route correctly implements:
- JSDoc comments on all handlers (per coding guidelines)
- API key authentication via
getApiKeyAccountId(never usingaccount_idin body)- Zod validation for DELETE body via
validateDisconnectConnectorBody- Consistent CORS headers and error handling
Based on learnings and coding guidelines, the authentication and validation patterns are properly followed.
CLAUDE.md (1)
62-79: LGTM!Excellent documentation of the authentication pattern. The example code matches the actual implementation in
app/api/connectors/route.ts, and the security rationale clearly explains whyaccount_idshould never be in request bodies.lib/composio/getCallbackUrl.ts (1)
1-34: LGTM on SRP compliance!The extraction of
getFrontendBaseUrlto its own file properly addresses the Single Responsibility Principle. This file now has a single purpose: building callback URLs. Based on learnings and coding guidelines for SRP.lib/composio/toolRouter/getTools.ts (1)
7-22: Clear tool typing and allowlist.The interface and explicit allowlist make the exposed surface area clear and controlled.
lib/mcp/tools/composio/registerComposioTools.ts (1)
26-70: Tool description and registration flow look solid.The category examples and success/error handling are clear and helpful.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const composioToolSchema = z.object({ | ||
| account_id: z | ||
| .string() | ||
| .min(1) | ||
| .describe("The user's account ID (from system prompt)"), | ||
| room_id: z | ||
| .string() | ||
| .optional() | ||
| .describe("The current chat room ID for OAuth redirect (from URL path)"), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the file structure around the target file
git ls-files lib/mcp/tools/composio/ | head -20Repository: Recoupable-com/Recoup-API
Length of output: 148
🏁 Script executed:
# Read the full registerComposioTools.ts file
cat -n lib/mcp/tools/composio/registerComposioTools.tsRepository: Recoupable-com/Recoup-API
Length of output: 2879
🏁 Script executed:
# Check for auth-related files and context
fd -e ts -e tsx lib/ | grep -i auth | head -20Repository: Recoupable-com/Recoup-API
Length of output: 390
🏁 Script executed:
# Look for how account_id is used throughout the codebase
rg "account_id" --type ts --type tsx | head -30Repository: Recoupable-com/Recoup-API
Length of output: 96
🏁 Script executed:
# Find auth-related files
fd -type f -name "*auth*" lib/Repository: Recoupable-com/Recoup-API
Length of output: 239
🏁 Script executed:
# Look at getComposioTools function
fd -type f lib/composio/toolRouter.tsRepository: Recoupable-com/Recoup-API
Length of output: 239
🏁 Script executed:
# Search for account_id usage and validation
rg "account_id" -A 3 -B 3 lib/Repository: Recoupable-com/Recoup-API
Length of output: 50381
🏁 Script executed:
# Look for MCP server setup and auth context
fd -type f lib/mcp/ | head -20Repository: Recoupable-com/Recoup-API
Length of output: 239
🏁 Script executed:
# Check for x-api-key or authentication patterns
rg "x-api-key|apiKey|principal|userId|authenticate" -A 2 lib/ | head -40Repository: Recoupable-com/Recoup-API
Length of output: 2166
🏁 Script executed:
cat -n lib/composio/toolRouter.tsRepository: Recoupable-com/Recoup-API
Length of output: 127
🏁 Script executed:
# Look for getComposioTools implementation
rg "getComposioTools" -A 10 -B 2 lib/composio/Repository: Recoupable-com/Recoup-API
Length of output: 1211
🏁 Script executed:
# Search for MCP server setup and auth
fd . lib/mcp/ -name "*.ts" | head -20Repository: Recoupable-com/Recoup-API
Length of output: 299
🏁 Script executed:
# Look for x-api-key authentication
rg "x-api-key|serverClient" lib/mcp/ -A 2 -B 2Repository: Recoupable-com/Recoup-API
Length of output: 51
🏁 Script executed:
# Check if there's any auth context in the MCP server or similar
rg "authenticate|auth|principal|context" lib/mcp/ -lRepository: Recoupable-com/Recoup-API
Length of output: 2238
🏁 Script executed:
# Read the full getComposioTools implementation
cat lib/composio/toolRouter/getTools.tsRepository: Recoupable-com/Recoup-API
Length of output: 1652
🏁 Script executed:
# Check createToolRouterSession to see how userId is used
cat lib/composio/toolRouter/createSession.tsRepository: Recoupable-com/Recoup-API
Length of output: 857
🏁 Script executed:
# Look for how MCP server is initialized and if it has auth context
rg "registerTool|McpServer" lib/mcp/ -A 3 | head -50Repository: Recoupable-com/Recoup-API
Length of output: 4372
🏁 Script executed:
# Search for where registerComposioTools is called to see if auth context is available
rg "registerComposioTools" -B 5 -A 5 lib/Repository: Recoupable-com/Recoup-API
Length of output: 1904
🏁 Script executed:
# Check if there's any server-side auth for MCP
fd . lib/ -name "*server*" -o -name "*auth*" | grep -E "mcp|server"Repository: Recoupable-com/Recoup-API
Length of output: 299
🏁 Script executed:
# Find where registerAllTools is called and how MCP server is set up
rg "registerAllTools|McpServer" lib/ -B 5 -A 10 | head -80Repository: Recoupable-com/Recoup-API
Length of output: 6923
🏁 Script executed:
# Look for the main MCP server initialization file
fd . lib/mcp/ -name "*.ts" | grep -v test | head -15Repository: Recoupable-com/Recoup-API
Length of output: 299
🏁 Script executed:
# Search for x-api-key usage and how auth is handled
rg "x-api-key|apiKey" -B 3 -A 3 lib/Repository: Recoupable-com/Recoup-API
Length of output: 15266
🏁 Script executed:
# Look for serverClient imports to understand auth pattern
rg "serverClient|@/lib/supabase/serverClient" lib/mcp/ -B 2 -A 2Repository: Recoupable-com/Recoup-API
Length of output: 51
🏁 Script executed:
# Look for where the MCP server is instantiated and how it's used
rg "new McpServer|McpServer(" -B 5 -A 10 lib/Repository: Recoupable-com/Recoup-API
Length of output: 167
🏁 Script executed:
# Search for MCP server in main app routes
fd . -name "*route*" -o -name "*server*" | grep -E "route|server" | head -20Repository: Recoupable-com/Recoup-API
Length of output: 299
🏁 Script executed:
# Check if there's an MCP endpoint that validates auth
rg "registerAllTools|callTool" -B 10 -A 5 lib/Repository: Recoupable-com/Recoup-API
Length of output: 1025
🏁 Script executed:
# Look for how account context is passed through the MCP system
rg "getApiKeyAccountId|validateAccountIdHeaders" lib/mcp/ -B 3 -A 3Repository: Recoupable-com/Recoup-API
Length of output: 51
🏁 Script executed:
# Check if there's a context or middleware that extracts authenticated account_id in MCP
cat lib/accounts/getAccountIdHandler.tsRepository: Recoupable-com/Recoup-API
Length of output: 1329
🏁 Script executed:
# Find where MCP server is exposed via HTTP endpoint
rg "mcp|McpServer" app/ -l 2>/dev/null | head -10Repository: Recoupable-com/Recoup-API
Length of output: 85
🏁 Script executed:
# Look for HTTP routes that handle MCP requests
fd . app/ -name "*.ts" 2>/dev/null | head -20Repository: Recoupable-com/Recoup-API
Length of output: 51
🏁 Script executed:
# Check if there's an MCP endpoint and how it's authenticated
find app -name "*mcp*" -o -name "*chat*" 2>/dev/null | head -10Repository: Recoupable-com/Recoup-API
Length of output: 90
🏁 Script executed:
# Search for callTool or tool execution in HTTP handlers
rg "callTool|registerTool" app/ -B 5 -A 5 2>/dev/null | head -60Repository: Recoupable-com/Recoup-API
Length of output: 51
🏁 Script executed:
# Look at app structure
ls -la app/ 2>/dev/null | head -20Repository: Recoupable-com/Recoup-API
Length of output: 542
🏁 Script executed:
# Read the MCP route handler to see how tools are called and authenticated
cat app/mcp/route.tsRepository: Recoupable-com/Recoup-API
Length of output: 1103
Pass authenticated account_id from MCP context instead of accepting it as tool input.
The composio tool accepts account_id directly from the LLM input (lines 11–14) without validating it against the authenticated principal. With x-api-key auth, an attacker with a valid API key can call this tool with a different account_id to access another user's Composio OAuth connections and tokens, enabling cross-tenant access.
The MCP route handler (app/mcp/route.ts) does not extract or pass the authenticated account_id from the x-api-key header through the tool execution context. Extract the authenticated account_id server-side (via x-api-key validation) and remove the account_id parameter from the schema, or validate that the provided account_id matches the authenticated principal before calling getComposioTools.
🤖 Prompt for AI Agents
In `@lib/mcp/tools/composio/registerComposioTools.ts` around lines 10 - 19, The
composio tool currently accepts account_id from LLM input (composioToolSchema)
which allows forging; change it to not accept account_id from tool input by
removing account_id from composioToolSchema and instead obtain the authenticated
principal's account_id in the MCP request handling code and pass that
server-side into getComposioTools (or, if you prefer a minimal change, validate
that the parsed account_id equals the authenticated principal before calling
getComposioTools). Update any call sites that construct tool context to use the
server-extracted account_id and remove/ignore the incoming account_id field so
OAuth token access is always tied to the authenticated principal.
…mcp/tools/index.ts
…point - Create validateAuthorizeConnectorBody.ts with Zod schema - Validate connector (required string) and callback_url (optional URL) - Update route to use validation function per coding guidelines
- Add verifyConnectorOwnership() to check connected_account_id belongs to user - DELETE /api/connectors now returns 403 if user doesn't own the connection - Prevents authorization bypass where users could disconnect other users' connectors
- Remove unsafe type assertion 'as Record<string, ComposioTool>' - Add isComposioTool() type guard with runtime validation - Use Object.prototype.hasOwnProperty for safe property access - Validates description (string), inputSchema (exists), execute (function) Addresses CodeRabbit's critical issue about unsafe type casting on SDK return values.
- Replace custom ComposioTool interface with Vercel AI SDK's Tool type - Import Tool and ToolSet types from 'ai' package - Update isValidTool() to check for 'parameters' (SDK format) instead of 'inputSchema' - Use ToolSet as return type for getComposioTools() - Maintains runtime validation while using correct SDK types This aligns with Tool Router's actual return type when using VercelProvider: session.tools() returns ToolSet (Record<string, Tool>) from Vercel AI SDK.
- Check for VERCEL_URL to support preview deployments - Preview URLs need https:// prepended (VERCEL_URL doesn't include protocol) - Maintains production (chat.recoupable.com) and local (localhost:3001) behavior Fixes OAuth redirect issue where preview deployments would incorrectly redirect to localhost, which is unreachable.
- Check VERCEL_GIT_COMMIT_REF for 'test' branch specifically - Use stable test frontend URL for test deployments - Maintains preview URL fallback for other feature branches - Easier to update TEST_FRONTEND_URL constant if domain changes
Match API pattern: test-recoup-api -> test-recoup-chat
- Remove verbose description constant and inline it - Remove separate schema type definition - Inline schema directly in registerTool call - Keep same functionality: passes raw results from getComposioTools - Tool filtering handled by ALLOWED_TOOLS in getTools.ts
- Remove getToolResultSuccess and getToolResultError wrapper imports - Use getCallToolResult(JSON.stringify(...)) directly - Follows KISS: pass raw results without unnecessary wrappers
lib/composio/getFrontendBaseUrl.ts
Outdated
| } | ||
|
|
||
| // Local development fallback | ||
| return "http://localhost:3001"; |
There was a problem hiding this comment.
Why are you using localhost:3001 instead of localhost:3000?
There was a problem hiding this comment.
| return "http://localhost:3001"; | |
| return "http://localhost:3000"; |
- Frontend (Recoup-Chat) runs on port 3000, not 3001 - API (Recoup-API) runs on port 3001 - getFrontendBaseUrl should return frontend port
- Remove JSDoc comments - Remove try/catch wrapper - Simplify description and schema - Just pass through results from getComposioTools directly
- Add ENABLED_TOOLKITS constant for easy expansion later - Pass toolkits option to session.create() per Composio docs - Currently only googlesheets is enabled
| { | ||
| description: "Get Composio tools for Google Sheets integration.", | ||
| inputSchema: z.object({ | ||
| account_id: z.string().min(1), | ||
| room_id: z.string().optional(), | ||
| }), | ||
| }, |
There was a problem hiding this comment.
There should not be a schema wrapper for composio tools.
- they should be passed directly, as-in, to the registerTool function.
- Remove account_id from composio tool input schema - Get accountId from MCP auth info via resolveAccountId - Keep room_id as optional input for OAuth redirect - Follows API auth pattern used by other MCP tools
- Update CLAUDE.md Authentication section - Add MCP tools pattern using resolveAccountId - Clarify both API keys and Privy tokens resolve to accountId
| server.registerTool( | ||
| "composio", | ||
| { | ||
| description: "Get Composio tools for Google Sheets integration.", | ||
| inputSchema: composioSchema, |
There was a problem hiding this comment.
KISS principle
- actual: registerComposioTools still defines a tool with an inputSchema
- required: directly call
server.registerToolon the results fromgetComposioTools
…sage examples
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.