[FEATURE] sse attempt #2 #115
Conversation
|
CodeAnt AI is reviewing your PR. |
WalkthroughThe changes add Server-Sent Events (SSE) support to Desktop Commander MCP, enabling remote connectivity via HTTP. Documentation in the README is updated to describe SSE usage. New scripts, dependencies, and a dedicated HTTP server using Express are introduced. Command-line options allow running in STDIO, HTTP/SSE, or combined modes, with session management and protocol support. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant MainServer
participant HTTPServer
User->>CLI: Run desktop-commander-sse [--sse-only] [--port=PORT]
CLI->>MainServer: Spawn with --sse [--sse-only] --port=PORT
MainServer->>HTTPServer: Start HTTP/SSE server on PORT
User->>HTTPServer: Connect via /mcp (GET/POST)
HTTPServer->>MainServer: Establish MCP session/transport
HTTPServer-->>User: Stream events/responses via SSE
User->>HTTPServer: Send JSON-RPC requests (POST)
HTTPServer->>MainServer: Route requests to session transport
MainServer-->>HTTPServer: Respond with results/errors
HTTPServer-->>User: Send JSON-RPC responses/events
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
codeant-ai
bot
added
the
size:XL
Pull Request Feedback 🔍
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
src/server.ts (2)
58-59: Consider using a configurable logging mechanism.The added console.log statement will print debugging information for every resources/list request. While useful during development, this could create excessive noise in production logs.
Consider using a debug flag or structured logging approach:
-server.setRequestHandler(ListResourcesRequestSchema, async (request) => { - console.log('Handling resources/list request in server:', request); +server.setRequestHandler(ListResourcesRequestSchema, async (request) => { + if (process.env.DEBUG) { + console.log('Handling resources/list request in server:', request); + }
67-68: Consider using a configurable logging mechanism.Similar to the previous handler, this console.log statement will print debug information for every prompts/list request.
Consider using a debug flag or structured logging approach:
-server.setRequestHandler(ListPromptsRequestSchema, async (request) => { - console.log('Handling prompts/list request in server:', request); +server.setRequestHandler(ListPromptsRequestSchema, async (request) => { + if (process.env.DEBUG) { + console.log('Handling prompts/list request in server:', request); + }src/start-sse-server.ts (1)
31-45: Consider extracting the argument filtering logic for better readability.The argument filtering logic in the spawn call is complex and could be made more readable.
Extract the filtering logic to a separate function:
+ // Filter out already processed arguments + function filterProcessedArgs(args: string[], portArgIndex: number) { + return args.filter(arg => + arg !== '--port' && + arg !== (portArgIndex !== -1 ? args[portArgIndex + 1] : '') && + arg !== '--http-only' && + arg !== '--sse-only' + ); + } // Start the main server script with appropriate arguments const serverProcess = spawn('node', [ join(__dirname, 'index.js'), '--sse', '--port', port.toString(), ...(httpOnly ? ['--sse-only'] : []), - ...args.filter(arg => - arg !== '--port' && - arg !== (portArgIndex !== -1 ? args[portArgIndex + 1] : '') && - arg !== '--http-only' && - arg !== '--sse-only' - ) + ...filterProcessedArgs(args, portArgIndex) ], { stdio: 'inherit' });README.md (3)
2-3: Heading level skips directly from H1 to H3 – use an H2 insteadStatic-analysis (MD001) is correct: jumping from
#to###breaks the incremental heading hierarchy and hurts accessibility / ToC generation.-### Search, update, manage files and run terminal commands with AI (now with SSE support) +## Search, update, manage files and run terminal commands with AI (now with SSE support)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2-2: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
61-67: Consider mentioning authentication expectations up-frontThe new SSE bullet list is great, but it would help users if you briefly call out that no authentication is yet implemented (the roadmap hints at “Advanced SSE authentication”). A one-liner could avoid accidental exposure when developers bind to 0.0.0.0 in production.
330-331: Changelog entry date format differs from previous rowsThe earlier items use
DD-MM-YYYY, but the new row is13-05-2025(same format? double-check). Stick to one convention to keep the list sortable.src/http-server.ts (3)
63-76: Overly permissive CORS and direct mutation of request headers
Access-Control-Allow-Headers: *is rejected by some browsers (the spec only allows explicit lists). Specify the expected headers instead (e.g.Content-Type, MCP-Session-Id).- Directly mutating
req.headers.acceptafter Express has parsed headers can lead to subtle bugs. Prefer cloning into a local variable passed to the transport, or use a proxy header (x-accept-override) rather than altering incoming data.These tighten security and avoid undefined behaviour in Node’s HTTP parser.
14-55:getAllToolsFromServeris temporary but shipped to productionA hard-coded tool list will drift from the real server capabilities. At minimum, add a TODO comment that clearly points to the follow-up task, or better, call
await server.listTools()if such API exists.- // For now, we'll just return a dummy list of tools - // In a real implementation, you would get this from the server + // TODO: Replace with server.listTools() once SDK exposes it
442-457: Session-ID extraction duplicates logic – extract helperHeader vs query param parsing is repeated in three blocks. Extract to a small helper to reduce copy-paste and the risk of future divergence.
function getSessionId(req: Request): string | undefined { return (req.headers['mcp-session-id'] as string) ?? (req.query.session_id as string); }[maintainability]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
README.md(6 hunks)package.json(3 hunks)src/http-server.ts(1 hunks)src/index.ts(3 hunks)src/server.ts(1 hunks)src/start-sse-server.ts(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
2-2: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (7)
src/index.ts (1)
126-151: LGTM! Clean implementation of the HTTP/SSE server integration.The HTTP/SSE server integration is well-structured with proper error handling and fallback to STDIO transport on failure. The cleanup handler ensures the HTTP server is properly terminated on process exit.
src/start-sse-server.ts (1)
47-60: LGTM! Good process management.The signal handling and exit code propagation are well implemented. The code correctly forwards termination signals to the child process and ensures the parent process exits with the same code as the child.
package.json (5)
15-16: LGTM! Binary entries properly configured.The new binary entry for the SSE server is correctly configured.
32-33: LGTM! Helpful npm scripts for SSE mode.The new scripts make it easy for users to start the server in either dual mode or SSE-only mode.
63-65: LGTM! Appropriate keywords added.The added keywords accurately reflect the new SSE functionality and will help with package discoverability.
68-69:❓ Verification inconclusive
Verify compatibility with the updated SDK version.
The SDK has been updated from an earlier version to ^1.11.2, which might introduce breaking changes.
Please verify that the application works correctly with the updated SDK version:
🏁 Script executed:
#!/bin/bash # Look for any imports that might be affected by SDK version change rg -A 2 -B 2 '@modelcontextprotocol/sdk' --type=tsLength of output: 1133
Verify @modelcontextprotocol/sdk ^1.11.2 compatibility
The SDK bump to ^1.11.2 may include breaking changes. Please check that all imports and exports you’re using still exist and behave as expected:
• src/server.ts
– Server from@modelcontextprotocol/sdk/server/index.js
– CallToolRequestSchema, ListPromptsRequestSchema, CallToolRequest from@modelcontextprotocol/sdk/types.js
• src/http-server.ts
– Server and StreamableHTTPServerTransport from@modelcontextprotocol/sdk/server
– isInitializeRequest from@modelcontextprotocol/sdk/types.js
• src/custom-stdio.ts
– StdioServerTransport from@modelcontextprotocol/sdk/server/stdio.jsRun your unit and integration tests against 1.11.2, or temporarily pin the old version to confirm no regressions. If any exports have been renamed, removed or moved, update the import paths accordingly and adjust your code to any new API signatures.
72-72:✅ Verification successful
Note Express 5.x is in beta status.
Express 5.1.0 is specified in dependencies, but Express 5.x is still in beta and might have API differences compared to the stable 4.x series.
🌐 Web query:
Is Express 5.1.0 stable or still in beta? What are the key differences from Express 4?💡 Result:
Express 5.1.0 is the latest stable release of the Express framework, having been tagged as
lateston npm as of March 31, 2025. (expressjs.com) This release signifies the transition from theCURRENTphase to theACTIVEphase in Express's support timeline, indicating its stability and readiness for production use.Key Differences Between Express 5.1.0 and Express 4:
Node.js Version Support:
- Express 4: Supports Node.js versions starting from 0.10.0.
- Express 5.1.0: Drops support for Node.js versions prior to v18, requiring Node.js v18 or higher. (expressjs.com)
Routing and Path Matching:
- Express 4: Allows the use of sub-expression regular expressions in route paths, which can be vulnerable to Regular Expression Denial of Service (ReDoS) attacks.
- Express 5.1.0: Removes support for sub-expression regular expressions to enhance security. It introduces simplified route patterns, such as using
{}for optional parameters and*for splats, improving readability and security. (expressjs.com)Promise Handling:
- Express 4: Does not natively handle rejected promises in middleware and route handlers.
- Express 5.1.0: Introduces native support for handling rejected promises in middleware and route handlers, forwarding the rejected value as an
Errorto the error-handling middleware. This simplifies error handling in asynchronous code. (expressjs.com)Body Parser Changes:
- Express 4: Requires the use of external middleware like
body-parserfor parsing request bodies.- Express 5.1.0: Integrates body parsing capabilities directly into the framework, including support for Brotli compression and the ability to customize the URL-encoded body depth. (expressjs.com)
Deprecation and Removal of Methods:
- Express 4: Includes several deprecated methods and properties, such as
app.del(),app.param(fn), and pluralized method names likereq.acceptsCharset().- Express 5.1.0: Removes these deprecated methods and properties to streamline the API and improve consistency. For example,
app.del()is replaced withapp.delete(), and pluralized methods likereq.acceptsCharsets()are introduced. (expressjs.com)ETag Support in
res.sendFile():
- Express 4: Does not provide an option to customize ETag generation in
res.sendFile().- Express 5.1.0: Adds support for the
etagoption inres.sendFile(), allowing developers to control ETag generation for file responses. (expressjs.com)Support for
Uint8Arrayinres.send():
- Express 4: Does not support sending
Uint8Arrayinstances directly in responses.- Express 5.1.0: Adds support for
Uint8Arrayinres.send(), enabling more efficient handling of binary data in responses. (expressjs.com)These changes in Express 5.1.0 aim to enhance security, performance, and developer experience by modernizing the framework and removing outdated practices.
Citations:
- 1: https://expressjs.com/2025/03/31/v5-1-latest-release.html?utm_source=openai
- 2: https://expressjs.com/2024/10/15/v5-release?utm_source=openai
- 3: https://expressjs.com/2024/10/15/v5-release?utm_source=openai
- 4: https://expressjs.com/en/guide/migrating-5?utm_source=openai
- 5: https://expressjs.com/en/guide/migrating-5?utm_source=openai
- 6: https://expressjs.com/en/guide/migrating-5?utm_source=openai
- 7: https://expressjs.com/2025/03/31/v5-1-latest-release.html?utm_source=openai
- 8: https://expressjs.com/2025/03/31/v5-1-latest-release.html?utm_source=openai
Express 5.1.0 is now a stable release
The dependency update to"express": "^5.1.0"is safe for production—Express 5.1.0 was promoted to thelatesttag on npm as of March 31, 2025.Please note that v5 includes several breaking changes compared to 4.x:
- Requires Node.js v18 or higher
- Drops sub-expression RegExp support in route paths
- Native promise rejection handling in middleware/route handlers
- Built-in body parsing (including Brotli), replacing the old
body-parserpackage- Removal of deprecated methods (e.g.
app.del()→app.delete(), pluralizedreq.acceptsCharsets(), etc.)- New options for ETag generation in
res.sendFile()andUint8Arraysupport inres.send()Ensure your code has been updated accordingly and refer to the official migration guide:
https://expressjs.com/en/guide/migrating-5.html
| // Parse command-line arguments | ||
| const args = process.argv.slice(2); | ||
| const httpServerEnabled = args.includes('--http') || args.includes('--sse'); | ||
| const httpServerPort = getArgValue(args, '--port') || 3000; | ||
|
|
||
| // Function to extract value after an argument | ||
| function getArgValue(args: string[], flag: string): number | null { | ||
| const index = args.indexOf(flag); | ||
| if (index !== -1 && index + 1 < args.length) { | ||
| const value = parseInt(args[index + 1], 10); | ||
| return isNaN(value) ? null : value; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add port validation to prevent invalid configuration.
The getArgValue function parses port values but doesn't validate that they're within the valid port range (1-65535).
Add validation to ensure the port is within a valid range:
function getArgValue(args: string[], flag: string): number | null {
const index = args.indexOf(flag);
if (index !== -1 && index + 1 < args.length) {
const value = parseInt(args[index + 1], 10);
- return isNaN(value) ? null : value;
+ if (isNaN(value)) return null;
+ // Validate port is in valid range
+ if (flag === '--port' && (value < 1 || value > 65535)) {
+ console.error(`Invalid port number: ${value}. Must be between 1-65535`);
+ return null;
+ }
+ return value;
}
return null;
}📝 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.
| // Parse command-line arguments | |
| const args = process.argv.slice(2); | |
| const httpServerEnabled = args.includes('--http') || args.includes('--sse'); | |
| const httpServerPort = getArgValue(args, '--port') || 3000; | |
| // Function to extract value after an argument | |
| function getArgValue(args: string[], flag: string): number | null { | |
| const index = args.indexOf(flag); | |
| if (index !== -1 && index + 1 < args.length) { | |
| const value = parseInt(args[index + 1], 10); | |
| return isNaN(value) ? null : value; | |
| } | |
| return null; | |
| } | |
| // Function to extract value after an argument | |
| function getArgValue(args: string[], flag: string): number | null { | |
| const index = args.indexOf(flag); | |
| if (index !== -1 && index + 1 < args.length) { | |
| const value = parseInt(args[index + 1], 10); | |
| if (isNaN(value)) return null; | |
| // Validate port is in valid range | |
| if (flag === '--port' && (value < 1 || value > 65535)) { | |
| console.error(`Invalid port number: ${value}. Must be between 1-65535`); | |
| return null; | |
| } | |
| return value; | |
| } | |
| return null; | |
| } |
| // Get port from command line args or use default | ||
| const args = process.argv.slice(2); | ||
| let port = DEFAULT_PORT; | ||
| const portArgIndex = args.indexOf('--port'); | ||
| if (portArgIndex !== -1 && portArgIndex + 1 < args.length) { | ||
| const portArg = parseInt(args[portArgIndex + 1], 10); | ||
| if (!isNaN(portArg)) { | ||
| port = portArg; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add port validation for security and reliability.
Similar to the issue in index.ts, this file parses port values without validating the range.
Add validation to ensure the port is within the valid range (1-65535):
if (portArgIndex !== -1 && portArgIndex + 1 < args.length) {
const portArg = parseInt(args[portArgIndex + 1], 10);
if (!isNaN(portArg)) {
+ if (portArg < 1 || portArg > 65535) {
+ console.error(`Invalid port number: ${portArg}. Must be between 1-65535`);
+ process.exit(1);
+ }
port = portArg;
}
}📝 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.
| // Get port from command line args or use default | |
| const args = process.argv.slice(2); | |
| let port = DEFAULT_PORT; | |
| const portArgIndex = args.indexOf('--port'); | |
| if (portArgIndex !== -1 && portArgIndex + 1 < args.length) { | |
| const portArg = parseInt(args[portArgIndex + 1], 10); | |
| if (!isNaN(portArg)) { | |
| port = portArg; | |
| } | |
| } | |
| // Get port from command line args or use default | |
| const args = process.argv.slice(2); | |
| let port = DEFAULT_PORT; | |
| const portArgIndex = args.indexOf('--port'); | |
| if (portArgIndex !== -1 && portArgIndex + 1 < args.length) { | |
| const portArg = parseInt(args[portArgIndex + 1], 10); | |
| if (!isNaN(portArg)) { | |
| if (portArg < 1 || portArg > 65535) { | |
| console.error(`Invalid port number: ${portArg}. Must be between 1-65535`); | |
| process.exit(1); | |
| } | |
| port = portArg; | |
| } | |
| } |
| ### Using Server-Sent Events (SSE) | ||
|
|
||
| Desktop Commander can also be run as an HTTP server with Server-Sent Events (SSE) support, allowing remote connections from MCP clients: | ||
|
|
||
| ```bash | ||
| # Install and run with SSE enabled (dual mode - both STDIO and HTTP) | ||
| npx @wonderwhy-er/desktop-commander-sse start:sse | ||
|
|
||
| # Run in SSE-only mode (HTTP server only, no STDIO) | ||
| npx @wonderwhy-er/desktop-commander-sse start:sse-only | ||
|
|
||
| # Specify a custom port (default is 3000) | ||
| npx @wonderwhy-er/desktop-commander-sse start:sse --port 8080 | ||
| ``` | ||
|
|
||
| When running in SSE mode, the server provides an HTTP endpoint at `/mcp` that implements the MCP Streamable HTTP transport protocol. This allows you to connect to the server remotely using any MCP client that supports this protocol. | ||
|
|
||
| The server supports: | ||
| - GET requests for establishing Server-Sent Events connections | ||
| - POST requests for sending commands to the server | ||
| - Session management for multiple concurrent connections | ||
| - CORS headers for cross-origin requests from web clients (including claude.ai, cursor.sh, etc.) | ||
|
|
||
| The HTTP server is accessible at `http://localhost:3000/mcp` by default. | ||
|
|
There was a problem hiding this comment.
Documented SSE endpoint (/mcp) does not match the implementation (/messages)
src/http-server.ts uses /messages for the streaming endpoint, yet the README instructs users to connect to /mcp. This mismatch will lead to 404s and support questions.
Please align the docs with the code (or vice-versa) before shipping.
| // Handle requests to the MCP endpoint and the messages endpoint | ||
| app.all(['/mcp', '/messages', '/messages/:sessionId'], async (req, res) => { | ||
| try { | ||
| // Log the incoming request details for debugging |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate route definitions cause maintenance overhead & unreachable code
/messages is registered twice: once with a bespoke handler (line 95) and again in the catch-all list (line 269). Because Express matches in registration order, the second handler is never executed for /messages. Consider:
- Removing the first specialised handler and extending the generic one, or
- Mounting the specialised handler under a different path.
This will eliminate ~250 lines of duplicate logic for tools/list, resources/list, etc.
| // Add a specific handler for /messages route | ||
| app.all('/messages', async (req: Request, res: Response) => { | ||
| try { | ||
| console.log('Handling dedicated /messages route'); | ||
|
|
||
| // For GET requests to /messages, send a simple SSE stream | ||
| if (req.method === 'GET') { | ||
| console.log('GET on /messages - setting up SSE stream'); | ||
|
|
||
| // Set SSE headers | ||
| res.setHeader('Content-Type', 'text/event-stream'); | ||
| res.setHeader('Cache-Control', 'no-cache'); | ||
| res.setHeader('Connection', 'keep-alive'); | ||
|
|
||
| // Create a new session ID | ||
| const sessionId = randomUUID(); | ||
| console.log('Creating session for /messages:', sessionId); | ||
|
|
||
| // Create a new transport for this session | ||
| const transport = new StreamableHTTPServerTransport({ | ||
| sessionIdGenerator: () => sessionId, | ||
| enableJsonResponse: false // Use SSE mode | ||
| }); | ||
|
|
||
| // Store the transport for future requests | ||
| transports[sessionId] = transport; | ||
|
|
||
| // Connect to the server | ||
| await server.connect(transport); | ||
|
|
||
| // Get tools for the response | ||
| const allTools = await getAllToolsFromServer(server); | ||
| const toolsJson = JSON.stringify(allTools); | ||
|
|
||
| // Send endpoint event immediately | ||
| res.write(`event: endpoint\ndata: /messages/?session_id=${sessionId}\n\n`); | ||
| console.log('Sent endpoint event for direct /messages route'); | ||
|
|
||
| // Send initialization message - CRITICAL! | ||
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":0,"result":{"protocolVersion":"2024-11-05","capabilities":{"experimental":{},"prompts":{"listChanged":false},"resources":{"subscribe":false,"listChanged":false},"tools":{"listChanged":false}},"serverInfo":{"name":"desktop-commander-sse","version":"0.1.39"}}}\n\n`); | ||
| console.log('Sent initialization message'); | ||
|
|
||
| // Send tools list message with actual tools | ||
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":1,"result":{"tools":${toolsJson}}}\n\n`); | ||
| console.log('Sent tools list message'); | ||
|
|
||
| // Send empty resources message | ||
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":3,"result":{"resources":[]}}\n\n`); | ||
| console.log('Sent empty resources message'); | ||
|
|
||
| // Send empty prompts message | ||
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":4,"result":{"prompts":[]}}\n\n`); | ||
| console.log('Sent empty prompts message'); | ||
|
|
||
| // Start ping interval | ||
| const pingInterval = setInterval(() => { | ||
| if (!res.writableEnded) { | ||
| const timestamp = new Date().toISOString(); | ||
| res.write(`: ping - ${timestamp}\n\n`); | ||
| } else { | ||
| clearInterval(pingInterval); | ||
| } | ||
| }, 15000); | ||
|
|
||
| // Clean up resources on close | ||
| res.on('close', () => { | ||
| clearInterval(pingInterval); | ||
| delete transports[sessionId]; | ||
| console.log('Closed /messages SSE connection'); | ||
| }); | ||
|
|
||
| // Keep the connection open | ||
| return; | ||
| } |
There was a problem hiding this comment.
GET / messages never forwards subsequent MCP traffic – call transport.handleRequest
The code hand-crafts the handshake but never hands control over to StreamableHTTPServerTransport, so any later server-initiated messages (tool output, notifications, etc.) are silently dropped.
- // Connect to the server
- await server.connect(transport);
- ...
- // Send empty prompts message
- res.write(`event: message\ndata: {"jsonrpc":"2.0","id":4,"result":{"prompts":[]}}\n\n`);
- console.log('Sent empty prompts message');
-
- // Start ping interval
+ // Connect to the server and delegate the stream handling
+ await server.connect(transport);
+
+ // Let the transport take over so all further MCP messages flow
+ await transport.handleRequest(req, res);
+
+ // Start ping interval *after* the transport is active📝 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.
| // Add a specific handler for /messages route | |
| app.all('/messages', async (req: Request, res: Response) => { | |
| try { | |
| console.log('Handling dedicated /messages route'); | |
| // For GET requests to /messages, send a simple SSE stream | |
| if (req.method === 'GET') { | |
| console.log('GET on /messages - setting up SSE stream'); | |
| // Set SSE headers | |
| res.setHeader('Content-Type', 'text/event-stream'); | |
| res.setHeader('Cache-Control', 'no-cache'); | |
| res.setHeader('Connection', 'keep-alive'); | |
| // Create a new session ID | |
| const sessionId = randomUUID(); | |
| console.log('Creating session for /messages:', sessionId); | |
| // Create a new transport for this session | |
| const transport = new StreamableHTTPServerTransport({ | |
| sessionIdGenerator: () => sessionId, | |
| enableJsonResponse: false // Use SSE mode | |
| }); | |
| // Store the transport for future requests | |
| transports[sessionId] = transport; | |
| // Connect to the server | |
| await server.connect(transport); | |
| // Get tools for the response | |
| const allTools = await getAllToolsFromServer(server); | |
| const toolsJson = JSON.stringify(allTools); | |
| // Send endpoint event immediately | |
| res.write(`event: endpoint\ndata: /messages/?session_id=${sessionId}\n\n`); | |
| console.log('Sent endpoint event for direct /messages route'); | |
| // Send initialization message - CRITICAL! | |
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":0,"result":{"protocolVersion":"2024-11-05","capabilities":{"experimental":{},"prompts":{"listChanged":false},"resources":{"subscribe":false,"listChanged":false},"tools":{"listChanged":false}},"serverInfo":{"name":"desktop-commander-sse","version":"0.1.39"}}}\n\n`); | |
| console.log('Sent initialization message'); | |
| // Send tools list message with actual tools | |
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":1,"result":{"tools":${toolsJson}}}\n\n`); | |
| console.log('Sent tools list message'); | |
| // Send empty resources message | |
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":3,"result":{"resources":[]}}\n\n`); | |
| console.log('Sent empty resources message'); | |
| // Send empty prompts message | |
| res.write(`event: message\ndata: {"jsonrpc":"2.0","id":4,"result":{"prompts":[]}}\n\n`); | |
| console.log('Sent empty prompts message'); | |
| // Start ping interval | |
| const pingInterval = setInterval(() => { | |
| if (!res.writableEnded) { | |
| const timestamp = new Date().toISOString(); | |
| res.write(`: ping - ${timestamp}\n\n`); | |
| } else { | |
| clearInterval(pingInterval); | |
| } | |
| }, 15000); | |
| // Clean up resources on close | |
| res.on('close', () => { | |
| clearInterval(pingInterval); | |
| delete transports[sessionId]; | |
| console.log('Closed /messages SSE connection'); | |
| }); | |
| // Keep the connection open | |
| return; | |
| } | |
| // Connect to the server and delegate the stream handling | |
| await server.connect(transport); | |
| // Let the transport take over so all further MCP messages flow | |
| await transport.handleRequest(req, res); | |
| // Start ping interval *after* the transport is active | |
| const pingInterval = setInterval(() => { | |
| if (!res.writableEnded) { | |
| const timestamp = new Date().toISOString(); | |
| res.write(`: ping - ${timestamp}\n\n`); | |
| } else { | |
| clearInterval(pingInterval); | |
| } | |
| }, 15000); | |
| // Clean up resources on close | |
| res.on('close', () => { | |
| clearInterval(pingInterval); | |
| delete transports[sessionId]; | |
| console.log('Closed /messages SSE connection'); | |
| }); | |
| // Keep the connection open | |
| return; |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| // Add CORS headers for cross-origin requests and handle Accept headers | ||
| app.use((req: Request, res: Response, next: NextFunction) => { | ||
| // Add CORS headers | ||
| res.header('Access-Control-Allow-Origin', '*'); |
There was a problem hiding this comment.
Suggestion: Replace the open CORS policy with a configuration that uses an environment variable to restrict allowed origins. [cors-configuration]
| res.header('Access-Control-Allow-Origin', '*'); | |
| const allowedOrigin = process.env.ALLOWED_ORIGIN || '*'; | |
| res.header('Access-Control-Allow-Origin', allowedOrigin); |
| try { | ||
| // Log the incoming request details for debugging | ||
| console.log(`[${new Date().toISOString()}] ${req.method} ${req.originalUrl} - Body method: ${req.body?.method || 'none'}, ID: ${req.body?.id || 'none'}`); | ||
| console.log('Headers:', JSON.stringify(req.headers)); |
There was a problem hiding this comment.
Suggestion: Avoid detailed logging of HTTP headers in production to prevent accidental leakage of sensitive data. [sensitive-data-logging]
| console.log('Headers:', JSON.stringify(req.headers)); | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.log('Headers:', JSON.stringify(req.headers)); | |
| } |
| return [ | ||
| { | ||
| name: "read_file", | ||
| description: "Read the complete contents of a file from the file system.", | ||
| inputSchema: { | ||
| type: "object", | ||
| properties: { | ||
| path: { type: "string" }, | ||
| isUrl: { type: "boolean", default: false } | ||
| }, | ||
| required: ["path"] | ||
| } | ||
| }, | ||
| { | ||
| name: "write_file", | ||
| description: "Write content to a file.", | ||
| inputSchema: { | ||
| type: "object", | ||
| properties: { | ||
| path: { type: "string" }, | ||
| content: { type: "string" } | ||
| }, | ||
| required: ["path", "content"] | ||
| } | ||
| }, | ||
| { | ||
| name: "execute_command", | ||
| description: "Execute a terminal command.", | ||
| inputSchema: { | ||
| type: "object", | ||
| properties: { | ||
| command: { type: "string" }, | ||
| timeout_ms: { type: "number" } | ||
| }, | ||
| required: ["command"] | ||
| } | ||
| } | ||
| ]; |
There was a problem hiding this comment.
Suggestion: Restrict exposure of sensitive tool operations by conditionally returning them based on the NODE_ENV. [conditional-exposure]
| return [ | |
| { | |
| name: "read_file", | |
| description: "Read the complete contents of a file from the file system.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| path: { type: "string" }, | |
| isUrl: { type: "boolean", default: false } | |
| }, | |
| required: ["path"] | |
| } | |
| }, | |
| { | |
| name: "write_file", | |
| description: "Write content to a file.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| path: { type: "string" }, | |
| content: { type: "string" } | |
| }, | |
| required: ["path", "content"] | |
| } | |
| }, | |
| { | |
| name: "execute_command", | |
| description: "Execute a terminal command.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| command: { type: "string" }, | |
| timeout_ms: { type: "number" } | |
| }, | |
| required: ["command"] | |
| } | |
| } | |
| ]; | |
| if (process.env.NODE_ENV === 'production') { | |
| // In production, disable dangerous tool operations. | |
| return []; | |
| } else { | |
| return [ | |
| { | |
| name: "read_file", | |
| description: "Read the complete contents of a file from the file system.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| path: { type: "string" }, | |
| isUrl: { type: "boolean", default: false } | |
| }, | |
| required: ["path"] | |
| } | |
| }, | |
| { | |
| name: "write_file", | |
| description: "Write content to a file.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| path: { type: "string" }, | |
| content: { type: "string" } | |
| }, | |
| required: ["path", "content"] | |
| } | |
| }, | |
| { | |
| name: "execute_command", | |
| description: "Execute a terminal command.", | |
| inputSchema: { | |
| type: "object", | |
| properties: { | |
| command: { type: "string" }, | |
| timeout_ms: { type: "number" } | |
| }, | |
| required: ["command"] | |
| } | |
| } | |
| ]; | |
| } |
| function getArgValue(args: string[], flag: string): number | null { | ||
| const index = args.indexOf(flag); | ||
| if (index !== -1 && index + 1 < args.length) { | ||
| const value = parseInt(args[index + 1], 10); | ||
| return isNaN(value) ? null : value; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Suggestion: Validate the HTTP port argument to ensure it falls within the valid range (1-65535) to avoid misconfiguration. [input-validation]
| function getArgValue(args: string[], flag: string): number | null { | |
| const index = args.indexOf(flag); | |
| if (index !== -1 && index + 1 < args.length) { | |
| const value = parseInt(args[index + 1], 10); | |
| return isNaN(value) ? null : value; | |
| } | |
| return null; | |
| } | |
| function getArgValue(args: string[], flag: string): number | null { | |
| const index = args.indexOf(flag); | |
| if (index !== -1 && index + 1 < args.length) { | |
| const value = parseInt(args[index + 1], 10); | |
| return (!isNaN(value) && value > 0 && value < 65536) ? value : null; | |
| } | |
| return null; | |
| } |
| "start:sse": "node dist/start-sse-server.js", | ||
| "start:sse-only": "node dist/start-sse-server.js --sse-only", |
There was a problem hiding this comment.
Suggestion: Enhance the SSE server startup scripts by leveraging PM2 clustering and enforcing a production environment for improved performance and security. [Clustering & Security Optimization]
| "start:sse": "node dist/start-sse-server.js", | |
| "start:sse-only": "node dist/start-sse-server.js --sse-only", | |
| "start:sse": "NODE_ENV=production pm2 start dist/start-sse-server.js --name sse-server -i max", | |
| "start:sse-only": "NODE_ENV=production pm2 start dist/start-sse-server.js --name sse-server -i max -- --sse-only", |
|
CodeAnt AI finished reviewing your PR. |
|
That is not how its done you just need alternative to stdio server from sdk and should work |
User description
So, I thought, yeah we can just use the MCP library and it should just work..
CodeAnt-AI Description
src/http-server.ts) supporting Server-Sent Events (SSE) for the MCP protocol, with robust session management, CORS, and compatibility with multiple MCP clients.src/index.ts), allowing command-line configuration for port and mode (HTTP/SSE-only or dual).src/start-sse-server.ts) to simplify launching the server in SSE/HTTP mode, with signal handling and argument parsing.package.jsonandpackage-lock.jsonto support Express 5.x, @types/express, and the latest MCP SDK, ensuring compatibility and stability.This PR adds full Server-Sent Events (SSE) support to Desktop Commander, enabling remote connections via HTTP and compatibility with modern MCP clients. It provides a robust, session-aware HTTP server, updated documentation, and improved developer experience for running and managing the server in various modes.
Changes walkthrough
http-server.ts
Add Express-based HTTP/SSE server with MCP Streamable HTTP supportsrc/http-server.ts
Server-Sent Events (SSE) support.
for MCP protocol.
session-aware and stateless requests.
handling.
index.ts
Integrate HTTP/SSE server startup and CLI argument parsingsrc/index.ts
selection.
shutdown.
server.ts
Add logging to resources/list and prompts/list handlerssrc/server.ts
resources/listandprompts/listrequest handlersfor better traceability.
start-sse-server.ts
Add CLI script for starting server in SSE/HTTP modesrc/start-sse-server.ts
port and mode arguments.
package.json
Add SSE server entrypoint, scripts, and update dependenciespackage.json
SSE/HTTP mode.
@modelcontextprotocol/sdk.
README.md
Document Server-Sent Events (SSE) support and usageREADME.md
configuration options.
package-lock.json
Update dependencies for SSE/HTTP server and SDK upgradepackage-lock.json
latest @modelcontextprotocol/sdk.
numbers.
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit