From 260bdd4c1ac1496bf14dc5cec669b5ef6500cddc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:53:26 +0000 Subject: [PATCH] fix(mcp): scope request logs to server by durable id, not recycled name key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCP request logs were joined to their server via serverKey, a slug derived from the server's name. Deleting a server never cleaned up its logs, and re-creating a server under the same name regenerated the identical key — so the new server's Logs tab silently showed the deleted server's history interleaved with its own. Add serverId to mcp_request_logs as the durable match (stamped on every write from now on); query by serverId primarily, falling back to serverKey scoped by projectId and capped at the server's own createdAt for rows written before this migration, so a recycled key can never pull in a predecessor's history. Also drop the cluster instance-assignment cache entry on delete, which had the same recycled-key problem for node pinning. --- src/__tests__/integration/db-parity.test.ts | 6 +- .../integration/mcp-composite.test.ts | 9 +- .../integration/mcp-log-redaction.test.ts | 11 ++- src/lib/database/index.ts | 1 + src/lib/database/mongodb/indexManifest.ts | 5 +- src/lib/database/mongodb/mcp-server.mixin.ts | 86 ++++++++++--------- src/lib/database/provider/contract.ts | 7 +- src/lib/database/provider/types.extended.ts | 22 +++++ src/lib/database/sqlite/base.ts | 17 ++++ src/lib/database/sqlite/mcp-server.mixin.ts | 53 +++++++++--- src/lib/database/sqlite/schema.ts | 4 + src/lib/services/mcp/mcpService.ts | 42 +++++++-- src/server/api/plugins/client-mcp.ts | 2 + src/server/api/plugins/mcp.ts | 10 ++- src/server/api/plugins/public-mcp.ts | 2 + src/server/api/routes/mcp/[id]/logs/route.ts | 4 +- src/server/api/routes/mcp/[id]/route.ts | 4 +- 17 files changed, 208 insertions(+), 77 deletions(-) diff --git a/src/__tests__/integration/db-parity.test.ts b/src/__tests__/integration/db-parity.test.ts index 9128c0d0..523b52cc 100644 --- a/src/__tests__/integration/db-parity.test.ts +++ b/src/__tests__/integration/db-parity.test.ts @@ -897,7 +897,11 @@ describeForEachProvider('MCP Hub servers + audit logs', (getDb) => { errorMessage: 'nope', }); - const logs = await db.listMcpRequestLogs('srv-a'); + const logs = await db.listMcpRequestLogs({ + serverId: 'nonexistent-server-id', + serverKey: 'srv-a', + projectId: 'proj-1', + }); expect(logs).toHaveLength(1); expect(logs[0].callerType).toBe('public'); expect(logs[0].transport).toBe('jsonrpc'); diff --git a/src/__tests__/integration/mcp-composite.test.ts b/src/__tests__/integration/mcp-composite.test.ts index be7d65d1..72431144 100644 --- a/src/__tests__/integration/mcp-composite.test.ts +++ b/src/__tests__/integration/mcp-composite.test.ts @@ -252,7 +252,7 @@ describe('composite execution — real routed call, member-scoped log, self-refe }); it('writes a member-scoped log row tagged with the composite key, separate from the composite\'s own log', async () => { - const memberLogs = await listMcpRequestLogs(TENANT_DB_NAME, memberA.key, { limit: 10 }); + const memberLogs = await listMcpRequestLogs(TENANT_DB_NAME, memberA, { limit: 10 }); const viaComposite = memberLogs.filter((l) => l.viaServerKey === composite.key); expect(viaComposite.length).toBeGreaterThan(0); expect(viaComposite[0].transport).toBe('internal'); @@ -357,7 +357,12 @@ describe('sqlite viaServerKey column', () => { it('is persisted and queryable end to end', async () => { const db = await getDatabase(); await db.switchToTenant(TENANT_DB_NAME); - const logs = await db.listMcpRequestLogs(memberA.key, { limit: 50 }); + const logs = await db.listMcpRequestLogs({ + serverId: String(memberA._id), + serverKey: memberA.key, + projectId: memberA.projectId, + createdAt: memberA.createdAt, + }, { limit: 50 }); expect(logs.some((l) => l.viaServerKey)).toBe(true); }); }); diff --git a/src/__tests__/integration/mcp-log-redaction.test.ts b/src/__tests__/integration/mcp-log-redaction.test.ts index 78630585..cb2ac691 100644 --- a/src/__tests__/integration/mcp-log-redaction.test.ts +++ b/src/__tests__/integration/mcp-log-redaction.test.ts @@ -23,11 +23,18 @@ import { reloadConfig } from '@/lib/core/config'; import { disconnectDatabase } from '@/lib/database'; import { logMcpRequest, listMcpRequestLogs } from '@/lib/services/mcp'; import { LOG_SECRET_MASK } from '@/lib/services/logRedaction'; +import type { IMcpServer } from '@/lib/database'; const TENANT_DB_NAME = 'mcp_logredact_tenant'; const TENANT_ID = 'tenant-logredact'; const SERVER_KEY = 'srv-logredact'; const SECRET = 'Bearer sk-live-echoed-credential-1234567890'; +// These logs are written with no real MCP server record backing them (only +// `logMcpRequest`'s legacy serverKey field) — listMcpRequestLogs now scopes +// by serverId primarily, falling back to serverKey for pre-migration rows. +// A synthetic server stub with no serverId/createdAt match exercises that +// fallback path the same way a real legacy row would. +const SERVER_STUB = { _id: 'nonexistent-server-id', key: SERVER_KEY } as IMcpServer; beforeAll(() => { reloadConfig(); @@ -62,7 +69,7 @@ describe('logMcpRequest — secret scrubbing at the persistence boundary', () => [SECRET], ); - const logs = await listMcpRequestLogs(TENANT_DB_NAME, SERVER_KEY, { limit: 10 }); + const logs = await listMcpRequestLogs(TENANT_DB_NAME, SERVER_STUB, { limit: 10 }); expect(logs.length).toBe(1); const stored = JSON.stringify(logs[0].responsePayload); @@ -91,7 +98,7 @@ describe('logMcpRequest — secret scrubbing at the persistence boundary', () => [SECRET], ); - const errorLog = (await listMcpRequestLogs(TENANT_DB_NAME, SERVER_KEY, { limit: 10 })) + const errorLog = (await listMcpRequestLogs(TENANT_DB_NAME, SERVER_STUB, { limit: 10 })) .find((l) => l.toolName === 'failing'); expect(errorLog?.errorMessage).toBeDefined(); expect(errorLog?.errorMessage).not.toContain('sk-live-echoed-credential-1234567890'); diff --git a/src/lib/database/index.ts b/src/lib/database/index.ts index 6df2d1ae..32110a3f 100644 --- a/src/lib/database/index.ts +++ b/src/lib/database/index.ts @@ -320,6 +320,7 @@ export type { IMcpToolAnnotations, IMcpRequestLog, IMcpRequestAggregate, + McpLogServerScope, IMcpAuditLog, McpAuditAction, McpServerStatus, diff --git a/src/lib/database/mongodb/indexManifest.ts b/src/lib/database/mongodb/indexManifest.ts index ee018e8e..059b050d 100644 --- a/src/lib/database/mongodb/indexManifest.ts +++ b/src/lib/database/mongodb/indexManifest.ts @@ -153,7 +153,10 @@ export const TENANT_DB_INDEXES: Record = { websearch_run_logs: [{ key: { searchKey: 1, createdAt: -1 }, options: { name: 'idx_search_createdAt' } }], agent_conversations: [{ key: { agentKey: 1, updatedAt: -1 }, options: { name: 'idx_agent_updatedAt' } }], agent_versions: [{ key: { agentId: 1, version: -1 }, options: { name: 'idx_agent_version' } }], - mcp_request_logs: [{ key: { serverKey: 1, createdAt: -1 }, options: { name: 'idx_server_createdAt' } }], + mcp_request_logs: [ + { key: { serverKey: 1, createdAt: -1 }, options: { name: 'idx_server_createdAt' } }, + { key: { serverId: 1, createdAt: -1 }, options: { name: 'idx_serverId_createdAt' } }, + ], tool_request_logs: [{ key: { toolKey: 1, createdAt: -1 }, options: { name: 'idx_tool_createdAt' } }], ocr_job_items: [{ key: { jobId: 1, index: 1 }, options: { name: 'idx_job_index' } }], batch_job_items: [{ key: { batchId: 1, index: 1 }, options: { name: 'idx_batch_index' } }], diff --git a/src/lib/database/mongodb/mcp-server.mixin.ts b/src/lib/database/mongodb/mcp-server.mixin.ts index 1138a8c6..25d18388 100644 --- a/src/lib/database/mongodb/mcp-server.mixin.ts +++ b/src/lib/database/mongodb/mcp-server.mixin.ts @@ -10,11 +10,50 @@ import type { IMcpAuditLog, IMcpRequestLog, IMcpRequestAggregate, + McpLogServerScope, McpServerStatus, } from '../provider.interface'; import type { Constructor } from './types'; import { MongoDBProviderBase, COLLECTIONS } from './base'; +/** + * Match a server's request logs by durable `serverId` primarily. The second + * `$or` branch only extends the match to rows written before `serverId` + * existed (`serverId` absent) — scoped to this server's own `serverKey` + + * `projectId`, and no older than this server's own `createdAt`, so a + * deleted-and-recreated server can never adopt its predecessor's logs just + * because they share the same name-derived key. + */ +function mcpLogScopeFilter(scope: McpLogServerScope): Record { + const legacy: Record = { + serverId: { $exists: false }, + serverKey: scope.serverKey, + }; + if (scope.projectId !== undefined) legacy.projectId = scope.projectId; + if (scope.createdAt) legacy.createdAt = { $gte: scope.createdAt }; + return { $or: [{ serverId: scope.serverId }, legacy] }; +} + +/** Combine the scope filter with the caller's own query options via `$and` (the scope filter already uses `$or`, which can't be merged by object-spread without colliding keys). */ +function buildMcpLogFilter( + scope: McpLogServerScope, + options?: { status?: string; from?: Date; to?: Date; keyword?: string }, +): Record { + const clauses: Record[] = [mcpLogScopeFilter(scope)]; + if (options?.status) clauses.push({ status: options.status }); + if (options?.from || options?.to) { + const range: Record = {}; + if (options.from) range.$gte = options.from; + if (options.to) range.$lte = options.to; + clauses.push({ createdAt: range }); + } + if (options?.keyword?.trim()) { + const keywordRegex = new RegExp(options.keyword.trim(), 'i'); + clauses.push({ $or: [{ toolName: keywordRegex }, { errorMessage: keywordRegex }] }); + } + return clauses.length === 1 ? clauses[0] : { $and: clauses }; +} + export function McpServerMixin>(Base: TBase) { return class McpServerOps extends Base { // ── MCP Server CRUD ────────────────────────────────────────────── @@ -129,7 +168,7 @@ export function McpServerMixin>(B } async listMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { limit?: number; skip?: number; @@ -140,20 +179,7 @@ export function McpServerMixin>(B }, ): Promise { const db = this.getTenantDb(); - const filter: Record = { serverKey }; - if (options?.status) filter.status = options.status; - if (options?.from || options?.to) { - filter.createdAt = {}; - if (options.from) (filter.createdAt as Record).$gte = options.from; - if (options.to) (filter.createdAt as Record).$lte = options.to; - } - if (options?.keyword?.trim()) { - const keywordRegex = new RegExp(options.keyword.trim(), 'i'); - filter.$or = [ - { toolName: keywordRegex }, - { errorMessage: keywordRegex }, - ]; - } + const filter = buildMcpLogFilter(scope, options); const docs = await db .collection(COLLECTIONS.mcpRequestLogs) .find(filter) @@ -183,40 +209,20 @@ export function McpServerMixin>(B } async countMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; status?: string; keyword?: string }, ): Promise { const db = this.getTenantDb(); - const filter: Record = { serverKey }; - - if (options?.status) filter.status = options.status; - if (options?.from || options?.to) { - filter.createdAt = {}; - if (options.from) (filter.createdAt as Record).$gte = options.from; - if (options.to) (filter.createdAt as Record).$lte = options.to; - } - if (options?.keyword?.trim()) { - const keywordRegex = new RegExp(options.keyword.trim(), 'i'); - filter.$or = [ - { toolName: keywordRegex }, - { errorMessage: keywordRegex }, - ]; - } - + const filter = buildMcpLogFilter(scope, options); return db.collection(COLLECTIONS.mcpRequestLogs).countDocuments(filter); } async aggregateMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; groupBy?: 'hour' | 'day' | 'month' }, ): Promise { const db = this.getTenantDb(); - const match: Record = { serverKey }; - if (options?.from || options?.to) { - match.createdAt = {}; - if (options.from) (match.createdAt as Record).$gte = options.from; - if (options.to) (match.createdAt as Record).$lte = options.to; - } + const match = buildMcpLogFilter(scope, { from: options?.from, to: options?.to }); const pipeline = [ { $match: match }, @@ -284,7 +290,7 @@ export function McpServerMixin>(B } return { - serverKey, + serverKey: scope.serverKey, totalRequests: (agg?.totalRequests as number) ?? 0, successCount: (agg?.successCount as number) ?? 0, errorCount: (agg?.errorCount as number) ?? 0, diff --git a/src/lib/database/provider/contract.ts b/src/lib/database/provider/contract.ts index 46e932b1..b2290080 100644 --- a/src/lib/database/provider/contract.ts +++ b/src/lib/database/provider/contract.ts @@ -47,6 +47,7 @@ import type { IMcpAuditLog, IMcpRequestAggregate, IMcpRequestLog, + McpLogServerScope, IMcpServer, IModel, IModelUsageAggregate, @@ -1343,7 +1344,7 @@ export interface DatabaseProvider extends EnterpriseDbMethods { log: Omit, ): Promise; listMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { limit?: number; skip?: number; @@ -1354,11 +1355,11 @@ export interface DatabaseProvider extends EnterpriseDbMethods { }, ): Promise; countMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; status?: string; keyword?: string }, ): Promise; aggregateMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; groupBy?: 'hour' | 'day' | 'month' }, ): Promise; listRecentMcpRequestLogs(options?: { diff --git a/src/lib/database/provider/types.extended.ts b/src/lib/database/provider/types.extended.ts index c00e7a5f..5845cf3d 100644 --- a/src/lib/database/provider/types.extended.ts +++ b/src/lib/database/provider/types.extended.ts @@ -332,6 +332,14 @@ export interface IMcpRequestLog extends IUsageAttributionFields { _id?: ObjectId | string; tenantId: string; projectId?: string; + /** + * Durable reference to the server that wrote this row. Always stamped on + * new rows (see `logMcpRequest`); absent only on rows written before this + * field existed. `serverKey` is a slug of the server's *name* and gets + * recycled if the server is deleted and a new one is created with the + * same name — `serverId` is what actually scopes a server's Logs tab. + */ + serverId?: string; serverKey: string; toolName: string; status: 'success' | 'error'; @@ -383,6 +391,20 @@ export interface IMcpAuditLog { createdAt?: Date; } +/** + * Scopes an MCP request-log query to one server. `serverId` is the primary + * match; `serverKey`/`projectId`/`createdAt` only extend the match to rows + * written before `serverId` existed, and only up to this server's own + * `createdAt` — so a deleted-and-recreated server's recycled key can't pull + * in the predecessor's history. See `mcpLogScope` in mcpService.ts. + */ +export interface McpLogServerScope { + serverId: string; + serverKey: string; + projectId?: string; + createdAt?: Date; +} + export interface IMcpRequestAggregate { serverKey: string; totalRequests: number; diff --git a/src/lib/database/sqlite/base.ts b/src/lib/database/sqlite/base.ts index f4b0fd19..fe24ca74 100644 --- a/src/lib/database/sqlite/base.ts +++ b/src/lib/database/sqlite/base.ts @@ -863,6 +863,13 @@ export class SQLiteProviderBase { // records the composite's key here so the member's own Logs tab can show // "via " alongside calls that hit it directly. this.ensureTableColumn(db, TABLES.mcpRequestLogs, 'viaServerKey', 'viaServerKey TEXT'); + // serverId: durable FK to mcp_servers.id, replacing serverKey (a slug of + // the server's name) as the log-query scope. serverKey is recycled when + // a server is deleted and recreated under the same name, which used to + // make a new server's Logs tab show its predecessor's history — see + // mcpLogScopeClause in mcp-server.mixin.ts. Rows written before this + // column existed are matched by the legacy fallback in that clause. + this.ensureTableColumn(db, TABLES.mcpRequestLogs, 'serverId', 'serverId TEXT'); // Evaluation: dataset items moved to their own table; the denormalised // itemCount rides on the dataset row. Suites were missing the @@ -938,6 +945,16 @@ export class SQLiteProviderBase { ON ${TABLES.modelUsageLogs}(tenantId, userId, createdAt DESC); `); + // mcp_request_logs.serverId index — must be created here (after + // applyTenantMigrations), same ordering constraint as above: serverId + // reaches legacy DBs via ensureTableColumn, and referencing it in + // TENANT_SCHEMA_SQL directly aborted the whole schema exec on tenants + // created before the column existed. + db.exec(` + CREATE INDEX IF NOT EXISTS idx_mcp_request_logs_serverId + ON ${TABLES.mcpRequestLogs}(serverId); + `); + // usage_daily unique dimension index v3 (adds metadataKey, on top of v2's // agentKey). Same ordering constraint as above: agentKey/metadataKey // reach legacy DBs via ensureTableColumn. v2 would reject rows differing diff --git a/src/lib/database/sqlite/mcp-server.mixin.ts b/src/lib/database/sqlite/mcp-server.mixin.ts index 277935cd..710c4191 100644 --- a/src/lib/database/sqlite/mcp-server.mixin.ts +++ b/src/lib/database/sqlite/mcp-server.mixin.ts @@ -13,11 +13,38 @@ import type { IMcpAuditLog, IMcpRequestLog, IMcpRequestAggregate, + McpLogServerScope, McpServerStatus, } from '../provider.interface'; import type { Constructor, SqliteRow } from './types'; import { SQLiteProviderBase, TABLES } from './base'; +/** + * Build the WHERE clause matching a server's request logs by durable + * `serverId` primarily. The `OR` branch only extends the match to rows + * written before `serverId` existed (`serverId IS NULL`) — scoped to this + * server's own `serverKey` + `projectId`, and no older than this server's + * own `createdAt`, so a deleted-and-recreated server can never adopt its + * predecessor's logs just because they share the same name-derived key. + * Mutates `params` with the scope's bound parameters. + */ +function mcpLogScopeClause(scope: McpLogServerScope, params: Record): string { + params.scopeServerId = scope.serverId; + params.scopeServerKey = scope.serverKey; + const legacyParts = ['serverId IS NULL', 'serverKey = @scopeServerKey']; + if (scope.projectId !== undefined) { + params.scopeProjectId = scope.projectId; + legacyParts.push('projectId = @scopeProjectId'); + } else { + legacyParts.push('projectId IS NULL'); + } + if (scope.createdAt) { + params.scopeCreatedAt = scope.createdAt.toISOString(); + legacyParts.push('createdAt >= @scopeCreatedAt'); + } + return `(serverId = @scopeServerId OR (${legacyParts.join(' AND ')}))`; +} + export function McpServerMixin>(Base: TBase) { return class McpServerOps extends Base { // ── MCP Server CRUD ────────────────────────────────────────────── @@ -178,11 +205,11 @@ export function McpServerMixin>(Ba db.prepare(` INSERT INTO ${TABLES.mcpRequestLogs} - (id, tenantId, projectId, serverKey, toolName, status, + (id, tenantId, projectId, serverId, serverKey, toolName, status, requestPayload, responsePayload, errorMessage, latencyMs, callerTokenId, callerType, callerUserId, transport, sourceType, sessionId, viaServerKey, userId, apiTokenId, actorType, createdAt) - VALUES (@id, @tenantId, @projectId, @serverKey, @toolName, @status, + VALUES (@id, @tenantId, @projectId, @serverId, @serverKey, @toolName, @status, @requestPayload, @responsePayload, @errorMessage, @latencyMs, @callerTokenId, @callerType, @callerUserId, @transport, @sourceType, @sessionId, @viaServerKey, @userId, @apiTokenId, @actorType, @createdAt) @@ -190,6 +217,7 @@ export function McpServerMixin>(Ba id, tenantId: log.tenantId, projectId: log.projectId ?? null, + serverId: log.serverId ?? null, serverKey: log.serverKey, toolName: log.toolName, status: log.status, @@ -214,7 +242,7 @@ export function McpServerMixin>(Ba } async listMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { limit?: number; skip?: number; @@ -225,8 +253,8 @@ export function McpServerMixin>(Ba }, ): Promise { const db = this.getTenantDb(); - const clauses: string[] = ['serverKey = @serverKey']; - const params: Record = { serverKey }; + const params: Record = {}; + const clauses: string[] = [mcpLogScopeClause(scope, params)]; if (options?.status) { clauses.push('status = @status'); params.status = options.status; } if (options?.from) { clauses.push('createdAt >= @from'); params.from = options.from.toISOString(); } if (options?.to) { clauses.push('createdAt <= @to'); params.to = options.to.toISOString(); } @@ -263,12 +291,12 @@ export function McpServerMixin>(Ba } async countMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; status?: string; keyword?: string }, ): Promise { const db = this.getTenantDb(); - const clauses: string[] = ['serverKey = @serverKey']; - const params: Record = { serverKey }; + const params: Record = {}; + const clauses: string[] = [mcpLogScopeClause(scope, params)]; if (options?.status) { clauses.push('status = @status'); params.status = options.status; } if (options?.from) { clauses.push('createdAt >= @from'); params.from = options.from.toISOString(); } @@ -287,12 +315,12 @@ export function McpServerMixin>(Ba } async aggregateMcpRequestLogs( - serverKey: string, + scope: McpLogServerScope, options?: { from?: Date; to?: Date; groupBy?: 'hour' | 'day' | 'month' }, ): Promise { const db = this.getTenantDb(); - const clauses: string[] = ['serverKey = @serverKey']; - const params: Record = { serverKey }; + const params: Record = {}; + const clauses: string[] = [mcpLogScopeClause(scope, params)]; if (options?.from) { clauses.push('createdAt >= @from'); params.from = options.from.toISOString(); } if (options?.to) { clauses.push('createdAt <= @to'); params.to = options.to.toISOString(); } const where = `WHERE ${clauses.join(' AND ')}`; @@ -342,7 +370,7 @@ export function McpServerMixin>(Ba })); return { - serverKey, + serverKey: scope.serverKey, totalRequests: (totalsRow?.totalRequests as number) ?? 0, successCount: (totalsRow?.successCount as number) ?? 0, errorCount: (totalsRow?.errorCount as number) ?? 0, @@ -444,6 +472,7 @@ export function McpServerMixin>(Ba _id: r.id as string, tenantId: r.tenantId as string, projectId: r.projectId as string | undefined, + serverId: (r.serverId as string | null) ?? undefined, serverKey: r.serverKey as string, toolName: r.toolName as string, status: r.status as IMcpRequestLog['status'], diff --git a/src/lib/database/sqlite/schema.ts b/src/lib/database/sqlite/schema.ts index a08aee9e..3741538e 100644 --- a/src/lib/database/sqlite/schema.ts +++ b/src/lib/database/sqlite/schema.ts @@ -1669,6 +1669,7 @@ export const TENANT_SCHEMA_SQL = ` id TEXT PRIMARY KEY, tenantId TEXT NOT NULL, projectId TEXT, + serverId TEXT, serverKey TEXT NOT NULL, toolName TEXT NOT NULL, status TEXT NOT NULL, @@ -1690,6 +1691,9 @@ export const TENANT_SCHEMA_SQL = ` ); CREATE INDEX IF NOT EXISTS idx_mcp_request_logs_serverKey ON mcp_request_logs(serverKey); CREATE INDEX IF NOT EXISTS idx_mcp_request_logs_createdAt ON mcp_request_logs(createdAt); + -- idx_mcp_request_logs_serverId is created in applyTenantIndexes (base.ts), + -- after applyTenantMigrations ensures the serverId column on legacy DBs — + -- referencing it here would abort this whole script on pre-existing tenants. -- MCP Audit Logs CREATE TABLE IF NOT EXISTS mcp_audit_logs ( diff --git a/src/lib/services/mcp/mcpService.ts b/src/lib/services/mcp/mcpService.ts index 1c12d6ca..9c5392d3 100644 --- a/src/lib/services/mcp/mcpService.ts +++ b/src/lib/services/mcp/mcpService.ts @@ -11,6 +11,7 @@ import type { IMcpStdioConfig, IMcpTool, IMcpToolAnnotations, + McpLogServerScope, McpSourceType, } from '@/lib/database'; import type { @@ -30,7 +31,7 @@ import { import { recordUsageEvent, resolveUsageAttribution } from '@/lib/services/usage/usageEvents'; import { safeFetch } from '@/lib/security/outboundFetch'; import { normalizeApiSpec, type SpecFormatHint } from '@/lib/services/specImport'; -import { routeInstanceCall } from '@/lib/core/cluster'; +import { deleteInstanceAssignment, routeInstanceCall } from '@/lib/core/cluster'; import type { QueuePayload } from '@/lib/core/queue'; import { mcpEntityId } from './mcpEntityId'; import { @@ -1323,6 +1324,13 @@ export async function deleteMcpServer( const deleted = await db.deleteMcpServer(serverId); if (deleted && existing) { + // Drop the node-pinning cache entry, keyed by tenantId+key: without this, + // a server recreated under the same name would silently inherit the + // deleted server's cluster placement (including a stale "strictly + // assigned to an offline node" error). + void deleteInstanceAssignment('mcp', mcpEntityId(existing.tenantId, existing.key)) + .catch((error) => logger.warn('Failed to clear MCP instance assignment', { serverId, error })); + // Release a sandbox-backed runtime if one exists (best-effort). if (existing.stdioConfig?.executionMode === 'sandbox' && mcpSandboxRunner.current) { void mcpSandboxRunner.current @@ -1386,6 +1394,22 @@ export async function listMcpServers( // ── Request logging ─────────────────────────────────────────────────────── +/** + * Scope a request-log query to `server`. `serverId` is the durable match; + * the rest only extend it to rows written before `serverId` existed, capped + * at this server's own `createdAt` so a deleted-and-recreated server (which + * regenerates the same name-derived `key`) can't pull in its predecessor's + * history. + */ +function mcpLogScope(server: IMcpServer): McpLogServerScope { + return { + serverId: String(server._id), + serverKey: server.key, + projectId: server.projectId, + createdAt: server.createdAt, + }; +} + /** * Outbound secret values that could be echoed back into a logged response for * this server: the caller's applied runtime-header values plus the server's @@ -1456,7 +1480,7 @@ export async function logMcpRequest( export async function listMcpRequestLogs( tenantDbName: string, - serverKey: string, + server: IMcpServer, options?: { limit?: number; skip?: number; @@ -1468,27 +1492,27 @@ export async function listMcpRequestLogs( ) { const db = await getDatabase(); await db.switchToTenant(tenantDbName); - return db.listMcpRequestLogs(serverKey, options); + return db.listMcpRequestLogs(mcpLogScope(server), options); } export async function countMcpRequestLogs( tenantDbName: string, - serverKey: string, + server: IMcpServer, options?: { from?: Date; to?: Date; status?: string; keyword?: string }, ) { const db = await getDatabase(); await db.switchToTenant(tenantDbName); - return db.countMcpRequestLogs(serverKey, options); + return db.countMcpRequestLogs(mcpLogScope(server), options); } export async function aggregateMcpRequestLogs( tenantDbName: string, - serverKey: string, + server: IMcpServer, options?: { from?: Date; to?: Date; groupBy?: 'hour' | 'day' | 'month' }, ) { const db = await getDatabase(); await db.switchToTenant(tenantDbName); - return db.aggregateMcpRequestLogs(serverKey, options); + return db.aggregateMcpRequestLogs(mcpLogScope(server), options); } // ── Audit logging ───────────────────────────────────────────────────────── @@ -1549,7 +1573,7 @@ export async function getMcpMonitorSnapshot( const entries: McpServerMonitorEntry[] = []; for (const server of servers) { - const aggregate = await db.aggregateMcpRequestLogs(server.key, { from, groupBy: 'hour' }); + const aggregate = await db.aggregateMcpRequestLogs(mcpLogScope(server), { from, groupBy: 'hour' }); const sourceType = resolveSourceType(server); let kind: McpServerMonitorEntry['runtime']['kind'] = 'openapi'; @@ -1814,6 +1838,7 @@ async function executeCompositeTool( void logMcpRequest(tenantDbName, { tenantId: m.tenantId, projectId: m.projectId, + serverId: String(m._id), serverKey: m.key, toolName: tool.origin.realName, status: 'success', @@ -1834,6 +1859,7 @@ async function executeCompositeTool( void logMcpRequest(tenantDbName, { tenantId: m.tenantId, projectId: m.projectId, + serverId: String(m._id), serverKey: m.key, toolName: tool.origin.realName, status: 'error', diff --git a/src/server/api/plugins/client-mcp.ts b/src/server/api/plugins/client-mcp.ts index d3a64e83..93c53593 100644 --- a/src/server/api/plugins/client-mcp.ts +++ b/src/server/api/plugins/client-mcp.ts @@ -211,6 +211,7 @@ async function runToolCall( void logMcpRequest(log.tenantDbName, { tenantId: log.tenantId, projectId: log.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'success', @@ -236,6 +237,7 @@ async function runToolCall( void logMcpRequest(log.tenantDbName, { tenantId: log.tenantId, projectId: log.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'error', diff --git a/src/server/api/plugins/mcp.ts b/src/server/api/plugins/mcp.ts index ae8e9ad1..3a398231 100644 --- a/src/server/api/plugins/mcp.ts +++ b/src/server/api/plugins/mcp.ts @@ -471,11 +471,11 @@ export const mcpApiPlugin: FastifyPluginAsync = async (app) => { }; if (query.includeLogs === 'true') { - payload.logs = await listMcpRequestLogs(session.tenantDbName, server.key, { limit: 50 }); + payload.logs = await listMcpRequestLogs(session.tenantDbName, server, { limit: 50 }); } if (query.includeAggregate === 'true') { - payload.aggregate = await aggregateMcpRequestLogs(session.tenantDbName, server.key, { + payload.aggregate = await aggregateMcpRequestLogs(session.tenantDbName, server, { groupBy: 'day', }); } @@ -697,8 +697,8 @@ export const mcpApiPlugin: FastifyPluginAsync = async (app) => { }; const [logs, total] = await Promise.all([ - listMcpRequestLogs(session.tenantDbName, server.key, filter), - countMcpRequestLogs(session.tenantDbName, server.key, { + listMcpRequestLogs(session.tenantDbName, server, filter), + countMcpRequestLogs(session.tenantDbName, server, { from, keyword: filter.keyword, status: query.status, @@ -783,6 +783,7 @@ export const mcpApiPlugin: FastifyPluginAsync = async (app) => { void logMcpRequest(session.tenantDbName, { tenantId: session.tenantId, projectId: server.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'success', @@ -805,6 +806,7 @@ export const mcpApiPlugin: FastifyPluginAsync = async (app) => { void logMcpRequest(session.tenantDbName, { tenantId: session.tenantId, projectId: server.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'error', diff --git a/src/server/api/plugins/public-mcp.ts b/src/server/api/plugins/public-mcp.ts index 7deb7e45..8101700c 100644 --- a/src/server/api/plugins/public-mcp.ts +++ b/src/server/api/plugins/public-mcp.ts @@ -103,6 +103,7 @@ async function runPublicToolCall( void logMcpRequest(tenant.dbName, { tenantId: server.tenantId, projectId: server.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'success', @@ -122,6 +123,7 @@ async function runPublicToolCall( void logMcpRequest(tenant.dbName, { tenantId: server.tenantId, projectId: server.projectId, + serverId: String(server._id), serverKey: server.key, toolName, status: 'error', diff --git a/src/server/api/routes/mcp/[id]/logs/route.ts b/src/server/api/routes/mcp/[id]/logs/route.ts index b234cacf..8c170253 100644 --- a/src/server/api/routes/mcp/[id]/logs/route.ts +++ b/src/server/api/routes/mcp/[id]/logs/route.ts @@ -40,7 +40,7 @@ export async function GET( const resolvedSkip = Number.isNaN(skip) ? (page - 1) * limit : Math.max(skip, 0); - const logs = await listMcpRequestLogs(tenantDbName, server.key, { + const logs = await listMcpRequestLogs(tenantDbName, server, { limit, skip: resolvedSkip, status, @@ -49,7 +49,7 @@ export async function GET( keyword, }); - const total = await countMcpRequestLogs(tenantDbName, server.key, { + const total = await countMcpRequestLogs(tenantDbName, server, { status, from, to, diff --git a/src/server/api/routes/mcp/[id]/route.ts b/src/server/api/routes/mcp/[id]/route.ts index 7f5bf220..88859a57 100644 --- a/src/server/api/routes/mcp/[id]/route.ts +++ b/src/server/api/routes/mcp/[id]/route.ts @@ -38,12 +38,12 @@ export async function GET( const payload: Record = { server: serializeMcpServerFull(server) }; if (includeLogs) { - const logs = await listMcpRequestLogs(tenantDbName, server.key, { limit: 50 }); + const logs = await listMcpRequestLogs(tenantDbName, server, { limit: 50 }); payload.logs = logs; } if (includeAggregate) { - const aggregate = await aggregateMcpRequestLogs(tenantDbName, server.key, { groupBy: 'day' }); + const aggregate = await aggregateMcpRequestLogs(tenantDbName, server, { groupBy: 'day' }); payload.aggregate = aggregate; }