diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index 0ac801d4c..9334c888b 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -1,8 +1,8 @@ import { readEnv } from '../app-config'; import { sessions } from './sessionManager'; -import { revokeUserSessions, revokeUserSessionsForClient } from './sessionManager'; +import { invalidateMcpSessions, revokeUserSessions, revokeUserSessionsForClient } from './sessionManager'; -export { revokeUserSessions, revokeUserSessionsForClient }; +export { invalidateMcpSessions, revokeUserSessions, revokeUserSessionsForClient }; /** * Process-wide MCP state: the tuning knobs, the per-user rate-limit window, @@ -64,16 +64,6 @@ const sessionSweepInterval = setInterval(() => { // Prevent the interval from keeping the process alive if nothing else is running sessionSweepInterval.unref(); -/** Invalidate all active MCP sessions (call when addon state changes so sessions re-create with updated tools). */ -export function invalidateMcpSessions(): void { - for (const [sid, session] of sessions) { - try { session.server.close(); } catch { /* ignore */ } - try { session.transport.close(); } catch { /* ignore */ } - sessions.delete(sid); - } - console.log('[MCP] All sessions invalidated due to addon state change'); -} - /** Close all active MCP sessions (call during graceful shutdown). */ export function closeMcpSessions(): void { clearInterval(sessionSweepInterval); diff --git a/server/src/mcp/sessionManager.ts b/server/src/mcp/sessionManager.ts index f71b15f39..a0e260d83 100644 --- a/server/src/mcp/sessionManager.ts +++ b/server/src/mcp/sessionManager.ts @@ -42,6 +42,26 @@ export function revokeUserSessionsForClient(userId: number, clientId: string): v } } +/** + * Terminate every active MCP session, so the next request re-initializes and + * picks up a changed tool surface. + * + * Lives here rather than in the barrel because callers outside src/mcp/ need + * it: index.ts evaluates readEnv().mcp at module scope and installs the sweep + * interval, so importing the barrel from a domain module drags both into every + * test file that mocks app-config with a partial export set. This module has + * no module-scope side effects. + * + * Unconditional by design. The advertised surface is per-session (it depends on + * the caller's scopes and grants), so there is no ctx-free set to diff, and + * tools/list_changed is not an alternative: no event store is configured and + * the surface is frozen at initialize anyway. + */ +export function invalidateMcpSessions(): void { + for (const [sid, session] of sessions) closeSession(sid, session); + console.log('[MCP] All sessions invalidated after a tool-surface change'); +} + /** * Close the least-recently-active session for a user so a new one can take its slot, * and return its id (null when the user has no sessions). diff --git a/server/tests/unit/mcp/sessionManager.test.ts b/server/tests/unit/mcp/sessionManager.test.ts index b451aaaf4..934c54d9c 100644 --- a/server/tests/unit/mcp/sessionManager.test.ts +++ b/server/tests/unit/mcp/sessionManager.test.ts @@ -1,9 +1,10 @@ /** - * Unit tests for MCP sessionManager — SESS-001 to SESS-016. - * Covers revokeUserSessions, revokeUserSessionsForClient and evictOldestSessionForUser. + * Unit tests for MCP sessionManager — SESS-001 to SESS-020. + * Covers revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser + * and invalidateMcpSessions. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { sessions, revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser, McpSession } from '../../../src/mcp/sessionManager'; +import { sessions, revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser, invalidateMcpSessions, McpSession } from '../../../src/mcp/sessionManager'; function makeSession(overrides: Partial = {}): McpSession { return { @@ -179,3 +180,42 @@ describe('evictOldestSessionForUser', () => { expect(sessions.has('sid-1')).toBe(false); }); }); + +describe('invalidateMcpSessions', () => { + it('SESS-017: drops every session regardless of user or client', () => { + sessions.set('sid-1', makeSession({ userId: 1, clientId: 'client-a' })); + sessions.set('sid-2', makeSession({ userId: 2, clientId: null })); + + invalidateMcpSessions(); + + expect(sessions.size).toBe(0); + }); + + it('SESS-018: closes both halves of every session', () => { + const a = makeSession({ userId: 1 }); + const b = makeSession({ userId: 2 }); + sessions.set('sid-1', a); + sessions.set('sid-2', b); + + invalidateMcpSessions(); + + expect(a.server.close).toHaveBeenCalledOnce(); + expect(a.transport.close).toHaveBeenCalledOnce(); + expect(b.server.close).toHaveBeenCalledOnce(); + expect(b.transport.close).toHaveBeenCalledOnce(); + }); + + it('SESS-019: still empties the map when close() throws', () => { + const s = makeSession({ userId: 1 }); + (s.server.close as ReturnType).mockImplementation(() => { throw new Error('close failed'); }); + sessions.set('sid-1', s); + + expect(() => invalidateMcpSessions()).not.toThrow(); + expect(sessions.size).toBe(0); + }); + + it('SESS-020: is a no-op on an empty map', () => { + expect(() => invalidateMcpSessions()).not.toThrow(); + expect(sessions.size).toBe(0); + }); +});