feat(official-mcp-registry): Implement Federated Registry Provider for local and official package support #103
Conversation
Seey215
commented
Nov 4, 2025
Seey215
commented
- Added FederatedRegistryProvider to prioritize local package queries before official ones.
- Developed OfficialRegistryProvider to interact with the official API for package data.
- Established registry-factory for initializing and retrieving registry providers.
…r local and official package support - Added FederatedRegistryProvider to prioritize local package queries before official ones. - Developed OfficialRegistryProvider to interact with the official API for package data. - Established registry-factory for initializing and retrieving registry providers.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a federated registry system that enables the application to fetch MCP server packages from both local and official registries. The implementation follows a provider pattern with local-first fallback to the official registry API.
- Adds registry provider infrastructure with three providers: Local, Official, and Federated
- Implements transformation utilities to convert official API responses to the local package format
- Integrates federated registry lookup into the existing PackageSO factory method
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 43 comments.
Show a summary per file
| File | Description |
|---|---|
| src/domains/registry/registry-types.ts | Defines the IRegistryProvider interface and RegistrySource type |
| src/domains/registry/registry-schema.ts | Zod schemas for official registry API data structures |
| src/domains/registry/registry-utils.ts | Transformation logic to convert official API responses to local format |
| src/domains/registry/registry-factory.ts | Factory pattern implementation with singleton providers |
| src/domains/registry/providers/official-registry-provider.ts | HTTP client for official registry API with timeout handling |
| src/domains/registry/providers/local-registry-provider.ts | Async adapter wrapper for PackageRepository |
| src/domains/registry/providers/federated-registry-provider.ts | Local-first federated query implementation |
| src/domains/registry/index.ts | Public API exports for the registry module |
| src/domains/package/package-so.ts | Updated to use federated registry provider |
| src/domains/package/package-handler.ts | Exports repository for factory initialization |
| src/api/index.ts | Initializes registry factory on startup |
| src/domains/registry/tests/*.test.ts | Comprehensive test coverage for all providers and utilities |
| src/domains/package/package-so.test.ts | Updated tests for registry integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/domains/package/package-so.ts
Outdated
| const packageInfo = allPackages[packageName] || {}; | ||
| return new PackageSO(packageName, config, packageInfo, repository, executor); | ||
|
|
||
| return new PackageSO(packageName, config as MCPServerPackageConfig, packageInfo, executor); |
There was a problem hiding this comment.
The type assertion 'as MCPServerPackageConfig' is unnecessary here. The config is already checked for null on line 44, and the IRegistryProvider.getPackageConfig signature returns 'Promise<unknown | null>'. Since the method is typed to return 'unknown', consider updating the IRegistryProvider interface to have a generic type parameter or change the return type to 'Promise<MCPServerPackageConfig | null>' for type safety without requiring assertions.
| return new PackageSO(packageName, config as MCPServerPackageConfig, packageInfo, executor); | |
| return new PackageSO(packageName, config, packageInfo, executor); |
| }; | ||
|
|
||
| vi.spyOn(mockRepository, "exists").mockReturnValue(true); | ||
| vi.spyOn(mockRepository, "exists").mockReturnValue(true); |
There was a problem hiding this comment.
Line 52 is a duplicate of line 51. This duplicated spy setup should be removed.
| vi.spyOn(mockRepository, "exists").mockReturnValue(true); |
| * @param packageName - 包名 | ||
| * @returns 包配置,如果不存在返回 null | ||
| */ | ||
| getPackageConfig(packageName: string): Promise<unknown | null>; |
There was a problem hiding this comment.
The return type 'Promise<unknown | null>' is too generic. Consider using a generic type parameter or returning 'Promise<MCPServerPackageConfig | null>' directly to provide better type safety for consumers of this interface and eliminate the need for type assertions.
| getPackageConfig(packageName: string): Promise<unknown | null>; | |
| getPackageConfig(packageName: string): Promise<MCPServerPackageConfig | null>; |
| * 转换单个服务器配置 | ||
| * @param server - 官方服务器配置 | ||
| * @returns 本地格式配置,如果不符合条件返回 null |
There was a problem hiding this comment.
The JSDoc comments are in Chinese while the rest of the codebase appears to use English. For consistency and accessibility, consider translating these comments to English. For example: '转换单个服务器配置' should be 'Transform a single server configuration'.
| * 批量转换并过滤服务器配置 | ||
| * @param servers - 官方服务器配置数组 | ||
| * @returns 本地格式配置数组 |
There was a problem hiding this comment.
The JSDoc comments are in Chinese while the rest of the codebase appears to use English. For consistency, consider translating these comments to English. For example: '批量转换并过滤服务器配置' should be 'Batch transform and filter server configurations'.
| // 3. 转换为本地格式 | ||
| const config: MCPServerPackageConfig = { | ||
| type: "mcp-server", | ||
| runtime: "node", // npm 包默认为 node |
There was a problem hiding this comment.
The inline comment is in Chinese while the rest of the codebase appears to use English. For consistency, consider translating to English: 'npm packages default to node'.
| runtime: "node", // npm 包默认为 node | |
| runtime: "node", // npm packages default to node |
| runtime: "node", // npm 包默认为 node | ||
| packageName: pkg.identifier, // 使用 npm 包名作为 packageName |
There was a problem hiding this comment.
The inline comment is in Chinese while the rest of the codebase appears to use English. For consistency, consider translating to English: 'Use npm package name as packageName'.
| runtime: "node", // npm 包默认为 node | |
| packageName: pkg.identifier, // 使用 npm 包名作为 packageName | |
| runtime: "node", // npm package defaults to node | |
| packageName: pkg.identifier, // Use npm package name as packageName |
| name: server.title || server.name, | ||
| description: server.description, | ||
| url: server.repository?.url, | ||
| // 转换环境变量 |
There was a problem hiding this comment.
The inline comment is in Chinese while the rest of the codebase appears to use English. For consistency, consider translating to English: 'Transform environment variables'.
| // 转换环境变量 | |
| // Transform environment variables |
src/domains/package/package-so.ts
Outdated
| executor: ToolExecutor, | ||
| ): Promise<PackageSO> { | ||
| const config = repository.getPackageConfig(packageName); | ||
| // Use FederatedRegistryProvider to support both local and official packages |
There was a problem hiding this comment.
[nitpick] This comment should clarify that the federated provider checks local packages first before falling back to the official registry, as this is the key behavior that differs from the previous implementation.
| // Use FederatedRegistryProvider to support both local and official packages | |
| // Use FederatedRegistryProvider, which checks for local packages first and falls back to the official registry if not found locally. | |
| // This is the key behavior that differs from the previous implementation. |
src/domains/package/package-so.ts
Outdated
| throw new Error(`Package '${packageName}' not found`); | ||
| } | ||
|
|
||
| // Get package metadata from local repository |
There was a problem hiding this comment.
[nitpick] This comment could be more specific - it's retrieving category and validation metadata from the local packages-list.json index, which may not exist for official packages. Consider clarifying: 'Get package metadata (category, validated) from local repository index if available'.
| // Get package metadata from local repository | |
| // Get package metadata (category, validated) from local repository index if available. | |
| // Note: This may not exist for official packages. |
…itialization logic
…finitions for package configuration
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
