P3 — Typed SPA chat (Phase P webUI) #1151
@@ -0,0 +1,44 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { McpClientService } from './mcp-client.service.js';
|
||||
|
||||
const MCP_LEAK_MARKER = 'MCP_LEAK_MARKER /srv/secret';
|
||||
|
||||
describe('McpClientService — failed connect error sanitization', () => {
|
||||
const originalMcpServers = process.env['MCP_SERVERS'];
|
||||
|
||||
beforeEach(() => {
|
||||
process.env['MCP_SERVERS'] = JSON.stringify([
|
||||
{ name: 'leaky-server', url: 'http://localhost:9999/mcp' },
|
||||
]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
if (originalMcpServers === undefined) {
|
||||
delete process.env['MCP_SERVERS'];
|
||||
} else {
|
||||
process.env['MCP_SERVERS'] = originalMcpServers;
|
||||
}
|
||||
});
|
||||
|
||||
it('stores a generic serverEntry.error while logging the raw exception server-side', async () => {
|
||||
vi.spyOn(Client.prototype, 'connect').mockRejectedValue(new Error(MCP_LEAK_MARKER));
|
||||
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
|
||||
|
||||
const service = new McpClientService();
|
||||
await service.onModuleInit();
|
||||
|
||||
const statuses = service.getServerStatuses();
|
||||
expect(statuses).toHaveLength(1);
|
||||
expect(statuses[0]?.connected).toBe(false);
|
||||
expect(statuses[0]?.error).toBe('Connection failed (see server logs).');
|
||||
expect(statuses[0]?.error).not.toContain(MCP_LEAK_MARKER);
|
||||
|
||||
const loggedRawMarker = errorSpy.mock.calls.some((call) =>
|
||||
call.some((arg) => typeof arg === 'string' && arg.includes(MCP_LEAK_MARKER)),
|
||||
);
|
||||
expect(loggedRawMarker).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -189,7 +189,7 @@ export class McpClientService implements OnModuleInit, OnModuleDestroy {
|
||||
);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
serverEntry.error = message;
|
||||
serverEntry.error = 'Connection failed (see server logs).';
|
||||
serverEntry.connected = false;
|
||||
this.logger.error(`Failed to connect to MCP server "${config.name}": ${message}`);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import type { SlashCommandPayload, SystemReloadPayload } from '@mosaicstack/types';
|
||||
import { ReloadService } from './reload.service.js';
|
||||
import { CommandExecutorService } from '../commands/command-executor.service.js';
|
||||
|
||||
function createMockCommandRegistry() {
|
||||
return {
|
||||
@@ -104,3 +107,79 @@ describe('ReloadService', () => {
|
||||
expect(() => service.registerPlugin('my-plugin', {})).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('ReloadService — /reload command sanitizes plugin errors', () => {
|
||||
it('generic per-plugin errors reach the chat surface while raw markers stay server-side only', async () => {
|
||||
const registry = {
|
||||
getManifest: vi.fn().mockReturnValue({
|
||||
version: 1,
|
||||
commands: [
|
||||
{ name: 'reload', aliases: [], scope: 'core', execution: 'socket', available: true },
|
||||
],
|
||||
skills: [],
|
||||
}),
|
||||
};
|
||||
const reloadService = new ReloadService(registry as never);
|
||||
|
||||
const RELOAD_LOAD_LEAK_MARKER = 'RELOAD_LOAD_LEAK_MARKER /srv/load-secret';
|
||||
const RELOAD_UNLOAD_LEAK_MARKER = 'RELOAD_UNLOAD_LEAK_MARKER /srv/unload-secret';
|
||||
|
||||
reloadService.registerPlugin('unload-fails', {
|
||||
pluginName: 'unload-fails',
|
||||
onLoad: vi.fn().mockResolvedValue(undefined),
|
||||
onUnload: vi.fn().mockRejectedValue(new Error(RELOAD_UNLOAD_LEAK_MARKER)),
|
||||
});
|
||||
reloadService.registerPlugin('load-fails', {
|
||||
pluginName: 'load-fails',
|
||||
onLoad: vi.fn().mockRejectedValue(new Error(RELOAD_LOAD_LEAK_MARKER)),
|
||||
onUnload: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
|
||||
const broadcastReload = vi.fn();
|
||||
const mockChatGateway = { broadcastReload };
|
||||
const mockAgentService = { getSession: vi.fn(), applyAgentConfig: vi.fn() };
|
||||
const mockSystemOverride = { set: vi.fn(), get: vi.fn(), clear: vi.fn() };
|
||||
const mockSessionGC = { sweepOrphans: vi.fn() };
|
||||
const mockBrain = { agents: { findByName: vi.fn(), findById: vi.fn(), create: vi.fn() } };
|
||||
|
||||
const executor = new CommandExecutorService(
|
||||
registry as never,
|
||||
mockAgentService as never,
|
||||
mockSystemOverride as never,
|
||||
mockSessionGC as never,
|
||||
null,
|
||||
mockBrain as never,
|
||||
reloadService,
|
||||
mockChatGateway as never,
|
||||
null,
|
||||
);
|
||||
|
||||
const payload: SlashCommandPayload = { command: 'reload', conversationId: 'conv-1' };
|
||||
const result = await executor.execute(payload, { userId: 'user-1', tenantId: 'user-1' });
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.message).toContain('unload-fails: unload failed (internal error)');
|
||||
expect(result.message).toContain('load-fails: load failed (internal error)');
|
||||
expect(result.message).not.toContain(RELOAD_UNLOAD_LEAK_MARKER);
|
||||
expect(result.message).not.toContain(RELOAD_LOAD_LEAK_MARKER);
|
||||
|
||||
expect(broadcastReload).toHaveBeenCalledOnce();
|
||||
const broadcastPayload = broadcastReload.mock.calls[0]?.[0] as SystemReloadPayload;
|
||||
expect(broadcastPayload.message).toContain('unload-fails: unload failed (internal error)');
|
||||
expect(broadcastPayload.message).toContain('load-fails: load failed (internal error)');
|
||||
expect(broadcastPayload.message).not.toContain(RELOAD_UNLOAD_LEAK_MARKER);
|
||||
expect(broadcastPayload.message).not.toContain(RELOAD_LOAD_LEAK_MARKER);
|
||||
|
||||
const loggedUnloadMarker = errorSpy.mock.calls.some((call) =>
|
||||
call.some((arg) => typeof arg === 'string' && arg.includes(RELOAD_UNLOAD_LEAK_MARKER)),
|
||||
);
|
||||
const loggedLoadMarker = errorSpy.mock.calls.some((call) =>
|
||||
call.some((arg) => typeof arg === 'string' && arg.includes(RELOAD_LOAD_LEAK_MARKER)),
|
||||
);
|
||||
expect(loggedUnloadMarker).toBe(true);
|
||||
expect(loggedLoadMarker).toBe(true);
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,7 +58,8 @@ export class ReloadService implements OnApplicationBootstrap, OnApplicationShutd
|
||||
await plugin.onUnload();
|
||||
reloaded.push(name);
|
||||
} catch (err) {
|
||||
errors.push(`${name}: unload failed — ${err}`);
|
||||
this.logger.error(`Plugin "${name}" failed during onUnload: ${err}`);
|
||||
errors.push(`${name}: unload failed (internal error)`);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -69,7 +70,8 @@ export class ReloadService implements OnApplicationBootstrap, OnApplicationShutd
|
||||
try {
|
||||
await plugin.onLoad();
|
||||
} catch (err) {
|
||||
errors.push(`${name}: load failed — ${err}`);
|
||||
this.logger.error(`Plugin "${name}" failed during onLoad: ${err}`);
|
||||
errors.push(`${name}: load failed (internal error)`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user