feat(mcp-proxy): singleton mode to prevent multiple instances#43
Conversation
Use a lockfile + HTTP bridge so only one mcp-proxy process runs at a time. Subsequent spawns forward calls to the primary via StreamableHTTP.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 30 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughIntroduces a multi-instance coordination system for MCP proxy using filesystem-based locking. A primary instance starts an HTTP server on a singleton port, while bridge instances proxy requests through the primary. New modules provide lock management and bridge server functionality with fallback transport support. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Process
participant Lock as Lock File
participant Primary as Primary Server<br/>(McpProxyServer + HTTP)
participant Bridge as Bridge Server
participant Upstream as Upstream MCP<br/>Server
App->>Lock: tryAcquireLock(port)
alt Lock Acquired (PRIMARY)
Lock->>App: true
App->>Primary: new McpProxyServer()
App->>Primary: startHttpTransport(9200)
Primary->>Primary: Create HTTP server on port 9200
Note over Primary: Listening for client requests
App->>App: setupGracefulShutdown()
else Lock Exists (BRIDGE)
Lock->>App: false
App->>Lock: readLock()
Lock->>App: {pid, port: 9200}
App->>Bridge: new BridgeServer(9200)
App->>Bridge: start()
Bridge->>Bridge: ensureClient() to Primary:9200
Note over Bridge: Ready to proxy requests
end
rect rgba(100, 150, 200, 0.5)
Note over Primary,Bridge: Request Flow
Client->>Primary: POST /mcp (primary mode)
Primary->>Upstream: callTool()
Upstream->>Primary: result
Primary->>Client: {content: [...]}
Client->>Bridge: Tool call (bridge mode)
Bridge->>Primary: callTool() via HTTP
Primary->>Upstream: callTool()
Upstream->>Primary: result
Primary->>Bridge: {content: [...]}
Bridge->>Client: {content: [...]}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/mcp-proxy/src/bridge.ts`:
- Around line 79-103: The ensureClient method currently assigns this.client
before its connect() completes, causing races; change it to memoize the
in-flight Promise (e.g., this.clientPromise) instead of publishing this.client
early: on first call create the Client instance and set this.clientPromise =
(async () => { try connect using StreamableHTTPClientTransport(url) catch try
SSEClientTransport and re-create client; } )(), await that promise in concurrent
callers, only assign this.client after a successful connection, and ensure the
promise is cleared or rejected on failure so future attempts can retry; update
references in ensureClient, connect, StreamableHTTPClientTransport,
SSEClientTransport and primaryUrl usage accordingly.
In `@packages/mcp-proxy/src/index.ts`:
- Around line 24-28: The current wrapper calls releaseLock() before awaiting
originalCleanup(), allowing a replacement process to acquire PRIMARY while the
old server is still closing; change server.cleanup to await originalCleanup()
first and then call releaseLock() (or call releaseLock() from a finally block
after awaiting originalCleanup()) so the singleton lock is only freed after
originalCleanup() completes; update the wrapper around
originalCleanup/server.cleanup to reflect this ordering and preserve error
propagation.
In `@packages/mcp-proxy/src/server.ts`:
- Around line 499-540: The createServer request handler currently awaits
this.server.connect(transport) and transport.handleRequest(req, res) without
local error handling; wrap both async blocks (the POST session creation path
that calls this.server.connect and the branch that calls
transport.handleRequest) in a try/catch so any rejection is caught, log the
error (including transport.sessionId or sessionId when available), and send an
HTTP 5xx response (e.g., 500) with a JSON error message instead of letting the
rejection bubble to the process-level handler; ensure you still clean up the
transport in the catch if it was created and avoid swallowing errors silently.
In `@packages/mcp-proxy/src/singleton.ts`:
- Around line 29-43: The race occurs because the code creates the lock file with
openSync(lockPath, "wx") then calls writeFileSync(lockPath, ...) which can leave
a window where readLock() sees an empty/unreadable file; instead write the JSON
payload to the already-open file descriptor so the file is atomically populated
before close. Modify tryAcquireLock to replace writeFileSync(lockPath,
JSON.stringify(info)) with writing to the fd (e.g., use writeSync(fd,
JSON.stringify(info)) and optionally fsync before closeSync(fd)), keeping the
rest of the flow (readLock, unlinkSync, retry) unchanged and referencing the
same lockPath, LockInfo and tryAcquireLock symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f13b8237-7f9e-4789-a33f-dee8677be741
📒 Files selected for processing (4)
packages/mcp-proxy/src/bridge.tspackages/mcp-proxy/src/index.tspackages/mcp-proxy/src/server.tspackages/mcp-proxy/src/singleton.ts
…ring, connection race, HTTP error handling
Summary
/tmp/mcp-proxy.lockwithfs.open('wx')) with PID liveness check to handle stale locksHow it works
New files
singleton.ts— lock file management (acquire, release, stale detection)bridge.ts— stdio↔HTTP bridge for secondary instancesModified files
server.ts— addedstartHttpTransport()for bridge connectionsindex.ts— startup decision logic (PRIMARY vs BRIDGE)Config
MCP_PROXY_SINGLETON_PORT(default: 9200)MCP_PROXY_LOCK_DIR(default: /tmp)No changes needed in agent MCP configs — interface remains stdio.
Summary by CodeRabbit
Release Notes