chore: migrate to updated registry server API#474
Conversation
Regenerate client from latest swagger and update all call sites to
match the breaking API path changes in toolhive-registry-server:
- GET /extension/v0/registries → GET /v1/registries
- GET /registry/v0.1/servers → GET /registry/{registryName}/v0.1/servers
- GET /registry/v0.1/servers/{serverName}/versions/{version}
→ GET /registry/{registryName}/v0.1/servers/{serverName}/versions/{version}
Update MSW fixtures and test imports accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details on the pull request process.
This review will be automatically dismissed once you add the justification section.
There was a problem hiding this comment.
Pull request overview
Migrates ToolHive Cloud UI to the updated toolhive-registry-server API by regenerating the OpenAPI client and updating server actions, MSW mocks, and tests to match the new endpoint structure (notably /v1/registries and registry-scoped server routes).
Changes:
- Regenerated OpenAPI spec and hey-api generated TypeScript types/SDK.
- Updated catalog list/detail server actions and page logic to use
/v1/registriesand/registry/{registryName}/v0.1/.... - Updated MSW fixtures/handlers and adjusted test imports to the new fixture paths.
Reviewed changes
Copilot reviewed 13 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
swagger.json |
Updated OpenAPI spec reflecting new v1 registry endpoints and registry-scoped server routes. |
src/generated/types.gen.ts |
Regenerated API types for new endpoints and response shapes. |
src/generated/sdk.gen.ts |
Regenerated SDK functions to match the updated API surface. |
src/generated/index.ts |
Updated generated exports to align with the regenerated SDK/types. |
src/app/catalog/actions.ts |
Switched registry/server fetching to new endpoints; added default-registry behavior. |
src/app/catalog/page.tsx |
Updated catalog page data loading to use registry-scoped server listing. |
src/app/catalog/actions.test.ts |
Updated tests to import the new server-list fixture. |
src/app/catalog/[repoName]/[serverName]/[version]/actions.ts |
Updated server-detail fetch to use registry-scoped server detail endpoint (now requires registryName). |
src/mocks/handlers.ts |
(Indirectly impacted) Custom handlers still take precedence over auto-generated handlers. |
src/mocks/server-detail/index.ts |
Updated custom MSW route to include :registryName in the path and updated fixture import. |
src/mocks/fixtures/v1_registries/get.ts |
Added fixture for the new GET /v1/registries endpoint. |
src/mocks/fixtures/registry_registryName_v0_1_servers/get.ts |
Added fixture for registry-scoped server listing, plus scenarios. |
src/mocks/fixtures/extension_v0_registries/get.ts |
Repurposed legacy fixture file to mock v1 registries response. |
src/mocks/fixtures/registry_v0_1_servers/get.ts |
Updated legacy aggregated server-list fixture types to the new response type. |
src/mocks/fixtures/registry_v0_1_servers__serverName__versions__version_/get.ts |
Updated legacy server-detail fixture types to the new response type. |
src/lib/utils.test.ts |
Updated fixture import to the new registry-scoped servers fixture. |
src/lib/schemas/server-meta.test.ts |
Updated fixture import to the new registry-scoped servers fixture. |
| if (!registries.data) { | ||
| return []; | ||
| } | ||
| // console.log("registries", JSON.stringify(registries.data, null, 2)); |
There was a problem hiding this comment.
There’s a commented-out console.log left in getRegistries(). This adds noise and is easy to forget; please remove it (or gate behind a debug flag/env var if you need occasional inspection).
| // console.log("registries", JSON.stringify(registries.data, null, 2)); |
| export async function getServers( | ||
| params?: ServerListParams, | ||
| ): Promise<ServerListResult> { | ||
| const api = await getAuthenticatedClient(); | ||
| const servers = await api.getRegistryV01Servers({ | ||
| client: api.client, | ||
| query: { | ||
| version: "latest", | ||
| cursor: params?.cursor || undefined, | ||
| limit: params?.limit, | ||
| search: params?.search || undefined, | ||
| }, | ||
| }); | ||
| const registries = await getRegistries(); | ||
| const defaultRegistry = registries[0]?.name; | ||
|
|
||
| if (servers.error) { | ||
| console.error("[catalog] Failed to fetch servers:", servers.error); | ||
| throw servers.error; | ||
| } | ||
|
|
||
| if (!servers.data) { | ||
| if (!defaultRegistry) { | ||
| return { servers: [] }; | ||
| } | ||
|
|
||
| const data = servers.data; | ||
| const items = Array.isArray(data?.servers) ? data.servers : []; | ||
|
|
||
| return { | ||
| servers: items | ||
| .map((item) => item?.server) | ||
| .filter((server): server is V0ServerJson => server != null), | ||
| nextCursor: data.metadata?.nextCursor, | ||
| }; | ||
| return getServersByRegistryName(defaultRegistry, params); | ||
| } |
There was a problem hiding this comment.
getServers() now calls getRegistries() and then getServersByRegistryName(), which results in multiple authenticated client creations and at least two network calls for every getServers() invocation. Since getServers() is used in several server routes/components, consider reusing a single authenticated client within getServers() (fetch registries and servers with the same client) and/or memoizing the default registry lookup per request (e.g., React/Next cache) to avoid repeated registry fetches.
| const registries = await getRegistries(); | ||
| const selectedRegistry = registryName ?? registries[0]?.name; | ||
|
|
There was a problem hiding this comment.
CatalogPage always awaits getRegistries() before fetching servers. When registryName is present in searchParams, the servers request doesn’t depend on registries and could be started in parallel (Promise.all) to avoid an unnecessary waterfall on the initial render.
| const registries = await getRegistries(); | |
| const selectedRegistry = registryName ?? registries[0]?.name; | |
| const registriesPromise = getRegistries(); | |
| if (registryName) { | |
| const [registries, { servers, nextCursor }] = await Promise.all([ | |
| registriesPromise, | |
| getServersByRegistryName(registryName, paginationParams), | |
| ]); | |
| return ( | |
| <ServersWrapper | |
| servers={servers} | |
| registries={registries} | |
| nextCursor={nextCursor} | |
| /> | |
| ); | |
| } | |
| const registries = await registriesPromise; | |
| const selectedRegistry = registries[0]?.name; |
| const registriesResult = await api.getV1Registries({ client: api.client }); | ||
| const registryName = registriesResult.data?.registries?.[0]?.name; | ||
|
|
||
| if (!registryName) { | ||
| return { | ||
| error: new Error("No registry available"), | ||
| data: null, | ||
| response: null, | ||
| }; |
There was a problem hiding this comment.
If no registry is available, getServerDetails() returns { error: Error, data: null, response: null }. The calling page only checks response?.status === 404, so this case will render an essentially empty detail page instead of failing fast. Prefer throwing (or returning a response/status that the page can handle) when the registry list is empty or the registries call errors, so this surfaces via error.tsx rather than silently continuing.
| const registriesResult = await api.getV1Registries({ client: api.client }); | |
| const registryName = registriesResult.data?.registries?.[0]?.name; | |
| if (!registryName) { | |
| return { | |
| error: new Error("No registry available"), | |
| data: null, | |
| response: null, | |
| }; | |
| const { error: registriesError, data: registriesData } = | |
| await api.getV1Registries({ client: api.client }); | |
| if (registriesError) { | |
| throw registriesError; | |
| } | |
| const registryName = registriesData?.registries?.[0]?.name; | |
| if (!registryName) { | |
| throw new Error("No registry available"); |
| import type { GetV1RegistriesResponse } from "@api/types.gen"; | ||
| import { AutoAPIMock } from "@mocks"; | ||
|
|
||
| export const mockedGetExtensionV0Registries = | ||
| AutoAPIMock<GetExtensionV0RegistriesResponse>({ | ||
| registries: [ | ||
| { | ||
| name: "Ut est", | ||
| createdAt: "2024-01-01T00:00:00.000Z", | ||
| creationType: "CONFIG", | ||
| filterConfig: { | ||
| names: { | ||
| exclude: ["tempor in Lorem"], | ||
| include: ["occaecat id"], | ||
| }, | ||
| tags: { | ||
| exclude: ["dolor dolore"], | ||
| include: ["Ut tempor sit anim enim"], | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }); | ||
| export const mockedGetV1Registries = AutoAPIMock<GetV1RegistriesResponse>({ | ||
| registries: [ | ||
| { | ||
| name: "Ut est", | ||
| createdAt: "2024-01-01T00:00:00.000Z", | ||
| creationType: "CONFIG", | ||
| }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
This fixture directory/file corresponds to the removed /extension/v0/registries endpoint but now defines mockedGetV1Registries. Since /v1/registries has its own fixture (src/mocks/fixtures/v1_registries/get.ts) and this file isn’t referenced, keeping it is confusing and risks future accidental imports. Consider deleting this legacy fixture folder/file.
| import type { GetRegistryByRegistryNameV01ServersResponse } from "@api/types.gen"; | ||
| import { AutoAPIMock } from "@mocks"; | ||
| import { HttpResponse } from "msw"; | ||
|
|
||
| export const mockedGetRegistryV01Servers = | ||
| AutoAPIMock<GetRegistryV01ServersResponse>({ | ||
| AutoAPIMock<GetRegistryByRegistryNameV01ServersResponse>({ |
There was a problem hiding this comment.
This legacy fixture (registry_v0_1_servers) appears to be unused after the API migration (call sites now import the registry_registryName_v0_1_servers fixture). Keeping both increases maintenance burden and makes it unclear which fixture is authoritative. Consider deleting this old fixture directory/file if it’s no longer needed.
| import type { GetRegistryByRegistryNameV01ServersByServerNameVersionsByVersionResponse } from "@api/types.gen"; | ||
| import { AutoAPIMock } from "@mocks"; | ||
|
|
||
| export const mockedGetRegistryV01ServersByServerNameVersionsByVersion = | ||
| AutoAPIMock<GetRegistryV01ServersByServerNameVersionsByVersionResponse>({ | ||
| server: { | ||
| name: "awslabs/aws-nova-canvas", | ||
| title: "AWS Nova Canvas MCP Server", | ||
| version: "1.0.0", | ||
| description: | ||
| "Image generation using Amazon Nova Canvas. A Model Context Protocol server that integrates with AWS services for AI-powered image generation.\n\nAmazon Nova Canvas is a cutting-edge image generation service that leverages advanced AI models to create high-quality images from text descriptions. This MCP server provides seamless integration with AWS services, allowing you to generate images programmatically within your applications.\n\nKey Features:\n- High-quality image generation with customizable parameters\n- Support for multiple image formats (PNG, JPEG, WebP)\n- Configurable image dimensions and aspect ratios\n- Advanced prompt engineering capabilities\n- Cost-effective pricing with pay-as-you-go model\n- Enterprise-grade security and compliance\n- Real-time generation with low latency\n- Batch processing support for multiple images\n\nUse Cases:\n- Content creation for marketing and advertising\n- Product visualization and mockups\n- Social media content generation\n- E-commerce product images\n- Game asset creation\n- Architectural visualization\n- Educational materials and illustrations\n\nThis server requires valid AWS credentials with appropriate permissions to access the Amazon Nova Canvas service. Make sure your IAM role has the necessary policies attached before using this integration.\n\nFor more information about pricing, limits, and best practices, please refer to the official AWS documentation.", | ||
| repository: { | ||
| source: "github", | ||
| id: "awslabs", | ||
| url: "https://github.com/awslabs/aws-nova-canvas", | ||
| }, | ||
| websiteUrl: "https://github.com/awslabs/aws-nova-canvas", | ||
| icons: [ | ||
| { | ||
| src: "https://www.amazon.com/favicon.ico", | ||
| sizes: ["32x32"], | ||
| mimeType: "image/x-icon", | ||
| }, | ||
| ], | ||
| packages: [ | ||
| { | ||
| version: "1.0.0", | ||
| environmentVariables: [ | ||
| { | ||
| name: "AWS_ACCESS_KEY_ID", | ||
| }, | ||
| { | ||
| name: "AWS_SECRET_ACCESS_KEY", | ||
| }, | ||
| ], | ||
| AutoAPIMock<GetRegistryByRegistryNameV01ServersByServerNameVersionsByVersionResponse>( | ||
| { |
There was a problem hiding this comment.
This server-detail fixture export isn’t referenced anywhere (server detail responses appear to be handled by the custom MSW handler in src/mocks/server-detail). Since the filename/path no longer matches the new endpoint structure and it’s unused, consider removing it to avoid confusion and duplicated mock data.
Summary
swagger.json(toolhive-registry-servermain)API changes
GET /extension/v0/registriesGET /v1/registriesGET /registry/v0.1/serversGET /registry/{registryName}/v0.1/serversGET /registry/v0.1/servers/{serverName}/versions/{version}GET /registry/{registryName}/v0.1/servers/{serverName}/versions/{version}Test plan
pnpm type-checkpassespnpm lintpassespnpm test— 194/194 tests pass🤖 Generated with Claude Code