feat: add Google Drive integration via Composio #103
Conversation
- Add generic Composio toolkit infrastructure (toolkits.ts, authenticateToolkit.ts, getConnectedAccount.ts, refreshConnectedAccount.ts) - Add /api/connectedAccounts/googleDrive/refresh endpoint - Refactor Google Sheets to use shared Composio utilities (DRY) - Add /api/connectedAccounts/googleSheets/refresh endpoint (moved from generic /refresh)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes refactor the Composio integration layer from toolkit-specific implementations to generalized utilities. New modules in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as API Route
participant AuthToolkit as authenticateToolkit
participant GetAcct as getConnectedAccount
participant RefreshAcct as refreshConnectedAccount
participant ComposioClient as Composio Client
participant Backend as Composio Backend
Client->>Route: POST /api/connectedAccounts/[toolkit]/refresh
Route->>GetAcct: getConnectedAccount(toolkitKey, accountId)
GetAcct->>ComposioClient: Query connected accounts
alt No accounts found
GetAcct->>AuthToolkit: authenticateToolkit(toolkitKey, userId)
AuthToolkit->>ComposioClient: Initiate OAuth
AuthToolkit-->>GetAcct: connectionRequest
GetAcct->>ComposioClient: Retry query connected accounts
end
ComposioClient-->>GetAcct: userAccounts
GetAcct-->>Route: connectedAccount
Route->>RefreshAcct: refreshConnectedAccount(toolkitKey, accountId, redirectUrl)
RefreshAcct->>Backend: POST /api/v3/connected_accounts/{id}/refresh
Backend-->>RefreshAcct: ConnectedAccountRefreshResponse
RefreshAcct-->>Route: refreshResponse
Route-->>Client: JSON with refresh status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In @lib/composio/getConnectedAccount.ts:
- Around line 28-34: The code calls authenticateToolkit(toolkitKey, accountId,
options) which starts an interactive OAuth flow and then immediately retries
composio.connectedAccounts.list — this will usually still be empty; instead stop
retrying: remove the second call to composio.connectedAccounts.list after
authenticateToolkit and change getConnectedAccount (or the function containing
this block) to return the connection request result (or a flag/object indicating
the OAuth flow was initiated) so the caller can perform the redirect/consent and
retry list after the callback completes; update any callers to handle the
connection-request response rather than relying on an immediate retry.
In @lib/composio/refreshConnectedAccount.ts:
- Around line 27-31: The declared return type of refreshConnectedAccount is
inaccurate: change the function signature from
Promise<ConnectedAccountRefreshResponse | null> to
Promise<ConnectedAccountRefreshResponse> because the implementation never
returns null (it throws on errors and returns parsed JSON via the response
handling in refreshConnectedAccount). Update the return type in the
refreshConnectedAccount declaration accordingly and verify callers don't rely on
a null return (adjust callers if any assume null).
🧹 Nitpick comments (4)
app/api/connectedAccounts/googleSheets/refresh/route.ts (1)
40-48: Fragile error classification using string matching.Determining status code by checking if error message includes "not found" is brittle. If the underlying error message format changes or if other errors contain "not found", this will misclassify them.
Consider having
refreshConnectedAccountthrow typed errors or use error codes instead.Example: Custom error classes
In a shared errors file:
export class NotFoundError extends Error { constructor(message: string) { super(message); this.name = 'NotFoundError'; } }Then in the route:
} catch (error) { const errorMessage = error instanceof Error ? error.message : "Internal server error"; - const statusCode = errorMessage.includes("not found") ? 404 : 500; + const statusCode = error instanceof NotFoundError ? 404 : 500; return NextResponse.json( { error: errorMessage }, { status: statusCode, headers: getCorsHeaders() }, ); }app/api/connectedAccounts/googleDrive/refresh/route.ts (2)
40-48: Fragile error classification using string matching (duplicated pattern).This error handling pattern is identical to the Google Sheets refresh route. Determining status code by checking if error message includes "not found" is brittle and duplicated across routes.
Consider having
refreshConnectedAccountthrow typed errors, and potentially extracting the common error handling logic into a shared utility.Example: Shared error handler
Create a shared handler in
lib/api/errorHandlers.ts:export function handleRefreshError(error: unknown): { error: string; status: number } { if (error instanceof NotFoundError) { return { error: error.message, status: 404 }; } const errorMessage = error instanceof Error ? error.message : "Internal server error"; return { error: errorMessage, status: 500 }; }Then in both routes:
} catch (error) { - const errorMessage = - error instanceof Error ? error.message : "Internal server error"; - const statusCode = errorMessage.includes("not found") ? 404 : 500; - + const { error: errorMessage, status: statusCode } = handleRefreshError(error); + return NextResponse.json( { error: errorMessage }, { status: statusCode, headers: getCorsHeaders() }, ); }
1-50: Consider extracting common route logic.The Google Drive and Google Sheets refresh routes are nearly identical except for the toolkit key ("GOOGLE_DRIVE" vs "GOOGLE_SHEETS"). This duplication could be reduced with a shared handler or route factory.
Example: Shared route handler
Create
lib/api/createRefreshRoute.ts:export function createRefreshRouteHandler(toolkitKey: ComposioToolkitKey) { return async function POST(request: NextRequest) { try { const body = await request.json(); const { accountId, redirectUrl } = body; if (!accountId) { return NextResponse.json( { error: "accountId is required" }, { status: 400, headers: getCorsHeaders() }, ); } const response = await refreshConnectedAccount( toolkitKey, accountId, redirectUrl, ); return NextResponse.json( { message: "Connected account refreshed successfully", ...response }, { status: 200, headers: getCorsHeaders() }, ); } catch (error) { // ... error handling } }; }Then in each route:
export const POST = createRefreshRouteHandler("GOOGLE_DRIVE");lib/composio/refreshConnectedAccount.ts (1)
17-17: Consider making the API base URL configurable.The hardcoded Composio API base URL limits flexibility for testing and different environments. Consider moving this to an environment variable (e.g.,
COMPOSIO_API_BASE_URL) with the current value as a fallback default.♻️ Suggested improvement
-const COMPOSIO_API_BASE_URL = "https://backend.composio.dev"; +const COMPOSIO_API_BASE_URL = process.env.COMPOSIO_API_BASE_URL || "https://backend.composio.dev";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/api/connectedAccounts/googleDrive/refresh/route.tsapp/api/connectedAccounts/googleSheets/refresh/route.tslib/composio/authenticateToolkit.tslib/composio/getConnectedAccount.tslib/composio/googleSheets/authenticateGoogleSheetsToolkit.tslib/composio/googleSheets/getConnectedAccount.tslib/composio/googleSheets/refreshConnectedAccount.tslib/composio/refreshConnectedAccount.tslib/composio/toolkits.ts
💤 Files with no reviewable changes (3)
- lib/composio/googleSheets/getConnectedAccount.ts
- lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts
- lib/composio/googleSheets/refreshConnectedAccount.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/api/**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
All API routes should have JSDoc comments
Files:
app/api/connectedAccounts/googleSheets/refresh/route.tsapp/api/connectedAccounts/googleDrive/refresh/route.ts
🧬 Code graph analysis (4)
lib/composio/authenticateToolkit.ts (2)
lib/composio/toolkits.ts (2)
ComposioToolkitKey(20-20)getToolkitAuthConfigId(33-44)lib/composio/client.ts (1)
getComposioClient(7-12)
lib/composio/getConnectedAccount.ts (3)
lib/composio/toolkits.ts (2)
ComposioToolkitKey(20-20)getToolkitConfig(26-28)lib/composio/client.ts (1)
getComposioClient(7-12)lib/composio/authenticateToolkit.ts (1)
authenticateToolkit(13-28)
lib/composio/refreshConnectedAccount.ts (3)
lib/composio/toolkits.ts (2)
ComposioToolkitKey(20-20)getToolkitConfig(26-28)lib/composio/getComposioApiKey.ts (1)
getComposioApiKey(6-14)lib/composio/getConnectedAccount.ts (1)
getConnectedAccount(15-37)
app/api/connectedAccounts/googleDrive/refresh/route.ts (2)
lib/networking/getCorsHeaders.ts (1)
getCorsHeaders(6-12)lib/composio/refreshConnectedAccount.ts (1)
refreshConnectedAccount(27-72)
⏰ 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 (7)
lib/composio/authenticateToolkit.ts (1)
1-28: LGTM! Clean implementation of toolkit authentication.The function is well-documented and follows a clear pattern: retrieve auth config, get client, initiate connection. Error handling is appropriately delegated to
getToolkitAuthConfigId.app/api/connectedAccounts/googleSheets/refresh/route.ts (2)
5-13: LGTM! Standard CORS preflight handler.
23-28: LGTM! Proper validation of required parameter.lib/composio/toolkits.ts (1)
1-44: LGTM! Well-structured centralized configuration.The toolkit configuration follows a clean pattern:
- Single source of truth for toolkit configs
- Proper TypeScript typing with const assertion
- Clear helper functions with appropriate error handling for missing environment variables
app/api/connectedAccounts/googleDrive/refresh/route.ts (1)
2-2: LGTM! Successfully refactored to use shared utilities.The route now uses the centralized
refreshConnectedAccountfunction with the "GOOGLE_DRIVE" toolkit key, aligning with the new architecture.Also applies to: 16-16, 30-34
lib/composio/refreshConnectedAccount.ts (2)
1-3: LGTM!The imports are clean and all are utilized within the function.
5-15: LGTM!The interface correctly models the Composio API response with appropriate nullable types and status literals.
| if (userAccounts.items.length === 0) { | ||
| await authenticateToolkit(toolkitKey, accountId, options); | ||
| userAccounts = await composio.connectedAccounts.list({ | ||
| userIds: [accountId], | ||
| toolkitSlugs: [config.slug], | ||
| }); | ||
| } |
There was a problem hiding this comment.
Logic issue: OAuth flow requires user interaction before retry.
After calling authenticateToolkit on line 29, the code immediately retries the account list. However, authenticateToolkit initiates an OAuth flow that requires user interaction (redirect, consent, callback). The retry will likely return empty because the OAuth flow hasn't completed yet.
Consider one of these approaches:
- Return the connection request to the caller for handling
- Document this expected behavior clearly
- Remove the retry and let the caller handle the flow
Proposed approach 1: Return connection request
export async function getConnectedAccount(
toolkitKey: ComposioToolkitKey,
accountId: string,
options?: CreateConnectedAccountOptions,
) {
const config = getToolkitConfig(toolkitKey);
const composio = getComposioClient();
let userAccounts = await composio.connectedAccounts.list({
userIds: [accountId],
toolkitSlugs: [config.slug],
});
if (userAccounts.items.length === 0) {
- await authenticateToolkit(toolkitKey, accountId, options);
- userAccounts = await composio.connectedAccounts.list({
- userIds: [accountId],
- toolkitSlugs: [config.slug],
- });
+ const connectionRequest = await authenticateToolkit(toolkitKey, accountId, options);
+ // Return connection request for caller to handle OAuth flow
+ return { items: [], connectionRequest };
}
return userAccounts;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @lib/composio/getConnectedAccount.ts around lines 28 - 34, The code calls
authenticateToolkit(toolkitKey, accountId, options) which starts an interactive
OAuth flow and then immediately retries composio.connectedAccounts.list — this
will usually still be empty; instead stop retrying: remove the second call to
composio.connectedAccounts.list after authenticateToolkit and change
getConnectedAccount (or the function containing this block) to return the
connection request result (or a flag/object indicating the OAuth flow was
initiated) so the caller can perform the redirect/consent and retry list after
the callback completes; update any callers to handle the connection-request
response rather than relying on an immediate retry.
| export async function refreshConnectedAccount( | ||
| toolkitKey: ComposioToolkitKey, | ||
| accountId: string, | ||
| redirectUrl?: string, | ||
| ): Promise<ConnectedAccountRefreshResponse | null> { |
There was a problem hiding this comment.
Fix return type inconsistency.
The function signature declares Promise<ConnectedAccountRefreshResponse | null>, but the implementation never returns null. It either throws an error (lines 37, 43, 66) or returns the parsed JSON response (line 71). The return type should be Promise<ConnectedAccountRefreshResponse> to accurately reflect the actual behavior.
🔧 Proposed fix
-): Promise<ConnectedAccountRefreshResponse | null> {
+): Promise<ConnectedAccountRefreshResponse> {Also applies to: 71-71
🤖 Prompt for AI Agents
In @lib/composio/refreshConnectedAccount.ts around lines 27 - 31, The declared
return type of refreshConnectedAccount is inaccurate: change the function
signature from Promise<ConnectedAccountRefreshResponse | null> to
Promise<ConnectedAccountRefreshResponse> because the implementation never
returns null (it throws on errors and returns parsed JSON via the response
handling in refreshConnectedAccount). Update the return type in the
refreshConnectedAccount declaration accordingly and verify callers don't rely on
a null return (adjust callers if any assume null).
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| console.error("Error refreshing connected account:", error); | ||
|
|
||
| const errorMessage = error instanceof Error ? error.message : "Internal server error"; | ||
| const errorMessage = |
There was a problem hiding this comment.
Missing error logging in the catch block. The old endpoint logged all errors with console.error(), but the new implementation doesn't, reducing observability in production.
View Details
📝 Patch Details
diff --git a/app/api/connectedAccounts/googleDrive/refresh/route.ts b/app/api/connectedAccounts/googleDrive/refresh/route.ts
index a2bccc3..b427726 100644
--- a/app/api/connectedAccounts/googleDrive/refresh/route.ts
+++ b/app/api/connectedAccounts/googleDrive/refresh/route.ts
@@ -38,6 +38,7 @@ export async function POST(request: NextRequest) {
{ status: 200, headers: getCorsHeaders() },
);
} catch (error) {
+ console.error("Error refreshing connected account:", error);
const errorMessage =
error instanceof Error ? error.message : "Internal server error";
const statusCode = errorMessage.includes("not found") ? 404 : 500;
diff --git a/app/api/connectedAccounts/googleSheets/refresh/route.ts b/app/api/connectedAccounts/googleSheets/refresh/route.ts
index 4b6fb47..42a8b4a 100644
--- a/app/api/connectedAccounts/googleSheets/refresh/route.ts
+++ b/app/api/connectedAccounts/googleSheets/refresh/route.ts
@@ -38,6 +38,7 @@ export async function POST(request: NextRequest) {
{ status: 200, headers: getCorsHeaders() },
);
} catch (error) {
+ console.error("Error refreshing connected account:", error);
const errorMessage =
error instanceof Error ? error.message : "Internal server error";
const statusCode = errorMessage.includes("not found") ? 404 : 500;
Analysis
Missing error logging in refresh endpoints
What fails: The POST handlers in app/api/connectedAccounts/googleDrive/refresh/route.ts and app/api/connectedAccounts/googleSheets/refresh/route.ts do not log errors to the server console when refresh operations fail, reducing observability in production.
How to reproduce:
// Call the endpoint with an invalid or expired accountId that will cause refreshConnectedAccount() to throw
const response = await fetch('/api/connectedAccounts/googleDrive/refresh', {
method: 'POST',
body: JSON.stringify({ accountId: 'invalid-id', redirectUrl: 'https://example.com' })
});Result: When the underlying API call fails, no error is logged to the server console. The error is sent to the client but server-side observability is lost.
Expected behavior: Errors should be logged server-side with console.error("Error refreshing connected account:", error); matching:
- The original endpoint implementation (git commit e528311: removed file had this logging)
- Similar error handling patterns in the codebase (
app/api/image/generate/route.ts,app/api/x402/image/generate/route.ts)
References:
- Original implementation logged errors before refactoring in commit e528311
- Similar endpoints use console.error for production monitoring
There was a problem hiding this comment.
why do we need an API endpoint for refreshing the sheets token?
| if (!accountId) { | ||
| return NextResponse.json( | ||
| { error: "accountId is required" }, | ||
| { status: 400, headers: getCorsHeaders() }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
use api key instead of accountId
- accountId is not secure.
- api key is secure.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.