Support MCP( WIP) #5974
Support MCP( WIP) #5974
Conversation
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
WalkthroughThis pull request introduces a comprehensive implementation of the Message Control Protocol (MCP) in the NextChat application. The changes include creating new modules for MCP client management, logging, and configuration. The project now supports dynamic client initialization, server interaction, and enhanced logging capabilities. The modifications span across multiple files, adding new functionality for handling MCP actions, configuring servers, and integrating MCP processing into the existing chat system. Changes
Sequence DiagramsequenceDiagram
participant App as NextChat App
participant MCP as MCP Client Manager
participant Server as MCP Server
App->>MCP: Initialize Clients
MCP->>Server: Connect to Servers
Server-->>MCP: Return Server Capabilities
MCP-->>App: Clients Ready
App->>MCP: Execute MCP Action
MCP->>Server: Send Request
Server-->>MCP: Process Request
MCP-->>App: Return Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Your build has completed! |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
app/store/chat.ts (1)
429-447: Validate JSON parsing & protect against malformed message content
- The regular expression approach to identify ```json:mcp blocks is functional, but consider graceful fallbacks if the block is missing or incomplete.
- You already catch JSON parsing errors, which is good. In case of invalid JSON from user-provided data, ensuring a user-facing error response could be beneficial.
- If multiple MCP calls can happen concurrently, verify the system’s concurrency handling to avoid race conditions.
app/mcp/actions.ts (1)
1-33: Leverage stronger typing and ensure concurrency safety
- Currently,
fsClientis typed asany—consider adding an explicit interface or type to make maintenance easier and reduce accidental misuse.- When multiple calls happen in quick succession, ensure
initFileSystemClientcan safely handle simultaneous initialization requests.app/mcp/logger.ts (1)
1-60: Ensure cross-platform log compatibility
- ANSI color codes might not display as intended across all terminals (e.g., some CI/CD environments or Windows shells).
- Consider adding timestamps or structured logging for easier troubleshooting and correlation with other logs.
app/mcp/client.ts (4)
1-5: Use absolute imports selectively.Imports from the "@modelcontextprotocol" package and Zod library are fine. However, ensure that these external imports remain minimal for faster builds and better maintainability. If future code only requires submodules, consider selective imports from large packages (e.g.,
import { ... } from "@modelcontextprotocol/sdk/client").
6-10: Clarify optional fields inServerConfiginterface.Currently,
argsandenvare optional. Provide doc comments describing the scenarios in which these fields would be omitted to improve code clarity.
44-47: ValidatePrimitivestructure and values.As
Primitive.valueis typed toany, adding a Zod schema or another validation layer for resource, tool, or prompt objects can improve type safety and help catch unexpected data structures from the server.
83-87: Use structured logging for request execution.Currently, you only do
console.log(r)after executing the request. Consider usinglogger.infoor a more structured approach to log request/response pairs for debugging and auditing.... const r = client.request(request, z.any()); - console.log(r); + logger.info("Request response:", r); return r; }tsconfig.json (1)
26-26: Cleanup references to removed files ininclude.Removing
"app/calcTextareaHeight.ts"from theincludearray is fine. Ensure that all references (e.g., imports) to that file are also removed throughout the codebase to avoid confusion.package.json (1)
25-25: Leverage the@modelcontextprotocol/sdkdependency effectively.Ensure that all relevant modules from the new SDK are utilized. If only a small subset is needed, consider partial imports or code splitting to reduce bundle size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
app/mcp/actions.ts(1 hunks)app/mcp/client.ts(1 hunks)app/mcp/example.ts(1 hunks)app/mcp/logger.ts(1 hunks)app/mcp/mcp_config.ts(1 hunks)app/store/chat.ts(2 hunks)next.config.mjs(3 hunks)package.json(2 hunks)tsconfig.json(2 hunks)
🔇 Additional comments (8)
app/store/chat.ts (1)
32-32: Add test coverage for 'executeMcpAction' usage
The new import integrates MCP action processing into the chat flow. To reduce the risk of unhandled edge cases (e.g., unavailable server, invalid JSON structures, etc.), please ensure the code path invoking executeMcpAction is tested thoroughly.
app/mcp/mcp_config.ts (1)
1-40: Secure storage of credentials and externalize paths
- The placeholders for API keys and tokens should never be replaced by real secrets in the repository; instead, reading from environment variables or a vault is recommended.
- The path
"/Users/kadxy/Desktop"is hard-coded. Consider making this path configurable for different environments.
app/mcp/example.ts (1)
1-92: Harden the file reading mechanism
- The code showcases reading arbitrary files from a specified path. Confirm that server-side controls or directory whitelisting is in place to guard against unauthorized access.
- Good job using Zod schemas for validation—this helps ensure the integrity of server responses.
app/mcp/client.ts (1)
49-81: Parallel asynchronous operations are well handled; keep an eye on performance.
The approach of collecting promises in an array and awaiting them simultaneously is a good practice. For large-scale data, ensure the server handles concurrent calls effectively to avoid resource contention or throttling.
next.config.mjs (2)
35-35: Ensure experimental features are stable enough for production.
serverActions: true is an experimental feature. Confirm that your production environment accepts potential breaking changes or instabilities. Consider adding a feature gate or fallback logic.
75-78: Maintain consistent pattern for rewrite placeholders.
Rewrite placeholders, such as :resource_name and :deploy_name, are used in the destination URL. Confirm that you handle all edge cases (like missing or invalid resource_name / deploy_name). Potentially add server-side validation to avoid runtime errors.
tsconfig.json (1)
3-3: Check compatibility of ES2022 target.
Confirm that your deployment environment supports ES2022 features, or use a transpilation step if targeting older browsers. Some hosting platforms may not yet fully support ES2022 natively.
package.json (1)
57-58: Check versions & synergy for zod and zustand.
zod is now widely used for schema validation. Keep an eye on potential conflicts when using zustand for state management if any store-based validations are needed.
app/mcp/client.ts
Outdated
| export async function createClient( | ||
| serverConfig: ServerConfig, | ||
| name: string, | ||
| ): Promise<Client> { | ||
| logger.info(`Creating client for server ${name}`); | ||
|
|
||
| const transport = new StdioClientTransport({ | ||
| command: serverConfig.command, | ||
| args: serverConfig.args, | ||
| env: serverConfig.env, | ||
| }); | ||
| const client = new Client( | ||
| { | ||
| name: `nextchat-mcp-client-${name}`, | ||
| version: "1.0.0", | ||
| }, | ||
| { | ||
| capabilities: { | ||
| roots: { | ||
| // listChanged indicates whether the client will emit notifications when the list of roots changes. | ||
| // listChanged 指示客户端在根列表更改时是否发出通知。 | ||
| listChanged: true, | ||
| }, | ||
| }, | ||
| }, | ||
| ); | ||
| await client.connect(transport); | ||
| return client; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider robust error handling for client creation.
When creating the client and connecting to the transport (line 40), include error handling for potential failures (e.g., invalid command, environment issues). This ensures unexpected errors are caught gracefully.
...
await client.connect(transport);
+ // TODO: Consider wrapping this in try/catch:
+ // try {
+ // await client.connect(transport);
+ // } catch (err) {
+ // logger.error('Failed to connect to transport', err);
+ // throw err;
+ // }
return client;
}Committable suggestion skipped: line range outside the PR's diff.
| @@ -49,11 +50,12 @@ | |||
| "remark-breaks": "^3.0.2", | |||
| "remark-gfm": "^3.0.1", | |||
| "remark-math": "^5.1.1", | |||
| "rt-client": "https://github.com/Azure-Samples/aoai-realtime-audio-sdk/releases/download/js/v0.5.0/rt-client-0.5.0.tgz", | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: The rt-client dependency is actively used and cannot be removed
The rt-client package is an essential dependency that is actively used in the realtime chat functionality:
- It's imported and used extensively in
app/components/realtime-chat/realtime-chat.tsx - Core functionalities depend on it:
- Real-time audio streaming and processing
- Voice chat features
- Server communication for real-time responses
- Audio transcription and playback
Removing this dependency would break the real-time chat feature. The package should be kept unless there's a planned replacement or the real-time chat functionality is being removed.
🔗 Analysis chain
Confirm removal of the deprecated rt-client dependency.
Verify that no modules still rely on rt-client. Skipping this verification might lead to runtime errors if references remain.
Let me gather more information about the actual usage of rt-client in the code.
Let me check the actual usage in the realtime-chat.tsx file to understand if this is an active dependency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for references to rt-client in the codebase:
rg 'rt-client' -A 3 -B 3
Length of output: 2671
Script:
#!/bin/bash
# Check for imports and actual usage of rt-client in the realtime components
ast-grep --pattern 'import { $$$ } from "rt-client"'
# Check for any direct usage of rt-client package
rg -l 'new RTClient|RTClient\.'
Length of output: 675
Script:
#!/bin/bash
# Check the implementation in realtime-chat.tsx
cat app/components/realtime-chat/realtime-chat.tsx
Length of output: 10998
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
app/mcp/actions.ts (3)
7-17: Global variables and maps
Storing clients in a globalMapand a globalinitializedflag may be problematic in multi-instance or serverless environments. Consider a more robust design (e.g., an initialization manager) if concurrency or horizontal scaling are required.
18-51: initializeMcpClients function
- Code loops over
conf.mcpServersand initializes each client with proper logging.- Catches errors and tracks failed clients in
errorClients.- Marks
initialized = trueto prevent reinitialization.- Logs final status and available clients.
This flow is user-friendly and understandable. Consider adding retries or fallback logic for partial initialization failures.
53-70: executeMcpAction function
- Fetches the client from
clientsMapby ID and executes the request.- Logs any execution errors and rethrows them.
Consider returning a standardized error object for better client-side handling, especially if this function may be extended for user-facing error messages in the future.app/store/chat.ts (2)
363-367: MCP check on new messages
Line 366 callscheckMcpJsonwhenever a new message arrives. This integration ensures automatic detection of MCP requests without additional code in the message pipeline. Be sure to handle potential performance impacts if message volume is very high.
773-798: checkMcpJson function
- Validates if message content matches MCP JSON format.
- On success, extracts the MCP payload and invokes
executeMcpAction.- Uses
onUserInputto feed back results as a new message.
This is a clean, modular approach. However, watch for large JSON payloads or malicious content. Consider adding rate-limits or size checks for production.app/mcp/utils.ts (2)
1-3: isMcpJson function
Returns the match result of a regex. This function is concise but slightly at risk of returning null or array. Consider returning a boolean explicitly (!!content.match(...)) if you only need a truthy check.
5-11: extractMcpJson function
CapturesclientIdand parses JSON from the second capture group. The approach is straightforward. A try-catch block for JSON parsing (or an upfront validity check) might guard against syntax errors or maliciously malformed JSON.app/page.tsx (1)
9-10: Await MCP initialization
Blocking initialization ensures MCP dependencies are ready before UI rendering. Ensure that this call won’t degrade page responsiveness for users if the initialization is slow. Consider deferring or parallelizing if needed.app/mcp/logger.ts (1)
24-65: Logging methods
- Provides functions for info, success, error, warn, and debug with consistent output formatting.
debugis controlled by a flag to prevent noisy logs.- A structured
formatMessagemethod safely handles objects.
Well-structured, though consider a fallback or custom transport in production to integrate with external logging systems (e.g., Winston, Bunyan).app/mcp/client.ts (3)
32-35: Document or remove commented capabilities configuration.The commented capabilities configuration lacks explanation. Either document why it's preserved for future use or remove it if unnecessary.
28-28: Consider extracting version to a configuration constant.The hardcoded version "1.0.0" should be moved to a configuration constant for better maintainability.
+const CLIENT_VERSION = "1.0.0"; export async function createClient( serverConfig: ServerConfig, name: string, ): Promise<Client> { // ... const client = new Client( { name: `nextchat-mcp-client-${name}`, - version: "1.0.0", + version: CLIENT_VERSION, }, // ... );
52-76: Refactor duplicate promise handling pattern.The promise handling pattern is repeated for resources, tools, and prompts. Consider extracting this to a helper function.
+const createPrimitivePromise = ( + client: Client, + type: Primitive["type"], + listFn: () => Promise<{ [key: string]: any[] }> +) => { + return listFn().then((result) => { + const items = result[`${type}s`]; + items.forEach((item) => primitives.push({ type, value: item })); + }); +}; if (capabilities?.resources) { - promises.push( - client.listResources().then(({ resources }) => { - resources.forEach((item) => - primitives.push({ type: "resource", value: item }), - ); - }), - ); + promises.push(createPrimitivePromise(client, "resource", () => client.listResources())); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.eslintignore(1 hunks)app/mcp/actions.ts(1 hunks)app/mcp/client.ts(1 hunks)app/mcp/example.ts(1 hunks)app/mcp/logger.ts(1 hunks)app/mcp/mcp_config.json(1 hunks)app/mcp/utils.ts(1 hunks)app/page.tsx(1 hunks)app/store/chat.ts(4 hunks)package.json(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- .eslintignore
- app/mcp/mcp_config.json
🚧 Files skipped from review as they are similar to previous changes (2)
- app/mcp/example.ts
- package.json
🔇 Additional comments (10)
app/mcp/actions.ts (3)
1-2: Use of "use server" directive
These lines establish a server-only execution context. Be mindful of referencing browser-specific APIs in server files to avoid runtime errors.
3-6: Import dependencies
Imports for createClient, executeRequest, MCPClientLogger, and the JSON config look consistent with the intended usage. Ensure that underlying modules handle errors gracefully.
72-77: getAvailableClients function
Filters out clients that failed to initialize. This is straightforward and aligns with the error management strategy. Ensure future expansions remain in sync (e.g., re-checking previously error-prone clients).
app/store/chat.ts (2)
32-33: New MCP imports
Imports from mcp/actions and mcp/utils introduce the MCP functionality into the chat store. This is a logical location given the store’s role in handling messages.
434-434: Asynchronous onFinish callback
This line suggests the onFinish callback is asynchronous. Confirm that all references to onFinish handle promises and potential errors correctly to avoid unhandled promise rejections.
app/page.tsx (1)
4-4: Import initializeMcpClients
This sets up a new dependency on actions.ts. Verify that all required environment variables and config references are available at runtime.
app/mcp/logger.ts (2)
1-10: Color codes definition
Defines ANSI codes for styling the log output. Straightforward and correct for Node.js environments.
12-22: Logger constructor and fields
The prefix and debugMode options provide flexible logging. This is a neat approach for customizing logs.
app/mcp/client.ts (2)
1-10: LGTM! Well-structured imports and interface definition.
The ServerConfig interface is well-designed with appropriate optional parameters for args and env configuration.
14-40: Add error handling for client creation and connection.
The client creation process should include proper error handling for transport creation and connection failures.
|
|
||
| interface Primitive { | ||
| type: "resource" | "tool" | "prompt"; | ||
| value: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace 'any' type with a more specific type.
Using 'any' type reduces type safety. Consider defining specific types for resource, tool, and prompt values.
| /** Execute a request */ | ||
| export async function executeRequest(client: Client, request: any) { | ||
| return client.request(request, z.any()); | ||
| } |
There was a problem hiding this comment.
Improve request validation and error handling.
The current implementation has several concerns:
- Using
z.any()bypasses the benefits of Zod's type validation - No request validation before execution
- No error handling for failed requests
Consider implementing proper validation and error handling:
-export async function executeRequest(client: Client, request: any) {
- return client.request(request, z.any());
+export async function executeRequest(client: Client, request: unknown) {
+ const requestSchema = z.object({
+ // Define your request schema here
+ // Example:
+ // method: z.string(),
+ // params: z.record(z.unknown())
+ });
+
+ try {
+ const validatedRequest = requestSchema.parse(request);
+ return await client.request(validatedRequest, z.unknown());
+ } catch (error) {
+ if (error instanceof z.ZodError) {
+ throw new Error(`Invalid request format: ${error.message}`);
+ }
+ throw error;
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
📝 补充信息 | Additional Information
Summary by CodeRabbit
Documentation
New Features
Chores
Configuration