-
Notifications
You must be signed in to change notification settings - Fork 489
Closed
Description
Bug Description
When adding, editing, or importing MCP servers, success toasts and auto-tests execute before syncSettingsToServer() completes. This causes "Server not found" errors during auto-test because the server hasn't been persisted to disk yet.
Impact
- Severity: High - Breaks the "add server and test immediately" user flow
- User Experience: Users see success toast, then immediately see "Server not found" error
- Affected Operations: Add server, Edit server, Import servers, Toggle enabled, Delete server
- Data Integrity: UI state and disk state temporarily out of sync
Affected Code
File: apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts
Affected Functions:
- Lines 276-290:
handleSave(edit flow) - Lines 307-343:
handleSecurityWarningConfirm(add/import flows) - Lines 345-356:
handleToggleEnabled - Lines 358-370:
handleDelete
Current Behavior (Incorrect)
const handleSecurityWarningConfirm = async () => {
if (pendingServerData.type === 'add' && pendingServerData.serverData) {
addMCPServer(pendingServerData.serverData); // Updates UI state only
toast.success('MCP server added'); // ❌ Shows before sync completes
await syncSettingsToServer(); // Still writing to disk...
handleCloseDialog();
}
};Sequence of Events:
- User adds MCP server
addMCPServer()updates Zustand store (UI state)- ❌ Success toast appears immediately
- ❌ Auto-test fires (if enabled)
- Auto-test calls backend → "Server not found" (not on disk yet)
syncSettingsToServer()completes (finally written to disk)
Expected Behavior
Wait for syncSettingsToServer() to complete before showing success feedback or triggering dependent operations.
Proposed Solution
Pattern: Await sync completion and handle errors before success notifications
const handleSecurityWarningConfirm = async () => {
if (pendingServerData.type === 'add' && pendingServerData.serverData) {
addMCPServer(pendingServerData.serverData);
// ✅ Wait for sync to complete
const syncSuccess = await syncSettingsToServer();
if (!syncSuccess) {
toast.error('Failed to save MCP server to disk');
return;
}
toast.success('MCP server added'); // ✅ Only after sync succeeds
handleCloseDialog();
}
};Apply to all 4 affected functions:
handleSave(edit existing server)handleSecurityWarningConfirm(add new / import multiple)handleToggleEnabled(enable/disable toggle)handleDelete(remove server)
Testing
Before Fix:
- Add new MCP server with auto-test enabled
- See success toast immediately
- See "Server not found" error from auto-test
- Refresh → Server appears (sync completed in background)
After Fix:
- Add new MCP server with auto-test enabled
- Brief wait while sync completes
- Success toast appears
- Auto-test runs successfully (server already on disk)
Dependencies
- Requires: HTTP error handling fix (see related issue) for proper error visibility
- Enables: Correct JSON format persistence (see related issue about array format)
Additional Context
syncSettingsToServer()is async and takes 50-200ms depending on disk I/O- This is a timing bug, not a logic bug - the operations were correct but out of order
- No breaking changes - only improves operation sequencing
- Performance impact: User sees ~100ms delay before success toast (acceptable trade-off)
coderabbitai
Metadata
Metadata
Assignees
Labels
No labels