feat: MCP server support #638
Conversation
RohitR311
added
Type: Feature
WalkthroughA new MCP worker module was introduced, enabling the server to interact with a backend web scraping API via the Model Context Protocol (MCP). The server startup now conditionally launches this worker as a child process based on environment variables. Supporting scripts, dependencies, and TypeScript configuration for the MCP worker were also added. Changes
Sequence Diagram(s)sequenceDiagram
participant Server
participant MCP Worker (Child Process)
participant Backend API
Server->>MCP Worker (Child Process): Forks process if MCP is enabled
MCP Worker (Child Process)->>Backend API: Authenticated API requests (robots, runs, etc.)
Backend API-->>MCP Worker (Child Process): API responses (robot data, run results, etc.)
MCP Worker (Child Process)-->>Server: Logs, status, and error messages (via IPC)
Assessment against linked issues
Poem
✨ 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 (
|
![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 (2)
package.json (1)
113-125: Duplicate@types/nodeentry
@types/nodeis already listed in"dependencies". Keep it only indevDependencies(usual) to avoid version skew.server/src/mcp-worker.ts (1)
22-29: Environment validation OK – just a minor UX tweakIf
MCP_API_KEYis missing you throw synchronously; consider emitting a clearer message with instructions before exiting.Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(3 hunks)server/src/mcp-worker.ts(1 hunks)server/src/mcp/index.ts(1 hunks)server/src/server.ts(4 hunks)server/tsconfig.mcp.json(1 hunks)
server/tsconfig.mcp.json
Outdated
| "outDir": "./dist", | ||
| "rootDir": "./", | ||
| "strict": true, | ||
| "esModuleInterop": true, | ||
| "skipLibCheck": true, | ||
| "forceConsistentCasingInFileNames": true, | ||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "sourceMap": true | ||
| }, | ||
| "include": [ | ||
| "mcp/**/*", | ||
| "src/mcp-worker.ts", | ||
| "src/logger.ts", | ||
| "src/types/**/*" | ||
| ], |
There was a problem hiding this comment.
Build will miss src/mcp/index.ts and output to the wrong place
include: "mcp/**/*" does not match the actual source directory (src/mcp/).
Moreover, outDir: "./dist" writes to server/dist, but npm run mcp:start tries to run dist/mcp/index.js from the repo root, so the file won’t be found.
- "rootDir": "./",
- "include": [
- "mcp/**/*",
- "src/mcp-worker.ts",
- "src/logger.ts",
- "src/types/**/*"
- ],
- "outDir": "./dist",
+ "rootDir": "./src",
+ "include": [
+ "src/mcp/**/*",
+ "src/mcp-worker.ts",
+ "src/logger.ts",
+ "src/types/**/*"
+ ],
+ "outDir": "../dist"(Adjust scripts accordingly.)
📝 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.
| "outDir": "./dist", | |
| "rootDir": "./", | |
| "strict": true, | |
| "esModuleInterop": true, | |
| "skipLibCheck": true, | |
| "forceConsistentCasingInFileNames": true, | |
| "declaration": true, | |
| "declarationMap": true, | |
| "sourceMap": true | |
| }, | |
| "include": [ | |
| "mcp/**/*", | |
| "src/mcp-worker.ts", | |
| "src/logger.ts", | |
| "src/types/**/*" | |
| ], | |
| "compilerOptions": { | |
| "outDir": "../dist", | |
| "rootDir": "./src", | |
| "strict": true, | |
| "esModuleInterop": true, | |
| "skipLibCheck": true, | |
| "forceConsistentCasingInFileNames": true, | |
| "declaration": true, | |
| "declarationMap": true, | |
| "sourceMap": true | |
| }, | |
| "include": [ | |
| "src/mcp/**/*", | |
| "src/mcp-worker.ts", | |
| "src/logger.ts", | |
| "src/types/**/*" | |
| ], |
🤖 Prompt for AI Agents
In server/tsconfig.mcp.json lines 6 to 21, the "include" path "mcp/**/*" does
not match the actual source directory "src/mcp/", causing src/mcp/index.ts to be
missed in the build. Also, "outDir" is set to "./dist" which outputs to
server/dist, but the runtime expects dist/mcp/index.js at the repo root. Fix
this by updating "include" to "src/mcp/**/*" and changing "outDir" to "../dist"
or another path that outputs to the repo root's dist folder. Adjust related npm
scripts to match the new output location.
| mcpWorkerProcess.on('exit', (code: any) => { | ||
| logger.log('info', `MCP Worker exited with code: ${code}`); | ||
| }); | ||
|
|
||
| logger.log('info', 'MCP Server started with stdio transport in worker process'); | ||
| } catch (error: any) { | ||
| logger.log('error', `Failed to initialize MCP Server: ${error.message}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Leak: worker isn’t killed on server shutdown
mcpWorkerProcess is scoped to initializeMCPServer; the top-level SIGINT handler can’t access it, leaving the child orphaned.
Expose a module-level variable and kill it alongside the other workers.
-let mcpWorkerProcess: ChildProcess | undefined;
+let mcpWorkerProcess: ReturnType<typeof fork> | undefined;
...
if (mcpWorkerProcess) mcpWorkerProcess.kill();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/src/server.ts around lines 118 to 126, the mcpWorkerProcess variable
is declared inside the initializeMCPServer function, making it inaccessible to
the top-level SIGINT handler, which causes the worker process to remain running
after server shutdown. To fix this, declare mcpWorkerProcess as a module-level
variable outside the function so it can be accessed globally, then update the
SIGINT handler to check if mcpWorkerProcess exists and call its kill method to
properly terminate the worker during shutdown.
| const mcpWorkerPath = path.resolve(__dirname, './mcp-worker.ts'); | ||
| const mcpWorkerProcess = fork(mcpWorkerPath, [], { | ||
| execArgv: process.env.NODE_ENV === 'production' ? [] : ['--inspect=5861'], | ||
| env: { | ||
| ...process.env, | ||
| MCP_WORKER: 'true' | ||
| } | ||
| }); | ||
|
|
||
| mcpWorkerProcess.on('message', (message: any) => { |
There was a problem hiding this comment.
Child process will fail to launch in both prod and dev
fork(...'mcp-worker.ts')→ Node can’t execute TypeScript without-r ts-node/register.- In production the compiled file will be
mcp-worker.js, so the path is wrong.
-const mcpWorkerPath = path.resolve(__dirname, './mcp-worker.ts');
+const isTsRuntime = /\.ts$/.test(__filename); // crude check
+const mcpWorkerPath = path.resolve(
+ __dirname,
+ isTsRuntime ? './mcp-worker.ts' : './mcp-worker.js'
+);
+
+const extraExec = isTsRuntime ? ['-r', 'ts-node/register'] : [];-execArgv: process.env.NODE_ENV === 'production' ? [] : ['--inspect=5861'],
+execArgv: [
+ ...(process.env.NODE_ENV === 'production' ? [] : ['--inspect=5861']),
+ ...extraExec
+],📝 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.
| const mcpWorkerPath = path.resolve(__dirname, './mcp-worker.ts'); | |
| const mcpWorkerProcess = fork(mcpWorkerPath, [], { | |
| execArgv: process.env.NODE_ENV === 'production' ? [] : ['--inspect=5861'], | |
| env: { | |
| ...process.env, | |
| MCP_WORKER: 'true' | |
| } | |
| }); | |
| mcpWorkerProcess.on('message', (message: any) => { | |
| // determine whether we're running on .ts or .js at runtime | |
| const isTsRuntime = /\.ts$/.test(__filename); // crude check | |
| const mcpWorkerPath = path.resolve( | |
| __dirname, | |
| isTsRuntime ? './mcp-worker.ts' : './mcp-worker.js' | |
| ); | |
| // if we're in TS runtime, preload ts-node to handle .ts files | |
| const extraExec = isTsRuntime ? ['-r', 'ts-node/register'] : []; | |
| const mcpWorkerProcess = fork(mcpWorkerPath, [], { | |
| execArgv: [ | |
| ...(process.env.NODE_ENV === 'production' ? [] : ['--inspect=5861']), | |
| ...extraExec | |
| ], | |
| env: { | |
| ...process.env, | |
| MCP_WORKER: 'true' | |
| } | |
| }); | |
| mcpWorkerProcess.on('message', (message: any) => { | |
| // ... | |
| }); |
🤖 Prompt for AI Agents
In server/src/server.ts around lines 101 to 110, the child process fails to
launch because the forked path points to a TypeScript file without using ts-node
in development, and the path is incorrect for production where the compiled
JavaScript file should be used. Fix this by conditionally setting the worker
path to 'mcp-worker.ts' with the '-r ts-node/register' execArgv in development,
and to 'mcp-worker.js' in production without extra execArgv. Adjust the fork
call accordingly to handle both environments correctly.
package.json
Outdated
| "migration:generate": "sequelize-cli migration:generate --name", | ||
| "mcp:build": "tsc --project server/tsconfig.mcp.json", | ||
| "mcp:start": "node dist/mcp/index.js", | ||
| "mcp:dev": "ts-node mcp/index.ts", | ||
| "mcp:inspector": "npx @modelcontextprotocol/inspector dist/mcp/index.js" | ||
| }, |
There was a problem hiding this comment.
mcp:* scripts reference non-existent paths
The MCP server source sits under server/src/mcp/…, not project-root mcp/….
-"mcp:build": "tsc --project server/tsconfig.mcp.json",
-"mcp:start": "node dist/mcp/index.js",
-"mcp:dev": "ts-node mcp/index.ts",
+"mcp:build": "tsc --project server/tsconfig.mcp.json",
+"mcp:start": "node server/dist/src/mcp/index.js",
+"mcp:dev": "ts-node server/src/mcp/index.ts",Adjust any other tooling (inspector path, etc.) after the build-output fix in tsconfig.
📝 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.
| "migration:generate": "sequelize-cli migration:generate --name", | |
| "mcp:build": "tsc --project server/tsconfig.mcp.json", | |
| "mcp:start": "node dist/mcp/index.js", | |
| "mcp:dev": "ts-node mcp/index.ts", | |
| "mcp:inspector": "npx @modelcontextprotocol/inspector dist/mcp/index.js" | |
| }, | |
| "migration:generate": "sequelize-cli migration:generate --name", | |
| "mcp:build": "tsc --project server/tsconfig.mcp.json", | |
| "mcp:start": "node server/dist/src/mcp/index.js", | |
| "mcp:dev": "ts-node server/src/mcp/index.ts", | |
| "mcp:inspector": "npx @modelcontextprotocol/inspector dist/mcp/index.js" | |
| }, |
🤖 Prompt for AI Agents
In package.json lines 102 to 107, the mcp:* scripts incorrectly reference paths
starting with "mcp/" at the project root, but the source files are actually
under "server/src/mcp/". Update all mcp:* script paths to point to the correct
locations under "server/src/mcp/" for source files and "dist/mcp/" for build
outputs as appropriate. Also, verify and adjust the inspector script path to
align with the corrected build output location after fixing the tsconfig build
output path.
server/src/mcp/index.ts
Outdated
| } else { | ||
| // HTTP transport for web-based MCP clients | ||
| transport = new StreamableHTTPServerTransport({ | ||
| sessionIdGenerator: undefined // Stateless for simplicity | ||
| }); | ||
| logger.log('info', `Starting Maxun MCP server with HTTP transport on port ${this.config.httpPort}`); | ||
| } | ||
|
|
||
| await this.mcpServer.connect(transport); |
There was a problem hiding this comment.
httpPort is ignored
You log the chosen port but never pass it to StreamableHTTPServerTransport, so it binds to the default.
-transport = new StreamableHTTPServerTransport({
- sessionIdGenerator: undefined // Stateless for simplicity
-});
+transport = new StreamableHTTPServerTransport({
+ port: this.config.httpPort ?? 8080,
+ sessionIdGenerator: undefined // Stateless for simplicity
+});📝 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.
| } else { | |
| // HTTP transport for web-based MCP clients | |
| transport = new StreamableHTTPServerTransport({ | |
| sessionIdGenerator: undefined // Stateless for simplicity | |
| }); | |
| logger.log('info', `Starting Maxun MCP server with HTTP transport on port ${this.config.httpPort}`); | |
| } | |
| await this.mcpServer.connect(transport); | |
| } else { | |
| // HTTP transport for web-based MCP clients | |
| transport = new StreamableHTTPServerTransport({ | |
| port: this.config.httpPort ?? 8080, | |
| sessionIdGenerator: undefined // Stateless for simplicity | |
| }); | |
| logger.log('info', `Starting Maxun MCP server with HTTP transport on port ${this.config.httpPort}`); | |
| } | |
| await this.mcpServer.connect(transport); |
🤖 Prompt for AI Agents
In server/src/mcp/index.ts around lines 595 to 603, the httpPort value from
this.config is logged but not passed to the StreamableHTTPServerTransport
constructor, causing it to bind to the default port instead of the configured
one. Fix this by adding the httpPort property with the value
this.config.httpPort to the options object when creating the
StreamableHTTPServerTransport instance.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
server/src/mcp-worker.ts (4)
4-5: Avoid the extranode-fetchdependency on Node ≥ 18Node 18+ ships a built-in, standards-compliant
fetchimplementation. Keepingnode-fetchadds an unnecessary runtime dependency and almost doubles the bundle size.-import fetch from 'node-fetch'; +// Use the built-in global fetch (Node 18+) or fall back for Node 16 builds +const fetchFn: typeof fetch = + typeof fetch !== 'undefined' ? fetch : (await import('node-fetch')).default as any;(If Node 16 is still a hard requirement, gate the dynamic import behind a version check.)
9-13: Leverage a proper logger instead ofconsole.error
console.erroris fine during prototyping but quickly becomes inadequate (no log levels, timestamps, or sinks). Consider swapping in a minimal winston/pino instance so this worker can be monitored in production just like the main server.
20-27: CheckBACKEND_URLandMCP_API_KEYup-front, fail fastYou already guard
MCP_API_KEY—good. It would be equally valuable to validateBACKEND_URL(e.g., ensure it is a valid URL; fail if it is empty) so mis-configuration is caught at boot instead of via mysterious ECONNREFUSED later on.
49-54: Return richer error information to aid debuggingThe backend often responds with a JSON body containing
message/code. Swallowing that detail forces consumers to dig through server logs. Parse and bubble it up when present:-if (!response.ok) { - throw new Error(`API request failed: ${response.status} ${response.statusText}`); -} +if (!response.ok) { + let body: any = null; + try { body = await response.json(); } catch {} + const msg = body?.message ?? response.statusText; + throw new Error(`API request failed (${response.status}): ${msg}`); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(3 hunks)server/src/mcp-worker.ts(1 hunks)server/tsconfig.mcp.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/tsconfig.mcp.json
- package.json
🔇 Additional comments (1)
server/src/mcp-worker.ts (1)
371-373: Environment-driven autostart can create duplicate workersUsing both
MCP_WORKER==='true'andrequire.main===modulemeans a singlenode dist/mcp-worker.jsrun withMCP_WORKER=truesatisfies both conditions and risks starting the worker twice. Combine them:-if (process.env.MCP_WORKER === 'true' || require.main === module) { +if (process.env.MCP_WORKER === 'true' || require.main === module) { main(); }→ choose one predicate, not both (likely
require.main === module).
| try { | ||
| const transport = new StdioServerTransport(); | ||
| await this.mcpServer.connect(transport); | ||
| log('Maxun MCP Worker connected and ready'); | ||
| } catch (error: any) { | ||
| log(`Failed to start MCP Worker: ${error.message}`); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async stop() { | ||
| try { | ||
| await this.mcpServer.close(); | ||
| log('Maxun MCP Worker stopped'); | ||
| } catch (error: any) { | ||
| log(`Error stopping MCP Worker: ${error.message}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Graceful shutdown still calls process.exit(0) prematurely
process.exit short-circuits any pending event-loop tasks (e.g., unresolved promises inside the SDK). When the handler is async, simply letting the program finish naturally after await worker.stop() is enough.
-process.on('SIGTERM', async () => {
- await worker.stop();
- process.exit(0);
-});
+process.on('SIGTERM', () => worker.stop());Repeat for SIGINT.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/src/mcp-worker.ts around lines 328 to 346, the graceful shutdown calls
process.exit(0) prematurely, which can interrupt pending async tasks. Remove any
calls to process.exit in the shutdown handlers and ensure the program exits
naturally after awaiting worker.stop(). Apply the same fix for the SIGINT
handler to avoid premature termination.
| try { | ||
| const data = await this.makeApiRequest('/api/robots'); | ||
|
|
||
| return { | ||
| content: [{ | ||
| type: "text", | ||
| text: `Found ${data.robots.totalCount} robots:\n\n${JSON.stringify(data.robots.items, null, 2)}` | ||
| }] | ||
| }; | ||
| } catch (error: any) { | ||
| return { | ||
| content: [{ | ||
| type: "text", | ||
| text: `Error fetching robots: ${error.message}` | ||
| }], | ||
| isError: true | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DRY up repeated try/catch & message formatting
Every tool repeats the same:
try { await makeApiRequest… } catch (e) { return {isError:true,…} }- Converting objects to human-readable text via
JSON.stringify.
Consider a small helper:
private async toolHandler<T>(fn: () => Promise<T>, onSuccess: (data:T)=>string) {
try {
const data = await fn();
return { content: [{ type: "text", text: onSuccess(data) }] };
} catch (err: any) {
return { content: [{ type:"text", text: String(err?.message ?? err) }], isError:true };
}
}Then:
this.mcpServer.tool("list_robots", {}, () =>
this.toolHandler(
() => this.makeApiRequest("/api/robots"),
data => `Found ${data.robots.totalCount} robots:\n\n${JSON.stringify(data.robots.items, null, 2)}`
)
);Benefits:
• Less duplication, easier maintenance
• Consistent error formatting
• Central place to extend (metrics, retries, etc.)
Also applies to: 90-107, 118-174
🤖 Prompt for AI Agents
In server/src/mcp-worker.ts around lines 62 to 79, the try/catch blocks and
JSON.stringify formatting for API responses are repeated across multiple tools.
Refactor by creating a private helper method that accepts a function returning a
promise and a success formatting callback, handling the try/catch and error
formatting centrally. Replace the existing code with calls to this helper to
reduce duplication, ensure consistent error handling, and simplify future
enhancements. Apply the same refactor to similar code blocks in lines 90-107 and
118-174.
| private async makeApiRequest(endpoint: string, options: any = {}) { | ||
| const url = `${this.apiUrl}${endpoint}`; | ||
| const headers = { | ||
| 'Content-Type': 'application/json', | ||
| 'x-api-key': this.apiKey, | ||
| ...options.headers | ||
| }; | ||
|
|
||
| const response = await fetch(url, { | ||
| ...options, | ||
| headers |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout / abort handling to fetch calls
All HTTP requests are open-ended; if the backend stalls the worker will hang forever and block the MCP tool response. Wrap the request in an AbortController with a sane default (e.g., 30 s) and surface a timeout error to the client.
-const response = await fetch(url, {
- ...options,
- headers
-});
+const controller = new AbortController();
+const timeoutId = setTimeout(() => controller.abort(), 30_000);
+const response = await fetchFn(url, {
+ ...options,
+ headers,
+ signal: controller.signal
+}).finally(() => clearTimeout(timeoutId));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/src/mcp-worker.ts around lines 36 to 46, the fetch call lacks timeout
or abort handling, causing potential indefinite hangs if the backend stalls. To
fix this, create an AbortController instance, set a timeout (e.g., 30 seconds)
that triggers the controller's abort method, and pass the controller's signal to
the fetch options. Also, handle the abort error to surface a clear timeout error
to the client.
What this PR does?
Brings support for configuring MCP server for local LLMs
MCP.mp4
Closes: #470
Summary by CodeRabbit
New Features
Chores