mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 22:46:32 +00:00
refactor(mcp): move invalidateMcpSessions into sessionManager
The body was a hand-inlined duplicate of sessionManager's own closeSession helper, so it moves next to revokeUserSessions and reuses it. The barrel re-exports it, leaving admin.service.ts and admin.controller.ts untouched. The reason to move it is that callers outside src/mcp/ are coming. 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, which is the hazard kill-switch.ts documents. sessionManager.ts has no module-scope side effects. The log line loses "due to addon state change"; addons stop being the only thing that invalidates a session surface. SESS-017 to SESS-020 cover it.
This commit is contained in:
+2
-12
@@ -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);
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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> = {}): 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<typeof vi.fn>).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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user