-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add Connectors page UI with disconnect functionality #1455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 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. 📝 WalkthroughWalkthroughThis PR refactors Google Sheets-specific connector authentication into a generic, extensible connectors management system. It introduces a new settings page for managing connector accounts, removes hardcoded Google Sheets logic, and adds reusable components for displaying connectors and handling OAuth callbacks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ConnectorsPage as Connectors Page
participant useConnectors as useConnectors Hook
participant API as API Server
participant Composio as Composio Service
User->>ConnectorsPage: Open Settings/Connectors
ConnectorsPage->>useConnectors: Trigger fetch on mount
useConnectors->>API: GET /connectors
API->>useConnectors: Return connector list
useConnectors->>ConnectorsPage: Display connectors
ConnectorsPage->>User: Show installed & available
User->>ConnectorsPage: Click "Connect" on unavailable connector
ConnectorsPage->>useConnectors: authorize(connector)
useConnectors->>API: POST /authorize/connector
API->>Composio: Initiate OAuth flow
Composio->>API: Return redirect URL
API->>useConnectors: Return redirect URL
useConnectors->>User: Navigate to Composio OAuth
User->>Composio: Grant permissions
Composio->>User: Redirect to app with connected=true
User->>ConnectorsPage: Return from OAuth
ConnectorsPage->>ConnectionSuccessBanner: Detect query param
ConnectionSuccessBanner->>User: Show success banner (5s)
ConnectorsPage->>useConnectors: Refetch connectors
useConnectors->>API: GET /connectors
API->>useConnectors: Return updated list
useConnectors->>ConnectorsPage: Update displayed connectors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 565e6c8dfa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const results = result?.data?.results; | ||
| const firstResult = results ? Object.values(results)[0] : null; | ||
| const redirectUrl = firstResult?.redirect_url; | ||
| const connector = firstResult?.toolkit || "Connector"; | ||
| const status = firstResult?.status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pick the auth result with redirect/status instead of first entry
The component always uses Object.values(results)[0] to decide the connector name and whether to render the auth UI. If results contains multiple entries and the first one lacks redirect_url/status=Active, this returns null even though another entry does have an auth redirect (your caller explicitly checks for “any” auth result before rendering). That means users can get no connect CTA after a successful composio response with multiple results. Consider selecting the first entry that has redirect_url or status rather than assuming the first map entry is the auth one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @components/ConnectorsPage/ConnectorCard.tsx:
- Around line 132-137: The DropdownMenu trigger button (wrapped by
DropdownMenuTrigger and rendering the MoreVertical icon) lacks accessible text
for screen readers; update the button element to include an aria-label (e.g.,
aria-label="Open connector menu") and appropriate aria-haspopup="menu" and
aria-expanded bound to the menu state (or let the DropdownMenu expose expanded
state) so assistive tech can identify and toggle the menu; ensure these
attributes are added to the actual button inside DropdownMenuTrigger (asChild)
and keep keyboard focus/interaction intact.
In @components/VercelChat/ConnectionSuccessBanner.tsx:
- Around line 41-46: The dismiss button rendered near the X icon has no
accessible name; update the button used with onClick={() => setShow(false)} to
provide an accessible label by adding either an aria-label (e.g.,
aria-label="Close notification" or "Dismiss") or an inner screen-reader-only
text node (e.g., a span with visually-hidden/sr-only class) so assistive
technologies can announce its purpose; keep the existing X icon for visual users
and ensure the same label is applied to the element containing the X component.
In @components/VercelChat/tools/composio/ComposioAuthResult.tsx:
- Around line 80-92: The anchor element with href={redirectUrl} has an
unnecessary onClick that prevents default and manually sets
window.location.href; remove the entire onClick handler from the <a> so the
native navigation and browser behaviors are preserved (keep href={redirectUrl}
and the existing className, getIcon usage, and displayName span); if
pre-navigation logic is required, move it into a validated handler that does not
call e.preventDefault() or use Next.js router for client-side routing instead of
overriding default anchor behavior.
In @hooks/useConnectors.ts:
- Around line 42-70: The fetchConnectors function can update state after
unmount; wrap the network call with an AbortController: in the component
useEffect that calls fetchConnectors, create const controller = new
AbortController() and pass controller.signal into fetch (e.g., fetch(url, {
signal: controller.signal })), catch DOMException with name "AbortError" and
avoid calling setError for that case, and call controller.abort() in the
useEffect cleanup to prevent state updates on an unmounted component;
alternatively modify fetchConnectors to accept an AbortSignal parameter and
check signal.aborted before calling setConnectors/setIsLoading/setError.
In @lib/consts.ts:
- Around line 4-9: The repo is missing documentation for the new environment
toggle NEXT_PUBLIC_USE_LOCAL_API used by IS_LOCAL and NEW_API_BASE_URL in
lib/consts.ts; add a line to .env.example declaring
NEXT_PUBLIC_USE_LOCAL_API=false so developers see the default/local API toggle
and can override it during development.
🧹 Nitpick comments (17)
lib/chat/setupToolsForRequest.ts (2)
27-35: Consider adding error handling for external service calls.The function makes several async calls to external services (
getOrCreatePurchaserAccount,createMCPClient,mcpClient.tools()) without explicit error handling. If any of these fail, the error propagates without meaningful context, making debugging harder.Per the coding guidelines for
lib/**/*.ts, proper error handling is expected.♻️ Suggested improvement with error handling
export async function setupToolsForRequest( body: ChatRequestBody ): Promise<ToolSet> { const { excludeTools } = body; const localTools = getMcpTools(); - const account = await getOrCreatePurchaserAccount(); + const account = await getOrCreatePurchaserAccount().catch((error) => { + throw new Error(`Failed to get purchaser account: ${error.message}`); + }); - const mcpClient = await createMCPClient({ - transport: new StreamableHTTPClientTransport( - new URL("/mcp", NEW_API_BASE_URL) - ), - }).then((client) => withPayment(client, { account, network: "base" })); + const mcpClient = await createMCPClient({ + transport: new StreamableHTTPClientTransport( + new URL("/mcp", NEW_API_BASE_URL) + ), + }) + .then((client) => withPayment(client, { account, network: "base" })) + .catch((error) => { + throw new Error(`Failed to create MCP client: ${error.message}`); + }); - const mcpClientTools = await mcpClient.tools(); + const mcpClientTools = await mcpClient.tools().catch((error) => { + throw new Error(`Failed to retrieve MCP tools: ${error.message}`); + });
36-39: Tool spread order is intentional but worth documenting.The spread order
{ ...mcpClientTools, ...localTools }means local tools will override MCP client tools if there are naming collisions. This is likely intentional to allow local overrides, but a brief inline comment would make this explicit for future maintainers.📝 Optional: Add clarifying comment
const allTools: ToolSet = { ...mcpClientTools, - ...localTools, + ...localTools, // Local tools take precedence over MCP tools };components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx (1)
54-59: LGTM! Clean implementation using theasChildpattern.The
asChildprop correctly delegates the clickable behavior to the Next.jsLink, preserving proper navigation semantics and accessibility. The implementation is consistent with Radix UI patterns.One minor observation: this component mixes
lucide-reacticons (Plug,Moon,Sun,Monitor,Check) with@tabler/icons-react(IconUser,IconLogout). Consider standardizing on one library for consistency, though this is not blocking.lib/connectors/connectorMetadata.ts (1)
14-38: Well-organized metadata structure.Good use of comments to categorize connectors. The Record type with explicit interface provides type safety while remaining extensible.
Note: There's a minor mismatch between connectors listed here and those in
formatConnectorName.ts. This file is missing:perplexityai,codeinterpreter,serpapi,firecrawl. Similarly, this file includesoutlook,airtable,linear,jira,hubspot,components/ConnectorsPage/index.ts (1)
1-2: Consider exporting prop types for external consumers.Per coding guidelines, component prop types should be exported with explicit
ComponentNamePropsnaming convention. IfConnectorCardPropsis intended for external use (e.g., custom card rendering), consider adding it to the exports:export { ConnectorsPage } from "./ConnectorsPage"; -export { ConnectorCard } from "./ConnectorCard"; +export { ConnectorCard, type ConnectorCardProps } from "./ConnectorCard";This enables consumers to type their own wrappers or compose these components more easily.
app/settings/connectors/page.tsx (1)
1-18: Clean page setup with appropriate Suspense boundary.The client component wrapping and Suspense usage is correct for a page that relies on client-side hooks like
useSearchParams.Minor accessibility enhancement: consider adding
aria-busyoraria-liveattributes to the loading fallback for screen reader users:fallback={ - <div className="flex h-full items-center justify-center"> + <div className="flex h-full items-center justify-center" role="status" aria-label="Loading connectors"> <p className="text-muted-foreground">Loading...</p> </div> }components/ConnectorsPage/ConnectorsPage.tsx (3)
24-32: Consider using Next.js router instead of window.history.replaceState.Direct manipulation of
window.historybypasses Next.js navigation state management. Using the router ensures consistency:♻️ Suggested refactor
+"use client"; + +import { Fragment } from "react"; +import { usePrivy } from "@privy-io/react-auth"; +import { useSearchParams, useRouter } from "next/navigation"; ... export function ConnectorsPage() { + const router = useRouter(); ... useEffect(() => { if (searchParams.get("connected") === "true") { setShowSuccess(true); refetch(); - window.history.replaceState({}, "", "/settings/connectors"); + router.replace("/settings/connectors"); const timer = setTimeout(() => setShowSuccess(false), 5000); return () => clearTimeout(timer); } - }, [searchParams, refetch]); + }, [searchParams, refetch, router]);
34-34: Prefernullover empty<Fragment />for rendering nothing.Returning
nullis the idiomatic React pattern for conditional non-rendering and avoids the overhead of creating a fragment node:- if (!ready) return <Fragment />; + if (!ready) return null;With this change, the
Fragmentimport on line 3 can also be removed.
70-79: Addaria-labelfor screen reader accessibility.Per coding guidelines, UI components should provide proper ARIA attributes. The
titleattribute isn't reliably announced by screen readers:<button onClick={() => refetch()} disabled={isLoading} className="p-2 rounded-lg hover:bg-muted transition-colors disabled:opacity-50" - title="Refresh" + aria-label="Refresh connectors" + title="Refresh" >components/VercelChat/ToolComponents.tsx (1)
538-559: Add a composio-specific skeleton to match the established pattern.Composio currently lacks a loading skeleton in
getToolCallComponentand falls through to the generic loader ("Using composio" + spinner). While this works functionally, the codebase consistently pairs result components with dedicated skeletons—25+ tools follow this pattern (ImageSkeleton, YouTubeAccessSkeleton, TasksSkeleton, etc.).Since
ComposioAuthResultexists as a specialized result component, consider addingComposioAuthResultSkeletonto provide contextual loading feedback and maintain consistency with the component architecture.components/ConnectorsPage/ConnectorCard.tsx (3)
49-72: Consider extracting icon mapping to module scope for reuse and performance.The
getConnectorIconfunction recreates theiconsobject on every call. Moving the icon mapping outside the function improves performance and could be reused by other components (likeComposioAuthResultwhich has similar icon logic).♻️ Suggested refactor
+const CONNECTOR_ICONS: Record<string, (size: number, className: string) => React.ReactNode> = { + googlesheets: (size, className) => <SiGooglesheets size={size} className={className} color="#34A853" />, + googledrive: (size, className) => <SiGoogledrive size={size} className={className} color="#4285F4" />, + // ... other icons +}; function getConnectorIcon(slug: string, size = 24) { - const iconProps = { size, className: "flex-shrink-0" }; - - const icons: Record<string, React.ReactNode> = { - googlesheets: <SiGooglesheets {...iconProps} color="#34A853" />, - // ... - }; - - return icons[slug] || <Link2 size={size} className="flex-shrink-0 text-muted-foreground" />; + const iconFactory = CONNECTOR_ICONS[slug]; + return iconFactory + ? iconFactory(size, "flex-shrink-0") + : <Link2 size={size} className="flex-shrink-0 text-muted-foreground" />; }
87-97: Consider user feedback when redirect URL is not returned.When
onConnectreturnsnull(no redirect URL), the user sees the loading state briefly clear with no indication of what happened. This could leave users confused.💡 Suggestion to improve UX
const handleConnect = async () => { setIsConnecting(true); try { const redirectUrl = await onConnect(connector.slug); if (redirectUrl) { window.location.href = redirectUrl; + } else { + // Consider showing a toast or error state + console.warn(`No redirect URL returned for ${connector.slug}`); } } finally { setIsConnecting(false); } };
162-169: Add focus-visible styles for keyboard navigation.The "Enable" button lacks visible focus states, which is important for keyboard navigation accessibility.
♿ Add focus-visible styles
<button onClick={handleConnect} disabled={isConnecting} - className="inline-flex items-center gap-1.5 px-4 py-2 text-sm font-medium text-foreground bg-transparent border border-border hover:bg-muted rounded-lg transition-colors disabled:opacity-50" + className="inline-flex items-center gap-1.5 px-4 py-2 text-sm font-medium text-foreground bg-transparent border border-border hover:bg-muted rounded-lg transition-colors disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2" >hooks/useConnectors.ts (1)
136-143: Consider returning aclearErrorfunction for better error state management.Currently, errors persist until the next operation. Providing a way to clear errors gives consumers more control over the UI state.
💡 Optional enhancement
+ const clearError = useCallback(() => setError(null), []); + return { connectors, isLoading, error, + clearError, refetch: fetchConnectors, authorize, disconnect, };components/VercelChat/tools/composio/ComposioAuthResult.tsx (3)
7-21: Consider a more precise type for the results structure.The
Record<string, ...>type suggests multiple results, but the component only uses the first one. If only one result is expected, the type could be more explicit. If multiple results are possible, the component might need to handle them.💡 Type refinement suggestion
interface ComposioAuthResultProps { result: { data?: { - results?: Record< - string, - { - toolkit?: string; - status?: string; - redirect_url?: string; - instruction?: string; - } - >; + results?: { + [key: string]: { + toolkit?: string; + status?: string; + redirect_url?: string; + instruction?: string; + }; + }; }; }; }Or if only one result is truly expected, consider documenting this in the interface or adjusting the API contract.
36-41: Icon logic duplicates ConnectorCard'sgetConnectorIcon- consider consolidating.This
getIconfunction duplicates icon selection logic that exists inConnectorCard.tsx. Consider extracting a shared utility tolib/connectors/for consistency and maintainability (DRY principle).♻️ Consolidation suggestion
Create a shared utility at
lib/connectors/getConnectorIcon.tsx:// lib/connectors/getConnectorIcon.tsx import { FileSpreadsheet, Link2 } from "lucide-react"; import { SiGooglesheets, /* other icons */ } from "@icons-pack/react-simple-icons"; export function getConnectorIcon(slug: string, size = 24, className = "flex-shrink-0") { // Consolidated icon mapping }Then import and use in both components.
100-102: Dual export pattern is fine but consider consistency.Having both named and default exports works, but the codebase should be consistent. If other components in this directory use only named exports, consider aligning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package.jsonis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (25)
app/settings/connectors/page.tsxcomponents/ConnectorsPage/ConnectorCard.tsxcomponents/ConnectorsPage/ConnectorsPage.tsxcomponents/ConnectorsPage/index.tscomponents/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/VercelChat/ToolComponents.tsxcomponents/VercelChat/chat.tsxcomponents/VercelChat/tools/composio/ComposioAuthResult.tsxcomponents/VercelChat/tools/googleSheets/GoogleSheetsLoginLoading.tsxcomponents/VercelChat/tools/googleSheets/GoogleSheetsLoginResult.tsxhooks/useConnectors.tshooks/useGoogleSheetsLogin.tslib/agents/googleSheetsAgent/getGoogleSheetsTools.tslib/agents/googleSheetsAgent/index.tslib/chat/setupToolsForRequest.tslib/composio/client.tslib/composio/googleSheets/authenticateGoogleSheetsToolkit.tslib/composio/googleSheets/fetchConnectedAccountsRefresh.tslib/composio/googleSheets/getConnectedAccount.tslib/connectors/connectorMetadata.tslib/connectors/formatConnectorName.tslib/connectors/index.tslib/consts.tslib/tools/composio/googleSheetsLoginTool.ts
💤 Files with no reviewable changes (10)
- hooks/useGoogleSheetsLogin.ts
- lib/composio/googleSheets/fetchConnectedAccountsRefresh.ts
- components/VercelChat/tools/googleSheets/GoogleSheetsLoginLoading.tsx
- lib/composio/googleSheets/authenticateGoogleSheetsToolkit.ts
- lib/tools/composio/googleSheetsLoginTool.ts
- lib/agents/googleSheetsAgent/getGoogleSheetsTools.ts
- lib/composio/client.ts
- lib/agents/googleSheetsAgent/index.ts
- components/VercelChat/tools/googleSheets/GoogleSheetsLoginResult.tsx
- lib/composio/googleSheets/getConnectedAccount.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/component-design.mdc)
**/*.{tsx,ts,jsx,js}: Use semantic HTML elements appropriate to the component's role
Ensure keyboard navigation and focus management in UI components
Provide proper ARIA roles/states and test with screen readers
Start with semantic HTML first, then augment with ARIA if needed
Support both controlled and uncontrolled state in components
Wrap a single HTML element per component (no multiple root elements)
Favor composition over inheritance in component design
Expose clear APIs via props/slots for component customization
UseasChildpattern for flexible element types in components
Support polymorphism withasprop when appropriate
Use@radix-ui/react-*primitives for behavior and accessibility
Useclass-variance-authority(CVA) for component variants
Use@radix-ui/react-slotfor implementingasChildpattern
Use@radix-ui/react-use-controllable-statefor state management
Follow shadcn/ui component structure and naming conventions
Usecn()utility combiningclsxandtailwind-mergefor class merging
Export both component and variant functions (e.g.,Button, buttonVariants)
Use@radix-ui/react-iconsfor UI component icons (not Lucide React)
Uselucide-reactfor application icons and illustrations
Useframer-motionfor animations and transitions
Usenext-themesfor theme management
Usetailwindcss-animatefor CSS animations
Usesonnerfor toast notifications
Useembla-carousel-reactfor carousels
Usereact-resizable-panelsfor resizable layouts
ProvidedefaultValueandonValueChangeprops for state management
Usedata-stateattribute for visual component states (open/closed, loading, etc.)
Usedata-slotattribute for component identification
Use kebab-case naming for data attributes (e.g.,data-slot="form-field")
Follow class order in Tailwind: base → variants → conditionals → user overrides
Define component variants outside components to avoid recreation on re-render
Implement focus trapping in modal/dialog components
Store...
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tslib/connectors/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxlib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tscomponents/ConnectorsPage/ConnectorsPage.tsxhooks/useConnectors.tscomponents/VercelChat/chat.tsxlib/chat/setupToolsForRequest.tslib/consts.tscomponents/VercelChat/tools/composio/ComposioAuthResult.tsxapp/settings/connectors/page.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/component-design.mdc)
**/*.{tsx,ts}: Extend native HTML attributes usingReact.ComponentProps<"element">pattern
Export component prop types with explicitComponentNamePropsnaming convention
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tslib/connectors/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxlib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tscomponents/ConnectorsPage/ConnectorsPage.tsxhooks/useConnectors.tscomponents/VercelChat/chat.tsxlib/chat/setupToolsForRequest.tslib/consts.tscomponents/VercelChat/tools/composio/ComposioAuthResult.tsxapp/settings/connectors/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/stagehand.mdc)
**/*.{ts,tsx}: Plan instructions using Stagehandobservemethod before executing actions (e.g.,const results = await page.observe("Click the sign in button");)
Cache the results ofobserveto avoid unexpected DOM changes
Keepactactions atomic and specific (e.g., "Click the sign in button" not "Sign in to the website"). Avoid actions that are more than one step.
Use variable substitution with thevariablesparameter inactfor dynamic form filling (e.g.,action: "Enter Name: %name%", variables: { name: "John Doe" })
Always use Zod schemas for structured data extraction with theextractmethod (e.g.,schema: z.object({ text: z.string() }))
Wrap array extractions in a single object when extracting multiple items usingextract(e.g.,schema: z.object({ buttons: z.array(z.string()) }))
Use explicitinstructionandschemaparameters when callingextractmethod
Import Stagehand types from@browserbasehq/stagehandpackage (e.g.,import { Stagehand, Page, BrowserContext } from "@browserbasehq/stagehand";)
Initialize Stagehand withnew Stagehand()constructor and callawait stagehand.init()before using page automation methods
Implement main automation logic in functions that accept{ page, context, stagehand }parameters
Provide specific instructions to agents instead of vague ones (e.g., "Navigate to products page and filter by 'Electronics'" not "Do some stuff on this page")
Break down complex agent tasks into smaller steps for better execution
Use error handling with try/catch blocks when executing agent tasks
Combine agents for navigation with traditional methods for precise data extraction
**/*.{ts,tsx}: Create single-responsibility components with obvious data flow
Use strict TypeScript types with zero 'any' types
Follow Next.js optimization guides for performance
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tslib/connectors/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxlib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tscomponents/ConnectorsPage/ConnectorsPage.tsxhooks/useConnectors.tscomponents/VercelChat/chat.tsxlib/chat/setupToolsForRequest.tslib/consts.tscomponents/VercelChat/tools/composio/ComposioAuthResult.tsxapp/settings/connectors/page.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,js}: Write minimal code - use only the absolute minimum code needed
Ensure code is self-documenting through precise naming with verbs for functions and nouns for variables
Add short comments only when necessary to clarify non-obvious logic
Implement built-in security practices for authentication and data handling
Consider if code can be split into smaller functions before implementing
Avoid unnecessary abstractions during implementation
Ensure code clarity is suitable for understanding by junior developers
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tslib/connectors/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxlib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tscomponents/ConnectorsPage/ConnectorsPage.tsxhooks/useConnectors.tscomponents/VercelChat/chat.tsxlib/chat/setupToolsForRequest.tslib/consts.tscomponents/VercelChat/tools/composio/ComposioAuthResult.tsxapp/settings/connectors/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Single Responsibility Principle: One function per file with clear naming
Remove all console.log statements before merging to production
Write comments to explain 'why', not 'what'; prefer self-documenting code
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tslib/connectors/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxlib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tscomponents/ConnectorsPage/ConnectorsPage.tsxhooks/useConnectors.tscomponents/VercelChat/chat.tsxlib/chat/setupToolsForRequest.tslib/consts.tscomponents/VercelChat/tools/composio/ComposioAuthResult.tsxapp/settings/connectors/page.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/components/**/*.{ts,tsx}: Use @radix-ui/react-* primitives with class-variance-authority (CVA) for component styling
Use cn() utility for class merging in components
Always include accessibility in components: use semantic HTML, ARIA attributes, and keyboard navigation
Files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/index.tscomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxcomponents/ConnectorsPage/ConnectorsPage.tsxcomponents/VercelChat/chat.tsxcomponents/VercelChat/tools/composio/ComposioAuthResult.tsx
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/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/VercelChat/ConnectionSuccessBanner.tsxcomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/ToolComponents.tsxcomponents/ConnectorsPage/ConnectorsPage.tsxcomponents/VercelChat/chat.tsxcomponents/VercelChat/tools/composio/ComposioAuthResult.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/connectors/index.tslib/connectors/formatConnectorName.tslib/connectors/connectorMetadata.tslib/chat/setupToolsForRequest.tslib/consts.ts
hooks/**/*.ts
⚙️ CodeRabbit configuration file
hooks/**/*.ts: For custom hooks, ensure:
- Single responsibility per hook
- Return consistent interface
- Handle edge cases and errors
- Use proper dependency arrays
- Keep hooks focused and reusable
- Follow naming convention (use prefix)
- DRY: Extract common hook logic into shared utilities
- KISS: Avoid complex hook compositions, prefer simple state management
Files:
hooks/useConnectors.ts
**/lib/consts.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Store shared constants in
lib/consts.tsincludingRECOUP_FROM_EMAILfor email sender address and platform-specific configurations
Files:
lib/consts.ts
🧠 Learnings (11)
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Use `lucide-react` for application icons and illustrations
Applied to files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsxcomponents/ConnectorsPage/ConnectorCard.tsxcomponents/VercelChat/tools/composio/ComposioAuthResult.tsx
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Use `role="menu"` and `role="menuitem"` for dropdown menu components
Applied to files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Use `radix-ui/react-*` primitives for behavior and accessibility
Applied to files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Use `radix-ui/react-icons` for UI component icons (not Lucide React)
Applied to files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Handle enter/space activation in dropdown menu components
Applied to files:
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Export both component and variant functions (e.g., `Button, buttonVariants`)
Applied to files:
components/ConnectorsPage/index.ts
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Expose clear APIs via props/slots for component customization
Applied to files:
components/ConnectorsPage/index.ts
📚 Learning: 2025-11-29T16:42:25.945Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: .cursor/rules/component-design.mdc:0-0
Timestamp: 2025-11-29T16:42:25.945Z
Learning: Applies to **/*.{tsx,ts} : Export component prop types with explicit `ComponentNameProps` naming convention
Applied to files:
components/ConnectorsPage/index.ts
📚 Learning: 2026-01-07T17:34:21.923Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:34:21.923Z
Learning: Applies to **/lib/chat/toolChains/**/*.{ts,tsx} : MCP tools like `send_email` are provided by the recoup-api MCP server; reference tool names in tool chains at `lib/chat/toolChains/` instead of defining tools locally
Applied to files:
lib/chat/setupToolsForRequest.ts
📚 Learning: 2026-01-07T17:34:21.923Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:34:21.923Z
Learning: Applies to **/lib/consts.ts : Store shared constants in `lib/consts.ts` including `RECOUP_FROM_EMAIL` for email sender address and platform-specific configurations
Applied to files:
lib/consts.ts
📚 Learning: 2026-01-07T17:34:21.923Z
Learnt from: CR
Repo: Recoupable-com/Recoup-Chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:34:21.923Z
Learning: Use Next.js 16 with App Router and React 19
Applied to files:
app/settings/connectors/page.tsx
🧬 Code graph analysis (7)
components/Sidebar/UserProfileDropdown/UserProfileDropdown.tsx (1)
components/ui/dropdown-menu.tsx (1)
DropdownMenuItem(189-189)
components/VercelChat/ToolComponents.tsx (1)
components/VercelChat/tools/composio/ComposioAuthResult.tsx (1)
ComposioAuthResult(29-100)
lib/connectors/formatConnectorName.ts (1)
lib/connectors/index.ts (1)
formatConnectorName(2-2)
components/ConnectorsPage/ConnectorsPage.tsx (2)
hooks/useConnectors.ts (1)
useConnectors(34-144)components/ConnectorsPage/ConnectorCard.tsx (1)
ConnectorCard(78-174)
hooks/useConnectors.ts (1)
lib/consts.ts (1)
NEW_API_BASE_URL(5-9)
components/VercelChat/chat.tsx (1)
components/VercelChat/ConnectionSuccessBanner.tsx (1)
ConnectionSuccessBanner(11-50)
components/VercelChat/tools/composio/ComposioAuthResult.tsx (1)
lib/connectors/formatConnectorName.ts (1)
formatConnectorName(23-35)
⏰ 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). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: Run evaluations
🔇 Additional comments (15)
lib/chat/setupToolsForRequest.ts (1)
11-19: Clear and accurate documentation.The docblock properly reflects the architectural shift from direct Google Sheets tool integration to the Composio Tool Router approach via MCP. This aligns well with the PR's goal of genericizing connector management.
components/VercelChat/chat.tsx (2)
87-90: LGTM! Proper Suspense boundary foruseSearchParams.The
Suspensewrapper is required sinceConnectionSuccessBannerusesuseSearchParams, which suspends in Next.js App Router. Thefallback={null}is appropriate here since the banner should only appear conditionally after OAuth returns.
126-129: LGTM! Memoization with appropriate comparison.The custom comparison function correctly prevents unnecessary re-renders by checking only the props that matter (
idandreportId). Context-based state changes will still trigger re-renders as expected.lib/connectors/formatConnectorName.ts (1)
23-35: Clean pure function with good encapsulation.The function follows single responsibility and is well-documented. The known connector map handles the common cases properly.
One consideration: the fallback logic handles CamelCase inputs well but won't split lowercase compound words (e.g.,
"newconnector"becomes"Newconnector"not"New Connector"). This is acceptable since unknown connectors would likely come from an API with consistent casing, but worth noting if the behavior ever seems unexpected.components/VercelChat/ConnectionSuccessBanner.tsx (1)
17-29: Well-structured effect with proper cleanup.The effect correctly:
- Checks for the OAuth return parameter
- Shows the banner and cleans the URL immediately
- Sets up auto-dismiss with proper cleanup on unmount
One edge case to be aware of: if the user navigates away and back quickly (within 5 seconds), the timer from the first render would be cleaned up, but the banner won't reappear since
connected=truewas already removed from the URL. This is likely the desired behavior.lib/connectors/connectorMetadata.ts (1)
44-49: LGTM! Clean fallback implementation.The function properly handles case insensitivity and provides a sensible default description. The nullish coalescing with
||is appropriate here since empty strings aren't valid metadata values.lib/connectors/index.ts (1)
1-2: LGTM!Clean barrel exports that centralize connector utilities. The inline
typeexport forConnectorMetais correct and follows TypeScript best practices for type-only imports.components/ConnectorsPage/ConnectorsPage.tsx (2)
88-144: LGTM!Well-structured conditional rendering with proper semantic HTML (
<main>,<section>). The separation of "Installed Connectors" and "Available Connectors" sections provides clear information architecture. The responsive grid (grid-cols-1 lg:grid-cols-2) adapts well to different viewports.
8-8: The import path is consistent with the actual filename (providers/UserProvder.tsx), so it's not incorrect as written. However, the provider filename itself contains a typo and should be renamed fromUserProvder.tsxtoUserProvider.tsx. This would require updating 50+ import statements across the codebase and should be addressed as a separate refactor effort.Likely an incorrect or invalid review comment.
components/ConnectorsPage/ConnectorCard.tsx (1)
110-123: LGTM! Clean semantic structure.Good use of semantic HTML with proper heading hierarchy (
h3) and descriptive text structure. The truncate classes handle overflow gracefully.hooks/useConnectors.ts (3)
72-101: Theauthorizefunction accessesaccountIdbut it's not properly guarded in dependencies.The function uses
accountIdwhich is correctly in the dependency array, butsetErroris also used. WhilesetErroris stable (from useState), it's good practice to be explicit. More importantly, the error from authorize silently swallows the error by only setting state - consider whether the caller needs more feedback.The implementation correctly handles the authorization flow with proper error handling and returns null on failure, allowing callers to handle the case appropriately.
103-130: LGTM! Disconnect implementation follows good patterns.The disconnect function properly:
- Uses the
connectedAccountIdparameter directly (doesn't needaccountId)- Refreshes the list after disconnect
- Returns a boolean for success/failure feedback
5-5: The import path is correct and will not cause build failures.The provider file actually exists as
UserProvder.tsx, so the import statement at line 5 matches the actual filename. While the provider filename itself has an unusual spelling (missing the 'i'), the import path is consistent with the existing codebase structure and is valid.Likely an incorrect or invalid review comment.
components/VercelChat/tools/composio/ComposioAuthResult.tsx (2)
45-60: LGTM! Clean connected state UI.Good use of semantic color tokens with proper dark mode variants. The success state is clear and accessible with both icon and text indication.
62-65: Consider handling the "no redirect URL and not active" state more gracefully.Returning
nullwhen there's no redirect URL and status isn't "active" might be intentional, but it could leave users confused if there's an error state that should be displayed. Consider whether this edge case needs feedback.Is it expected that
redirectUrlcould be absent whilestatusis neither "active" nor "initiated"? If so, the current silent return is fine. If this represents an error state, consider displaying feedback.
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <button className="p-1.5 rounded-md hover:bg-muted transition-colors"> | ||
| <MoreVertical className="h-4 w-4 text-muted-foreground" /> | ||
| </button> | ||
| </DropdownMenuTrigger> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add accessible label to the dropdown trigger button.
The dropdown trigger button only contains an icon without accessible text. Screen reader users won't know what this button does.
As per coding guidelines, components should include proper ARIA attributes and keyboard navigation support.
♿ Accessibility fix
<DropdownMenuTrigger asChild>
- <button className="p-1.5 rounded-md hover:bg-muted transition-colors">
+ <button
+ className="p-1.5 rounded-md hover:bg-muted transition-colors"
+ aria-label={`Options for ${formatConnectorName(connector.name, connector.slug)}`}
+ >
<MoreVertical className="h-4 w-4 text-muted-foreground" />
</button>
</DropdownMenuTrigger>🤖 Prompt for AI Agents
In @components/ConnectorsPage/ConnectorCard.tsx around lines 132 - 137, The
DropdownMenu trigger button (wrapped by DropdownMenuTrigger and rendering the
MoreVertical icon) lacks accessible text for screen readers; update the button
element to include an aria-label (e.g., aria-label="Open connector menu") and
appropriate aria-haspopup="menu" and aria-expanded bound to the menu state (or
let the DropdownMenu expose expanded state) so assistive tech can identify and
toggle the menu; ensure these attributes are added to the actual button inside
DropdownMenuTrigger (asChild) and keep keyboard focus/interaction intact.
| <button | ||
| onClick={() => setShow(false)} | ||
| className="ml-2 p-1 rounded hover:bg-green-100 dark:hover:bg-green-900/50 transition-colors" | ||
| > | ||
| <X className="h-4 w-4 text-green-600 dark:text-green-400" /> | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add accessible label to dismiss button.
The close button lacks an accessible name for screen reader users. Per coding guidelines, keyboard navigation and ARIA attributes should be properly implemented.
♿ Proposed fix for accessibility
<button
onClick={() => setShow(false)}
+ aria-label="Dismiss notification"
className="ml-2 p-1 rounded hover:bg-green-100 dark:hover:bg-green-900/50 transition-colors"
>
<X className="h-4 w-4 text-green-600 dark:text-green-400" />
</button>📝 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.
| <button | |
| onClick={() => setShow(false)} | |
| className="ml-2 p-1 rounded hover:bg-green-100 dark:hover:bg-green-900/50 transition-colors" | |
| > | |
| <X className="h-4 w-4 text-green-600 dark:text-green-400" /> | |
| </button> | |
| <button | |
| onClick={() => setShow(false)} | |
| aria-label="Dismiss notification" | |
| className="ml-2 p-1 rounded hover:bg-green-100 dark:hover:bg-green-900/50 transition-colors" | |
| > | |
| <X className="h-4 w-4 text-green-600 dark:text-green-400" /> | |
| </button> |
🤖 Prompt for AI Agents
In @components/VercelChat/ConnectionSuccessBanner.tsx around lines 41 - 46, The
dismiss button rendered near the X icon has no accessible name; update the
button used with onClick={() => setShow(false)} to provide an accessible label
by adding either an aria-label (e.g., aria-label="Close notification" or
"Dismiss") or an inner screen-reader-only text node (e.g., a span with
visually-hidden/sr-only class) so assistive technologies can announce its
purpose; keep the existing X icon for visual users and ensure the same label is
applied to the element containing the X component.
| <a | ||
| href={redirectUrl} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| if (redirectUrl) { | ||
| window.location.href = redirectUrl; | ||
| } | ||
| }} | ||
| className="w-full inline-flex items-center justify-center bg-green-600 hover:bg-green-700 text-white rounded-md px-4 py-2 text-sm font-medium transition-colors cursor-pointer" | ||
| > | ||
| {getIcon("h-4 w-4")} | ||
| <span className="ml-2">Connect {displayName}</span> | ||
| </a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify the anchor element - the onClick handler is redundant.
The <a> tag already has href={redirectUrl}, so preventing default and manually setting window.location.href to the same URL is unnecessary. This pattern also bypasses browser security features like target validation.
🔧 Simplified implementation
<a
href={redirectUrl}
- onClick={(e) => {
- e.preventDefault();
- if (redirectUrl) {
- window.location.href = redirectUrl;
- }
- }}
className="w-full inline-flex items-center justify-center bg-green-600 hover:bg-green-700 text-white rounded-md px-4 py-2 text-sm font-medium transition-colors cursor-pointer"
>
{getIcon("h-4 w-4")}
<span className="ml-2">Connect {displayName}</span>
</a>If you need tracking or validation before navigation, consider using Next.js's useRouter or adding the logic without preventDefault.
📝 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.
| <a | |
| href={redirectUrl} | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| if (redirectUrl) { | |
| window.location.href = redirectUrl; | |
| } | |
| }} | |
| className="w-full inline-flex items-center justify-center bg-green-600 hover:bg-green-700 text-white rounded-md px-4 py-2 text-sm font-medium transition-colors cursor-pointer" | |
| > | |
| {getIcon("h-4 w-4")} | |
| <span className="ml-2">Connect {displayName}</span> | |
| </a> | |
| <a | |
| href={redirectUrl} | |
| className="w-full inline-flex items-center justify-center bg-green-600 hover:bg-green-700 text-white rounded-md px-4 py-2 text-sm font-medium transition-colors cursor-pointer" | |
| > | |
| {getIcon("h-4 w-4")} | |
| <span className="ml-2">Connect {displayName}</span> | |
| </a> |
🤖 Prompt for AI Agents
In @components/VercelChat/tools/composio/ComposioAuthResult.tsx around lines 80
- 92, The anchor element with href={redirectUrl} has an unnecessary onClick that
prevents default and manually sets window.location.href; remove the entire
onClick handler from the <a> so the native navigation and browser behaviors are
preserved (keep href={redirectUrl} and the existing className, getIcon usage,
and displayName span); if pre-navigation logic is required, move it into a
validated handler that does not call e.preventDefault() or use Next.js router
for client-side routing instead of overriding default anchor behavior.
hooks/useConnectors.ts
Outdated
| const fetchConnectors = useCallback(async () => { | ||
| if (!accountId) { | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| const response = await fetch( | ||
| `${NEW_API_BASE_URL}/api/connectors?account_id=${accountId}`, | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to fetch connectors"); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| const visible = data.data.connectors.filter( | ||
| (c: ConnectorInfo) => !HIDDEN_CONNECTORS.includes(c.slug.toLowerCase()), | ||
| ); | ||
| setConnectors(visible); | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : "Unknown error"); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, [accountId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add AbortController to prevent state updates on unmounted component.
The fetchConnectors function doesn't handle component unmounting. If the component unmounts while the fetch is in flight, it will attempt to set state on an unmounted component.
🔧 Suggested fix with AbortController
- const fetchConnectors = useCallback(async () => {
+ const fetchConnectors = useCallback(async (signal?: AbortSignal) => {
if (!accountId) {
setIsLoading(false);
return;
}
setIsLoading(true);
setError(null);
try {
const response = await fetch(
`${NEW_API_BASE_URL}/api/connectors?account_id=${accountId}`,
+ { signal }
);
if (!response.ok) {
throw new Error("Failed to fetch connectors");
}
const data = await response.json();
const visible = data.data.connectors.filter(
(c: ConnectorInfo) => !HIDDEN_CONNECTORS.includes(c.slug.toLowerCase()),
);
setConnectors(visible);
} catch (err) {
+ if (err instanceof Error && err.name === 'AbortError') return;
setError(err instanceof Error ? err.message : "Unknown error");
} finally {
setIsLoading(false);
}
}, [accountId]);Then update the useEffect:
useEffect(() => {
- fetchConnectors();
- }, [fetchConnectors]);
+ const controller = new AbortController();
+ fetchConnectors(controller.signal);
+ return () => controller.abort();
+ }, [fetchConnectors]);🤖 Prompt for AI Agents
In @hooks/useConnectors.ts around lines 42 - 70, The fetchConnectors function
can update state after unmount; wrap the network call with an AbortController:
in the component useEffect that calls fetchConnectors, create const controller =
new AbortController() and pass controller.signal into fetch (e.g., fetch(url, {
signal: controller.signal })), catch DOMException with name "AbortError" and
avoid calling setError for that case, and call controller.abort() in the
useEffect cleanup to prevent state updates on an unmounted component;
alternatively modify fetchConnectors to accept an AbortSignal parameter and
check signal.aborted before calling setConnectors/setIsLoading/setError.
lib/consts.ts
Outdated
| export const IS_LOCAL = process.env.NEXT_PUBLIC_USE_LOCAL_API === "true"; | ||
| export const NEW_API_BASE_URL = IS_LOCAL | ||
| ? "http://localhost:3000" | ||
| : IS_PROD | ||
| ? "https://recoup-api.vercel.app" | ||
| : "https://test-recoup-api.vercel.app"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for NEXT_PUBLIC_USE_LOCAL_API references
echo "=== Checking for NEXT_PUBLIC_USE_LOCAL_API references ==="
rg "NEXT_PUBLIC_USE_LOCAL_API" -n
echo ""
echo "=== Checking for .env files ==="
fd -e env | grep -E "\.(env|example)" | head -20
echo ""
echo "=== Checking contents of .env.example if it exists ==="
if [ -f ".env.example" ]; then
echo "Found .env.example:"
cat .env.example
else
echo ".env.example not found"
fiRepository: Recoupable-com/Recoup-Chat
Length of output: 992
Add NEXT_PUBLIC_USE_LOCAL_API to .env.example for developer reference.
The implementation in lib/consts.ts is clean and follows Single Responsibility nicely—the three-way conditional (local → prod → test) provides clear precedence for development workflows. However, the new NEXT_PUBLIC_USE_LOCAL_API environment variable should be documented in .env.example so developers understand this local development toggle. This aligns with the established pattern of documenting all platform-specific configurations.
Add this line to .env.example:
NEXT_PUBLIC_USE_LOCAL_API=false
🤖 Prompt for AI Agents
In @lib/consts.ts around lines 4 - 9, The repo is missing documentation for the
new environment toggle NEXT_PUBLIC_USE_LOCAL_API used by IS_LOCAL and
NEW_API_BASE_URL in lib/consts.ts; add a line to .env.example declaring
NEXT_PUBLIC_USE_LOCAL_API=false so developers see the default/local API toggle
and can override it during development.
| * Get branded icon for a connector. | ||
| * Uses Simple Icons for brand logos, falls back to Lucide for others. | ||
| */ | ||
| function getConnectorIcon(slug: string, size = 24) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SRP - new lib for getConnectorIcon
| <DropdownMenuItem asChild className="cursor-pointer"> | ||
| <Link href="/settings/connectors"> | ||
| <Plug className="h-4 w-4" /> | ||
| Connectors | ||
| </Link> | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open Closed Principle
- new component for
components/VercelChat/chat.tsx
Outdated
| {/* Success banner for OAuth returns */} | ||
| <Suspense fallback={null}> | ||
| <ConnectionSuccessBanner /> | ||
| </Suspense> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SRP
- actual: added a tool specific component to the main shared chat component
- required: remove this for now. if it's worth it, make a new ticket / pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this component. if it's important, make a different pull request just for banner logic
| const [isConnecting, setIsConnecting] = useState(false); | ||
| const [isDisconnecting, setIsDisconnecting] = useState(false); | ||
| const meta = getConnectorMeta(connector.slug); | ||
|
|
||
| const handleConnect = async () => { | ||
| setIsConnecting(true); | ||
| try { | ||
| const redirectUrl = await onConnect(connector.slug); | ||
| if (redirectUrl) { | ||
| window.location.href = redirectUrl; | ||
| } | ||
| } finally { | ||
| setIsConnecting(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleDisconnect = async () => { | ||
| if (!connector.connectedAccountId) return; | ||
|
|
||
| setIsDisconnecting(true); | ||
| try { | ||
| await onDisconnect(connector.connectedAccountId); | ||
| } finally { | ||
| setIsDisconnecting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SRP
- actual: state and handler functions defined in HTML component
- required: new
hookforuseConnectorHandlers.ts
| <GoogleSheetsLoginResult /> | ||
| </div> | ||
| ); | ||
| } else if (toolName === "composio") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why I did not want a wrapper component for composio tools.
KISS principle
- actual: creating wrapper tool ui logic.
- required: directly reference specific tool name from composio
lib/connectors/connectorMetadata.ts
Outdated
| googledocs: { description: "Create and edit documents" }, | ||
| googlecalendar: { description: "Manage events and schedules" }, | ||
| gmail: { description: "Search, read, and send emails" }, | ||
|
|
||
| // Communication | ||
| slack: { description: "Post messages across your workspace" }, | ||
| outlook: { description: "Search your emails and calendar events" }, | ||
|
|
||
| // Productivity | ||
| notion: { description: "Search and create content on your pages" }, | ||
| airtable: { description: "Manage bases, tables, and records" }, | ||
| linear: { description: "Track projects, issues, and workflows" }, | ||
| jira: { description: "Manage projects and track issues" }, | ||
| hubspot: { description: "Manage contacts and CRM data" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YAGNI
- actual: defining many items from composio which we are not currently using
- required: delete any items not currently being used.
lib/consts.ts
Outdated
| export const NEW_API_BASE_URL = IS_PROD | ||
| ? "https://recoup-api.vercel.app" | ||
| : "https://test-recoup-api.vercel.app"; | ||
| export const IS_LOCAL = process.env.NEXT_PUBLIC_USE_LOCAL_API === "true"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KISS principle
- actual: using custom ENV to determine local dev.
- required: delete this OR move it to a new pull request if this change is actually important
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SRP
- actual: nested, conditional, components defined inline
- required: new component file for each conditionally rendered component in this file
| const composioResult = result as { | ||
| data?: { | ||
| results?: Record<string, { redirect_url?: string; status?: string }>; | ||
| }; | ||
| }; | ||
| const results = composioResult?.data?.results; | ||
| const hasAuthResult = results | ||
| ? Object.values(results).some( | ||
| (r) => r.redirect_url || r.status?.toLowerCase() === "active", | ||
| ) | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single Responsibility Principle
- actual: special tool logic defined in shared routing component.
- required: move the logic into the ComposioAuthResult component to match the existing file pattern.
| const authorize = useCallback( | ||
| async (connector: string): Promise<string | null> => { | ||
| if (!accountId) return null; | ||
|
|
||
| try { | ||
| const response = await fetch( | ||
| `${NEW_API_BASE_URL}/api/connectors/authorize`, | ||
| { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| account_id: accountId, | ||
| connector, | ||
| }), | ||
| }, | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to authorize connector"); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| return data.data.redirectUrl; | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : "Unknown error"); | ||
| return null; | ||
| } | ||
| }, | ||
| [accountId], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The authorize and disconnect callback functions don't clear the error state before executing, causing old error messages to persist when attempting new operations after a failure.
View Details
📝 Patch Details
diff --git a/hooks/useConnectors.ts b/hooks/useConnectors.ts
index cbf6f901..f941ec37 100644
--- a/hooks/useConnectors.ts
+++ b/hooks/useConnectors.ts
@@ -73,6 +73,8 @@ export function useConnectors() {
async (connector: string): Promise<string | null> => {
if (!accountId) return null;
+ setError(null);
+
try {
const response = await fetch(
`${NEW_API_BASE_URL}/api/connectors/authorize`,
@@ -102,6 +104,8 @@ export function useConnectors() {
const disconnect = useCallback(
async (connectedAccountId: string): Promise<boolean> => {
+ setError(null);
+
try {
const response = await fetch(
`${NEW_API_BASE_URL}/api/connectors/disconnect`,
Analysis
Error state persists across authorize and disconnect operations in useConnectors hook
What fails: The authorize and disconnect callback functions in hooks/useConnectors.ts don't clear the error state before executing, causing old error messages from failed operations to persist when attempting new operations that may succeed.
How to reproduce:
- Open the connectors page
- Click "Enable" on a connector (e.g., Google Sheets)
- Wait for authorization to fail (error banner displays: "Failed to authorize connector")
- Click "Enable" on a different connector (e.g., Slack)
- Wait for authorization to succeed
- Observe: The error banner still displays "Failed to authorize connector" from step 3, even though the current operation succeeded
Expected behavior: When attempting a new authorize or disconnect operation, any previous error state should be cleared before executing, matching the pattern used in the fetchConnectors function (line 49) which correctly calls setError(null); at the start.
Root cause: The fetchConnectors function properly clears error state with setError(null); at line 49, but the authorize (lines 72-101) and disconnect (lines 103-130) callback functions lack this error clearing logic, causing stale error state to persist in the UI.
Fix applied: Added setError(null); at the beginning of both authorize (after line 74) and disconnect (after line 104) callbacks to match the defensive error clearing pattern used elsewhere in the hook.
lib/composio/getConnectorIcon.tsx
Outdated
| googledrive: <SiGoogledrive {...iconProps} color="#4285F4" />, | ||
| googledocs: <SiGoogledocs {...iconProps} color="#4285F4" />, | ||
| googlecalendar: <SiGooglecalendar {...iconProps} color="#4285F4" />, | ||
| gmail: <SiGmail {...iconProps} color="#EA4335" />, | ||
| outlook: <Mail size={size} className="shrink-0 text-[#0078D4]" />, | ||
| slack: <SiSlack {...iconProps} color="#4A154B" />, | ||
| github: ( | ||
| <SiGithub {...iconProps} className="shrink-0 dark:text-white" /> | ||
| ), | ||
| notion: ( | ||
| <SiNotion {...iconProps} className="shrink-0 dark:text-white" /> | ||
| ), | ||
| linear: <SiLinear {...iconProps} color="#5E6AD2" />, | ||
| jira: <SiJira {...iconProps} color="#0052CC" />, | ||
| airtable: <SiAirtable {...iconProps} color="#18BFFF" />, | ||
| hubspot: <SiHubspot {...iconProps} color="#FF7A59" />, | ||
| supabase: <SiSupabase {...iconProps} color="#3FCF8E" />, | ||
| twitter: <SiX {...iconProps} className="shrink-0 dark:text-white" />, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yagni principle
- actual: defining many unused icons
- required: remove unused icons
- Remove unused connectors from connectorMetadata.ts, keep only googlesheets (YAGNI) - Remove unused icons from getConnectorIcon.tsx (YAGNI) - Move composio auth checking logic from ToolComponents into ComposioAuthResult (SRP) - Fix ComposioAuthResult to pick auth result with redirect/status instead of first entry - Simplify anchor onClick handler in ComposioConnectPrompt (remove redundant preventDefault) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create lib/composio/types.ts for ComposioResultEntry interface - Create lib/composio/findAuthResult.ts - Create lib/composio/hasValidAuthData.ts - Update ComposioAuthResult to import from lib files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove comingSoon property from ConnectorMeta interface - Remove comingSoon fallback from getConnectorMeta - Remove isComingSoon logic from ConnectorCard - Delete unused ConnectorComingSoon component Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| onConnect={authorize} | ||
| onDisconnect={disconnect} | ||
| /> | ||
| {connectors.length === 0 && <ConnectorsEmptyState />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| {connectors.length === 0 && <ConnectorsEmptyState />} | |
| {connectors.length === 0 && !error && <ConnectorsEmptyState />} |
The empty state is displayed alongside the error banner when a fetch error occurs, which misleads users into thinking there are legitimately no connectors rather than showing that an error occurred.
View Details
Analysis
Empty state displayed alongside error banner when connector fetch fails
What fails: ConnectorsPage.tsx displays both the error banner and "No connectors available" empty state message simultaneously when the connector API fetch fails, creating a confusing mixed message to users.
How to reproduce:
- Open the Connectors page at
/settings/connectors - Trigger a connector fetch failure by:
- Simulating a network error (invalid API endpoint)
- Or blocking the API call to
{NEW_API_BASE_URL}/api/connectors
Result: Both error banner ("Failed to fetch connectors") and empty state message ("No connectors available") render together, creating conflicting messages - the empty state suggests zero results (legitimate scenario) while error banner indicates failure (error scenario).
Expected: When a fetch error occurs, only the error banner should display. The empty state should only display when the fetch succeeds with legitimately zero connectors.
Root cause: Line 74 of ConnectorsPage.tsx conditionally renders the empty state with only {connectors.length === 0 && <ConnectorsEmptyState />}, but when a fetch error occurs, both error is set and connectors remains an empty array, so the empty state renders alongside the error banner.
Fix: Updated ConnectorsPage.tsx line 74 to check for absence of error:
{connectors.length === 0 && !error && <ConnectorsEmptyState />}This ensures the empty state only displays when there is genuinely no data (successful fetch with zero results), not when an error prevented data retrieval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert changes to this file please.
| }; | ||
|
|
||
| return ( | ||
| icons[slug] || ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| icons[slug] || ( | |
| icons[slug.toLowerCase()] || ( |
The connector icon lookup doesn't normalize the slug to lowercase, while other functions like getConnectorMeta and formatConnectorName do. This causes icon lookups to fail if the API returns slugs with different casing.
View Details
Analysis
getConnectorIcon() doesn't normalize slug to lowercase, causing icon lookup failures
What fails: The getConnectorIcon() function in lib/composio/getConnectorIcon.tsx performs direct slug lookup without normalizing to lowercase, while the Composio API returns connector slugs in uppercase (e.g., GOOGLESHEETS). This causes branded icons to fail and fall back to a generic icon.
How to reproduce: When a connector with slug "GOOGLESHEETS" is rendered via ConnectorCard, the call to getConnectorIcon("GOOGLESHEETS", 22) attempts to lookup icons["GOOGLESHEETS"], but the icons map uses lowercase keys (googlesheets), resulting in undefined and returning the generic Link2 icon instead of the branded Google Sheets icon.
Expected behavior: Per Composio documentation, connector slugs are returned in uppercase (e.g., GOOGLESHEETS for Google Sheets, GOOGLEDRIVE for Google Drive). The function should normalize the slug to lowercase before lookup, consistent with getConnectorMeta() and formatConnectorName() which both normalize internally using .toLowerCase().
Result vs Expected:
- Current:
getConnectorIcon("GOOGLESHEETS")returns generic fallback icon (Link2) - Expected:
getConnectorIcon("GOOGLESHEETS")returns branded Google Sheets icon
The fix normalizes the slug lookup: icons[slug.toLowerCase()] instead of icons[slug], aligning with the pattern used in other connector utility functions and ensuring consistent behavior regardless of API response casing.
Changed from blocklist (HIDDEN_CONNECTORS) to allowlist (ALLOWED_CONNECTORS) approach to only display Google Sheets connector to end users. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| > | ||
| {getIcon("h-4 w-4")} | ||
| <span className="ml-2">Connect {displayName}</span> | ||
| </a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OAuth link navigates away instead of new tab
Medium Severity
The ComposioConnectPrompt component uses a plain anchor tag without target="_blank" for the OAuth redirect link. The previous GoogleSheetsLoginResult implementation used window.open(url, "_blank", "noopener,noreferrer") to open the OAuth flow in a new tab. Now clicking "Connect" navigates the user away from their chat conversation, potentially losing chat context and state.
| <DropdownMenuItem onClick={onReconnect} className="cursor-pointer"> | ||
| <RefreshCw className="h-4 w-4 mr-2" /> | ||
| Reconnect | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconnect button missing disabled state during operation
Medium Severity
The ConnectorConnectedMenu component receives isDisconnecting to disable the Disconnect button during operation, but isConnecting is not passed to disable the Reconnect button. The useConnectorHandlers hook tracks both states, and ConnectorEnableButton correctly uses isConnecting to disable itself, but ConnectorConnectedMenu lacks this prop entirely. Users can click "Reconnect" multiple times while an authorization request is in progress, potentially triggering multiple OAuth flows.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Introduces a dedicated Connectors UI and generalizes connector auth.
Settings > Connectorspage (ConnectorsPage, cards, sections, header, loading/empty/error/success states) with connect, reconnect, and disconnect actions viauseConnectors/useConnectorHandlers(calls/api/connectorsand/api/connectors/authorize); adds "Connectors" entry to the user profile menuComposioAuthResultand handlesCOMPOSIO_MANAGE_CONNECTIONS; updates tool display names/messagessetupToolsForRequest@icons-pack/react-simple-iconsdependencyWritten by Cursor Bugbot for commit ff929b6. This will update automatically on new commits. Configure here.