feat: add support for publishing external servers to collections#2332
feat: add support for publishing external servers to collections#2332
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 562d18e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🚀 Preview Environment (PR #2332)Preview URL: https://pr-2332.dev.getgram.ai
Gram Preview Bot |
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| } else { | ||
| setPhase("configure"); | ||
| } | ||
| }, [servers, existingSpecifiers]); | ||
| }, [servers, isServerAlreadyInstalled]); |
There was a problem hiding this comment.
🟡 Partition useEffect can reset user name edits when async useListToolsets data arrives during configure phase
The partition useEffect at client/dashboard/src/pages/catalog/useExternalMcpReleaseWorkflow.ts:238 depends on isServerAlreadyInstalled, which is re-created whenever existingSpecifiers or existingToolsetIds change. Because existingSpecifiers now incorporates data from useListToolsets (a new async data source added by this PR), it can change AFTER the initial partition has already run and the user has started editing server names.
Concrete scenario: a collection-backed server is already installed in the target project via toolsetOrigin but NOT via externalMcps. Initially, useListToolsets hasn't loaded, so isServerAlreadyInstalled returns false → name is set to the base name (e.g., "My Server"). The user starts typing a custom name. Then useListToolsets resolves, existingSpecifiers updates, isServerAlreadyInstalled changes identity → the effect re-runs → name is overwritten with the fork prefill ("My Server Copy"), losing the user's edits.
The phaseRef guard only protects deploying/complete/error phases — the configure phase is intentionally left unguarded, so this re-partition runs.
(Refers to lines 238-281)
Prompt for agents
The partition useEffect in useExternalMcpReleaseWorkflow (lines 238-281) re-runs whenever isServerAlreadyInstalled changes, which now happens when useListToolsets data loads. This can overwrite user name edits during the configure phase.
Possible approaches:
1. Track whether the user has manually edited any names (e.g., a hasUserEdited ref). If they have, skip the re-partition or merge changes carefully — only update the installed flag and fork name prefix for untouched entries.
2. Instead of re-running the full partition, apply the installed-state change incrementally: update existing configs in-place to add the fork suffix if they weren't already marked as installed, without touching user-edited names.
3. Ensure useListToolsets data is warm before the dialog opens (e.g., by calling it in the parent component like CatalogDetail or CollectionDetail), so the partition effect runs once with complete data.
The fix should preserve the current behavior where the phaseRef guard prevents re-partition after deployment starts.
Was this helpful? React with 👍 or 👎 to provide feedback.
| evolveForm: { | ||
| deploymentId: latestDeployment?.id, | ||
| nonBlocking: true, | ||
| upsertExternalMcps: serverConfigs.map((config) => { | ||
| const slug = generateSlug(config.server.registrySpecifier); | ||
| return { | ||
| registryId: config.server.registryId, | ||
| organizationMcpCollectionRegistryId: | ||
| config.server.organizationMcpCollectionRegistryId, | ||
| name: config.name, | ||
| slug, | ||
| registryServerSpecifier: config.server.registrySpecifier, | ||
| selectedRemotes: | ||
| config.selectedRemotes?.map((r) => r.url) ?? | ||
| config.server.remotes?.map((r) => r.url), | ||
| }; | ||
| }), | ||
| upsertExternalMcps: serverConfigs.map((config) => ({ | ||
| registryId: config.server.registryId, | ||
| organizationMcpCollectionRegistryId: | ||
| config.server.organizationMcpCollectionRegistryId, | ||
| name: config.name, | ||
| slug: generateSlug(config.name), | ||
| registryServerSpecifier: config.server.registrySpecifier, | ||
| selectedRemotes: | ||
| config.selectedRemotes?.map((r) => r.url) ?? | ||
| config.server.remotes?.map((r) => r.url), | ||
| })), | ||
| }, | ||
| }, | ||
| undefined, |
There was a problem hiding this comment.
🚩 Fork deployment relies on evolveDeployment upsert keying on slug, not specifier
The fork flow sends upsertExternalMcps entries with a NEW slug (e.g., "my-server-copy") but the SAME registryServerSpecifier as the original install (e.g., "org/my-server"). For forks to work correctly, evolveDeployment must upsert by slug (creating a new attachment row) rather than by registryServerSpecifier (which would update the original). The tests at useExternalMcpReleaseWorkflow.test.ts:881-895 confirm this expectation. If the server-side upsert key is the specifier rather than the slug, the fork would silently overwrite the original attachment. The test mocking makes this hard to verify end-to-end — worth confirming the server-side evolve implementation uses slug as the upsert key.
(Refers to lines 552-573)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The server-side evolve correctly uses slug (not registry_server_specifier) as the upsert key.
Follow up to #2299. We are removing the frontend restriction of installing servers from the catalogue more than once; instead, servers can be reinstalled as forks both from the catalogue and from a collection.