feat: MCP integration, model catalog refresh, and multi-chat fixes #27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Derive ACCEPTED_FILE_PICKER_TYPES from ALLOWED_FILE_TYPES so the file picker and validation logic stay in sync. Also adds MCP integration and cross-conversation memory planning documents. Co-authored-by: Cursor <cursoragent@cursor.com>
Scaffold the MCP (Model Context Protocol) integration layer: - Add mcpServers, mcpToolApprovals, mcpToolCallLog tables to Convex schema - Create Convex query/mutation functions for MCP server management - Add MCP constants to lib/config (limits, timeouts, circuit breaker) - Add ENABLE_MCP env flag as instant kill-switch - Pin @ai-sdk/mcp to exact version 1.0.18 - Include transport spike script and updated integration plan Co-authored-by: Cursor <cursoragent@cursor.com>
…espacing - Enhanced loadMCPToolsFromURL with dual transport support (HTTP/SSE), optional auth headers, and client reference return for lifecycle management - Added loadUserMcpTools orchestrator that loads enabled servers in parallel, namespaces tools, filters by approval status, and enforces per-request limits - Includes encrypted auth decryption (AES-256-GCM) and connection status tracking Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Wire MCP tools into the chat streaming pipeline with auth gating, feature flag, and proper client cleanup. Add MCP server CRUD API routes and settings UI for managing connections and tool approvals. Co-authored-by: Cursor <cursoragent@cursor.com>
…nd observability Add circuit breaker to skip MCP servers with consecutive failures, URL validation to block private/localhost endpoints (SSRF protection), and tool result truncation to prevent oversized payloads. Wire PostHog events for mcp_tool_load and mcp_tool_call for production observability. Includes Vitest setup and tests for all new modules. Co-authored-by: Cursor <cursoragent@cursor.com>
…pdate defaults - Add GPT-5.1 Instant and GPT-5.1 Thinking models with provider mappings - Remove deprecated models: GPT-3.5 Turbo, GPT-4 Turbo, GPT-4.1 Mini/Nano, GPT-4o Mini, o1, o3-mini, Claude 3 Haiku, Claude Opus 4 - Update default model from gpt-4.1-nano to gpt-5-mini - Add turbopackFileSystemCacheForDev comments in next.config.ts - Clarify SST cache troubleshooting docs (preventive vs active fix) - Add dev:clean command to AGENTS.md Co-authored-by: Cursor <cursoragent@cursor.com>
Set ENABLE_MCP=true in .env.example and drop 5 deprecated Gemini 1.5/2.0 models from the catalog, keeping only the latest 2.5 generation. Co-authored-by: Cursor <cursoragent@cursor.com>
…atalog MCP improvements: - Add fire-and-forget audit logging for MCP tool calls to Convex - Track failedServerCount (connection failures + circuit breaker skips) - Add proper HTTP 500 status to MCP test endpoint errors - Add console.error in catch blocks across MCP UI components - Add explanatory comment for MCPToolSet type cast escape hatch Model catalog updates: - Promote Gemini 3 Pro from preview to GA - Update OpenRouter Gemini 2.5 Pro/Flash to GA model IDs - Remove "preview" tag from o4-mini - Remove deprecated GPT-4.5 Preview from OpenRouter Co-authored-by: Cursor <cursoragent@cursor.com>
…s icon Remove the AppInfoTrigger component and simplify the user menu dropdown by showing only email. Switch settings icon from User02Icon to Settings01Icon. Co-authored-by: Cursor <cursoragent@cursor.com>
…/O4-mini - Rename gemini-3-pro to gemini-3-pro-preview to match actual API model ID - Update Codestral from open-mistral-7b to codestral-latest with correct pricing and metadata - Add missing OpenAI model types (o3, o4-mini variants) - Add codestral-latest to provider map and MistralModel type Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unused icon/collapsible props from SidebarList, always delegate to CollapsibleSection. Update chevron visibility so collapsed sections show the arrow at full opacity and expanded sections reveal it on hover. Co-authored-by: Cursor <cursoragent@cursor.com>
Add messageGroupId-based grouping and cacheAndAddMessage calls for both user and assistant messages in multi-chat. Use refs to avoid stale closures in onFinish callbacks, and extend ExtendedUIMessage with model and messageGroupId fields to remove unsafe type assertions. Co-authored-by: Cursor <cursoragent@cursor.com>
…atalog update - Group OpenRouter models in their own settings section with underlying provider icons and "via" labels instead of mixing with direct providers - Fix multi-chat message grouping so live useChat messages merge with persisted messages using text→groupId mapping - Fix React key uniqueness in multi-conversation response cards - Remove standalone Llama/Groq model definitions (available via OpenRouter) - Expand OpenRouter types: Claude Sonnet 4, Gemini 2.5, Perplexity Sonar family, GPT-4.1 variants, Llama 3.3 free Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent the same model's response from being added multiple times to a message group, both in the primary messageGroupId path and the fallback scan-backward path. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR delivers a comprehensive MCP integration, model catalog refresh, and critical multi-chat fixes. The implementation follows solid patterns with defense-in-depth security layers (SSRF validation, circuit breaker, result truncation), proper encryption for auth credentials, and extensive test coverage. Key changes:
Notable strengths:
Areas for attention:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ChatRoute as /api/chat
participant LoadTools as loadUserMcpTools()
participant Convex
participant MCPServer as MCP Server
participant CircuitBreaker
participant StreamText as streamText()
User->>ChatRoute: POST chat request
ChatRoute->>Convex: Fetch enabled MCP servers + approvals
Convex-->>ChatRoute: Server configs
ChatRoute->>LoadTools: Load MCP tools
LoadTools->>CircuitBreaker: Check circuit status
CircuitBreaker-->>LoadTools: Circuit closed (healthy)
par Connect to MCP Servers
LoadTools->>MCPServer: createMCPClient() with timeout
MCPServer-->>LoadTools: Client + tools
LoadTools->>CircuitBreaker: recordSuccess()
end
LoadTools->>LoadTools: Filter by approvals, namespace tools
LoadTools-->>ChatRoute: Merged tools + clients
ChatRoute->>StreamText: streamText(model, tools)
par Streaming Response
StreamText->>MCPServer: Execute tool calls
MCPServer-->>StreamText: Tool results
StreamText-->>User: Stream AI response
end
StreamText->>ChatRoute: onFinish callback
ChatRoute->>Convex: Log tool calls to mcpToolCallLog
ChatRoute->>ChatRoute: after() cleanup
ChatRoute->>LoadTools: Close MCP clients
LoadTools->>MCPServer: client.close()
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+44
to
+45
| // MCP state — declared outside try for cleanup in both after() and catch | ||
| let mcpClients: LoadToolsResult["clients"] = [] |
There was a problem hiding this comment.
MCP state declared outside try block but mcpClients may contain open connections if an error occurs after loadUserMcpTools but before streaming starts. The catch block closes them, but if error occurs before catch (e.g., during model creation), connections remain open.
Suggested change
| // MCP state — declared outside try for cleanup in both after() and catch | |
| let mcpClients: LoadToolsResult["clients"] = [] | |
| try { | |
| // MCP state — cleanup handled in both after() and catch | |
| let mcpClients: LoadToolsResult["clients"] = [] | |
| let mcpToolServerMap: LoadToolsResult["toolServerMap"] = new Map() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/api/chat/route.ts
Line: 44:45
Comment:
MCP state declared outside try block but mcpClients may contain open connections if an error occurs after `loadUserMcpTools` but before streaming starts. The catch block closes them, but if error occurs before catch (e.g., during model creation), connections remain open.
```suggestion
try {
// MCP state — cleanup handled in both after() and catch
let mcpClients: LoadToolsResult["clients"] = []
let mcpToolServerMap: LoadToolsResult["toolServerMap"] = new Map()
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+237
to
+242
| // ------------------------------------------------------------------------- | ||
| // 4. Collect tools from successful clients | ||
| // ------------------------------------------------------------------------- | ||
| const clients: MCPClient[] = [] | ||
| // MCPToolSet is an opaque mapped type from @ai-sdk/mcp that does not expose a | ||
| // mutable builder interface. We accumulate tools as Record<string, unknown> and |
There was a problem hiding this comment.
Type safety issue: mergedTools is built as Record<string, unknown> then cast to MCPToolSet. While this works, consider using type guards or asserting tool descriptor shape before insertion to catch SDK type changes at runtime.
The comment explains why this pattern is necessary (opaque mapped type), but runtime validation would add safety.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/mcp/load-tools.ts
Line: 237:242
Comment:
Type safety issue: `mergedTools` is built as `Record<string, unknown>` then cast to `MCPToolSet`. While this works, consider using type guards or asserting tool descriptor shape before insertion to catch SDK type changes at runtime.
The comment explains why this pattern is necessary (opaque mapped type), but runtime validation would add safety.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| export const DAILY_LIMIT_PRO_MODELS = 500 | ||
|
|
||
| export const NON_AUTH_ALLOWED_MODELS = ["gpt-4.1-nano"] | ||
| export const NON_AUTH_ALLOWED_MODELS = ["gpt-5-mini"] |
There was a problem hiding this comment.
Changed default from gpt-4.1-nano to gpt-5-mini. Verify the new model ID exists in your OpenAI provider configuration and is available to all users.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/config.ts
Line: 26:26
Comment:
Changed default from `gpt-4.1-nano` to `gpt-5-mini`. Verify the new model ID exists in your OpenAI provider configuration and is available to all users.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+124
to
+126
| name: v.string(), | ||
| url: v.string(), | ||
| transport: v.union(v.literal("http"), v.literal("sse")), |
There was a problem hiding this comment.
Auth fields use optional encryption. Validation logic in mcpServers.create/update enforces that when authType is "bearer" or "header", these fields are required (verified at convex/mcpServers.ts:167-178).
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/schema.ts
Line: 124:126
Comment:
Auth fields use optional encryption. Validation logic in `mcpServers.create/update` enforces that when `authType` is "bearer" or "header", these fields are required (verified at convex/mcpServers.ts:167-178).
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
264
to
287
| // PostHog: MCP tool call events — one event per tool invocation | ||
| // Iterates through all steps to find tool calls that map to MCP tools. | ||
| // durationMs per tool call requires wrapping tool.execute (future enhancement). | ||
| if (steps && mcpToolServerMap.size > 0) { | ||
| for (const step of steps) { | ||
| if (step.toolCalls) { | ||
| for (const toolCall of step.toolCalls) { | ||
| const serverInfo = mcpToolServerMap.get(toolCall.toolName) | ||
| if (serverInfo) { | ||
| phClient.capture({ | ||
| distinctId: userId, | ||
| event: "mcp_tool_call", | ||
| properties: { | ||
| toolName: serverInfo.displayName, | ||
| serverName: serverInfo.serverName, | ||
| serverId: serverInfo.serverId, | ||
| namespacedToolName: toolCall.toolName, | ||
| success: true, | ||
| chatId, | ||
| }, | ||
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
PostHog MCP tool call events only capture successful executions. Failed tool calls won't appear in PostHog analytics (only in Convex audit log). Consider capturing both success and failure events.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/api/chat/route.ts
Line: 264:287
Comment:
PostHog MCP tool call events only capture successful executions. Failed tool calls won't appear in PostHog analytics (only in Convex audit log). Consider capturing both success and failure events.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 58 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="lib/mcp/truncation.ts">
<violation number="1" location="lib/mcp/truncation.ts:43">
P2: When maxSize is smaller than the truncation suffix, this returns a string larger than maxSize, breaking the size limit guarantee. Consider truncating the suffix to fit maxSize bytes instead of returning it unmodified.</violation>
</file>
<file name="app/components/multi-chat/multi-conversation.tsx">
<violation number="1" location="app/components/multi-chat/multi-conversation.tsx:199">
P2: `response.message` is optional in rendering (loading/placeholder branch), but the key now unconditionally dereferences `response.message.id`, which will throw when message is undefined. Use a safe fallback for the key when the response is loading.</violation>
</file>
<file name="lib/mcp/url-validation.ts">
<violation number="1" location="lib/mcp/url-validation.ts:28">
P1: SSRF bypass via IPv6 addresses: The validation only blocks `[::1]` but misses other IPv6 private/local addresses. Attackers can use `http://[::]`, `http://[::ffff:127.0.0.1]` (IPv4-mapped localhost), `http://[fe80::1]` (link-local), or `http://[fd00::1]` (unique local) to access internal services.</violation>
</file>
<file name="app/api/chat/route.ts">
<violation number="1" location="app/api/chat/route.ts:281">
P2: The `success: true` is hardcoded for PostHog analytics, which will produce inaccurate metrics. Tool executions can fail, and the result status should be checked. Consider deriving success from the presence and content of the corresponding `toolResult`.</violation>
</file>
<file name="lib/file-handling.ts">
<violation number="1" location="lib/file-handling.ts:49">
P2: The accept list includes image/webp/.webp but validation only allows ALLOWED_FILE_TYPES, so users can pick .webp files and then hit a validation error. Either add image/webp to ALLOWED_FILE_TYPES (and MIME_TO_EXTENSIONS) or remove it from the accept list to keep them in sync.</violation>
</file>
<file name="lib/mcp/load-tools.ts">
<violation number="1" location="lib/mcp/load-tools.ts:219">
P2: Resource leak when MCP connection times out. When `Promise.race` rejects due to timeout, the underlying `createMCPClient` promise continues running. If it eventually resolves, the client is never closed. Track the promise and clean up orphaned clients.</violation>
</file>
<file name="app/api/mcp-servers/route.ts">
<violation number="1" location="app/api/mcp-servers/route.ts:146">
P2: PATCH handler allows empty strings after trimming. Unlike POST which validates `!name?.trim()`, the PATCH handler accepts whitespace-only values that trim to empty strings. Add validation to reject empty trimmed values for required fields.</violation>
</file>
<file name="convex/mcpServers.ts">
<violation number="1" location="convex/mcpServers.ts:33">
P1: Incomplete IPv6 SSRF protection allows bypass via IPv4-mapped IPv6 addresses (e.g., `[::ffff:127.0.0.1]`), link-local (`fe80::*`), or unique local (`fc00::/fd00::`) ranges. Consider adding checks for these IPv6 private ranges.</violation>
</file>
<file name="app/components/multi-chat/use-multi-chat.ts">
<violation number="1" location="app/components/multi-chat/use-multi-chat.ts:38">
P2: Handle `isDisconnect` in the onFinish guard; otherwise a disconnect can trigger the success callback with a partial message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…hat navigation - Add IPv6 private range detection (loopback, link-local, unique-local, IPv4-mapped) to both lib/mcp/url-validation.ts and convex/mcpServers.ts - Add DNS rebinding protection via validateResolvedUrl at connection time - Add runtime tool shape validation and orphaned client cleanup on timeout - Stop active streams on chat-to-chat navigation (single + multi-chat) - Handle isDisconnect callback, improve multi-chat duplicate detection - Move MCP client cleanup before potential throws, fix success tracking - Add image/webp to allowed file types, fix truncation edge cases - Add comprehensive test coverage for all changes Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
2 issues found across 15 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="lib/mcp/url-validation.ts">
<violation number="1" location="lib/mcp/url-validation.ts:19">
P2: Missing CGNAT range (100.64.0.0/10) in SSRF protection. This RFC 6598 range is used by cloud providers for internal networking and should be blocked to prevent SSRF attacks in cloud environments.</violation>
</file>
<file name="convex/mcpServers.ts">
<violation number="1" location="convex/mcpServers.ts:102">
P3: Error message is inconsistent with `lib/mcp/url-validation.ts`. When blocking private IPv6 ranges like `fe80::/10` (link-local) or `fc00::/7` (unique-local), this returns "Localhost and local network URLs are not allowed" which is misleading. The lib version correctly returns "Private IPv6 addresses are not allowed". Since the comment says to keep both in sync, consider using the same error message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…y for multi-chat Replace single shared messageGroupIdRef with per-model groupId tracking and a lifecycle-managed submission registry. Entries are only cleaned up when ALL models complete streaming, preventing premature bridge removal that caused duplicate message groups. Add server-side idempotency guard in convex/messages.ts to reject duplicate inserts from concurrent race conditions. Co-authored-by: Cursor <cursoragent@cursor.com>
…GNAT detection Clear useChat hook messages after persistence to prevent stale live messages from producing duplicate groups alongside persisted ones. Add three-tier group key fallback (submission registry → persisted reverse lookup → raw text). Also block CGNAT range (100.64.0.0/10, RFC 6598) in both server and client URL validation for MCP SSRF protection. Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
mcpServers,mcpToolApprovals,mcpToolCallLog), server-side tool loading with circuit breaker pattern, SSRF URL validation, result truncation, audit logging, and a settings UI for managing MCP servers and tool approvalsKey Changes
MCP Integration
convex/schema.ts— New tables for MCP servers, tool approvals, and call logsconvex/mcpServers.ts,convex/mcpToolApprovals.ts,convex/mcpToolCallLog.ts— CRUD + authlib/mcp/load-tools.ts— Server-side tool discovery with caching and circuit breakerlib/mcp/circuit-breaker.ts,lib/mcp/url-validation.ts,lib/mcp/truncation.ts— Safety layersapp/api/mcp-servers/— REST endpoints for server managementapp/api/chat/route.ts— MCP tool injection into chat streammcp-server-form.tsx,mcp-servers.tsx,mcp-tool-approvals.tsxModel Catalog
lib/models/data/openai.ts,gemini.ts,claude.ts,mistral.ts,openrouter.ts— Updatedlib/models/types.ts— AddedreasoningModelflagMulti-Chat
app/components/multi-chat/multi-chat.tsx— Persistence + deduplicationlib/chat-store/messages/provider.tsx— Message store fixesTest Plan
bun run lintandbun run typecheckpassbun run testfor MCP test suiteMade with Cursor
Summary by cubic
Integrates MCP tools into chat with secure server management and approvals, refreshes the model catalog, and stabilizes multi‑chat with lifecycle‑managed submissions, server‑side idempotency, and safer URL validation. Adds a settings UI, tests, and a feature flag; now also clears live messages after persistence to stop duplicate groups and blocks CGNAT ranges in SSRF checks.
New Features
Bug Fixes
Written for commit bd85e09. Summary will update on new commits.