mirror of
https://github.com/samanhappy/mcphub.git
synced 2025-12-23 18:29:21 -05:00
Add per-session server instance support to fix concurrent user isolation
Co-authored-by: samanhappy <2755122+samanhappy@users.noreply.github.com>
This commit is contained in:
@@ -47,7 +47,8 @@ MCPHub uses several configuration files:
|
||||
},
|
||||
"playwright": {
|
||||
"command": "npx",
|
||||
"args": ["@playwright/mcp@latest", "--headless"]
|
||||
"args": ["@playwright/mcp@latest", "--headless", "--isolated"],
|
||||
"perSession": true
|
||||
},
|
||||
"slack": {
|
||||
"command": "npx",
|
||||
@@ -101,8 +102,9 @@ MCPHub uses several configuration files:
|
||||
{
|
||||
"playwright": {
|
||||
"command": "npx",
|
||||
"args": ["@playwright/mcp@latest", "--headless"],
|
||||
"args": ["@playwright/mcp@latest", "--headless", "--isolated"],
|
||||
"timeout": 60000,
|
||||
"perSession": true,
|
||||
"env": {
|
||||
"PLAYWRIGHT_BROWSERS_PATH": "/tmp/browsers"
|
||||
}
|
||||
@@ -110,6 +112,8 @@ MCPHub uses several configuration files:
|
||||
}
|
||||
```
|
||||
|
||||
**Note**: The `--isolated` flag ensures each browser session is isolated, and `perSession: true` creates a separate server instance for each user session, preventing state leakage between concurrent users.
|
||||
|
||||
### File and System Servers
|
||||
|
||||
#### Filesystem Server
|
||||
|
||||
@@ -50,8 +50,9 @@ MCPHub 使用几个配置文件:
|
||||
},
|
||||
"playwright": {
|
||||
"command": "npx",
|
||||
"args": ["@playwright/mcp@latest", "--headless"],
|
||||
"timeout": 60000
|
||||
"args": ["@playwright/mcp@latest", "--headless", "--isolated"],
|
||||
"timeout": 60000,
|
||||
"perSession": true
|
||||
},
|
||||
"slack": {
|
||||
"command": "npx",
|
||||
@@ -111,8 +112,9 @@ MCPHub 使用几个配置文件:
|
||||
{
|
||||
"playwright": {
|
||||
"command": "npx",
|
||||
"args": ["@playwright/mcp@latest", "--headless"],
|
||||
"args": ["@playwright/mcp@latest", "--headless", "--isolated"],
|
||||
"timeout": 60000,
|
||||
"perSession": true,
|
||||
"env": {
|
||||
"PLAYWRIGHT_BROWSERS_PATH": "/tmp/browsers"
|
||||
}
|
||||
@@ -120,6 +122,8 @@ MCPHub 使用几个配置文件:
|
||||
}
|
||||
```
|
||||
|
||||
**注意**: `--isolated` 标志确保每个浏览器会话是隔离的,而 `perSession: true` 为每个用户会话创建单独的服务器实例,防止并发用户之间的状态泄漏。
|
||||
|
||||
### 文件和系统服务器
|
||||
|
||||
#### 文件系统服务器
|
||||
|
||||
@@ -14,8 +14,10 @@
|
||||
"command": "npx",
|
||||
"args": [
|
||||
"@playwright/mcp@latest",
|
||||
"--headless"
|
||||
]
|
||||
"--headless",
|
||||
"--isolated"
|
||||
],
|
||||
"perSession": true
|
||||
},
|
||||
"fetch": {
|
||||
"command": "uvx",
|
||||
|
||||
@@ -24,6 +24,10 @@ import { getServerDao, ServerConfigWithName } from '../dao/index.js';
|
||||
|
||||
const servers: { [sessionId: string]: Server } = {};
|
||||
|
||||
// Per-session server instances for servers with perSession=true
|
||||
// Key format: `${sessionId}:${serverName}`
|
||||
const perSessionServerInfos: { [key: string]: ServerInfo } = {};
|
||||
|
||||
const serverDao = getServerDao();
|
||||
|
||||
// Helper function to set up keep-alive ping for SSE connections
|
||||
@@ -79,6 +83,8 @@ export const getMcpServer = (sessionId?: string, group?: string): Server => {
|
||||
|
||||
export const deleteMcpServer = (sessionId: string): void => {
|
||||
delete servers[sessionId];
|
||||
// Clean up any per-session servers for this session
|
||||
cleanupPerSessionServers(sessionId);
|
||||
};
|
||||
|
||||
export const notifyToolChanged = async (name?: string) => {
|
||||
@@ -223,6 +229,144 @@ const createTransportFromConfig = (name: string, conf: ServerConfig): any => {
|
||||
return transport;
|
||||
};
|
||||
|
||||
// Helper function to get or create per-session server instance
|
||||
export const getOrCreatePerSessionServer = async (
|
||||
sessionId: string,
|
||||
serverName: string,
|
||||
serverConfig: ServerConfig,
|
||||
): Promise<ServerInfo> => {
|
||||
const key = `${sessionId}:${serverName}`;
|
||||
|
||||
// Return existing session server if it exists
|
||||
if (perSessionServerInfos[key]) {
|
||||
return perSessionServerInfos[key];
|
||||
}
|
||||
|
||||
console.log(`Creating per-session server instance for session ${sessionId}, server ${serverName}`);
|
||||
|
||||
// Create new transport for this session
|
||||
const transport = createTransportFromConfig(serverName, serverConfig);
|
||||
|
||||
const client = new Client(
|
||||
{
|
||||
name: `mcp-client-${serverName}-${sessionId}`,
|
||||
version: '1.0.0',
|
||||
},
|
||||
{
|
||||
capabilities: {
|
||||
prompts: {},
|
||||
resources: {},
|
||||
tools: {},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
// Get request options from server configuration, with fallbacks
|
||||
const serverRequestOptions = serverConfig.options || {};
|
||||
const requestOptions = {
|
||||
timeout: serverRequestOptions.timeout || 60000,
|
||||
resetTimeoutOnProgress: serverRequestOptions.resetTimeoutOnProgress || false,
|
||||
maxTotalTimeout: serverRequestOptions.maxTotalTimeout,
|
||||
};
|
||||
|
||||
// Create server info for this session
|
||||
const serverInfo: ServerInfo = {
|
||||
name: serverName,
|
||||
owner: serverConfig.owner,
|
||||
status: 'connecting',
|
||||
error: null,
|
||||
tools: [],
|
||||
prompts: [],
|
||||
client,
|
||||
transport,
|
||||
options: requestOptions,
|
||||
createTime: Date.now(),
|
||||
config: serverConfig,
|
||||
sessionId: sessionId,
|
||||
};
|
||||
|
||||
perSessionServerInfos[key] = serverInfo;
|
||||
|
||||
// Connect asynchronously
|
||||
client
|
||||
.connect(transport, requestOptions)
|
||||
.then(() => {
|
||||
console.log(`Successfully connected per-session client for server: ${serverName}, session: ${sessionId}`);
|
||||
const capabilities = client.getServerCapabilities();
|
||||
|
||||
if (capabilities?.tools) {
|
||||
client
|
||||
.listTools({}, requestOptions)
|
||||
.then((tools) => {
|
||||
console.log(`Successfully listed ${tools.tools.length} tools for per-session server: ${serverName}, session: ${sessionId}`);
|
||||
serverInfo.tools = tools.tools.map((tool) => ({
|
||||
name: `${serverName}${getNameSeparator()}${tool.name}`,
|
||||
description: tool.description || '',
|
||||
inputSchema: cleanInputSchema(tool.inputSchema || {}),
|
||||
}));
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error(`Failed to list tools for per-session server ${serverName}, session ${sessionId}:`, error);
|
||||
});
|
||||
}
|
||||
|
||||
if (capabilities?.prompts) {
|
||||
client
|
||||
.listPrompts({}, requestOptions)
|
||||
.then((prompts) => {
|
||||
console.log(`Successfully listed ${prompts.prompts.length} prompts for per-session server: ${serverName}, session: ${sessionId}`);
|
||||
serverInfo.prompts = prompts.prompts.map((prompt) => ({
|
||||
name: `${serverName}${getNameSeparator()}${prompt.name}`,
|
||||
title: prompt.title,
|
||||
description: prompt.description,
|
||||
arguments: prompt.arguments,
|
||||
}));
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error(`Failed to list prompts for per-session server ${serverName}, session ${sessionId}:`, error);
|
||||
});
|
||||
}
|
||||
|
||||
serverInfo.status = 'connected';
|
||||
serverInfo.error = null;
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error(`Failed to connect per-session client for server ${serverName}, session ${sessionId}:`, error);
|
||||
serverInfo.status = 'disconnected';
|
||||
serverInfo.error = `Failed to connect: ${error.stack}`;
|
||||
});
|
||||
|
||||
return serverInfo;
|
||||
};
|
||||
|
||||
// Helper function to clean up per-session servers for a session
|
||||
export const cleanupPerSessionServers = (sessionId: string): void => {
|
||||
const keysToDelete: string[] = [];
|
||||
|
||||
for (const key in perSessionServerInfos) {
|
||||
if (key.startsWith(`${sessionId}:`)) {
|
||||
const serverInfo = perSessionServerInfos[key];
|
||||
try {
|
||||
if (serverInfo.client) {
|
||||
serverInfo.client.close();
|
||||
}
|
||||
if (serverInfo.transport) {
|
||||
serverInfo.transport.close();
|
||||
}
|
||||
if (serverInfo.keepAliveIntervalId) {
|
||||
clearInterval(serverInfo.keepAliveIntervalId);
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn(`Error closing per-session server ${key}:`, error);
|
||||
}
|
||||
keysToDelete.push(key);
|
||||
}
|
||||
}
|
||||
|
||||
keysToDelete.forEach(key => delete perSessionServerInfos[key]);
|
||||
console.log(`Cleaned up ${keysToDelete.length} per-session servers for session ${sessionId}`);
|
||||
};
|
||||
|
||||
// Helper function to handle client.callTool with reconnection logic
|
||||
const callToolWithReconnect = async (
|
||||
serverInfo: ServerInfo,
|
||||
@@ -625,6 +769,45 @@ export const getServerByName = (name: string): ServerInfo | undefined => {
|
||||
return serverInfos.find((serverInfo) => serverInfo.name === name);
|
||||
};
|
||||
|
||||
// Get server by name with session support (for per-session servers)
|
||||
const getServerByNameWithSession = async (name: string, sessionId?: string): Promise<ServerInfo | undefined> => {
|
||||
// First check if this server is configured for per-session instances
|
||||
const serverConfig = await serverDao.findById(name);
|
||||
|
||||
if (serverConfig?.perSession && sessionId) {
|
||||
// Try to get or create per-session server
|
||||
const key = `${sessionId}:${name}`;
|
||||
if (perSessionServerInfos[key]) {
|
||||
return perSessionServerInfos[key];
|
||||
}
|
||||
// Create new per-session server instance
|
||||
return await getOrCreatePerSessionServer(sessionId, name, serverConfig);
|
||||
}
|
||||
|
||||
// Fall back to shared server
|
||||
return serverInfos.find((serverInfo) => serverInfo.name === name && !serverInfo.sessionId);
|
||||
};
|
||||
|
||||
// Get server by tool name with session support (for per-session servers)
|
||||
const getServerByToolWithSession = async (toolName: string, sessionId?: string): Promise<ServerInfo | undefined> => {
|
||||
// First try to find in per-session servers if sessionId is provided
|
||||
if (sessionId) {
|
||||
for (const key in perSessionServerInfos) {
|
||||
if (key.startsWith(`${sessionId}:`)) {
|
||||
const serverInfo = perSessionServerInfos[key];
|
||||
if (serverInfo.tools.some((tool) => tool.name === toolName)) {
|
||||
return serverInfo;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Fall back to shared servers
|
||||
return serverInfos.find((serverInfo) =>
|
||||
!serverInfo.sessionId && serverInfo.tools.some((tool) => tool.name === toolName)
|
||||
);
|
||||
};
|
||||
|
||||
// Filter tools by server configuration
|
||||
const filterToolsByConfig = async (serverName: string, tools: Tool[]): Promise<Tool[]> => {
|
||||
const serverConfig = await serverDao.findById(serverName);
|
||||
@@ -640,8 +823,8 @@ const filterToolsByConfig = async (serverName: string, tools: Tool[]): Promise<T
|
||||
});
|
||||
};
|
||||
|
||||
// Get server by tool name
|
||||
const getServerByTool = (toolName: string): ServerInfo | undefined => {
|
||||
// Get server by tool name (legacy - use getServerByToolWithSession instead)
|
||||
const _getServerByTool = (toolName: string): ServerInfo | undefined => {
|
||||
return serverInfos.find((serverInfo) => serverInfo.tools.some((tool) => tool.name === toolName));
|
||||
};
|
||||
|
||||
@@ -826,15 +1009,35 @@ Available servers: ${serversList}`;
|
||||
};
|
||||
}
|
||||
|
||||
const allServerInfos = getDataService()
|
||||
// Get shared servers
|
||||
let allServerInfos = getDataService()
|
||||
.filterData(serverInfos)
|
||||
.filter((serverInfo) => {
|
||||
if (serverInfo.enabled === false) return false;
|
||||
if (serverInfo.sessionId) return false; // Exclude per-session servers from shared list
|
||||
if (!group) return true;
|
||||
const serversInGroup = getServersInGroup(group);
|
||||
if (!serversInGroup || serversInGroup.length === 0) return serverInfo.name === group;
|
||||
return serversInGroup.includes(serverInfo.name);
|
||||
});
|
||||
|
||||
// Add per-session servers for this session
|
||||
if (sessionId) {
|
||||
const sessionServers = Object.values(perSessionServerInfos).filter(
|
||||
(serverInfo) => serverInfo.sessionId === sessionId && serverInfo.status === 'connected'
|
||||
);
|
||||
|
||||
// Filter session servers by group if applicable
|
||||
const filteredSessionServers = sessionServers.filter((serverInfo) => {
|
||||
if (serverInfo.enabled === false) return false;
|
||||
if (!group) return true;
|
||||
const serversInGroup = getServersInGroup(group);
|
||||
if (!serversInGroup || serversInGroup.length === 0) return serverInfo.name === group;
|
||||
return serversInGroup.includes(serverInfo.name);
|
||||
});
|
||||
|
||||
allServerInfos = [...allServerInfos, ...filteredSessionServers];
|
||||
}
|
||||
|
||||
const allTools = [];
|
||||
for (const serverInfo of allServerInfos) {
|
||||
@@ -998,17 +1201,13 @@ export const handleCallToolRequest = async (request: any, extra: any) => {
|
||||
}
|
||||
|
||||
const { arguments: toolArgs = {} } = request.params.arguments || {};
|
||||
const sessionId = extra?.sessionId;
|
||||
let targetServerInfo: ServerInfo | undefined;
|
||||
if (extra && extra.server) {
|
||||
targetServerInfo = getServerByName(extra.server);
|
||||
targetServerInfo = await getServerByNameWithSession(extra.server, sessionId);
|
||||
} else {
|
||||
// Find the first server that has this tool
|
||||
targetServerInfo = serverInfos.find(
|
||||
(serverInfo) =>
|
||||
serverInfo.status === 'connected' &&
|
||||
serverInfo.enabled !== false &&
|
||||
serverInfo.tools.some((tool) => tool.name === toolName),
|
||||
);
|
||||
// Find the first server that has this tool (session-aware)
|
||||
targetServerInfo = await getServerByToolWithSession(toolName, sessionId);
|
||||
}
|
||||
|
||||
if (!targetServerInfo) {
|
||||
@@ -1114,7 +1313,8 @@ export const handleCallToolRequest = async (request: any, extra: any) => {
|
||||
}
|
||||
|
||||
// Regular tool handling
|
||||
const serverInfo = getServerByTool(request.params.name);
|
||||
const sessionId = extra?.sessionId;
|
||||
const serverInfo = await getServerByToolWithSession(request.params.name, sessionId);
|
||||
if (!serverInfo) {
|
||||
throw new Error(`Server not found: ${request.params.name}`);
|
||||
}
|
||||
|
||||
@@ -178,6 +178,7 @@ export interface ServerConfig {
|
||||
enabled?: boolean; // Flag to enable/disable the server
|
||||
owner?: string; // Owner of the server, defaults to 'admin' user
|
||||
keepAliveInterval?: number; // Keep-alive ping interval in milliseconds (default: 60000ms for SSE servers)
|
||||
perSession?: boolean; // If true, creates a separate server instance for each session (useful for stateful servers like playwright)
|
||||
tools?: Record<string, { enabled: boolean; description?: string }>; // Tool-specific configurations with enable/disable state and custom descriptions
|
||||
prompts?: Record<string, { enabled: boolean; description?: string }>; // Prompt-specific configurations with enable/disable state and custom descriptions
|
||||
options?: Partial<Pick<RequestOptions, 'timeout' | 'resetTimeoutOnProgress' | 'maxTotalTimeout'>>; // MCP request options configuration
|
||||
@@ -239,6 +240,7 @@ export interface ServerInfo {
|
||||
enabled?: boolean; // Flag to indicate if the server is enabled
|
||||
keepAliveIntervalId?: NodeJS.Timeout; // Timer ID for keep-alive ping interval
|
||||
config?: ServerConfig; // Reference to the original server configuration for OpenAPI passthrough headers
|
||||
sessionId?: string; // Session ID for per-session server instances (undefined for shared servers)
|
||||
}
|
||||
|
||||
// Details about a tool available on the server
|
||||
|
||||
112
tests/services/perSessionServers.test.ts
Normal file
112
tests/services/perSessionServers.test.ts
Normal file
@@ -0,0 +1,112 @@
|
||||
import {
|
||||
getOrCreatePerSessionServer,
|
||||
cleanupPerSessionServers,
|
||||
handleCallToolRequest,
|
||||
} from '../../src/services/mcpService';
|
||||
import { ServerConfig } from '../../src/types';
|
||||
|
||||
// Mock the serverDao
|
||||
jest.mock('../../src/dao/index.js', () => ({
|
||||
getServerDao: () => ({
|
||||
findById: jest.fn((name: string) => {
|
||||
if (name === 'playwright') {
|
||||
return Promise.resolve({
|
||||
name: 'playwright',
|
||||
command: 'npx',
|
||||
args: ['@playwright/mcp@latest', '--headless', '--isolated'],
|
||||
perSession: true,
|
||||
enabled: true,
|
||||
});
|
||||
}
|
||||
return Promise.resolve(null);
|
||||
}),
|
||||
findAll: jest.fn(() => Promise.resolve([])),
|
||||
}),
|
||||
}));
|
||||
|
||||
// Mock the Client and Transport classes
|
||||
jest.mock('@modelcontextprotocol/sdk/client/index.js', () => ({
|
||||
Client: jest.fn().mockImplementation(() => ({
|
||||
connect: jest.fn(() => Promise.resolve()),
|
||||
close: jest.fn(),
|
||||
listTools: jest.fn(() => Promise.resolve({ tools: [] })),
|
||||
listPrompts: jest.fn(() => Promise.resolve({ prompts: [] })),
|
||||
getServerCapabilities: jest.fn(() => ({ tools: true, prompts: true })),
|
||||
callTool: jest.fn((params) => Promise.resolve({ content: [{ type: 'text', text: `Tool ${params.name} called` }] })),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('@modelcontextprotocol/sdk/client/stdio.js', () => ({
|
||||
StdioClientTransport: jest.fn().mockImplementation(() => ({
|
||||
close: jest.fn(),
|
||||
stderr: {
|
||||
on: jest.fn(),
|
||||
},
|
||||
})),
|
||||
}));
|
||||
|
||||
describe('Per-Session Server Instances', () => {
|
||||
afterEach(() => {
|
||||
// Clean up any created sessions
|
||||
cleanupPerSessionServers('session1');
|
||||
cleanupPerSessionServers('session2');
|
||||
});
|
||||
|
||||
it('should create separate server instances for different sessions', async () => {
|
||||
const config: ServerConfig = {
|
||||
command: 'npx',
|
||||
args: ['@playwright/mcp@latest', '--headless', '--isolated'],
|
||||
perSession: true,
|
||||
};
|
||||
|
||||
// Create server for session1
|
||||
const server1 = await getOrCreatePerSessionServer('session1', 'playwright', config);
|
||||
expect(server1).toBeDefined();
|
||||
expect(server1.sessionId).toBe('session1');
|
||||
|
||||
// Create server for session2
|
||||
const server2 = await getOrCreatePerSessionServer('session2', 'playwright', config);
|
||||
expect(server2).toBeDefined();
|
||||
expect(server2.sessionId).toBe('session2');
|
||||
|
||||
// They should be different instances
|
||||
expect(server1).not.toBe(server2);
|
||||
});
|
||||
|
||||
it('should reuse existing per-session server for the same session', async () => {
|
||||
const config: ServerConfig = {
|
||||
command: 'npx',
|
||||
args: ['@playwright/mcp@latest', '--headless', '--isolated'],
|
||||
perSession: true,
|
||||
};
|
||||
|
||||
// Create server for session1
|
||||
const server1 = await getOrCreatePerSessionServer('session1', 'playwright', config);
|
||||
|
||||
// Request the same server again
|
||||
const server2 = await getOrCreatePerSessionServer('session1', 'playwright', config);
|
||||
|
||||
// Should be the same instance
|
||||
expect(server1).toBe(server2);
|
||||
});
|
||||
|
||||
it('should clean up per-session servers when session ends', async () => {
|
||||
const config: ServerConfig = {
|
||||
command: 'npx',
|
||||
args: ['@playwright/mcp@latest', '--headless', '--isolated'],
|
||||
perSession: true,
|
||||
};
|
||||
|
||||
// Create server for session1
|
||||
const server1 = await getOrCreatePerSessionServer('session1', 'playwright', config);
|
||||
expect(server1).toBeDefined();
|
||||
|
||||
// Clean up session1
|
||||
cleanupPerSessionServers('session1');
|
||||
|
||||
// Create again should create a new instance (not the same object)
|
||||
const server2 = await getOrCreatePerSessionServer('session1', 'playwright', config);
|
||||
expect(server2).toBeDefined();
|
||||
expect(server2).not.toBe(server1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user