-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Refactor: Use MCP registry API as source of truth #492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Replace static github-mcp-registry.json with live API calls - Fetch from https://api.mcp.github.com/v0.1/servers/ with pagination support - Extract displayName from GitHub metadata for better matching - Implement smart matching logic: * Case-insensitive comparison * Match against both displayName and full name * Strip common suffixes like -mcp-server for pattern matching - Make build process async to support API calls - Cache registry data to only hit API once per build run - Remove obsolete github-mcp-registry.json file Benefits: - No more manual updates to registry data - Always uses latest MCP registry information - Improved server name matching resilience - Successfully loads all 54 servers from registry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the MCP registry data source from a static JSON file to live API calls, enabling automatic updates and eliminating manual maintenance. The implementation introduces async/await patterns, cursor-based pagination, and smart matching logic to handle server name variations.
Key Changes:
- Converted
loadMcpRegistryNames()to async function that fetches fromhttps://api.mcp.github.com/v0.1/servers/with pagination - Implemented smart matching logic supporting displayName, fullName, and partial name matching with suffix stripping
- Wrapped main execution in async function to support API calls during build process
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| eng/update-readme.mjs | Core refactor: async API fetching with pagination, smart matching logic, and parameter threading throughout the build pipeline |
| docs/README.agents.md | Generated documentation reflecting updated MCP server GitHub URLs from live registry data |
| collections/partners.md | Generated collection file with updated MCP server GitHub URLs |
| * - Normalizes names to lowercase and stores them in a Set for O(1) membership checks. | ||
| * - Fetches all pages from https://api.mcp.github.com/v0.1/servers/ using cursor-based pagination | ||
| * - Safely handles network errors or malformed JSON by returning an empty array. | ||
| * - Extracts server names from: data[].server.name |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSDoc comment states "Extracts server names from: data[].server.name" but the actual code accesses json?.servers (not data) and then entry?.server?.name. The documentation should accurately reflect the actual API response structure being accessed to avoid confusion for future maintainers.
| * - Extracts server names from: data[].server.name | |
| * - Extracts server names from: servers[].server.name |
| // Generate collections README | ||
| const collectionsReadme = buildCategoryReadme( | ||
| generateCollectionsSection, | ||
| COLLECTIONS_DIR, | ||
| collectionsHeader, | ||
| TEMPLATES.collectionsUsage | ||
| TEMPLATES.collectionsUsage, | ||
| registryNames | ||
| ); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line and subsequent lines are missing proper indentation. The code starting from line 917 (collectionsReadme declaration) should be indented at the same level as the previous statements (lines 894-915) since they are all part of the same try block that starts at line 876. This inconsistent indentation reduces code readability.
See below for a potential fix:
// Generate collections README
const collectionsReadme = buildCategoryReadme(
generateCollectionsSection,
COLLECTIONS_DIR,
collectionsHeader,
TEMPLATES.collectionsUsage,
registryNames
);
// Ensure docs directory exists for category outputs
if (!fs.existsSync(DOCS_DIR)) {
fs.mkdirSync(DOCS_DIR, { recursive: true });
}
// Write category outputs into docs folder
writeFileIfChanged(
path.join(DOCS_DIR, "README.instructions.md"),
instructionsReadme
);
writeFileIfChanged(path.join(DOCS_DIR, "README.prompts.md"), promptsReadme);
writeFileIfChanged(path.join(DOCS_DIR, "README.agents.md"), agentsReadme);
writeFileIfChanged(
path.join(DOCS_DIR, "README.collections.md"),
collectionsReadme
);
| throw new Error(`API returned status ${response.status}`); | ||
| } | ||
|
|
||
| const json = await response.json(); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding response body logging or context to the error message when JSON parsing fails. Currently, if response.json() throws an error (e.g., due to malformed JSON), the error message will only contain the JSON parsing error without information about which API call failed. This makes debugging production issues more difficult. Consider wrapping this in a try-catch to provide better error context.
| const json = await response.json(); | |
| let json; | |
| try { | |
| json = await response.json(); | |
| } catch (parseErr) { | |
| // Try to get the response body as text for debugging | |
| let responseText; | |
| try { | |
| responseText = await response.text(); | |
| } catch (textErr) { | |
| responseText = `<failed to read response body: ${textErr}>`; | |
| } | |
| // Limit the response body snippet to 500 chars to avoid log spam | |
| const snippet = responseText && responseText.length > 500 | |
| ? responseText.slice(0, 500) + '...' | |
| : responseText; | |
| throw new Error( | |
| `Failed to parse JSON from ${url}. Error: ${parseErr.message}. Response body: ${snippet}` | |
| ); | |
| } |
| } | ||
|
|
||
| // Check if serverName matches the displayName ignoring case | ||
| return entry.displayName === serverNameLower; |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The matching logic has redundant conditions. Line 435 checks if entry.displayName === serverNameLower, and line 451 repeats the exact same check. Since line 435 is inside the same conditional block and would already return true if this condition matched, the check on line 451 is unreachable and serves no purpose. Consider removing line 451 or restructuring the logic to eliminate this redundancy.
| return entry.displayName === serverNameLower; |
| const response = await fetch(url); | ||
|
|
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fetch call to the MCP registry API lacks timeout configuration. If the API becomes unresponsive, the build process could hang indefinitely. Consider adding a timeout using AbortController or a timeout option to fail fast and avoid blocking the build process. This is especially important since this is a build-time dependency that affects the entire README generation process.
| const response = await fetch(url); | |
| // Add timeout to fetch using AbortController | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 10000); // 10 seconds | |
| let response; | |
| try { | |
| response = await fetch(url, { signal: controller.signal }); | |
| } finally { | |
| clearTimeout(timeout); | |
| } | |
| do { | ||
| const url = cursor ? `${apiUrl}?cursor=${encodeURIComponent(cursor)}` : apiUrl; | ||
| const response = await fetch(url); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`API returned status ${response.status}`); | ||
| } | ||
|
|
||
| const json = await response.json(); | ||
| const servers = json?.servers || []; | ||
|
|
||
| // Extract server names and displayNames from the response | ||
| for (const entry of servers) { | ||
| const serverName = entry?.server?.name; | ||
| if (serverName) { | ||
| // Try to get displayName from GitHub metadata, fall back to server name | ||
| const displayName = | ||
| entry?.server?._meta?.["io.modelcontextprotocol.registry/publisher-provided"]?.github?.displayName || | ||
| serverName; | ||
|
|
||
| allServers.push({ | ||
| name: serverName, | ||
| displayName: displayName.toLowerCase(), | ||
| // Also store the original full name for matching | ||
| fullName: serverName.toLowerCase(), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Get next cursor for pagination | ||
| cursor = json?.metadata?.nextCursor || null; | ||
| } while (cursor); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pagination loop with a do-while structure could potentially run indefinitely if the API returns malformed pagination metadata (e.g., continuously returning the same cursor). Consider adding a safety limit (e.g., maximum number of pages or iterations) to prevent infinite loops in case of API bugs or unexpected behavior. This would make the build process more resilient to API issues.
Benefits:
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.Description
Type of Contribution
Additional Notes
By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.