Sweetmantech/myc 3470 composio google sheets#1379
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Rate limit exceeded@sweetmantech has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR refactors the routing and agent setup architecture by introducing specialized agent factory functions ( Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant getExecute
participant getRoutingDecision
participant generalAgent as getGeneralAgent
participant googleAgent as getGoogleSheetsAgent
participant agent as ToolLoopAgent
Client->>getExecute: setupChatRequest(body)
getExecute->>getRoutingDecision: getRoutingDecision(body)
getRoutingDecision->>generalAgent: getGeneralAgent(body)<br/>(compute fallback)
generalAgent-->>getRoutingDecision: RoutingDecision
alt routing decision targets googleSheetsAgent
getRoutingDecision->>googleAgent: getGoogleSheetsAgent(body)
googleAgent-->>getRoutingDecision: RoutingDecision
else fallback to general agent
getRoutingDecision-->>getExecute: generalAgentDecision
end
getExecute->>agent: agent.stream(chatConfig)
Note over agent: Uses model, instructions,<br/>tools, stopWhen from<br/>RoutingDecision
agent-->>getExecute: stream result
getExecute->>getExecute: handleChatCredits(usage)
getExecute-->>Client: merged UI message stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
lib/composio/client.ts (1)
4-25: Async singleton pattern is good; avoidNEXT_PUBLIC_for a secret API key.The cached async singleton looks clean and throwing on missing configuration is appropriate. However, using
NEXT_PUBLIC_COMPOSIO_API_KEYfor a server-only Composio token is risky:NEXT_PUBLIC_vars are intended for values safe to ship to the client and could be exposed if this helper is ever pulled into a client bundle. Prefer a non‑public env name (e.g.COMPOSIO_API_KEY) and update call sites, or at least document that this module must remain server-only.components/VercelChat/ChatInput.tsx (1)
18-24: Guard Sheets auth whenaccountIdis missing and tighten UX.
accountIdcan beundefineduntiluseUserresolves (or for users without an account), but the click handler always callsauthenticateGoogleSheetsToolkit(accountId). Consider disabling or hiding the “Connect Sheets” button untilaccountIdis truthy, or early‑return with a user message if it’s missing. You may also want to add a simple loading flag / try–catch around the async call to avoid repeated clicks and to surface failures to the user.Also applies to: 89-98
lib/chat/setupChatRequest.ts (1)
14-15: Harden Google Sheets agent routing with error handling and cleaner logging.The routing decision and conditional override of
config.tools/config.agentare structurally solid. To avoid brittle behavior, I’d recommend wrapping thegetGoogleSheetsAgent(accountId)call in a try/catch so Composio misconfigurations or missing Sheets connections don’t 500 the whole chat request—log the error and fall back to the baseconfigwhen Sheets isn’t available. Also consider removing or gating theconsole.log("routingDecision", routingDecision)behind a debug flag to keep production logs clean.Also applies to: 27-29, 54-55, 106-110
lib/chat/getExecute.ts (1)
15-15: Remove debug logging or use proper logging infrastructure.The console.log statement appears to be temporary debug code. Consider removing it or replacing it with a structured logging solution if agent visibility is needed in production.
lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts (1)
16-32: Enhance error handling with contextual information.The current error handling simply rethrows without adding context about what operation failed, making debugging difficult.
Apply this diff to improve error context:
try { const connectionRequest = await composio.connectedAccounts.initiate( userId, authConfigId! ); console.log( `SWEETS Visit this URL to authenticate Google Sheets: ${connectionRequest.redirectUrl}` ); // This will wait for the auth flow to be completed await connectionRequest.waitForConnection(); return connectionRequest.id; } catch (error) { - console.error("SWEETS error", error); - throw error; + throw new Error( + `Failed to authenticate Google Sheets for user ${userId}: ${error instanceof Error ? error.message : String(error)}`, + { cause: error } + ); }Based on coding guidelines (proper error handling).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.env.exampleis excluded by none and included by noneevals/catalog-opportunity-analysis.eval.tsis excluded by none and included by noneevals/catalog-songs-count.eval.tsis excluded by none and included by none
📒 Files selected for processing (8)
components/VercelChat/ChatInput.tsx(2 hunks)lib/agents/googleSheetsAgent/googleSheetsAgent.ts(1 hunks)lib/chat/getExecute.ts(1 hunks)lib/chat/setupChatRequest.ts(4 hunks)lib/chat/types.ts(2 hunks)lib/composio/client.ts(1 hunks)lib/composio/getComposioTools.ts(0 hunks)lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts(1 hunks)
💤 Files with no reviewable changes (1)
- lib/composio/getComposioTools.ts
🧰 Additional context used
📓 Path-based instructions (2)
components/**/*.tsx
⚙️ CodeRabbit configuration file
components/**/*.tsx: For React components, ensure:
- Single responsibility per component
- Proper prop interfaces (ISP)
- Use composition over inheritance
- Avoid prop drilling (use context or state management)
- Keep components under 200 lines
- Extract custom hooks for complex logic
- Use TypeScript interfaces for props
- DRY: Extract common UI patterns into reusable components
- KISS: Prefer simple component structure over complex abstractions
Files:
components/VercelChat/ChatInput.tsx
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/agents/googleSheetsAgent/googleSheetsAgent.tslib/composio/googleSheets/authenticateGoogleSheetsToolkit.tslib/chat/getExecute.tslib/chat/setupChatRequest.tslib/composio/client.tslib/chat/types.ts
🧬 Code graph analysis (4)
lib/agents/googleSheetsAgent/googleSheetsAgent.ts (2)
lib/composio/client.ts (1)
getComposioClient(12-25)lib/consts.ts (1)
DEFAULT_MODEL(41-41)
lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts (1)
lib/composio/client.ts (1)
getComposioClient(12-25)
lib/chat/getExecute.ts (3)
lib/chat/types.ts (1)
ChatRequest(13-24)lib/chat/setupChatRequest.ts (1)
setupChatRequest(16-113)lib/agents/googleSheetsAgent/googleSheetsAgent.ts (1)
getGoogleSheetsAgent(9-25)
lib/chat/setupChatRequest.ts (2)
lib/agents/routingAgent/getRoutingDecision.ts (1)
getRoutingDecision(10-40)lib/agents/googleSheetsAgent/googleSheetsAgent.ts (1)
getGoogleSheetsAgent(9-25)
⏰ 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). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
- GitHub Check: Run evaluations
🔇 Additional comments (1)
lib/chat/types.ts (1)
4-11: Agent field addition looks sound; confirm AI SDK usage aligns with this type.Adding
agent?: ToolLoopAgenttoChatConfigand importingToolLoopAgentfrom"ai"fits the new agent-based flow, and theexperimental_downloadchange is formatting-only. Please just double-check that whatever API you use to execute chats (e.g.,streamText/agents inaiv6) expectsagentin this shape and thatconfig.tools/config.agentare not conflicting in those call sites.Also applies to: 26-36
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚨 Review notes (focused, high-severity):
- 🔒 Security: now reads and removed . Do not expose this key to the client; revert to a server-only var (e.g., ) and keep the module server-only.
⚠️ Logic: adds but code expects ; this mismatch will crash at runtime. Align the env name in both places.- 🔒 Security: logs the Composio client and . These may leak sensitive info; remove the logs and avoid printing URLs or objects here.
⚠️ Logic: throws at module load when is missing. Move this validation into the function to avoid crashing import/build in non-sheets flows.⚠️ Logic: constructs a new Google Sheets agent even when is already set by . Use the provided instead to avoid divergent config.⚠️ Logic: awaits but uses without await, then calls . Ensure both paths return the same streaming result type before merging to the writer.- ⚡ Performance: Agent/tools are fetched twice (in and again in ); consolidate to a single creation to reduce latency.
- ✨ Improvement: Remove leftover debug logs (, , and logs) from production paths.
Happy to re-check after adjustments. ✅
There was a problem hiding this comment.
- 🔒 Security (lib/composio/client.ts:14): Do not use NEXT_PUBLIC_ for API keys; switch to a server-only env (e.g., COMPOSIO_API_KEY) to prevent exposure.
- 🔒 Security (lib/composio/client.ts:1): Add 'use server' to keep this module server-only and avoid accidental client bundling.
- 🧩 Config (.env.example:26): Name mismatch with code (uses NEXT_PUBLIC_COMPOSIO_API_KEY); align and avoid NEXT_PUBLIC for secrets.
⚠️ Logic (lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts:5): Avoid throwing at module load; move the env check inside the function to fail at call time.- 🔒 Security (lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts:22): Don’t log the auth redirect URL; treat it as sensitive.
- ⚡ Performance (lib/agents/googleSheetsAgent/googleSheetsAgent.ts:22): stepCountIs(111) can run long; cap steps to a small number (e.g., 1–3).
- ⚡ Performance (lib/agents/googleSheetsAgent/googleSheetsAgent.ts:15): Remove or guard the tools dump; logging full tool sets is noisy and heavy.
⚠️ Logic (lib/chat/getExecute.ts:20): Reusing chatConfig.agent is safer than re-creating the agent here to prevent divergence and extra work.
Summary: Focus on not exposing secrets, removing sensitive/verbose logs, reusing the configured agent, lowering the tool loop step cap, and aligning env names.
There was a problem hiding this comment.
🚨 Review notes (focused, high-severity):
- 🔒 Security:
lib/composio/client.tsnow readsprocess.env.NEXT_PUBLIC_COMPOSIO_API_KEYand removed 'use server'. Do not expose this key to the client; revert to a server-only var (e.g.,COMPOSIO_API_KEY) and keep the module server-only. ⚠️ Logic:.env.exampleaddsCOMPOSIO_API_KEYbut code expectsNEXT_PUBLIC_COMPOSIO_API_KEY; this mismatch will crash at runtime. Align the env name in both places.- 🔒 Security:
lib/composio/googleSheets/authenticateGoogleSheetsToolkit.tslogs the Composio client andredirectUrl. These may leak sensitive info; remove the logs and avoid printing URLs or objects here. ⚠️ Logic:authenticateGoogleSheetsToolkit.tsthrows at module load whenNEXT_PUBLIC_COMPOSIO_GOOGLE_SHEETS_AUTH_CONFIG_IDis missing. Move this validation into the function to avoid crashing import/build in non-sheets flows.⚠️ Logic:lib/chat/getExecute.tsconstructs a new Google Sheets agent even whenchatConfig.agentis already set bysetupChatRequest. Use the providedchatConfig.agentinstead to avoid divergent config.⚠️ Logic:getExecute.tsawaitsgoogleSheetsAgent.stream(chatConfig)but usesstreamText(chatConfig)without await, then callsresult.toUIMessageStream(). Ensure both paths return the same streaming result type before merging to the writer.- ⚡ Performance: Agent/tools are fetched twice (in
setupChatRequestand again ingetExecute); consolidate to a single creation to reduce latency. - ✨ Improvement: Remove leftover debug logs (
routingDecision,chatConfig agent, andSWEETSlogs) from production paths.
Happy to re-check after adjustments. ✅
There was a problem hiding this comment.
Summary: Focused on security and logic.
- 🔒 Avoid exposing Composio secrets; keep server-only and add 'use server'.
- ⚡ Reduce aggressive step limits to prevent runaway costs.
⚠️ Fix env var name mismatch and avoid module-load throws.- General: remove sensitive debug logs and reuse provided agent to prevent duplication.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Inline comments:
- 🔒 Security — lib/composio/client.ts: Reading
NEXT_PUBLIC_COMPOSIO_API_KEY(around new lines 513–519) exposes a server secret to the client; switch to a server-only var (e.g.,COMPOSIO_API_KEY), restore"use server", and keep this module server-only. ⚠️ Logic — .env.example: AddedCOMPOSIO_API_KEYbut code readsNEXT_PUBLIC_COMPOSIO_API_KEY; align names or the client will throw on startup when the expected var is missing.- 🔒 Security — lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts: Top-level
throwwhenNEXT_PUBLIC_COMPOSIO_GOOGLE_SHEETS_AUTH_CONFIG_IDis missing (lines ~621–627) will crash on import; move the check inside the function and fail gracefully. - 🔒 Security — lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts: Remove debug logs printing
authConfigId, the Composio client object, and the redirect URL (lines ~630–641); these can leak sensitive info. ⚠️ Logic — lib/agents/routingAgent/getRoutingDecision.ts: Passing bothmessagesandprompttoroutingAgent.generate(lines ~286–288) is ambiguous; provide one or ensure the API supports both to avoid unintended behavior.⚠️ Design — lib/agents/routingAgent/getRoutingDecision.ts: Function now returns aToolLoopAgentinstead of a routing decision; consider renaming (e.g.,getRoutedAgent) to match behavior and SRP.- ✅ Resolved — lib/chat/setupToolsForRequest.ts: Debug logging removed from tool setup; this looks addressed by recent changes.
- ✅ Resolved — lib/chat/getExecute.ts: Startup logging removed around stream initialization; this appears resolved.
Summary:
- 🔒 Replace public env usage with server-only for Composio and avoid top-level throws/log leaks in Sheets auth.
⚠️ Clarify routing call inputs and function intent; align env variable names between code and.env.example.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
lib/agents/routingAgent/index.ts (1)
1-2: Consider consolidating exports if dual pattern is unintentional.The
routingAgentis exported both as default (line 1) and as a named export (line 2). While valid, this dual pattern may be redundant unless intentional for backward compatibility. If both export styles are required, this is fine; otherwise, consider standardizing on one approach.lib/chat/getExecute.ts (2)
50-50: Address or track the TODO comment.The TODO acknowledges an SRP violation where
route_totool references itself. This self-referential structure can lead to maintenance issues and circular dependencies.The refactoring suggested in the previous comment addresses this concern. Would you like me to open an issue to track this if you prefer to defer the refactoring?
16-111: Decompose getExecute into smaller, focused functions.The main function body spans approximately 95 lines, nearly double the 50-line guideline. It handles multiple responsibilities: routing tool creation, step execution, stream merging, and credit handling.
Consider extracting:
- Routing tools creation (addressed in previous comment)
- Step execution logic:
async function executeAgentStep( stepConfig: ChatConfig, writer: UIMessageStreamWriter, body: ChatRequest ): Promise<{ shouldContinue: boolean }> { const result = await stepConfig.agent.stream(stepConfig); writer.merge(result.toUIMessageStream()); const done = await (result as unknown as { done?: boolean }).done; if (done) { const usage = await result.usage; await handleChatCredits({ usage, model: stepConfig.model, accountId: body.accountId, }); return { shouldContinue: false }; } return { shouldContinue: true }; }This improves testability and adheres to the Single Responsibility Principle.
Based on coding guidelines (SRP, functions under 50 lines, KISS).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/agents/routingAgent/index.ts(1 hunks)lib/chat/getExecute.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/agents/routingAgent/index.tslib/chat/getExecute.ts
🧬 Code graph analysis (1)
lib/chat/getExecute.ts (3)
lib/chat/types.ts (2)
ChatRequest(14-25)ChatConfig(37-53)lib/chat/setupChatRequest.ts (1)
setupChatRequest(9-73)lib/credits/handleChatCredits.ts (1)
handleChatCredits(18-43)
⏰ 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). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Run evaluations
- GitHub Check: code-review
🔇 Additional comments (1)
lib/agents/routingAgent/index.ts (1)
1-1: The refactoring has been completed correctly. All verified.✓
RoutingDecisiontype is properly exported fromlib/chat/types.ts
✓ All imports ofRoutingDecisioncorrectly reference the new location
✓getRoutingDecisionfunction remains properly exported fromlib/agents/routingAgent/index.ts
✓ No broken imports detected across the codebaseThe type migration maintains clean separation of concerns by centralizing type definitions while preserving function exports. No additional changes required.
There was a problem hiding this comment.
Summary\n- ✅ Resolved: GSheets agent console.log appears removed via refactor (googleSheetsAgent).\n- 🔒 auth toolkit: remove Composio client and OAuth URL logs (lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts L15, L22).\n-
There was a problem hiding this comment.
Inline comments
- 🔒 Security lib/composio/client.ts:14 — Reading
NEXT_PUBLIC_COMPOSIO_API_KEYexposes a secret to the client; switch to a server-only key (e.g.,COMPOSIO_API_KEY) and keep this module server-only. ⚠️ Logic lib/composio/client.ts:16 — Throwing on missing key will crash requests; fail fast at boot or return a safe fallback with clear logging.⚠️ Logic lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts:16 — Wrap Composio init in try/catch and fallback (e.g., to the general agent) to avoid a hard 500 when auth/config is missing.- ⚡ Performance lib/agents/routingAgent/getRoutingDecision.ts:14 — Defer building the general agent until after routing indicates it’s needed to avoid unnecessary work.
⚠️ Logic lib/chat/getExecute.ts:20 — Confirmresult.usageshape underagent.stream(...)matcheshandleChatCredits; guard if undefined to prevent incorrect deductions.⚠️ Types lib/chat/types.ts:31 —NoInferis referenced but not imported here; ensure it’s available or import the type fromai.
Summary
- Harden Composio integration: keep API key server-only, align env names with
.env.example, handle init errors gracefully, defer heavy agent construction, and verify credit usage handling with the new streaming call.
There was a problem hiding this comment.
Inline notes:\n\n- 🔒 Security: lib/composio/client.ts:14 — Using NEXT_PUBLIC_COMPOSIO_API_KEY exposes the secret. Use a server-only COMPOSIO_API_KEY and keep this module server-only.\n- 🔒 Security: lib/composio/client.ts:12 — Missing 'use server'; client imports could bundle the Composio SDK. Re-add 'use server'.\n-
There was a problem hiding this comment.
Inline comments:
- lib/chat/getExecute.ts: ⚡ Performance: Avoid awaiting the stream before merging to the writer; start piping immediately to preserve live streaming.
- lib/chat/getExecute.ts:
⚠️ Logic: Credits are no longer handled post-stream; call handleChatCredits with the final usage to keep billing consistent. - lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts:
⚠️ Logic: tools can be undefined when Composio client is unavailable or accountId is missing; default to {} and guard against missing accountId to prevent runtime errors.
Summary: Focused on streaming behavior and credit handling regressions plus a potential runtime nullability bug in the Sheets agent.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts (2)
22-22: Reduce excessive step count limit.
stepCountIs(111)contradicts the "single-step" intent documented above and creates risk:
- High token costs for long-running operations
- Increased latency
- Potential for runaway loops
For a specialized Google Sheets agent, 1-5 steps should suffice for most operations (e.g., read sheet → process → write back).
Apply this diff:
- const stopWhen = stepCountIs(111); + const stopWhen = stepCountIs(5);Based on coding guidelines (KISS principle) and past review comments.
10-13: Add guard for missing accountId.The
accountIdis destructured without validation. Ifbody.accountIdisundefinedor empty, the Composio client call on line 17 will fail or behave unexpectedly.Apply this diff to add validation:
export default async function getGoogleSheetsAgent( body: ChatRequest ): Promise<RoutingDecision> { const { accountId, model: bodyModel } = body; + + if (!accountId) { + throw new Error("accountId is required for Google Sheets agent"); + }
🧹 Nitpick comments (1)
lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts (1)
23-23: Enhance agent instructions for better specialization.The instruction "You are a Google Sheets agent." is too generic. Specialized agents should have clear, actionable instructions that guide the model's behavior and tool usage.
Apply this diff for more effective instructions:
- const instructions = "You are a Google Sheets agent."; + const instructions = `You are a specialized Google Sheets agent. Your role is to: +- Read, write, and manipulate Google Sheets data +- Parse user requests for spreadsheet operations +- Use Google Sheets tools to accomplish tasks efficiently +- Provide clear feedback about operations performed +Keep operations focused and complete tasks in minimal steps.`;Based on coding guidelines (single responsibility, clear purpose).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.env.exampleis excluded by none and included by none
📒 Files selected for processing (2)
lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts(1 hunks)lib/chat/getExecute.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/chat/getExecute.tslib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts
🧬 Code graph analysis (2)
lib/chat/getExecute.ts (1)
lib/chat/setupChatRequest.ts (1)
setupChatRequest(9-73)
lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts (3)
lib/chat/types.ts (2)
ChatRequest(14-25)RoutingDecision(27-35)lib/composio/client.ts (1)
getComposioClient(27-39)lib/consts.ts (1)
DEFAULT_MODEL(41-41)
⏰ 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). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Run evaluations
- GitHub Check: code-review
🔇 Additional comments (3)
lib/chat/getExecute.ts (2)
16-27: LGTM! Clean, linear flow.The refactoring from complex multi-step routing to direct agent streaming significantly improves maintainability. Credit handling is now guaranteed to execute after streaming completes, eliminating the previous issue where credits might not be deducted on early termination.
28-31: LGTM! Appropriate error handling.The try-catch block correctly logs and re-throws errors, allowing upstream handlers to manage the response while preserving debugging context.
lib/agents/googleSheetsAgent/getGoogleSheetsAgent.ts (1)
32-37: LGTM! Return structure matches RoutingDecision interface.The return object correctly implements the
RoutingDecisioninterface with all required fields (agent, model, instructions, stopWhen).
There was a problem hiding this comment.
- 🚨 Import-time throw in lib/composio/client.ts may crash imports when COMPOSIO_API_KEY is missing; move check into getComposioClient and return a safe fallback.
- 🔒 Re-add "use server" or keep server-only boundaries in lib/composio/client.ts to avoid bundling server code into client builds.
⚠️ In getGoogleSheetsAgent, handle missing/unauthorized Composio by defaulting to an empty tool set.⚠️ Verify agent.stream return shape matches streamText usage to avoid streaming regressions.⚠️ RoutingDecision.agent typing is over-specific; consider loosening to avoid type errors in non-Composio paths.
There was a problem hiding this comment.
✅ Resolved: The prior SRP concern about an inline routing/tools blob appears addressed by the new lib/agents/** split.
🔒 Security/Availability
if (!apiKey) {
console.warn("COMPOSIO_API_KEY not found in environment variables");
throw new Error("COMPOSIO_API_KEY not found in environment variables");
}🚨 Throwing at module import time can crash builds/requests when the env is unset and removes the server-only guard; move this check inside getComposioClient() and restore "use server" to prevent accidental client imports.
const tools = await composio.tools.get(accountId, {
toolkits: ["GOOGLESHEETS"],
});Please validate accountId (non-empty string) before calling and handle missing credentials gracefully; otherwise this will throw for unauthenticated users.
export interface RoutingDecision {
model: string;
instructions: string;
agent: ToolLoopAgent<never, VercelToolCollection, never>;Confining agent to VercelToolCollection may break type-compatibility for non-Composio agents (e.g., general agent tools); consider loosening to a generic ToolSet-compatible agent.
Summary: Focused fixes suggested for import-time throw in lib/composio/client.ts, accountId validation in the Sheets agent, and a typing constraint that could impact builds. ✅ SRP refactor looks good.
There was a problem hiding this comment.
- ✅ Resolved: Logging of Composio tools in the Google Sheets agent has been removed.
- 🔒 Security: In , add "use server" (or import ) to keep the client server-only and prevent bundling into the browser.
⚠️ Logic: In , avoid throwing at module load time when the env var is missing; move the check inside to prevent imports from crashing in non-runtime contexts.
There was a problem hiding this comment.
- ✅ Resolved: Logging of Composio tools in the Google Sheets agent has been removed.
- 🔒 Security: In
lib/composio/client.ts, add "use server" (or importserver-only) to keep the client server-only and prevent bundling@composio/coreinto the browser. ⚠️ Logic: Inlib/composio/client.ts, avoid throwing at module load time when the env var is missing; move the check insidegetComposioClient()to prevent imports from crashing in non-runtime contexts.
1 participant
Summary by CodeRabbit
New Features
Refactor