feat: multi-remote server selection for external MCPs#1792
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This comment has been minimized.
This comment has been minimized.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Adds TEXT[] column to store user-selected remote URLs for multi-remote MCP servers. When set, deployment processing will filter to only these remotes instead of auto-selecting. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Adds support for MCP servers that have multiple remote endpoints (like Salesforce which has different environments). Users can now select which remote endpoints to include when adding such servers. - Add ExternalMCPRemote type to Goa design - Add remotes field to ExternalMCPServer - Add selectRemotes workflow phase for multi-remote servers - Add UI for selecting remote endpoints with friendly display names - Add goBack navigation from configure to selectRemotes phase Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Remotes field population in registryclient.go to fix exhaustruct lint - Add TODO explaining selectedRemotes UI collects preferences but backend AddExternalMCPForm doesn't support remotes field yet Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When users select specific remotes for a multi-remote MCP server, those selections are now stored and used during deployment processing. - Add selected_remotes field to AddExternalMCPForm Goa design - Update SQL queries to include selected_remotes column - Pass selected remotes through ToolExtractor to RegistryClient - Filter remotes by user selection in GetServerDetails - Update frontend to send selected remote URLs Requires migration PR #1793 to be merged first. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests verify that: - Filtering remotes forces selection of SSE when streamable-http is excluded - When both are allowed, streamable-http is still preferred Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… added Addresses Devin review comment about missing Close button in footer when navigating back from configure phase with all servers already added. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
adaam2
force-pushed
the
fix/salesforce-multiple-remotes-v2
branch
from
a63e046 to
de4bbaa
Compare
adaam2
changed the title
|
|
||||||||||||||||
|
|
||||||||||||||||
- Don't select all remotes by default in selectRemotes phase - Show "Already in Project" title when all servers are already added - Hide misleading "Name the remaining servers" description Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Multi-remote servers (with selectedRemotes) are now always considered "new" since different remote selections make them distinct configs. This allows users to add multiple Salesforce instances with different endpoint selections. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add Title, IconURL, Meta, and Tools fields to satisfy exhaustruct linter. Tools are converted from registry format to API format. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Only track allDataLoaded based on the unfiltered query state. When a search query is active, don't update allDataLoaded based on its pagination state. Once allDataLoaded is true, keep it true - never reset. This prevents an infinite loop where typing search before loading all pages caused the state to flip-flop between true/false as the query key changed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Replace two separate HTTP calls to the same registry endpoint with a single fetchServerDetails call that returns all needed data (name, description, version, tools, remotes). This halves the latency and load on the upstream registry. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Initialize remoteIndex to -1 instead of 0 to detect when no remote matched the filter. When no remote matches, tools stays nil instead of incorrectly returning FirstRemote.Tools. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
adaam2
force-pushed
the
fix/salesforce-multiple-remotes-v2
branch
from
1581d71 to
8d8cba7
Compare
Add lastProcessedIndexRef to track which server index was last processed. If nextServer is called multiple times rapidly (e.g., double-click), subsequent calls for the same index are ignored, preventing duplicate entries in serverConfigs. Reset the ref when going back to selectRemotes phase to allow re-processing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Normalize empty search string to undefined for consistent query keys - Update fetchServerDetails to parse tools from all remotes (0-4) - Select tools from preferred remote (streamable-http > sse) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Return debouncedSearch from useInfiniteListMCPCatalog hook - Use debouncedSearch in allDataLoaded effect instead of searchQuery - Prevents premature allDataLoaded when clearing search during debounce Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The break statement was exiting the loop after finding the first streamable-http remote, which meant we weren't adding all remotes to the slice. This caused multi-remote servers like Salesforce to appear as single-remote, skipping the remote selection modal. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The previous logic picked whichever transport type appeared first, so an sse remote before a streamable-http remote would be incorrectly selected. Now matches registryclient.go behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| @layer utilities { | ||
| @keyframes scroll-dots { | ||
| from { | ||
| background-position: 0 0; | ||
| } | ||
| to { | ||
| background-position: 64px 64px; | ||
| } | ||
| } | ||
|
|
||
| .server-card:hover .scroll-dots-target { | ||
| animation: scroll-dots 3s linear infinite; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Duplicate @Keyframes scroll-dots definition causes CSS specificity conflicts
The scroll-dots keyframe animation and .server-card:hover .scroll-dots-target rule are defined in both client/dashboard/src/App.css:90-102 and client/dashboard/src/styles/globals.css:227-240. The App.css version additionally sets color: oklch(0.553 0.013 58.071 / 0.4) on hover, while the globals.css version inside @layer utilities does not. Because @layer utilities has lower specificity than unlayered styles, the App.css rule will always win at runtime, making the globals.css version dead code. If the intent was for the globals.css version to take effect (or vice versa), one should be removed to avoid confusion.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Auto-deploy when all servers were multi-remote (already configured in selectRemotes) | ||
| useEffect(() => { | ||
| if (!hasOnlySingleRemote && releaseState.canDeploy && hasNewServers) { | ||
| releaseState.startDeployment(); | ||
| } |
There was a problem hiding this comment.
🔴 Auto-deploy useEffect can fire repeatedly due to missing deps causing startDeployment called multiple times
In ConfigurePhaseContent, the auto-deploy effect at line 722-726 calls releaseState.startDeployment() when !hasOnlySingleRemote && releaseState.canDeploy && hasNewServers. However, releaseState.startDeployment is not included in the dependency array, and more critically, the effect's deps [hasOnlySingleRemote, releaseState.canDeploy, hasNewServers] can all remain truthy across multiple renders while the phase is still "configure" (before startDeployment changes the phase to "deploying"). Since startDeployment is async and state updates are batched, the effect can fire on consecutive renders before the phase transition occurs, potentially calling startDeployment() multiple times.
The problematic effect
useEffect(() => {
if (!hasOnlySingleRemote && releaseState.canDeploy && hasNewServers) {
releaseState.startDeployment();
}
}, [hasOnlySingleRemote, releaseState.canDeploy, hasNewServers]);This effect lacks a guard to prevent calling startDeployment() if it was already called. The startDeployment function itself only checks canDeploy and doesn't prevent duplicate calls.
Prompt for agents
In client/dashboard/src/pages/catalog/AddServerDialog.tsx, the useEffect at lines 722-726 can fire startDeployment() multiple times because there is no guard against re-invocation. Add a ref (e.g., `const hasAutoDeployedRef = useRef(false)`) that gets set to true before calling startDeployment, and checked at the start of the effect to prevent duplicate calls. Reset it when the component re-enters the configure phase (or when relevant state changes that should allow re-deployment).
Was this helpful? React with 👍 or 👎 to provide feedback.
| const fetchServerDetails = async () => { | ||
| setIsLoading(true); | ||
| setError(null); | ||
| fetchedForRef.current = serversKey; |
There was a problem hiding this comment.
🟡 useEnrichedServers sets fetchedForRef before async operation completes, preventing retries on error
In useEnrichedServers (AddServerDialog.tsx:229-276), fetchedForRef.current = serversKey is set at line 232 before the async Promise.all begins. If the fetch fails with an error that gets caught at line 265 (the outer catch), the fetchedForRef still equals serversKey. On the next render (same servers, same open=true), the check at line 225 if (fetchedForRef.current === serversKey) return; will skip the fetch, permanently blocking retry. The fallback at line 270 does set enrichedServers to the original servers, but the error state is also set, which causes the error dialog to render, and there's no retry mechanism since the effect won't re-fire.
Prompt for agents
In client/dashboard/src/pages/catalog/AddServerDialog.tsx, in the useEnrichedServers hook, move the line `fetchedForRef.current = serversKey` from line 232 (before the async operation) to inside the try block after setEnrichedServers(enriched) at line 264. This way, if the fetch fails, fetchedForRef won't be set, allowing the effect to retry when the user closes and reopens the dialog (since fetchedForRef is reset to null when open becomes false at line 215).
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
selectRemotesworkflow phase for configuring multi-remote servers before deploymentChanges
ExternalMCPRemotetype andremotesfield toExternalMCPServerin Goa designselected_remotesfield toAddExternalMCPFormfor storing user selectionsselected_remotesinexternal_mcp_attachmentsGetServerDetailsto filter remotes based on user selectionselectRemotesphase to the workflow for multi-remote server configurationgoBacknavigation to return from configure phase to selectRemotes phaseDependencies
selected_remotescolumn)Test plan
🤖 Generated with Claude Code