fix(core): Use slugs instead of ids to identify MCP registry servers (#30974)

This commit is contained in:
RomanDavydchuk 2026-05-28 16:49:36 +03:00 committed by GitHub
parent 93a176b997
commit 4722c4d582
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 207 additions and 75 deletions

View File

@ -0,0 +1,80 @@
import type { MigrationContext, ReversibleMigration } from '../migration-types';
export class UseSlugAsPrimaryKeyInMcpRegistryServer1784000000016 implements ReversibleMigration {
async up({
copyTable,
runQuery,
escape,
schemaBuilder: { createTable, dropTable, column },
}: MigrationContext) {
const tableName = 'mcp_registry_server';
const tempTableName = 'tmp_mcp_registry_server';
await createTable(tempTableName).withColumns(
column('slug').varchar(255).primary,
column('status')
.varchar(50)
.notNull.withEnumCheck(['active', 'deprecated'])
.comment(
'Server status in the MCP registry. Deprecated servers are not surfaced to users.',
),
column('version').varchar(50).notNull,
column('registryUpdatedAt').timestampNoTimezone(3).notNull,
column('data')
.json.notNull.default("'{}'")
.comment('JSON object containing server metadata (icons, remotes, tools, etc.)'),
).withTimestamps;
await copyTable(
tableName,
tempTableName,
['slug', 'status', 'version', 'registryUpdatedAt', 'data', 'createdAt', 'updatedAt'],
['slug', 'status', 'version', 'registryUpdatedAt', 'data', 'createdAt', 'updatedAt'],
);
await dropTable(tableName);
await runQuery(
`ALTER TABLE ${escape.tableName(tempTableName)} RENAME TO ${escape.tableName(tableName)}`,
);
}
async down({
copyTable,
runQuery,
escape,
schemaBuilder: { createTable, dropTable, column },
}: MigrationContext) {
const tableName = 'mcp_registry_server';
const tempTableName = 'tmp_mcp_registry_server';
await createTable(tempTableName)
.withColumns(
column('id').int.primary.autoGenerate2,
column('slug').varchar(255).notNull,
column('status')
.varchar(50)
.notNull.withEnumCheck(['active', 'deprecated'])
.comment(
'Server status in the MCP registry. Deprecated servers are not surfaced to users.',
),
column('version').varchar(50).notNull,
column('registryUpdatedAt').timestampNoTimezone(3).notNull,
column('data')
.json.notNull.default("'{}'")
.comment('JSON object containing server metadata (icons, remotes, tools, etc.)'),
)
.withUniqueConstraintOn(['slug']).withTimestamps;
await copyTable(
tableName,
tempTableName,
['slug', 'status', 'version', 'registryUpdatedAt', 'data', 'createdAt', 'updatedAt'],
['slug', 'status', 'version', 'registryUpdatedAt', 'data', 'createdAt', 'updatedAt'],
);
await dropTable(tableName);
await runQuery(
`ALTER TABLE ${escape.tableName(tempTableName)} RENAME TO ${escape.tableName(tableName)}`,
);
}
}

View File

@ -191,6 +191,7 @@ import { CreateInstanceAiObservationTables1784000000012 } from '../common/178400
import { SplitRedactionScopeInCustomRoles1784000000013 } from '../common/1784000000013-SplitRedactionScopeInCustomRoles';
import { PersistInstanceAiPendingConfirmations1784000000014 } from '../common/1784000000014-PersistInstanceAiPendingConfirmations';
import { AddSourceWorkflowIdToWorkflow1784000000015 } from '../common/1784000000015-AddSourceWorkflowIdToWorkflow';
import { UseSlugAsPrimaryKeyInMcpRegistryServer1784000000016 } from '../common/1784000000016-UseSlugAsPrimaryKeyInMcpRegistryServer';
import type { Migration } from '../migration-types';
export const postgresMigrations: Migration[] = [
@ -387,4 +388,5 @@ export const postgresMigrations: Migration[] = [
SplitRedactionScopeInCustomRoles1784000000013,
PersistInstanceAiPendingConfirmations1784000000014,
AddSourceWorkflowIdToWorkflow1784000000015,
UseSlugAsPrimaryKeyInMcpRegistryServer1784000000016,
];

View File

@ -184,6 +184,7 @@ import { CreateInstanceAiObservationTables1784000000012 } from '../common/178400
import { SplitRedactionScopeInCustomRoles1784000000013 } from '../common/1784000000013-SplitRedactionScopeInCustomRoles';
import { PersistInstanceAiPendingConfirmations1784000000014 } from '../common/1784000000014-PersistInstanceAiPendingConfirmations';
import { AddSourceWorkflowIdToWorkflow1784000000015 } from '../common/1784000000015-AddSourceWorkflowIdToWorkflow';
import { UseSlugAsPrimaryKeyInMcpRegistryServer1784000000016 } from '../common/1784000000016-UseSlugAsPrimaryKeyInMcpRegistryServer';
import type { Migration } from '../migration-types';
const sqliteMigrations: Migration[] = [
@ -373,6 +374,7 @@ const sqliteMigrations: Migration[] = [
SplitRedactionScopeInCustomRoles1784000000013,
PersistInstanceAiPendingConfirmations1784000000014,
AddSourceWorkflowIdToWorkflow1784000000015,
UseSlugAsPrimaryKeyInMcpRegistryServer1784000000016,
];
export { sqliteMigrations };

View File

@ -25,7 +25,7 @@ export class McpRegistryTestController {
const entities = [notionMockServer, linearMockServer].map(toEntity);
// Replace rather than upsert: a startup refresh can leave rows whose slug collides with our mocks at a different id, which `ON CONFLICT (id) DO UPDATE` does not cover.
// Replace rather than upsert to keep test seeds deterministic.
await this.repository.manager.transaction(async (manager) => {
await manager.createQueryBuilder().delete().from(McpRegistryServerEntity).execute();
await manager.insert(McpRegistryServerEntity, entities);

View File

@ -1,10 +1,9 @@
import { paginatedRequest, buildStrapiUpdateQuery } from '@/utils/strapi-utils';
import { paginatedRequest } from '@/utils/strapi-utils';
import { McpRegistryApiClient } from '../mcp-registry-api.client';
jest.mock('@/utils/strapi-utils', () => ({
paginatedRequest: jest.fn(),
buildStrapiUpdateQuery: jest.requireActual('@/utils/strapi-utils').buildStrapiUpdateQuery,
}));
const mockPaginatedRequest = paginatedRequest as jest.MockedFunction<typeof paginatedRequest>;
@ -86,10 +85,7 @@ describe('McpRegistryApiClient', () => {
});
it('should return servers from paginatedRequest', async () => {
const mockServers = [
{ id: 1, name: 'server-a' },
{ id: 2, name: 'server-b' },
];
const mockServers = [{ name: 'server-a' }, { name: 'server-b' }];
mockPaginatedRequest.mockResolvedValue(mockServers);
const result = await client.fetchAllServers();
@ -99,7 +95,7 @@ describe('McpRegistryApiClient', () => {
});
describe('fetchServersMetadata', () => {
it('should request only version and updatedAt fields with pageSize 500', async () => {
it('should request only slug, version and updatedAt fields with pageSize 500', async () => {
mockPaginatedRequest.mockResolvedValue([]);
await client.fetchServersMetadata();
@ -107,7 +103,7 @@ describe('McpRegistryApiClient', () => {
expect(mockPaginatedRequest).toHaveBeenCalledWith(
PRODUCTION_URL,
{
fields: ['version', 'updatedAt'],
fields: ['slug', 'version', 'updatedAt'],
pagination: { page: 1, pageSize: 500 },
},
{ throwOnError: true },
@ -116,8 +112,8 @@ describe('McpRegistryApiClient', () => {
it('should return metadata from paginatedRequest', async () => {
const mockMetadata = [
{ id: 1, version: '1.0.0', updatedAt: '2025-01-01' },
{ id: 2, version: '2.0.0', updatedAt: '2025-01-02' },
{ slug: 'server-a', version: '1.0.0', updatedAt: '2025-01-01' },
{ slug: 'server-b', version: '2.0.0', updatedAt: '2025-01-02' },
];
mockPaginatedRequest.mockResolvedValue(mockMetadata);
@ -127,16 +123,20 @@ describe('McpRegistryApiClient', () => {
});
});
describe('fetchServersByIds', () => {
it('should fetch servers using filter query built from ids', async () => {
describe('fetchServersBySlugs', () => {
it('should fetch servers using filter query built from slugs', async () => {
mockPaginatedRequest.mockResolvedValue([]);
await client.fetchServersByIds([1, 2, 3]);
await client.fetchServersBySlugs(['server-a', 'server-b', 'server-c']);
expect(mockPaginatedRequest).toHaveBeenCalledWith(
PRODUCTION_URL,
{
...buildStrapiUpdateQuery([1, 2, 3]),
filters: {
slug: {
$in: ['server-a', 'server-b', 'server-c'],
},
},
pagination: { page: 1, pageSize: 25 },
},
{ throwOnError: true },
@ -144,57 +144,69 @@ describe('McpRegistryApiClient', () => {
});
it('should return fetched servers', async () => {
const mockServers = [{ id: 1, name: 'server-a' }];
const mockServers = [{ name: 'server-a' }];
mockPaginatedRequest.mockResolvedValue(mockServers);
const result = await client.fetchServersByIds([1]);
const result = await client.fetchServersBySlugs(['server-a']);
expect(result).toEqual(mockServers);
});
it('should return empty array for empty ids', async () => {
const result = await client.fetchServersByIds([]);
it('should return empty array for empty slugs', async () => {
const result = await client.fetchServersBySlugs([]);
expect(result).toEqual([]);
expect(mockPaginatedRequest).not.toHaveBeenCalled();
});
it('should batch ids in chunks of 100', async () => {
const ids = Array.from({ length: 250 }, (_, i) => i + 1);
it('should batch slugs in chunks of 100', async () => {
const slugs = Array.from({ length: 250 }, (_, i) => `server-${i + 1}`);
mockPaginatedRequest.mockResolvedValue([]);
await client.fetchServersByIds(ids);
await client.fetchServersBySlugs(slugs);
expect(mockPaginatedRequest).toHaveBeenCalledTimes(3);
// First batch: ids 1-100
// First batch: slugs 1-100
expect(mockPaginatedRequest).toHaveBeenNthCalledWith(
1,
PRODUCTION_URL,
{
...buildStrapiUpdateQuery(ids.slice(0, 100)),
filters: {
slug: {
$in: slugs.slice(0, 100),
},
},
pagination: { page: 1, pageSize: 25 },
},
{ throwOnError: true },
);
// Second batch: ids 101-200
// Second batch: slugs 101-200
expect(mockPaginatedRequest).toHaveBeenNthCalledWith(
2,
PRODUCTION_URL,
{
...buildStrapiUpdateQuery(ids.slice(100, 200)),
filters: {
slug: {
$in: slugs.slice(100, 200),
},
},
pagination: { page: 1, pageSize: 25 },
},
{ throwOnError: true },
);
// Third batch: ids 201-250
// Third batch: slugs 201-250
expect(mockPaginatedRequest).toHaveBeenNthCalledWith(
3,
PRODUCTION_URL,
{
...buildStrapiUpdateQuery(ids.slice(200, 250)),
filters: {
slug: {
$in: slugs.slice(200, 250),
},
},
pagination: { page: 1, pageSize: 25 },
},
{ throwOnError: true },
@ -202,12 +214,12 @@ describe('McpRegistryApiClient', () => {
});
it('should concatenate results from all batches', async () => {
const ids = Array.from({ length: 150 }, (_, i) => i + 1);
const batch1 = [{ id: 1, name: 'server-1' }];
const batch2 = [{ id: 101, name: 'server-101' }];
const slugs = Array.from({ length: 150 }, (_, i) => `server-${i + 1}`);
const batch1 = [{ name: 'server-1' }];
const batch2 = [{ name: 'server-101' }];
mockPaginatedRequest.mockResolvedValueOnce(batch1).mockResolvedValueOnce(batch2);
const result = await client.fetchServersByIds(ids);
const result = await client.fetchServersBySlugs(slugs);
expect(result).toEqual([...batch1, ...batch2]);
});

View File

@ -59,7 +59,7 @@ function createService(options: CreateServiceOptions = {}) {
}
apiClient.fetchServersMetadata.mockResolvedValue([]);
apiClient.fetchServersByIds.mockResolvedValue([]);
apiClient.fetchServersBySlugs.mockResolvedValue([]);
apiClient.fetchAllServers.mockResolvedValue([notionMockServer, linearMockServer]);
repository.upsert.mockResolvedValue({} as never);
@ -164,12 +164,12 @@ describe('McpRegistryService', () => {
const setIntervalSpy = jest.spyOn(global, 'setInterval');
const metadata: McpRegistryServerMetadata[] = [
{
id: notionMockServer.id,
slug: notionMockServer.slug,
version: notionMockServer.version,
updatedAt: notionMockServer.updatedAt,
},
{
id: linearMockServer.id,
slug: linearMockServer.slug,
version: linearMockServer.version,
updatedAt: linearMockServer.updatedAt,
},
@ -179,7 +179,7 @@ describe('McpRegistryService', () => {
await service.onLeaderTakeover();
expect(apiClient.fetchServersByIds).not.toHaveBeenCalled();
expect(apiClient.fetchServersBySlugs).not.toHaveBeenCalled();
expect(repository.upsert).not.toHaveBeenCalled();
expect(push.broadcast).not.toHaveBeenCalled();
expect(publisher.publishCommand).not.toHaveBeenCalled();
@ -188,6 +188,40 @@ describe('McpRegistryService', () => {
service.shutdown();
});
it('onLeaderTakeover deprecates servers missing from metadata', async () => {
const metadata: McpRegistryServerMetadata[] = [
{
slug: notionMockServer.slug,
version: notionMockServer.version,
updatedAt: notionMockServer.updatedAt,
},
];
const { service, apiClient, repository, push, publisher } = createService({
storedServers: [notionMockServer, linearMockServer],
});
apiClient.fetchServersMetadata.mockResolvedValue(metadata);
await service.onLeaderTakeover();
expect(apiClient.fetchServersBySlugs).not.toHaveBeenCalled();
expect(repository.upsert).toHaveBeenCalledTimes(1);
const upsertEntities = repository.upsert.mock.calls[0][0];
expect(upsertEntities).toEqual([
{
...toEntity({
...linearMockServer,
status: 'deprecated',
}),
registryUpdatedAt: expect.any(Date),
},
]);
expect(repository.upsert.mock.calls[0][1]).toEqual(['slug']);
expect(push.broadcast).toHaveBeenCalledWith({ type: 'nodeDescriptionUpdated', data: {} });
expect(publisher.publishCommand).toHaveBeenCalledWith({ command: 'reload-mcp-registry' });
service.shutdown();
});
it('onLeaderTakeover fetches only changed servers and publishes reload', async () => {
const staleNotion: McpRegistryServer = {
...notionMockServer,
@ -196,12 +230,12 @@ describe('McpRegistryService', () => {
};
const metadata: McpRegistryServerMetadata[] = [
{
id: notionMockServer.id,
slug: notionMockServer.slug,
version: notionMockServer.version,
updatedAt: notionMockServer.updatedAt,
},
{
id: linearMockServer.id,
slug: linearMockServer.slug,
version: linearMockServer.version,
updatedAt: linearMockServer.updatedAt,
},
@ -210,12 +244,12 @@ describe('McpRegistryService', () => {
storedServers: [staleNotion, linearMockServer],
});
apiClient.fetchServersMetadata.mockResolvedValue(metadata);
apiClient.fetchServersByIds.mockResolvedValue([notionMockServer]);
apiClient.fetchServersBySlugs.mockResolvedValue([notionMockServer]);
await service.onLeaderTakeover();
expect(apiClient.fetchAllServers).not.toHaveBeenCalled();
expect(apiClient.fetchServersByIds).toHaveBeenCalledWith([notionMockServer.id]);
expect(apiClient.fetchServersBySlugs).toHaveBeenCalledWith([notionMockServer.slug]);
expect(repository.upsert).toHaveBeenCalledTimes(1);
const upsertEntities = repository.upsert.mock.calls[0][0];
expect(upsertEntities).toEqual([notionMockServer].map(toEntity));

View File

@ -1,10 +1,10 @@
import { Service } from '@n8n/di';
import { buildStrapiUpdateQuery, paginatedRequest } from '@/utils/strapi-utils';
import { paginatedRequest } from '@/utils/strapi-utils';
import type { McpRegistryServer } from './mcp-registry.types';
export type McpRegistryServerMetadata = Pick<McpRegistryServer, 'id' | 'version' | 'updatedAt'>;
export type McpRegistryServerMetadata = Pick<McpRegistryServer, 'slug' | 'version' | 'updatedAt'>;
const MCP_SERVERS_STAGING_URL = 'https://api-staging.n8n.io/api/mcp-servers';
const MCP_SERVERS_PRODUCTION_URL = 'https://api.n8n.io/api/mcp-servers';
@ -30,7 +30,7 @@ export class McpRegistryApiClient {
return await paginatedRequest<McpRegistryServerMetadata>(
this.getUrl(),
{
fields: ['version', 'updatedAt'],
fields: ['slug', 'version', 'updatedAt'],
pagination: { page: 1, pageSize: 500 },
},
{
@ -39,15 +39,18 @@ export class McpRegistryApiClient {
);
}
async fetchServersByIds(ids: number[]): Promise<McpRegistryServer[]> {
async fetchServersBySlugs(slugs: string[]): Promise<McpRegistryServer[]> {
const data: McpRegistryServer[] = [];
for (let i = 0; i < ids.length; i += STRAPI_ARRAY_LIMIT) {
const batch = ids.slice(i, i + STRAPI_ARRAY_LIMIT);
const qs = buildStrapiUpdateQuery(batch);
for (let i = 0; i < slugs.length; i += STRAPI_ARRAY_LIMIT) {
const batch = slugs.slice(i, i + STRAPI_ARRAY_LIMIT);
const batchData = await paginatedRequest<McpRegistryServer>(
this.getUrl(),
{
...qs,
filters: {
slug: {
$in: batch,
},
},
pagination: { page: 1, pageSize: 25 },
},
{

View File

@ -1,5 +1,5 @@
import { datetimeColumnType, JsonColumn, WithTimestamps } from '@n8n/db';
import { Column, Entity, Index, PrimaryColumn } from '@n8n/typeorm';
import { Column, Entity, PrimaryColumn } from '@n8n/typeorm';
export type McpRegistryServerData = {
name: string;
@ -29,11 +29,7 @@ export type McpRegistryServerData = {
@Entity('mcp_registry_server')
export class McpRegistryServerEntity extends WithTimestamps {
@PrimaryColumn('int')
id: number;
@Index({ unique: true })
@Column('varchar')
@PrimaryColumn('varchar')
slug: string;
@Column('varchar')

View File

@ -156,16 +156,27 @@ export class McpRegistryService {
private async refreshUpdatedServers(
existingServers: McpRegistryServer[],
): Promise<McpRegistryServer[] | null> {
const now = new Date().toISOString();
const metadata = await this.apiClient.fetchServersMetadata();
const existingById = new Map(existingServers.map((server) => [server.id, server]));
const idsToFetch = metadata
.filter((entry) => this.shouldFetchFullServer(entry, existingById.get(entry.id)))
.map(({ id }) => id);
if (idsToFetch.length === 0) {
const existingBySlug = new Map(existingServers.map((server) => [server.slug, server]));
const metadataSlugs = new Set(metadata.map(({ slug }) => slug));
const slugsToFetch = metadata
.filter((entry) => this.shouldFetchFullServer(entry, existingBySlug.get(entry.slug)))
.map(({ slug }) => slug);
const serversToDeprecate = existingServers
.filter((server) => !metadataSlugs.has(server.slug) && server.status !== 'deprecated')
.map((server) => ({ ...server, status: 'deprecated' as const, updatedAt: now }));
if (slugsToFetch.length === 0 && serversToDeprecate.length === 0) {
return null;
}
return await this.apiClient.fetchServersByIds(idsToFetch);
if (slugsToFetch.length === 0) {
return serversToDeprecate;
}
const updatedServers = await this.apiClient.fetchServersBySlugs(slugsToFetch);
return [...updatedServers, ...serversToDeprecate];
}
private shouldFetchFullServer(
@ -186,7 +197,9 @@ export class McpRegistryService {
// it will break workflows that use them.
// If we want to stop supporting a server,
// we will set its status to 'deprecated' instead.
await this.repository.upsert(entities, ['id']);
// If a server is removed from the remote API,
// it will be marked as deprecated as well.
await this.repository.upsert(entities, ['slug']);
}
private async refreshRegistryNodeTypes(releaseTypes: boolean): Promise<void> {

View File

@ -2,7 +2,7 @@ import type { McpRegistryServerEntity } from './mcp-registry-server.entity';
type McpRegistryServerUpsertRow = Pick<
McpRegistryServerEntity,
'id' | 'slug' | 'status' | 'version' | 'registryUpdatedAt' | 'data'
'slug' | 'status' | 'version' | 'registryUpdatedAt' | 'data'
>;
const serverStatuses = ['active', 'deprecated'] as const;
@ -13,7 +13,6 @@ type McpRegistryServerStatus = (typeof serverStatuses)[number];
* The shape of an entry returned by the MCP server registry.
*/
export type McpRegistryServer = {
id: number;
name: string;
slug: string;
title: string;
@ -56,7 +55,7 @@ export type McpRegistryTool = {
};
export function toEntity(server: McpRegistryServer): McpRegistryServerUpsertRow {
const { id, slug, status, version, updatedAt, ...rest } = server;
const { slug, status, version, updatedAt, ...rest } = server;
let mappedStatus = status;
// make sure that unknown statuses get mapped to a valid value
if (!serverStatuses.includes(status)) {
@ -64,7 +63,6 @@ export function toEntity(server: McpRegistryServer): McpRegistryServerUpsertRow
}
return {
id,
slug,
status: mappedStatus,
version,
@ -74,9 +72,8 @@ export function toEntity(server: McpRegistryServer): McpRegistryServerUpsertRow
}
export function fromEntity(entity: McpRegistryServerEntity): McpRegistryServer {
const { id, slug, status, version, registryUpdatedAt, data } = entity;
const { slug, status, version, registryUpdatedAt, data } = entity;
return {
id,
slug,
status,
version,

View File

@ -1,7 +1,6 @@
import type { McpRegistryServer } from './mcp-registry.types';
export const notionMockServer: McpRegistryServer = {
id: 42,
name: 'com.notion/mcp',
slug: 'notion',
title: 'Notion',
@ -41,7 +40,6 @@ export const notionMockServer: McpRegistryServer = {
};
export const linearMockServer: McpRegistryServer = {
id: 101,
name: 'app.linear/linear',
slug: 'linear',
title: 'Linear',

View File

@ -16,12 +16,7 @@ test.describe(
annotation: [{ type: 'owner', description: 'AI' }],
},
() => {
// NODE-5089 - ticket to fix and enable this test again
// eslint-disable-next-line playwright/no-skipped-test
test.skip('exposes Notion MCP as a tool with hidden connection fields', async ({
n8n,
api,
}) => {
test('exposes Notion MCP as a tool with hidden connection fields', async ({ n8n, api }) => {
await api.seedMcpRegistry();
await n8n.start.fromBlankCanvas();