diff --git a/server/src/nest/admin/admin.controller.ts b/server/src/nest/admin/admin.controller.ts index 5bd368f80..4639766ba 100644 --- a/server/src/nest/admin/admin.controller.ts +++ b/server/src/nest/admin/admin.controller.ts @@ -260,7 +260,12 @@ export class AdminController { // plugin that transitively depends on them) so a plugin never runs against a // disabled addon (#plugins dependencies). if (result.addon && result.addon.enabled === false && (body as { enabled?: boolean })?.enabled === false) { - await this.pluginRuntime.deactivateForDisabledAddon(id); + const stopped = await this.pluginRuntime.deactivateForDisabledAddon(id); + // A stopped plugin takes its MCP tools off the surface, and the addon that + // cascaded here is not necessarily an MCP-relevant one, so mcpAffected + // above does not cover this. Still conditional on something actually + // stopping, which keeps the #1414 rule that a no-op save kills nothing. + if (stopped.length && !result.mcpAffected) this.admin.invalidateMcpSessions(); } return { addon: result.addon }; } diff --git a/server/src/nest/plugins/plugins.controller.ts b/server/src/nest/plugins/plugins.controller.ts index 189951bad..7067d5720 100644 --- a/server/src/nest/plugins/plugins.controller.ts +++ b/server/src/nest/plugins/plugins.controller.ts @@ -14,6 +14,10 @@ import { devLinkEnabled } from './dev-link'; import { PluginActivateDto, PluginConfigDto, PluginEgressHostsDto, PluginInstallDto, PluginLinkDto, PluginRetrustDto, PluginUninstallDto, PluginUpdateDto } from './plugins.dto'; import { ManagedForbidden, isManagedBlocked, MANAGED_FORBIDDEN_ERROR } from '../common/managed'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; +// Straight from sessionManager, not the src/mcp barrel: that one evaluates +// readEnv().mcp at module scope and installs the sweep interval, which a domain +// module must not drag into every test that mocks app-config partially. +import { invalidateMcpSessions } from '../../mcp/sessionManager'; /** * Flatten a registry/install failure into the error envelope — CARRYING THE CODE. @@ -188,6 +192,7 @@ export class PluginsController { } throw new HttpException({ error: e instanceof Error ? e.message : 'activation failed' }, 400); } + invalidateMcpSessions(); return { status: this.runtime.isActive(id) ? 'active' : 'error' }; } @@ -197,6 +202,7 @@ export class PluginsController { // Cascade: disabling a plugin also disables everything that depends on it (a // dependent can't run without its dependency). The client refresh reflects it. await this.runtime.deactivateWithDependents(id); + invalidateMcpSessions(); return { status: 'inactive' }; } @@ -236,6 +242,7 @@ export class PluginsController { // (any path) releases a stale hold. Only after success — a failed update // changed nothing and must not touch the flag. await this.registry.recomputeUpdateHold(id, res.version, !!body?.version); + invalidateMcpSessions(); return res; } catch (e) { throw registryFailure(e, 'update failed'); @@ -274,7 +281,9 @@ export class PluginsController { if (!body?.version) throw new HttpException({ error: 'version is required' }, 400); if (!body?.publicKey) throw new HttpException({ error: 'publicKey is required' }, 400); try { - return await this.runtime.retrust(id, body.version, body.publicKey, { userId: user?.id ?? null, ip: getClientIp(req) }); + const res = await this.runtime.retrust(id, body.version, body.publicKey, { userId: user?.id ?? null, ip: getClientIp(req) }); + invalidateMcpSessions(); + return res; } catch (e) { throw registryFailure(e, 'retrust failed'); } @@ -284,6 +293,7 @@ export class PluginsController { @HttpCode(200) async uninstall(@Param('id') id: string, @Body() body: PluginUninstallDto) { await this.runtime.uninstall(id, !!body?.deleteData); + invalidateMcpSessions(); return { status: 'uninstalled' }; } diff --git a/server/tests/unit/plugins/mcp-session-invalidation.test.ts b/server/tests/unit/plugins/mcp-session-invalidation.test.ts new file mode 100644 index 000000000..486968803 --- /dev/null +++ b/server/tests/unit/plugins/mcp-session-invalidation.test.ts @@ -0,0 +1,87 @@ +/** + * MCP session invalidation on plugin lifecycle changes — MCPINV-001 to MCPINV-007. + * + * The advertised tool surface is frozen at initialize (no event store is + * configured, and tools/list_changed would not help), so anything that changes + * which plugin tools exist has to end the sessions that were built before it. + * + * Unconditional by design: the surface is per-session, because it depends on the + * caller's scopes and the plugin's grants, so there is no ctx-free set to diff. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { PluginsController } from '../../../src/nest/plugins/plugins.controller'; +import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; + +const invalidate = vi.fn(); +vi.mock('../../../src/mcp/sessionManager', () => ({ invalidateMcpSessions: () => invalidate() })); + +/* eslint-disable @typescript-eslint/no-explicit-any */ +const registry = { recomputeUpdateHold: vi.fn(async () => {}) } as any; +const ctrl = (runtime: any) => + new PluginsController({} as any, runtime, registry, { isManaged: () => false } as unknown as RuntimeEnvService); + +const runtime = () => + ({ + activate: vi.fn(async () => {}), + isActive: vi.fn(() => true), + deactivateWithDependents: vi.fn(async () => {}), + update: vi.fn(async () => ({ version: '1.1.0' })), + retrust: vi.fn(async () => ({ ok: true })), + uninstall: vi.fn(async () => {}), + }) as any; +/* eslint-enable @typescript-eslint/no-explicit-any */ + +beforeEach(() => { + invalidate.mockClear(); + process.env.TREK_PLUGINS_ENABLED = 'true'; +}); + +describe('plugin lifecycle invalidates MCP sessions', () => { + it('MCPINV-001: activate', async () => { + await ctrl(runtime()).activate('weather', { consent: true } as never); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('MCPINV-002: deactivate', async () => { + await ctrl(runtime()).deactivate('weather'); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('MCPINV-003: update', async () => { + // A version bump can change the declared tools, so it is a surface change + // even though the plugin stays active. + await ctrl(runtime()).update('weather', {} as never); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('MCPINV-004: uninstall', async () => { + await ctrl(runtime()).uninstall('weather', {} as never); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('MCPINV-005: retrust', async () => { + // Retrust installs a version, so it is an update by another name. + await ctrl(runtime()).retrust( + 'weather', + { version: '1.1.0', publicKey: 'k' } as never, + { id: 1 }, + { headers: {}, socket: {} } as never, + ); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('MCPINV-006: a failed activate does not invalidate', async () => { + // Nothing changed, so nobody's session should be dropped. + const r = runtime(); + r.activate = vi.fn(async () => { throw new Error('boom'); }); + await expect(ctrl(r).activate('weather', {} as never)).rejects.toThrow(); + expect(invalidate).not.toHaveBeenCalled(); + }); + + it('MCPINV-007: a failed update does not invalidate', async () => { + const r = runtime(); + r.update = vi.fn(async () => { throw new Error('boom'); }); + await expect(ctrl(r).update('weather', {} as never)).rejects.toThrow(); + expect(invalidate).not.toHaveBeenCalled(); + }); +});