fix: improve MCP server update and synchronization handling

- Added rollback functionality for server updates on sync failure to maintain local state integrity.
- Enhanced logic for identifying newly added servers during addition and import processes, ensuring accurate pending sync tracking.
- Implemented duplicate server name validation during configuration to prevent errors in server management.
This commit is contained in:
Test User
2025-12-30 01:47:55 -05:00
parent cf62dbbf7a
commit 5a0ad75059

View File

@@ -278,9 +278,12 @@ export function useMCPServers() {
// If editing an existing server, save directly (user already approved it) // If editing an existing server, save directly (user already approved it)
if (editingServer) { if (editingServer) {
const previousData = { ...editingServer };
updateMCPServer(editingServer.id, serverData); updateMCPServer(editingServer.id, serverData);
const syncSuccess = await syncSettingsToServer(); const syncSuccess = await syncSettingsToServer();
if (!syncSuccess) { if (!syncSuccess) {
// Rollback local state on sync failure
updateMCPServer(editingServer.id, previousData);
toast.error('Failed to save MCP server to disk'); toast.error('Failed to save MCP server to disk');
return; return;
} }
@@ -309,29 +312,26 @@ export function useMCPServers() {
if (!pendingServerData) return; if (!pendingServerData) return;
if (pendingServerData.type === 'add' && pendingServerData.serverData) { if (pendingServerData.type === 'add' && pendingServerData.serverData) {
// Get current server count to find the newly added one // Capture existing IDs before adding to find the new server reliably
const currentCount = mcpServers.length; const existingIds = new Set(mcpServers.map((s) => s.id));
addMCPServer(pendingServerData.serverData); addMCPServer(pendingServerData.serverData);
// Get the newly added server ID and mark it as pending sync // Find the newly added server by comparing IDs
const newServers = useAppStore.getState().mcpServers; const newServers = useAppStore.getState().mcpServers;
const newServerId = newServers[currentCount]?.id; const newServer = newServers.find((s) => !existingIds.has(s.id));
if (newServerId) { if (newServer) {
pendingSyncServerIdsRef.current.add(newServerId); pendingSyncServerIdsRef.current.add(newServer.id);
} }
const syncSuccess = await syncSettingsToServer(); const syncSuccess = await syncSettingsToServer();
// Clear pending sync and trigger auto-test after sync // Clear pending sync and trigger auto-test after sync
if (newServerId) { if (newServer) {
pendingSyncServerIdsRef.current.delete(newServerId); pendingSyncServerIdsRef.current.delete(newServer.id);
if (syncSuccess) { if (syncSuccess && newServer.enabled !== false) {
const newServer = useAppStore.getState().mcpServers.find((s) => s.id === newServerId);
if (newServer && newServer.enabled !== false) {
testServer(newServer, true); testServer(newServer, true);
} }
} }
}
if (!syncSuccess) { if (!syncSuccess) {
toast.error('Failed to save MCP server to disk'); toast.error('Failed to save MCP server to disk');
@@ -342,27 +342,24 @@ export function useMCPServers() {
toast.success('MCP server added'); toast.success('MCP server added');
handleCloseDialog(); handleCloseDialog();
} else if (pendingServerData.type === 'import' && pendingServerData.importServers) { } else if (pendingServerData.type === 'import' && pendingServerData.importServers) {
// Get current server count to find the newly added ones // Capture existing IDs before adding to find the new servers reliably
const currentCount = mcpServers.length; const existingIds = new Set(mcpServers.map((s) => s.id));
for (const serverData of pendingServerData.importServers) { for (const serverData of pendingServerData.importServers) {
addMCPServer(serverData); addMCPServer(serverData);
} }
// Get all newly added server IDs and mark them as pending sync // Find all newly added servers by comparing IDs
const newServers = useAppStore.getState().mcpServers.slice(currentCount); const newServers = useAppStore.getState().mcpServers.filter((s) => !existingIds.has(s.id));
const newServerIds = newServers.map((s) => s.id); newServers.forEach((s) => pendingSyncServerIdsRef.current.add(s.id));
newServerIds.forEach((id) => pendingSyncServerIdsRef.current.add(id));
const syncSuccess = await syncSettingsToServer(); const syncSuccess = await syncSettingsToServer();
// Clear pending sync and trigger auto-test after sync // Clear pending sync and trigger auto-test after sync
newServerIds.forEach((id) => pendingSyncServerIdsRef.current.delete(id)); newServers.forEach((s) => pendingSyncServerIdsRef.current.delete(s.id));
if (syncSuccess) { if (syncSuccess) {
const latestServers = useAppStore.getState().mcpServers; for (const server of newServers) {
for (const id of newServerIds) { if (server.enabled !== false) {
const server = latestServers.find((s) => s.id === id);
if (server && server.enabled !== false) {
testServer(server, true); testServer(server, true);
} }
} }
@@ -386,16 +383,19 @@ export function useMCPServers() {
const handleToggleEnabled = async (server: MCPServerConfig) => { const handleToggleEnabled = async (server: MCPServerConfig) => {
const wasDisabled = server.enabled === false; const wasDisabled = server.enabled === false;
const previousEnabled = server.enabled;
updateMCPServer(server.id, { enabled: !server.enabled }); updateMCPServer(server.id, { enabled: !server.enabled });
const syncSuccess = await syncSettingsToServer(); const syncSuccess = await syncSettingsToServer();
if (!syncSuccess) { if (!syncSuccess) {
// Rollback local state on sync failure
updateMCPServer(server.id, { enabled: previousEnabled });
toast.error('Failed to save settings to disk'); toast.error('Failed to save settings to disk');
return; return;
} }
toast.success(wasDisabled ? 'Server enabled' : 'Server disabled'); toast.success(wasDisabled ? 'Server enabled' : 'Server disabled');
// Auto-test if server was just enabled // Auto-test if server was just enabled
if (wasDisabled && syncSuccess) { if (wasDisabled) {
const updatedServer = useAppStore.getState().mcpServers.find((s) => s.id === server.id); const updatedServer = useAppStore.getState().mcpServers.find((s) => s.id === server.id);
if (updatedServer) { if (updatedServer) {
testServer(updatedServer, true); testServer(updatedServer, true);
@@ -736,12 +736,18 @@ export function useMCPServers() {
serversArray: Array<Record<string, unknown>> serversArray: Array<Record<string, unknown>>
): Promise<boolean> => { ): Promise<boolean> => {
// Validate all servers first // Validate all servers first
const names = new Set<string>();
for (const serverConfig of serversArray) { for (const serverConfig of serversArray) {
const name = serverConfig.name as string; const name = serverConfig.name as string;
if (!name || typeof name !== 'string') { if (!name || typeof name !== 'string') {
toast.error('Each server must have a name'); toast.error('Each server must have a name');
return false; return false;
} }
if (names.has(name)) {
toast.error(`Duplicate server name found: "${name}"`);
return false;
}
names.add(name);
const serverType = (serverConfig.type as string) || 'stdio'; const serverType = (serverConfig.type as string) || 'stdio';
if (serverType === 'stdio') { if (serverType === 'stdio') {