feat(agents): mcpd routes + RBAC + tool dispatcher (Stage 3)
Wires the Stage 2 services into HTTP. New routes:
GET /api/v1/agents — list
GET /api/v1/agents/:idOrName — describe
POST /api/v1/agents — create
PUT /api/v1/agents/:idOrName — update
DELETE /api/v1/agents/:idOrName — delete
GET /api/v1/projects/:p/agents — project-scoped list (mcplocal disco)
POST /api/v1/agents/:name/chat — chat (non-streaming or SSE stream)
POST /api/v1/agents/:name/threads — create thread explicitly
GET /api/v1/agents/:name/threads — list threads
GET /api/v1/threads/:id/messages — replay history
The chat endpoint reuses the SSE pattern from llm-infer.ts (same headers
incl. X-Accel-Buffering:no, same `data: …\n\n` framing, same `[DONE]`
terminator). Each ChatService chunk is one frame. Non-streaming returns
{threadId, assistant, turnIndex} as JSON.
RBAC mapping in main.ts:mapUrlToPermission:
- /agents/:name/{chat,threads*} → run:agents:<name>
- /threads/:id/* → view:agents (service-level owner check
handles fine-grained access since the URL doesn't carry the agent name)
- /agents and /agents/:idOrName → default {GET:view, POST:create,
PUT:edit, DELETE:delete} on resource 'agents'.
'agents' added to nameResolvers so RBAC's CUID→name lookup works.
ChatToolDispatcherImpl bridges ChatService to McpProxyService: it lists a
project's MCP servers, fans out tools/list calls to each, namespaces tool
names as `<server>__<tool>`, and routes tools/call back to the right
serverId on dispatch. tools/list errors on a single server are logged and
that server's tools are dropped from the turn's tool surface — one bad
server doesn't poison the whole list.
Tests:
agent-routes.test.ts (15) — full HTTP CRUD round-trip, 404/409 paths,
project-scoped list, non-streaming + SSE chat, thread create/list,
/threads/:id/messages replay, body-required 400.
chat-tool-dispatcher.test.ts (7) — empty list when no project / no
servers, namespacing + inputSchema forwarding, partial-failure
skipping with audit log, callTool dispatch shape, missing-server
rejection, JSON-RPC error surfacing.
All 22 new green; mcpd suite now 759/759 (was 737).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
256
src/mcpd/tests/agent-routes.test.ts
Normal file
256
src/mcpd/tests/agent-routes.test.ts
Normal file
@@ -0,0 +1,256 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import Fastify from 'fastify';
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import { registerAgentRoutes } from '../src/routes/agents.js';
|
||||
import { registerAgentChatRoutes } from '../src/routes/agent-chat.js';
|
||||
import { errorHandler } from '../src/middleware/error-handler.js';
|
||||
import { ConflictError, NotFoundError } from '../src/services/mcp-server.service.js';
|
||||
import type { AgentService, AgentView } from '../src/services/agent.service.js';
|
||||
import type { ChatService } from '../src/services/chat.service.js';
|
||||
|
||||
const NOW = new Date();
|
||||
|
||||
function makeView(overrides: Partial<AgentView> = {}): AgentView {
|
||||
return {
|
||||
id: 'agent-1',
|
||||
name: 'reviewer',
|
||||
description: '',
|
||||
systemPrompt: '',
|
||||
llm: { id: 'llm-1', name: 'qwen3-thinking' },
|
||||
project: null,
|
||||
proxyModelName: null,
|
||||
defaultParams: {},
|
||||
extras: {},
|
||||
ownerId: 'owner-1',
|
||||
version: 1,
|
||||
createdAt: NOW,
|
||||
updatedAt: NOW,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function mockAgentService(initial: AgentView[] = []): AgentService {
|
||||
const rows = new Map(initial.map((r) => [r.id, r]));
|
||||
return {
|
||||
list: vi.fn(async () => [...rows.values()]),
|
||||
listByProject: vi.fn(async (projectName: string) =>
|
||||
[...rows.values()].filter((r) => r.project?.name === projectName)),
|
||||
getById: vi.fn(async (id: string) => {
|
||||
const r = rows.get(id);
|
||||
if (!r) throw new NotFoundError(`Agent not found: ${id}`);
|
||||
return r;
|
||||
}),
|
||||
getByName: vi.fn(async (name: string) => {
|
||||
for (const r of rows.values()) if (r.name === name) return r;
|
||||
throw new NotFoundError(`Agent not found: ${name}`);
|
||||
}),
|
||||
create: vi.fn(async (input: unknown) => {
|
||||
const data = input as { name: string };
|
||||
for (const r of rows.values()) if (r.name === data.name) throw new ConflictError(`Agent already exists: ${data.name}`);
|
||||
const v = makeView({ id: `agent-${String(rows.size + 1)}`, name: data.name });
|
||||
rows.set(v.id, v);
|
||||
return v;
|
||||
}),
|
||||
update: vi.fn(async (id: string, input: unknown) => {
|
||||
const existing = rows.get(id);
|
||||
if (!existing) throw new NotFoundError(`Agent not found: ${id}`);
|
||||
const next = { ...existing, ...(input as Partial<AgentView>) };
|
||||
rows.set(id, next);
|
||||
return next;
|
||||
}),
|
||||
delete: vi.fn(async (id: string) => {
|
||||
if (!rows.has(id)) throw new NotFoundError(`Agent not found: ${id}`);
|
||||
rows.delete(id);
|
||||
}),
|
||||
upsertByName: vi.fn(),
|
||||
deleteByName: vi.fn(),
|
||||
} as unknown as AgentService;
|
||||
}
|
||||
|
||||
function mockChatService(): ChatService {
|
||||
return {
|
||||
chat: vi.fn(async (args: { agentName: string; userMessage?: string }) => ({
|
||||
threadId: 'thread-1', assistant: `echo: ${args.userMessage ?? ''}`, turnIndex: 1,
|
||||
})),
|
||||
chatStream: vi.fn(async function*() {
|
||||
yield { type: 'text' as const, delta: 'hi' };
|
||||
yield { type: 'final' as const, threadId: 'thread-1', turnIndex: 1 };
|
||||
}),
|
||||
createThread: vi.fn(async () => ({ id: 'thread-2' })),
|
||||
listThreads: vi.fn(async () => [
|
||||
{ id: 'thread-1', title: 't1', lastTurnAt: NOW, createdAt: NOW },
|
||||
]),
|
||||
listMessages: vi.fn(async () => []),
|
||||
} as unknown as ChatService;
|
||||
}
|
||||
|
||||
let app: FastifyInstance;
|
||||
|
||||
afterEach(async () => {
|
||||
if (app) await app.close();
|
||||
});
|
||||
|
||||
async function createApp(opts: { agents?: AgentService; chat?: ChatService } = {}): Promise<FastifyInstance> {
|
||||
app = Fastify({ logger: false });
|
||||
app.setErrorHandler(errorHandler);
|
||||
registerAgentRoutes(app, opts.agents ?? mockAgentService());
|
||||
registerAgentChatRoutes(app, opts.chat ?? mockChatService());
|
||||
await app.ready();
|
||||
return app;
|
||||
}
|
||||
|
||||
describe('Agent CRUD routes', () => {
|
||||
it('GET /api/v1/agents lists agents', async () => {
|
||||
await createApp({ agents: mockAgentService([makeView()]) });
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/agents' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json<unknown[]>()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('GET /api/v1/agents/:name resolves by name when not a CUID', async () => {
|
||||
await createApp({ agents: mockAgentService([makeView({ id: 'agent-1', name: 'reviewer' })]) });
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/agents/reviewer' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json<{ name: string }>().name).toBe('reviewer');
|
||||
});
|
||||
|
||||
it('GET /api/v1/agents/:id returns 404 when missing', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/agents/missing' });
|
||||
expect(res.statusCode).toBe(404);
|
||||
});
|
||||
|
||||
it('POST /api/v1/agents creates and returns 201', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents',
|
||||
payload: { name: 'deployer', llm: { name: 'qwen3-thinking' } },
|
||||
});
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(res.json<{ name: string }>().name).toBe('deployer');
|
||||
});
|
||||
|
||||
it('POST /api/v1/agents returns 409 on duplicate name', async () => {
|
||||
await createApp({ agents: mockAgentService([makeView({ id: 'a1', name: 'dup' })]) });
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents',
|
||||
payload: { name: 'dup', llm: { name: 'qwen3-thinking' } },
|
||||
});
|
||||
expect(res.statusCode).toBe(409);
|
||||
});
|
||||
|
||||
it('PUT /api/v1/agents/:name updates by name', async () => {
|
||||
await createApp({ agents: mockAgentService([makeView({ id: 'a1', name: 'editable' })]) });
|
||||
const res = await app.inject({
|
||||
method: 'PUT',
|
||||
url: '/api/v1/agents/editable',
|
||||
payload: { description: 'changed' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json<{ description: string }>().description).toBe('changed');
|
||||
});
|
||||
|
||||
it('DELETE /api/v1/agents/:name returns 204', async () => {
|
||||
await createApp({ agents: mockAgentService([makeView({ id: 'a1', name: 'doomed' })]) });
|
||||
const res = await app.inject({ method: 'DELETE', url: '/api/v1/agents/doomed' });
|
||||
expect(res.statusCode).toBe(204);
|
||||
});
|
||||
|
||||
it('GET /api/v1/projects/:name/agents lists project-scoped agents', async () => {
|
||||
await createApp({
|
||||
agents: mockAgentService([
|
||||
makeView({ id: 'a1', name: 'in', project: { id: 'p1', name: 'mcpctl-dev' } }),
|
||||
makeView({ id: 'a2', name: 'out' }),
|
||||
]),
|
||||
});
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/projects/mcpctl-dev/agents' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json<Array<{ name: string }>>().map((a) => a.name)).toEqual(['in']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Chat + threads routes', () => {
|
||||
it('POST /api/v1/agents/:name/chat (non-streaming) returns assistant body', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents/reviewer/chat',
|
||||
payload: { message: 'hi' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = res.json<{ threadId: string; assistant: string }>();
|
||||
expect(body.assistant).toContain('echo');
|
||||
expect(body.threadId).toBe('thread-1');
|
||||
});
|
||||
|
||||
it('POST /api/v1/agents/:name/chat rejects empty body with 400', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents/reviewer/chat',
|
||||
payload: {},
|
||||
});
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
|
||||
it('POST /api/v1/agents/:name/chat (streaming) emits SSE frames', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents/reviewer/chat',
|
||||
payload: { message: 'hi', stream: true },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers['content-type']).toContain('text/event-stream');
|
||||
const body = res.body;
|
||||
expect(body).toContain('data: ');
|
||||
expect(body).toContain('"type":"text"');
|
||||
expect(body).toContain('"type":"final"');
|
||||
expect(body.endsWith('data: [DONE]\n\n')).toBe(true);
|
||||
});
|
||||
|
||||
it('POST /api/v1/agents/:name/threads returns 201 with new thread id', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents/reviewer/threads',
|
||||
payload: { title: 'kickoff' },
|
||||
});
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(res.json<{ id: string }>().id).toBe('thread-2');
|
||||
});
|
||||
|
||||
it('GET /api/v1/agents/:name/threads lists threads', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/agents/reviewer/threads' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = res.json<Array<{ id: string }>>();
|
||||
expect(body).toHaveLength(1);
|
||||
expect(body[0]!.id).toBe('thread-1');
|
||||
});
|
||||
|
||||
it('GET /api/v1/threads/:id/messages returns the message log', async () => {
|
||||
await createApp();
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/threads/thread-1/messages' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json<unknown[]>()).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('mapUrlToPermission for agents', () => {
|
||||
// The mapping itself is tested implicitly through main.ts; this asserts the
|
||||
// shape we export for the chat URL → run:agents:<name>.
|
||||
it('routes /agents/:name/chat through run:agents:<name>', async () => {
|
||||
// Smoke check via the route working at all — RBAC integration is exercised
|
||||
// in main.ts tests; this just guards against regressions in the URL shape.
|
||||
await createApp();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/agents/r/chat',
|
||||
payload: { message: 'x' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
185
src/mcpd/tests/chat-tool-dispatcher.test.ts
Normal file
185
src/mcpd/tests/chat-tool-dispatcher.test.ts
Normal file
@@ -0,0 +1,185 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { ChatToolDispatcherImpl } from '../src/services/chat-tool-dispatcher.js';
|
||||
import { TOOL_NAME_SEPARATOR } from '../src/services/chat.service.js';
|
||||
import type { McpProxyService } from '../src/services/mcp-proxy-service.js';
|
||||
import type { IProjectRepository, ProjectWithRelations } from '../src/repositories/project.repository.js';
|
||||
|
||||
const NOW = new Date();
|
||||
|
||||
function makeProject(overrides: Partial<ProjectWithRelations> = {}): ProjectWithRelations {
|
||||
return {
|
||||
id: 'proj-1',
|
||||
name: 'mcpctl-dev',
|
||||
description: '',
|
||||
prompt: '',
|
||||
proxyModel: '',
|
||||
gated: true,
|
||||
llmProvider: null,
|
||||
llmModel: null,
|
||||
serverOverrides: null,
|
||||
ownerId: 'owner-1',
|
||||
version: 1,
|
||||
createdAt: NOW,
|
||||
updatedAt: NOW,
|
||||
servers: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function mockProjectRepo(p: ProjectWithRelations | null): IProjectRepository {
|
||||
return {
|
||||
findById: vi.fn(async () => p),
|
||||
findAll: vi.fn(),
|
||||
findByName: vi.fn(),
|
||||
create: vi.fn(),
|
||||
update: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
} as unknown as IProjectRepository;
|
||||
}
|
||||
|
||||
describe('ChatToolDispatcherImpl', () => {
|
||||
it('returns [] when project has no MCP servers', async () => {
|
||||
const proxy = { execute: vi.fn() } as unknown as McpProxyService;
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy,
|
||||
projects: mockProjectRepo(makeProject()),
|
||||
});
|
||||
const tools = await dispatcher.listTools('proj-1');
|
||||
expect(tools).toEqual([]);
|
||||
expect(proxy.execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns [] when projectId is null (unattached agent)', async () => {
|
||||
const proxy = { execute: vi.fn() } as unknown as McpProxyService;
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy,
|
||||
projects: mockProjectRepo(null),
|
||||
});
|
||||
expect(await dispatcher.listTools(null)).toEqual([]);
|
||||
});
|
||||
|
||||
it('namespaces tools as `<server>__<tool>` and forwards inputSchema', async () => {
|
||||
const proxy = {
|
||||
execute: vi.fn(async () => ({
|
||||
jsonrpc: '2.0' as const,
|
||||
id: 1,
|
||||
result: {
|
||||
tools: [
|
||||
{ name: 'query', description: 'do a query', inputSchema: { type: 'object', properties: { q: { type: 'string' } } } },
|
||||
{ name: 'ping' },
|
||||
],
|
||||
},
|
||||
})),
|
||||
} as unknown as McpProxyService;
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy,
|
||||
projects: mockProjectRepo(makeProject({
|
||||
servers: [{
|
||||
id: 'ps-1', projectId: 'proj-1', serverId: 'srv-grafana',
|
||||
server: { id: 'srv-grafana', name: 'grafana' },
|
||||
}],
|
||||
})),
|
||||
});
|
||||
const tools = await dispatcher.listTools('proj-1');
|
||||
expect(tools.map((t) => t.name)).toEqual([
|
||||
`grafana${TOOL_NAME_SEPARATOR}query`,
|
||||
`grafana${TOOL_NAME_SEPARATOR}ping`,
|
||||
]);
|
||||
expect(tools[0]!.parameters).toEqual({ type: 'object', properties: { q: { type: 'string' } } });
|
||||
// The 'ping' tool with no inputSchema gets a permissive default.
|
||||
expect(tools[1]!.parameters).toEqual({ type: 'object', properties: {} });
|
||||
});
|
||||
|
||||
it('skips servers whose tools/list errors out', async () => {
|
||||
const warn = vi.fn();
|
||||
const proxy = {
|
||||
execute: vi.fn(async ({ serverId }: { serverId: string }) => {
|
||||
if (serverId === 'srv-bad') {
|
||||
return { jsonrpc: '2.0' as const, id: 1, error: { code: -1, message: 'boom' } };
|
||||
}
|
||||
return {
|
||||
jsonrpc: '2.0' as const,
|
||||
id: 1,
|
||||
result: { tools: [{ name: 't1' }] },
|
||||
};
|
||||
}),
|
||||
} as unknown as McpProxyService;
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy,
|
||||
projects: mockProjectRepo(makeProject({
|
||||
servers: [
|
||||
{ id: 'ps-1', projectId: 'proj-1', serverId: 'srv-bad', server: { id: 'srv-bad', name: 'bad' } },
|
||||
{ id: 'ps-2', projectId: 'proj-1', serverId: 'srv-good', server: { id: 'srv-good', name: 'good' } },
|
||||
],
|
||||
})),
|
||||
logger: { warn },
|
||||
});
|
||||
const tools = await dispatcher.listTools('proj-1');
|
||||
expect(tools.map((t) => t.name)).toEqual([`good${TOOL_NAME_SEPARATOR}t1`]);
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ serverId: 'srv-bad' }),
|
||||
'tools/list failed',
|
||||
);
|
||||
});
|
||||
|
||||
it('callTool dispatches `tools/call` to the right serverId', async () => {
|
||||
const execute = vi.fn(async () => ({
|
||||
jsonrpc: '2.0' as const,
|
||||
id: 1,
|
||||
result: { content: [{ type: 'text', text: 'pong' }] },
|
||||
}));
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy: { execute } as unknown as McpProxyService,
|
||||
projects: mockProjectRepo(makeProject({
|
||||
servers: [{ id: 'ps-1', projectId: 'proj-1', serverId: 'srv-grafana', server: { id: 'srv-grafana', name: 'grafana' } }],
|
||||
})),
|
||||
});
|
||||
const result = await dispatcher.callTool({
|
||||
projectId: 'proj-1',
|
||||
serverName: 'grafana',
|
||||
toolName: 'ping',
|
||||
args: { q: 'cpu' },
|
||||
});
|
||||
expect(execute).toHaveBeenCalledWith({
|
||||
serverId: 'srv-grafana',
|
||||
method: 'tools/call',
|
||||
params: { name: 'ping', arguments: { q: 'cpu' } },
|
||||
});
|
||||
expect(result).toEqual({ content: [{ type: 'text', text: 'pong' }] });
|
||||
});
|
||||
|
||||
it('callTool throws when the server is not attached to the project', async () => {
|
||||
const execute = vi.fn();
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy: { execute } as unknown as McpProxyService,
|
||||
projects: mockProjectRepo(makeProject({ servers: [] })),
|
||||
});
|
||||
await expect(dispatcher.callTool({
|
||||
projectId: 'proj-1',
|
||||
serverName: 'grafana',
|
||||
toolName: 'ping',
|
||||
args: {},
|
||||
})).rejects.toThrow(/not attached/);
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('callTool surfaces JSON-RPC errors as exceptions', async () => {
|
||||
const execute = vi.fn(async () => ({
|
||||
jsonrpc: '2.0' as const,
|
||||
id: 1,
|
||||
error: { code: -1, message: 'tool blew up' },
|
||||
}));
|
||||
const dispatcher = new ChatToolDispatcherImpl({
|
||||
proxy: { execute } as unknown as McpProxyService,
|
||||
projects: mockProjectRepo(makeProject({
|
||||
servers: [{ id: 'ps-1', projectId: 'proj-1', serverId: 'srv-grafana', server: { id: 'srv-grafana', name: 'grafana' } }],
|
||||
})),
|
||||
});
|
||||
await expect(dispatcher.callTool({
|
||||
projectId: 'proj-1',
|
||||
serverName: 'grafana',
|
||||
toolName: 'ping',
|
||||
args: {},
|
||||
})).rejects.toThrow(/tool blew up/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user