chore(mcp): McpService.onModuleDestroy → destroyAllSessions + удаление мёртвого кода (#486)
onModuleDestroy чистил только sweep-таймер; loopback-collab-сессии держали доки открытыми на collab-сервере до идла — рестарт мог гонять с доком, запиненным умирающим воркером. Теперь дергает destroyAllSessions() через переопределяемый шов (для юнит-теста без ESM-пакета), best-effort. Плюс удалён мёртвый код: неиспользуемый импорт parseNodeArg в mcp/index.ts и мёртвые enum-члены SEARCH_REMOVE_* в queue.constants (подтверждено grep'ом — ни диспетчера, ни процессора). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,7 @@ import {
|
||||
} from './mcp-auth.helpers';
|
||||
import { JwtType } from '../../core/auth/dto/jwt-payload';
|
||||
import { CREDENTIALS_MISMATCH_MESSAGE } from '../../core/auth/auth.constants';
|
||||
import { McpService } from './mcp.service';
|
||||
|
||||
// The /mcp per-user auth decision logic is tested through the framework-free
|
||||
// `resolveMcpSessionConfig` helper that McpService delegates to. McpService
|
||||
@@ -1179,3 +1180,46 @@ describe('mapAuthResultToResponse (handle status/body mapping, refactor R2)', ()
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// #486: onModuleDestroy must ALSO tear down the live loopback CollabSessions, not
|
||||
// just clear the sweep timer — otherwise the embedded MCP's collab sockets keep
|
||||
// docs pinned open on the collab server past process exit. The teardown goes
|
||||
// through an overridable seam (destroyAllMcpSessions) so it can be spied without
|
||||
// loading the ESM-only @docmost/mcp package.
|
||||
describe('McpService.onModuleDestroy — CollabSession teardown (#486)', () => {
|
||||
function makeService(): McpService {
|
||||
// The constructor only stores its deps and starts the (unref'd) sweep timer,
|
||||
// so bare stubs suffice. onModuleDestroy clears that timer, so no leak.
|
||||
return new McpService(
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
);
|
||||
}
|
||||
|
||||
it('destroys all sessions AND clears the sweep timer on shutdown', async () => {
|
||||
const svc = makeService();
|
||||
const destroy = jest.fn().mockResolvedValue(undefined);
|
||||
(svc as any).destroyAllMcpSessions = destroy;
|
||||
const clearSpy = jest.spyOn(global, 'clearInterval');
|
||||
|
||||
await svc.onModuleDestroy();
|
||||
|
||||
expect(destroy).toHaveBeenCalledTimes(1);
|
||||
expect(clearSpy).toHaveBeenCalledWith((svc as any).sweepTimer);
|
||||
clearSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('swallows a teardown failure so shutdown never throws', async () => {
|
||||
const svc = makeService();
|
||||
(svc as any).destroyAllMcpSessions = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('collab teardown boom'));
|
||||
|
||||
await expect(svc.onModuleDestroy()).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -119,10 +119,42 @@ export class McpService implements OnModuleDestroy {
|
||||
this.sweepTimer.unref?.();
|
||||
}
|
||||
|
||||
onModuleDestroy(): void {
|
||||
async onModuleDestroy(): Promise<void> {
|
||||
clearInterval(this.sweepTimer);
|
||||
// Tear down any live loopback CollabSession providers at shutdown (#486). The
|
||||
// embedded MCP (and the in-app AI agent) open Hocuspocus collab sockets against
|
||||
// THIS process; without an explicit teardown those sessions keep their docs
|
||||
// "open" on the collab server and hold providers/buffers until they idle out,
|
||||
// so a restart can race a doc still pinned by the dying worker. Best-effort:
|
||||
// any failure is logged, never allowed to break shutdown.
|
||||
try {
|
||||
await this.destroyAllMcpSessions();
|
||||
} catch (err) {
|
||||
this.logger.error(
|
||||
'MCP CollabSession teardown on shutdown failed',
|
||||
err as Error,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve @docmost/mcp's `destroyAllSessions` and invoke it (#486). The live
|
||||
* CollabSession registry is a module-level singleton in the ESM package, shared
|
||||
* by every entry (`.`/`./http`), so this tears down ALL sessions regardless of
|
||||
* which surface opened them. The module is already loaded whenever MCP was used;
|
||||
* if it was never loaded (or is absent) the import + no-op is harmless.
|
||||
*
|
||||
* Held as an overridable field so a unit test can spy the teardown without
|
||||
* loading the ESM-only package or standing up the DI graph.
|
||||
*/
|
||||
private destroyAllMcpSessions: () => Promise<void> = async () => {
|
||||
const entry = require.resolve('@docmost/mcp');
|
||||
const mod = (await esmImport(pathToFileURL(entry).href)) as {
|
||||
destroyAllSessions?: () => void;
|
||||
};
|
||||
mod.destroyAllSessions?.();
|
||||
};
|
||||
|
||||
// Service account the embedded MCP uses to talk back to this Docmost
|
||||
// instance over loopback REST + the collaboration WebSocket. Now OPTIONAL:
|
||||
// it is only a fallback when no per-user Basic/Bearer credentials are sent.
|
||||
|
||||
@@ -31,9 +31,6 @@ export enum QueueJob {
|
||||
IMPORT_TASK = 'import-task',
|
||||
EXPORT_TASK = 'export-task',
|
||||
|
||||
SEARCH_REMOVE_PAGE = 'search-remove-page',
|
||||
SEARCH_REMOVE_ASSET = 'search-remove-attachment',
|
||||
SEARCH_REMOVE_FACE = 'search-remove-comment',
|
||||
TYPESENSE_FLUSH = 'typesense-flush',
|
||||
|
||||
PAGE_CREATED = 'page-created',
|
||||
|
||||
Reference in New Issue
Block a user