diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index 8f4e2002a5..a052544d11 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -769,5 +769,40 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); expect(res.body!.config).toEqual({ interface: { modelSelect: true } }); }); + + it('forwards baseOnly=true to getAppConfig when query param is the literal string "true"', async () => { + const getAppConfig = jest.fn().mockResolvedValue({ interface: { modelSelect: true } }); + const { handlers } = createHandlers({ getAppConfig }); + const req = mockReq({ query: { baseOnly: 'true' } }); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + expect(getAppConfig).toHaveBeenCalledWith(expect.objectContaining({ baseOnly: true })); + }); + + it('forwards baseOnly=false when the query param is missing, non-"true", or an array', async () => { + const cases: Array> = [ + {}, + { baseOnly: 'false' }, + { baseOnly: '1' }, + { baseOnly: ['true'] }, + { baseOnly: ['true', 'true'] }, + { baseOnly: { nested: 'true' } }, + ]; + + for (const query of cases) { + const getAppConfig = jest.fn().mockResolvedValue({ interface: { modelSelect: true } }); + const { handlers } = createHandlers({ getAppConfig }); + const req = mockReq({ query }); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + expect(getAppConfig).toHaveBeenCalledWith(expect.objectContaining({ baseOnly: false })); + } + }); }); }); diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index c1ce2cb13f..dc07a9d18f 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -108,6 +108,7 @@ export interface AdminConfigDeps { role?: string; userId?: string; tenantId?: string; + baseOnly?: boolean; }) => Promise; /** Invalidate all config-related caches after a mutation. */ invalidateConfigCaches?: (tenantId?: string) => Promise; @@ -211,8 +212,10 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) { return res.status(501).json({ error: 'Base config endpoint not configured' }); } + const baseOnly = (req.query as Record).baseOnly === 'true'; const appConfig = await getAppConfig({ tenantId: user.tenantId, + baseOnly, }); return res.status(200).json({ config: appConfig }); } catch (error) { diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index 81c59878ff..6c0e95b6ad 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -17,6 +17,62 @@ import { withTimeout } from '~/utils'; /** How long a failure stub is considered fresh before re-attempting inspection (5 minutes). */ const CONFIG_STUB_RETRY_MS = 5 * 60 * 1000; +/** + * Fields an admin override can legitimately set. Used to detect whether a + * resolved entry differs from its YAML base so unmodified YAML servers can + * skip lazy-init (avoids per-request inspect storms and prevents these + * servers from being cached in the config tier). + */ +const ADMIN_CONFIGURABLE_FIELDS = [ + 'type', + 'command', + 'args', + 'env', + 'stderr', + 'url', + 'headers', + 'proxy', + 'requiresOAuth', + 'apiKey', + 'oauth', + 'oauth_headers', + 'title', + 'description', + 'iconPath', + 'startup', + 'chatMenu', + 'serverInstructions', + 'customUserVars', + 'timeout', + 'sseReadTimeout', + 'initTimeout', +] as const; + +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a == null || b == null) return a === b; + if (typeof a !== typeof b) return false; + if (typeof a !== 'object') return false; + if (Array.isArray(a)) { + if (!Array.isArray(b) || a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (!deepEqual(a[i], b[i])) return false; + } + return true; + } + if (Array.isArray(b)) return false; + const aObj = a as Record; + const bObj = b as Record; + const aKeys = Object.keys(aObj); + const bKeys = Object.keys(bObj); + if (aKeys.length !== bKeys.length) return false; + for (const key of aKeys) { + if (!Object.prototype.hasOwnProperty.call(bObj, key)) return false; + if (!deepEqual(aObj[key], bObj[key])) return false; + } + return true; +} + const CONFIG_SERVER_INIT_TIMEOUT_MS = (() => { const raw = process.env.MCP_INIT_TIMEOUT_MS; if (raw == null) { @@ -133,38 +189,58 @@ export class MCPServersRegistry { } /** - * Returns the config for a single server. When `configServers` is provided, config-source - * servers are resolved from it directly (no global state, no cross-tenant race). + * Returns the config for a single server, mirroring the precedence used by + * getAllServerConfigs so list views and single-server lookups agree on + * the same name: + * 1. user-tier base entry wins absolutely over a config-tier candidate + * 2. healthy YAML/DB base wins over a failed (inspectionFailed) candidate + * 3. healthy candidate overlays its fields onto the base, preserving the + * base entry's source tag so downstream recovery routes correctly + * 4. with no base, the candidate is returned as-is (config-only server) + * + * readThroughCache memoizes only the global YAML/DB lookup; the per-call + * configServers candidate is tenant-scoped and is never cached, so a + * failed stub from one tenant can never satisfy a no-userId lookup from + * another. */ public async getServerConfig( serverName: string, userId?: string, configServers?: Record, ): Promise { - if (configServers?.[serverName]) { - return configServers[serverName]; - } + const candidate = configServers?.[serverName]; const cacheKey = this.getReadThroughCacheKey(serverName, userId); - + let base: t.ParsedServerConfig | undefined; if (await this.readThroughCache.has(cacheKey)) { - return await this.readThroughCache.get(cacheKey); + base = await this.readThroughCache.get(cacheKey); + } else { + const configFromYaml = await this.cacheConfigsRepo.get(serverName); + if (configFromYaml) { + base = configFromYaml; + } else { + base = await this.dbConfigsRepo.get(serverName, userId); + } + await this.readThroughCache.set(cacheKey, base); } - const configFromYaml = await this.cacheConfigsRepo.get(serverName); - if (configFromYaml) { - await this.readThroughCache.set(cacheKey, configFromYaml); - return configFromYaml; - } - - const configFromDB = await this.dbConfigsRepo.get(serverName, userId); - await this.readThroughCache.set(cacheKey, configFromDB); - return configFromDB; + if (!candidate) return base; + if (base?.source === 'user') return base; + if (candidate.inspectionFailed) return base ?? candidate; + return base ? { ...candidate, source: base.source } : candidate; } /** - * Returns all server configs visible to the given user. - * Operator-managed servers (YAML + Config) override User DB servers on name collisions. + * Returns the full server config map after merging YAML cache, Config-tier overrides, + * and User-DB entries. + * + * Precedence (lowest to highest): YAML cache > Config-tier overrides (success only) > User DB. + * Two guards keep the merge safe: + * 1. Config-tier entries carrying `inspectionFailed: true` never overlay an existing + * base entry; the healthy base is preserved for the duration of the retry window. + * 2. User-DB entries (`source: 'user'`) are never replaced by Config-tier overlays. + * On a successful overlay the base entry's `source` field is preserved so downstream + * recovery logic routes to the correct storage location. */ public async getAllServerConfigs( userId?: string, @@ -175,8 +251,17 @@ export class MCPServersRegistry { return this.getBaseServerConfigs(userId, role); } const base = await this.getBaseServerConfigs(userId, role); - this.warnOnOperatorManagedNameCollisions(configServers, base, 'Config'); - return { ...base, ...configServers }; + const result: Record = { ...base }; + for (const [name, override] of Object.entries(configServers)) { + if (result[name]?.source === 'user') { + logger.debug(`[MCP][config][${name}] Admin override shadowed by user-tier entry`); + continue; + } + if (override.inspectionFailed && result[name]) continue; + const baseSource = result[name]?.source; + result[name] = baseSource ? { ...override, source: baseSource } : override; + } + return result; } /** @@ -393,19 +478,16 @@ export class MCPServersRegistry { return {}; } - const yamlNames = await this.getYamlServerNames(); - const configServerEntries = Object.entries(resolvedMcpConfig).filter( - ([name]) => !yamlNames.has(name), - ); - - if (configServerEntries.length === 0) { - return {}; - } - const result: Record = {}; + /** Single snapshot of the YAML cache for the whole pass: in the Redis aggregate-key backend, every per-name get() reads and deserializes the full map, so N concurrent per-server lookups would issue N full-map reads. The snapshot also keeps the unchanged-YAML comparison consistent against one view of YAML across all entries. */ + const yamlSnapshot = await this.cacheConfigsRepo.getAll(); + const settled = await Promise.allSettled( - configServerEntries.map(async ([serverName, rawConfig]) => { + Object.entries(resolvedMcpConfig).map(async ([serverName, rawConfig]) => { + if (this.isUnmodifiedYamlServer(yamlSnapshot, serverName, rawConfig)) { + return; + } const parsed = await this.ensureSingleConfigServer(serverName, rawConfig); if (parsed) { result[serverName] = parsed; @@ -421,6 +503,31 @@ export class MCPServersRegistry { return result; } + /** + * Returns true when `rawConfig` matches the YAML cache entry for this server + * on every admin-configurable field, so an unmodified YAML-defined server + * can skip lazy-init and avoid being re-inspected or shadowed in the + * config tier. + */ + private isUnmodifiedYamlServer( + yamlSnapshot: Record, + serverName: string, + rawConfig: t.MCPOptions, + ): boolean { + const yamlEntry = yamlSnapshot[serverName]; + if (!yamlEntry || yamlEntry.source !== 'yaml') { + return false; + } + const yamlRecord = yamlEntry as unknown as Record; + const rawRecord = rawConfig as unknown as Record; + /** rawConfig is the pre-inspection MCPOptions; absent fields mean the admin didn't override and shouldn't count as a diff against inspector-derived values on the cached YAML entry. */ + return ADMIN_CONFIGURABLE_FIELDS.every((field) => { + const rawVal = rawRecord[field]; + if (rawVal === undefined) return true; + return deepEqual(yamlRecord[field], rawVal); + }); + } + /** * Ensures a single config-source server is initialized. * Cache key is scoped by config hash to prevent cross-tenant poisoning. diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index a1298dcd6e..b1a86864b0 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -141,7 +141,7 @@ describe('MCPServersRegistry', () => { } }); - it('should warn when config servers shadow DB servers', async () => { + it('should preserve the user-tier entry over a config-tier override on the same name without emitting a misleading shadow warning', async () => { const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(); const configServer = { ...testParsedConfig, @@ -152,10 +152,11 @@ describe('MCPServersRegistry', () => { jest.spyOn(registry['dbConfigsRepo'], 'getAll').mockResolvedValue({ slack: dbConfig }); try { - await registry.getAllServerConfigs('user-1', { slack: configServer }); + const result = await registry.getAllServerConfigs('user-1', { slack: configServer }); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Config MCP server')); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('slack')); + expect(result.slack.source).toBe('user'); + expect(result.slack.title).toBe('User Slack'); + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('Config MCP server')); } finally { warnSpy.mockRestore(); } @@ -402,4 +403,312 @@ describe('MCPServersRegistry', () => { }); }); }); + + describe('admin-panel overrides for YAML-defined servers', () => { + const yamlLangfuseConfig = Object.freeze({ + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'yaml', + updatedAt: FIXED_TIME, + }) as t.ParsedServerConfig; + + it('flows config-tier override on a YAML-defined server through to getAllServerConfigs', async () => { + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlLangfuseConfig); + + const overrideRawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + iconPath: 'https://example.com/icon.svg', + }; + + const configServers = await registry.ensureConfigServers({ + 'langfuse-docs': overrideRawConfig, + }); + + expect(configServers['langfuse-docs']).toBeDefined(); + expect(configServers['langfuse-docs'].iconPath).toBe('https://example.com/icon.svg'); + + const result = await registry.getAllServerConfigs('user-1', configServers); + + expect(result['langfuse-docs']).toBeDefined(); + expect(result['langfuse-docs'].iconPath).toBe('https://example.com/icon.svg'); + expect(result['langfuse-docs'].source).toBe('yaml'); + }); + + it('preserves user-DB tier (source: "user") over config-tier overrides', async () => { + const userDbEntry: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://user-defined.example.com/mcp', + requiresOAuth: false, + source: 'user', + dbId: 'user-db-id-123', + updatedAt: FIXED_TIME, + }; + + jest + .spyOn(registry['dbConfigsRepo'], 'getAll') + .mockResolvedValue({ 'shared-server': userDbEntry }); + + const configTierOverride: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://admin-override.example.com/mcp', + requiresOAuth: false, + source: 'config', + iconPath: 'https://example.com/admin-icon.svg', + updatedAt: FIXED_TIME, + }; + + const result = await registry.getAllServerConfigs('user-1', { + 'shared-server': configTierOverride, + }); + + expect(result['shared-server']).toBeDefined(); + expect(result['shared-server'].source).toBe('user'); + expect(result['shared-server'].url).toBe('https://user-defined.example.com/mcp'); + expect(result['shared-server'].dbId).toBe('user-db-id-123'); + }); + + it('still runs lazy-init for pure config-tier servers not present in YAML', async () => { + const configOnlyRawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://config-only.example.com/mcp', + requiresOAuth: false, + iconPath: 'https://example.com/config-only-icon.svg', + }; + + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + inspectSpy.mockClear(); + + const result = await registry.ensureConfigServers({ + 'config-only-server': configOnlyRawConfig, + }); + + expect(inspectSpy).toHaveBeenCalledTimes(1); + expect(inspectSpy).toHaveBeenCalledWith( + 'config-only-server', + configOnlyRawConfig, + undefined, + undefined, + undefined, + ); + expect(result['config-only-server']).toBeDefined(); + expect(result['config-only-server'].iconPath).toBe( + 'https://example.com/config-only-icon.svg', + ); + expect(result['config-only-server'].source).toBe('config'); + }); + + it('preserves YAML base entry when config-tier override reports inspectionFailed', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + headers: { Authorization: 'Bearer yaml-token' }, + source: 'yaml', + tools: 'yaml_tool_a, yaml_tool_b', + capabilities: '{"tools":{"listChanged":true}}', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const failedOverride: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'config', + inspectionFailed: true, + updatedAt: FIXED_TIME, + }; + + const result = await registry.getAllServerConfigs('user-1', { + 'langfuse-docs': failedOverride, + }); + + expect(result['langfuse-docs']).toBeDefined(); + expect(result['langfuse-docs'].source).toBe('yaml'); + expect(result['langfuse-docs'].inspectionFailed).toBeUndefined(); + expect(result['langfuse-docs'].tools).toBe('yaml_tool_a, yaml_tool_b'); + expect(result['langfuse-docs'].capabilities).toBe('{"tools":{"listChanged":true}}'); + expect((result['langfuse-docs'] as t.StreamableHTTPOptions).headers).toEqual({ + Authorization: 'Bearer yaml-token', + }); + }); + + it('healthy YAML entry survives end-to-end when inspect throws for YAML server', async () => { + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlLangfuseConfig); + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + inspectSpy.mockClear(); + inspectSpy.mockRejectedValueOnce(new Error('network timeout')); + + const configServers = await registry.ensureConfigServers({ + 'langfuse-docs': { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + iconPath: 'https://example.com/icon.svg', + } as t.MCPOptions, + }); + const result = await registry.getAllServerConfigs('user-1', configServers); + + expect(result['langfuse-docs'].source).toBe('yaml'); + expect(result['langfuse-docs'].inspectionFailed).toBeUndefined(); + expect(result['langfuse-docs'].url).toBe('https://langfuse.com/api/public/mcp'); + }); + + it('preserves YAML source tag when config-tier override succeeds', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'yaml', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const successfulOverride: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'config', + iconPath: 'https://example.com/icon.svg', + updatedAt: FIXED_TIME, + }; + + const result = await registry.getAllServerConfigs('user-1', { + 'langfuse-docs': successfulOverride, + }); + + expect(result['langfuse-docs']).toBeDefined(); + expect(result['langfuse-docs'].source).toBe('yaml'); + expect(result['langfuse-docs'].iconPath).toBe('https://example.com/icon.svg'); + }); + + it('skips lazy-init for YAML server with no admin override', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'yaml', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const yamlRawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + }; + + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + inspectSpy.mockClear(); + + const result = await registry.ensureConfigServers({ + 'langfuse-docs': yamlRawConfig, + }); + + expect(inspectSpy).not.toHaveBeenCalled(); + expect(result['langfuse-docs']).toBeUndefined(); + }); + + it('runs lazy-init for YAML server with admin override', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'yaml', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const overrideRawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + iconPath: 'https://x.com/icon.svg', + }; + + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + inspectSpy.mockClear(); + + await registry.ensureConfigServers({ + 'langfuse-docs': overrideRawConfig, + }); + + expect(inspectSpy).toHaveBeenCalledTimes(1); + }); + + it('getServerConfig falls through to YAML on failure stub', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'yaml', + tools: 'yaml_tool_a', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const failureStub: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + source: 'config', + inspectionFailed: true, + updatedAt: FIXED_TIME, + }; + + const result = await registry.getServerConfig('langfuse-docs', 'user-1', { + 'langfuse-docs': failureStub, + }); + + expect(result).toBeDefined(); + expect(result?.source).toBe('yaml'); + expect(result?.inspectionFailed).toBeUndefined(); + expect(result?.tools).toBe('yaml_tool_a'); + }); + + it('passes a fully merged config to lazy-init when override only adds new fields', async () => { + const yamlSeed: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + headers: { Authorization: 'Bearer yaml-token' }, + source: 'yaml', + updatedAt: FIXED_TIME, + }; + await registry['cacheConfigsRepo'].add('langfuse-docs', yamlSeed); + + const mergedRawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://langfuse.com/api/public/mcp', + requiresOAuth: false, + headers: { Authorization: 'Bearer yaml-token' }, + iconPath: 'https://example.com/icon.svg', + }; + + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + inspectSpy.mockClear(); + + const result = await registry.ensureConfigServers({ + 'langfuse-docs': mergedRawConfig, + }); + + expect(inspectSpy).toHaveBeenCalledTimes(1); + const firstCallArgs = inspectSpy.mock.calls[0]; + expect(firstCallArgs[0]).toBe('langfuse-docs'); + const passedConfig = firstCallArgs[1] as t.StreamableHTTPOptions & { + iconPath?: string; + requiresOAuth?: boolean; + }; + expect(passedConfig.type).toBe('streamable-http'); + expect(passedConfig.url).toBe('https://langfuse.com/api/public/mcp'); + expect(passedConfig.requiresOAuth).toBe(false); + expect(passedConfig.headers).toEqual({ Authorization: 'Bearer yaml-token' }); + expect(passedConfig.iconPath).toBe('https://example.com/icon.svg'); + + expect(result['langfuse-docs']).toBeDefined(); + expect(result['langfuse-docs'].iconPath).toBe('https://example.com/icon.svg'); + }); + }); }); diff --git a/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts b/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts index 9b601dcee2..5db4218a27 100644 --- a/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts +++ b/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts @@ -22,24 +22,21 @@ const mockMongoose = {} as typeof import('mongoose'); const sseConfig: t.MCPOptions = { type: 'sse', url: 'https://mcp.example.com/sse', -} as unknown as t.MCPOptions; +}; const altSseConfig: t.MCPOptions = { type: 'sse', url: 'https://mcp.other-tenant.com/sse', -} as unknown as t.MCPOptions; +}; const yamlConfig: t.MCPOptions = { type: 'stdio', command: 'node', args: ['tools.js'], -} as unknown as t.MCPOptions; +}; function makeParsedConfig(overrides: Partial = {}): t.ParsedServerConfig { return { - type: 'sse', - url: 'https://mcp.example.com/sse', - requiresOAuth: false, tools: 'tool_a, tool_b', capabilities: '{}', initDuration: 42, @@ -91,8 +88,9 @@ describe('MCPServersRegistry — ensureConfigServers', () => { ).toEqual({}); }); - it('should exclude YAML servers from config-source detection', async () => { + it('should skip unchanged YAML-named servers but still process config-only servers', async () => { await registry.addServer('yaml_server', yamlConfig, 'CACHE'); + inspectSpy.mockClear(); const result = await registry.ensureConfigServers({ yaml_server: yamlConfig, @@ -101,9 +99,17 @@ describe('MCPServersRegistry — ensureConfigServers', () => { expect(result).toHaveProperty('config_server'); expect(result).not.toHaveProperty('yaml_server'); + expect(inspectSpy).toHaveBeenCalledTimes(1); + expect(inspectSpy).toHaveBeenCalledWith( + 'config_server', + sseConfig, + undefined, + undefined, + undefined, + ); }); - it('should return empty when all servers are YAML', async () => { + it('should skip lazy-init for YAML-named servers with no admin override', async () => { await registry.addServer('yaml_a', yamlConfig, 'CACHE'); await registry.addServer('yaml_b', yamlConfig, 'CACHE'); inspectSpy.mockClear(); @@ -113,10 +119,74 @@ describe('MCPServersRegistry — ensureConfigServers', () => { yaml_b: yamlConfig, }); - expect(result).toEqual({}); + expect(result).not.toHaveProperty('yaml_a'); + expect(result).not.toHaveProperty('yaml_b'); expect(inspectSpy).not.toHaveBeenCalled(); }); + it('should lazy-init YAML-named server when admin override changes a field', async () => { + await registry.addServer('yaml_a', yamlConfig, 'CACHE'); + inspectSpy.mockClear(); + + const overrideConfig: t.MCPOptions = { ...yamlConfig, iconPath: 'https://x.com/icon.svg' }; + const result = await registry.ensureConfigServers({ + yaml_a: overrideConfig, + }); + + expect(result).toHaveProperty('yaml_a'); + expect(inspectSpy).toHaveBeenCalledTimes(1); + }); + + it('should lazy-init YAML server when admin overrides only the proxy field', async () => { + await registry.addServer('yaml_remote', sseConfig, 'CACHE'); + inspectSpy.mockClear(); + + const overrideConfig: t.MCPOptions = { + ...sseConfig, + proxy: 'http://proxy.example.com:8080', + } as t.MCPOptions; + const result = await registry.ensureConfigServers({ + yaml_remote: overrideConfig, + }); + + expect(result).toHaveProperty('yaml_remote'); + expect(inspectSpy).toHaveBeenCalledTimes(1); + }); + + it('should not re-init YAML server when only the difference is an inspector-derived field absent from rawConfig', async () => { + const yamlWithInferred: t.MCPOptions = { + ...sseConfig, + requiresOAuth: false, + } as t.MCPOptions; + await registry.addServer('yaml_remote', yamlWithInferred, 'CACHE'); + inspectSpy.mockClear(); + + const result = await registry.ensureConfigServers({ + yaml_remote: sseConfig, + }); + + expect(result).not.toHaveProperty('yaml_remote'); + expect(inspectSpy).not.toHaveBeenCalled(); + }); + + it('should issue a single batched YAML cache read regardless of how many servers are checked', async () => { + await registry.addServer('yaml_a', yamlConfig, 'CACHE'); + await registry.addServer('yaml_b', yamlConfig, 'CACHE'); + await registry.addServer('yaml_c', yamlConfig, 'CACHE'); + const cacheGetSpy = jest.spyOn(registry['cacheConfigsRepo'], 'get'); + const cacheGetAllSpy = jest.spyOn(registry['cacheConfigsRepo'], 'getAll'); + + await registry.ensureConfigServers({ + yaml_a: yamlConfig, + yaml_b: yamlConfig, + yaml_c: yamlConfig, + config_only: sseConfig, + }); + + expect(cacheGetSpy).not.toHaveBeenCalled(); + expect(cacheGetAllSpy).toHaveBeenCalledTimes(1); + }); + it('should lazy-initialize a config-source server and tag source as config', async () => { const result = await registry.ensureConfigServers({ my_server: sseConfig }); @@ -305,6 +375,89 @@ describe('MCPServersRegistry — ensureConfigServers', () => { expect(config?.source).toBe('config'); }); + it('should surface inspectionFailed stub for config-only server when no YAML/DB fallback exists', async () => { + inspectSpy.mockRejectedValueOnce(new Error('connection refused')); + const configServers = await registry.ensureConfigServers({ bad_config_only: sseConfig }); + expect(configServers.bad_config_only.inspectionFailed).toBe(true); + + const config = await registry.getServerConfig('bad_config_only', undefined, configServers); + expect(config).toBeDefined(); + expect(config?.inspectionFailed).toBe(true); + expect(config?.source).toBe('config'); + }); + + it('should prefer healthy YAML entry over inspectionFailed config-tier stub on the same name', async () => { + await registry.addServer('shared_name', yamlConfig, 'CACHE'); + + inspectSpy.mockRejectedValueOnce(new Error('connection refused')); + const configServers = await registry.ensureConfigServers({ shared_name: sseConfig }); + expect(configServers.shared_name.inspectionFailed).toBe(true); + + const config = await registry.getServerConfig('shared_name', undefined, configServers); + expect(config).toBeDefined(); + expect(config?.inspectionFailed).toBeUndefined(); + expect(config?.source).toBe('yaml'); + }); + + it('should prefer the user-tier DB entry over a config-tier candidate on the same name', async () => { + const dbConfig = makeParsedConfig({ + source: 'user', + title: 'User Slack', + } as Partial); + jest.spyOn(registry['dbConfigsRepo'], 'get').mockResolvedValue(dbConfig); + + const configCandidate = makeParsedConfig({ + source: 'config', + title: 'Config Slack', + } as Partial); + + const result = await registry.getServerConfig('slack', 'user-1', { + slack: configCandidate, + }); + expect(result?.source).toBe('user'); + expect((result as unknown as { title: string }).title).toBe('User Slack'); + }); + + it('should overlay healthy admin override fields onto a YAML base while preserving yaml source', async () => { + const yamlEntry = makeParsedConfig({ + ...sseConfig, + source: 'yaml', + title: 'YAML Title', + } as unknown as Partial); + await registry['cacheConfigsRepo'].add('shared', yamlEntry); + + const adminOverride = makeParsedConfig({ + ...sseConfig, + source: 'config', + title: 'Admin Title', + } as unknown as Partial); + + const result = await registry.getServerConfig('shared', undefined, { + shared: adminOverride, + }); + expect(result?.source).toBe('yaml'); + expect((result as unknown as { title: string }).title).toBe('Admin Title'); + }); + + it('should not leak a tenant-scoped failure stub into subsequent no-configServers calls', async () => { + inspectSpy.mockRejectedValueOnce(new Error('connection refused')); + const tenantA = await registry.ensureConfigServers({ tenant_a_only: sseConfig }); + expect(tenantA.tenant_a_only.inspectionFailed).toBe(true); + + const firstLookup = await registry.getServerConfig('tenant_a_only', undefined, tenantA); + expect(firstLookup?.inspectionFailed).toBe(true); + expect(firstLookup?.source).toBe('config'); + + const leakedLookup = await registry.getServerConfig('tenant_a_only'); + expect(leakedLookup).toBeUndefined(); + + const tenantB = await registry.ensureConfigServers({ tenant_a_only: altSseConfig }); + const tenantBLookup = await registry.getServerConfig('tenant_a_only', undefined, tenantB); + expect((tenantBLookup as unknown as { url: string }).url).toBe( + 'https://mcp.other-tenant.com/sse', + ); + }); + it('should not cross-contaminate between tenant configServers maps', async () => { const tenantA = await registry.ensureConfigServers({ srv: sseConfig }); const tenantB = await registry.ensureConfigServers({ srv: altSseConfig });