-
-
Notifications
You must be signed in to change notification settings - Fork 203
Mcpjam hosted multi tenancy #1461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7251452
9e3122a
36fbdfb
7c48103
89c6784
8a06acf
cc8d6fe
94c2bc9
6323c28
bc6c2e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Goal | ||
|
|
||
| We want to create a hosted version of MCPJam. It will be hosted via Docker on Railway. It must have everything the Desktop version has. | ||
|
|
||
| # Current limitations | ||
|
|
||
| Currently MCPJam was designed to be a Desktop app ran locally either through `npx`, Electron, Docker. The Hono backend has a MCPClientManager singleton. This means if we host it, everyone will be sharing the same MCPClientManager object, and we will have collisions. People will have access to other people's MCP servers and see everyone else's logs. | ||
|
|
||
| We cannot have this singleton behavior in a hosted version. Everyone should have their own MCPClientManager in isolation. | ||
|
|
||
| # Requirements | ||
|
|
||
| - We want to create a mono-repo that supports both the current local desktop, but also a hosted version. | ||
| - In hosted environment, everyone must have their own client manager in isolation. | ||
| - Try to scale, handle what happens when there are LOTS of people connected to the server. | ||
| - Must be deployable via Docker. | ||
| - Changes must be as minimalistic as possible. Least amount of impact and code required as possible. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import { | |
| sessionAuthMiddleware, | ||
| scrubTokenFromUrl, | ||
| } from "./middleware/session-auth"; | ||
| import { resolveRequestSessionId } from "./middleware/client-manager-session"; | ||
| import { originValidationMiddleware } from "./middleware/origin-validation"; | ||
| import { securityHeadersMiddleware } from "./middleware/security-headers"; | ||
|
|
||
|
|
@@ -82,8 +83,14 @@ function logBox(content: string, title?: string) { | |
| // Import routes and services | ||
| import mcpRoutes from "./routes/mcp/index"; | ||
| import appsRoutes from "./routes/apps/index"; | ||
| import { initElicitationCallback } from "./routes/mcp/elicitation"; | ||
| import { rpcLogBus } from "./services/rpc-log-bus"; | ||
| import { progressStore } from "./services/progress-store"; | ||
| import { tunnelManager } from "./services/tunnel-manager"; | ||
| import { | ||
| createClientManagerStore, | ||
| readSessionStoreOptionsFromEnv, | ||
| } from "./services/client-manager-store"; | ||
| import { | ||
| SERVER_PORT, | ||
| SERVER_HOSTNAME, | ||
|
|
@@ -218,24 +225,45 @@ if (!process.env.CONVEX_HTTP_URL) { | |
| ); | ||
| } | ||
|
|
||
| // Initialize centralized MCPJam Client Manager and wire RPC logging to SSE bus | ||
| const mcpClientManager = new MCPClientManager( | ||
| {}, | ||
| { | ||
| rpcLogger: ({ direction, message, serverId }) => { | ||
| rpcLogBus.publish({ | ||
| serverId, | ||
| direction, | ||
| timestamp: new Date().toISOString(), | ||
| message, | ||
| }); | ||
| }, | ||
| const mcpClientManagerStore = createClientManagerStore({ | ||
| hostedMode: HOSTED_MODE, | ||
| managerFactory: (sessionId) => { | ||
| const manager = new MCPClientManager( | ||
| {}, | ||
| { | ||
| rpcLogger: ({ direction, message, serverId }) => { | ||
| rpcLogBus.publish({ | ||
| sessionId, | ||
| serverId, | ||
| direction, | ||
| timestamp: new Date().toISOString(), | ||
| message, | ||
| }); | ||
| }, | ||
| progressHandler: ({ | ||
| serverId, | ||
| progressToken, | ||
| progress, | ||
| total, | ||
| message, | ||
| }) => { | ||
| progressStore.publish({ | ||
| sessionId, | ||
| serverId, | ||
| progressToken, | ||
| progress, | ||
| total, | ||
| message, | ||
| timestamp: new Date().toISOString(), | ||
| }); | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| initElicitationCallback(manager); | ||
| return manager; | ||
|
Comment on lines
+263
to
+264
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check how broadcastElicitation works and whether it's session-scoped
rg -n 'broadcastElicitation' --type=ts -C5
echo "---"
# Check the elicitation SSE endpoint for session filtering
rg -n 'elicitation.*stream\|elicitation.*subscribe\|elicitation.*sse' --type=ts -C5Repository: MCPJam/inspector Length of output: 2746 🏁 Script executed: # Find where elicitationSubscribers are registered
rg -n 'elicitationSubscribers' --type=ts -B3 -A3
echo "---"
# Look for the SSE endpoint registration
rg -n 'subscribe\|/elicitation' --type=ts -B2 -A10Repository: MCPJam/inspector Length of output: 2466 🏁 Script executed: # Get the full context of the SSE endpoint where subscribers are added
sed -n '80,120p' mcpjam-inspector/server/routes/mcp/elicitation.tsRepository: MCPJam/inspector Length of output: 1202 🏁 Script executed: # Find the complete route definition with endpoint path
rg -n 'elicitation\.get\|\.get.*subscribe\|\.get.*stream' mcpjam-inspector/server/routes/mcp/elicitation.ts -B5 -A20Repository: MCPJam/inspector Length of output: 42 🏁 Script executed: # Get file size and view the complete elicitation.ts
wc -l mcpjam-inspector/server/routes/mcp/elicitation.ts
echo "---"
cat -n mcpjam-inspector/server/routes/mcp/elicitation.tsRepository: MCPJam/inspector Length of output: 6013 Elicitation broadcasts leak across sessions in hosted mode. The Scoped subscribers by session (e.g., per-session Sets) or add 🤖 Prompt for AI Agents |
||
| }, | ||
| ); | ||
| // Middleware to inject client manager into context | ||
| app.use("*", async (c, next) => { | ||
| c.mcpClientManager = mcpClientManager; | ||
| await next(); | ||
| sessionStoreOptions: readSessionStoreOptionsFromEnv(), | ||
| }); | ||
|
|
||
| // ===== SECURITY MIDDLEWARE STACK ===== | ||
|
|
@@ -252,6 +280,14 @@ app.use("*", sessionAuthMiddleware); | |
|
|
||
| // ===== END SECURITY MIDDLEWARE ===== | ||
|
|
||
| // Resolve a manager only after the security stack has run. | ||
| app.use("*", async (c, next) => { | ||
| const sessionId = resolveRequestSessionId(c, HOSTED_MODE); | ||
| c.mcpSessionId = sessionId; | ||
| c.mcpClientManager = mcpClientManagerStore.getManager(sessionId); | ||
| await next(); | ||
| }); | ||
|
|
||
| // Middleware - only enable HTTP request logging in dev mode or when --verbose is passed | ||
| const enableHttpLogs = | ||
| process.env.NODE_ENV !== "production" || process.env.VERBOSE_LOGS === "true"; | ||
|
|
@@ -410,14 +446,16 @@ const server = serve({ | |
|
|
||
| // Handle graceful shutdown | ||
| process.on("SIGINT", async () => { | ||
| console.log("\n🛑 Shutting down gracefully..."); | ||
| appLogger.info("\n🛑 Shutting down gracefully..."); | ||
| await mcpClientManagerStore.dispose(); | ||
| await tunnelManager.closeAll(); | ||
| server.close(); | ||
| process.exit(0); | ||
| }); | ||
|
|
||
| process.on("SIGTERM", async () => { | ||
| console.log("\n🛑 Shutting down gracefully..."); | ||
| appLogger.info("\n🛑 Shutting down gracefully..."); | ||
| await mcpClientManagerStore.dispose(); | ||
| await tunnelManager.closeAll(); | ||
| server.close(); | ||
| process.exit(0); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { Hono } from "hono"; | ||
| import { resolveRequestSessionId } from "../client-manager-session.js"; | ||
|
|
||
| function createTestApp(hostedMode: boolean): Hono { | ||
| const app = new Hono(); | ||
|
|
||
| app.use("*", async (c, next) => { | ||
| (c as any).resolvedSessionId = resolveRequestSessionId(c, hostedMode); | ||
| await next(); | ||
| }); | ||
|
|
||
| app.get("/api/test", (c) => { | ||
| return c.json({ sessionId: (c as any).resolvedSessionId ?? null }); | ||
| }); | ||
|
|
||
| return app; | ||
| } | ||
|
|
||
| describe("resolveRequestSessionId", () => { | ||
| it("does not create a session when hosted mode is disabled", async () => { | ||
| const app = createTestApp(false); | ||
|
|
||
| const res = await app.request("/api/test"); | ||
| const data = await res.json(); | ||
|
|
||
| expect(data.sessionId).toBeNull(); | ||
| expect(res.headers.get("set-cookie")).toBeNull(); | ||
| }); | ||
|
|
||
| it("uses explicit session header when present", async () => { | ||
| const app = createTestApp(true); | ||
|
|
||
| const res = await app.request("/api/test", { | ||
| headers: { "x-mcpjam-session-id": "header-session-1" }, | ||
| }); | ||
| const data = await res.json(); | ||
|
|
||
| expect(data.sessionId).toBe("header-session-1"); | ||
| expect(res.headers.get("set-cookie")).toBeNull(); | ||
| }); | ||
|
|
||
| it("uses existing cookie session when present", async () => { | ||
| const app = createTestApp(true); | ||
|
|
||
| const res = await app.request("/api/test", { | ||
| headers: { Cookie: "mcpjam_session_id=cookie-session-1" }, | ||
| }); | ||
| const data = await res.json(); | ||
|
|
||
| expect(data.sessionId).toBe("cookie-session-1"); | ||
| expect(res.headers.get("set-cookie")).toBeNull(); | ||
| }); | ||
|
|
||
| it("creates a new session cookie when no session identifier exists", async () => { | ||
| const app = createTestApp(true); | ||
|
|
||
| const res = await app.request("/api/test"); | ||
| const data = await res.json(); | ||
| const setCookie = res.headers.get("set-cookie"); | ||
|
|
||
| expect(data.sessionId).toMatch( | ||
| /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, | ||
| ); | ||
| expect(setCookie).toContain(`mcpjam_session_id=${data.sessionId}`); | ||
| expect(setCookie).toContain("HttpOnly"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
Repository: MCPJam/inspector
Length of output: 1936
🏁 Script executed:
Repository: MCPJam/inspector
Length of output: 1453
🏁 Script executed:
Repository: MCPJam/inspector
Length of output: 12905
Extract the
createClientManagerStoreconfiguration into a shared factory function.Lines 90–132 in
app.tsand lines 228–267 inindex.tscontain identical store initialization logic. This duplicated block—including themanagerFactorywith RPC logger, progress handler, and elicitation callback setup—should be extracted to a reusable function (e.g.,createDefaultClientManagerStore()) in a shared utilities module. Both entry points can then invoke this function, eliminating the maintenance burden of keeping two identical copies in sync.🤖 Prompt for AI Agents