Skip to content

fix: enable MCP server management in web mode (CRITICAL) #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
juniorcammel wants to merge 8 commits into from
Closed

fix: enable MCP server management in web mode (CRITICAL) #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1f493d2
docs: document MCP server bug fixes and patches
junior-cammel Dec 29, 2025
3fbe662
Merge branch 'main' of https://github.com/juniorcammel/automaker
junior-cammel Dec 29, 2025
fa0d99c
docs: add GitHub issue references to MCP fixes documentation
junior-cammel Dec 29, 2025
e560949
docs: add Pull Request references to MCP fixes documentation
junior-cammel Dec 29, 2025
e9bac50
feat: add extended feature count options (200, 500, Custom)
junior-cammel Dec 29, 2025
5dda3ef
docs: add implementation plan for custom feature count
junior-cammel Dec 29, 2025
611d943
fix: resolve all 4 MCP server bugs for web and electron modes
junior-cammel Dec 29, 2025
ee41434
docs: document Bug #4 (web mode MCP fix) in CUSTOM_CHANGES.md
junior-cammel Dec 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
488 changes: 488 additions & 0 deletions .plans/custom-feature-count-implementation.md
View file
Open in desktop

Large diffs are not rendered by default.

527 changes: 527 additions & 0 deletions .plans/mcp-bugs-fix-plan.md
View file
Open in desktop

Large diffs are not rendered by default.

442 changes: 442 additions & 0 deletions CUSTOM_CHANGES.md
View file
Open in desktop

Large diffs are not rendered by default.

282 changes: 207 additions & 75 deletions apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,15 @@ export function useMCPServers() {
// If editing an existing server, save directly (user already approved it)
if (editingServer) {
updateMCPServer(editingServer.id, serverData);

const syncSuccess = await syncSettingsToServer();

if (!syncSuccess) {
toast.error('Failed to save MCP server to disk');
return;
}

toast.success('MCP server updated');
await syncSettingsToServer();
handleCloseDialog();
return;
}
Expand All @@ -303,14 +310,28 @@ export function useMCPServers() {

if (pendingServerData.type === 'add' && pendingServerData.serverData) {
addMCPServer(pendingServerData.serverData);

const syncSuccess = await syncSettingsToServer();

if (!syncSuccess) {
toast.error('Failed to save MCP server to disk');
return;
}

toast.success('MCP server added');
await syncSettingsToServer();
handleCloseDialog();
} else if (pendingServerData.type === 'import' && pendingServerData.importServers) {
for (const serverData of pendingServerData.importServers) {
addMCPServer(serverData);
}
await syncSettingsToServer();

const syncSuccess = await syncSettingsToServer();

if (!syncSuccess) {
toast.error('Failed to save MCP servers to disk');
return;
}

const count = pendingServerData.importServers.length;
toast.success(`Imported ${count} MCP server${count > 1 ? 's' : ''}`);
setIsImportDialogOpen(false);
Expand All @@ -323,13 +344,27 @@ export function useMCPServers() {

const handleToggleEnabled = async (server: MCPServerConfig) => {
updateMCPServer(server.id, { enabled: !server.enabled });
await syncSettingsToServer();

const syncSuccess = await syncSettingsToServer();

if (!syncSuccess) {
toast.error('Failed to save MCP server to disk');
return;
}

toast.success(server.enabled ? 'Server disabled' : 'Server enabled');
};

const handleDelete = async (id: string) => {
removeMCPServer(id);
await syncSettingsToServer();

const syncSuccess = await syncSettingsToServer();

if (!syncSuccess) {
toast.error('Failed to save changes to disk');
return;
}

setDeleteConfirmId(null);
toast.success('MCP server removed');
};
Expand Down Expand Up @@ -569,11 +604,11 @@ export function useMCPServers() {
};

const handleOpenGlobalJsonEdit = () => {
// Build the full mcpServers config object
const exportData: Record<string, Record<string, unknown>> = {};

for (const server of mcpServers) {
// Build array of servers with IDs preserved
const serversArray = mcpServers.map((server) => {
const serverConfig: Record<string, unknown> = {
id: server.id, // Preserve ID
name: server.name, // Preserve name
type: server.type || 'stdio',
};

Expand All @@ -596,102 +631,199 @@ export function useMCPServers() {
}
}

exportData[server.name] = serverConfig;
}
return serverConfig;
});

setGlobalJsonValue(JSON.stringify({ mcpServers: exportData }, null, 2));
setGlobalJsonValue(JSON.stringify({ mcpServers: serversArray }, null, 2));
setIsGlobalJsonEditOpen(true);
};

const handleSaveGlobalJsonEdit = async () => {
try {
const parsed = JSON.parse(globalJsonValue);

// Support both formats
// Support both formats: array and object
const servers = parsed.mcpServers || parsed;

if (typeof servers !== 'object' || Array.isArray(servers)) {
toast.error('Invalid format: expected object with server configurations');
if (Array.isArray(servers)) {
// Array format (new format with IDs)
await handleSaveGlobalJsonArray(servers);
} else if (typeof servers === 'object' && servers !== null) {
// Object format (legacy Claude Desktop format)
await handleSaveGlobalJsonObject(servers);
} else {
toast.error('Invalid format: expected array or object with server configurations');
return;
}

// Validate all servers first
for (const [name, config] of Object.entries(servers)) {
if (typeof config !== 'object' || config === null) {
toast.error(`Invalid config for "${name}"`);
return;
}
const syncSuccess = await syncSettingsToServer();

const serverConfig = config as Record<string, unknown>;
const serverType = (serverConfig.type as string) || 'stdio';
if (!syncSuccess) {
toast.error('Failed to save MCP servers to disk');
return;
}

if (serverType === 'stdio') {
if (!serverConfig.command || typeof serverConfig.command !== 'string') {
toast.error(`Command is required for "${name}" (stdio)`);
return;
}
} else if (serverType === 'sse' || serverType === 'http') {
if (!serverConfig.url || typeof serverConfig.url !== 'string') {
toast.error(`URL is required for "${name}" (${serverType})`);
return;
}
toast.success('MCP servers configuration updated');
setIsGlobalJsonEditOpen(false);
setGlobalJsonValue('');
} catch (error) {
toast.error('Invalid JSON: ' + (error instanceof Error ? error.message : 'Parse error'));
}
};
Comment on lines 641 to +672
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the new handleSaveGlobalJsonEdit function, the helper methods (handleSaveGlobalJsonArray and handleSaveGlobalJsonObject) display a toast for validation errors and then throw an error. The catch block in this function then catches that error and displays a second, more generic toast. This results in two error toasts being shown to the user for a single validation failure, which can be confusing.

To improve the user experience, the helper functions should return a boolean indicating success or failure instead of throwing an error for validation issues. This allows the main function to gracefully stop execution without triggering the catch block, ensuring only one clear error message is displayed.


// Helper: Process array format (with IDs)
const handleSaveGlobalJsonArray = async (serversArray: unknown[]) => {
// Validate all servers first
for (const config of serversArray) {
if (typeof config !== 'object' || config === null) {
toast.error('Invalid server config in array');
throw new Error('Invalid server config');
}

const serverConfig = config as Record<string, unknown>;
const name = serverConfig.name as string;
const serverType = (serverConfig.type as string) || 'stdio';

if (!name || typeof name !== 'string') {
toast.error('Server name is required');
throw new Error('Server name required');
}

if (serverType === 'stdio') {
if (!serverConfig.command || typeof serverConfig.command !== 'string') {
toast.error(`Command is required for "${name}" (stdio)`);
throw new Error('Command required');
}
} else if (serverType === 'sse' || serverType === 'http') {
if (!serverConfig.url || typeof serverConfig.url !== 'string') {
toast.error(`URL is required for "${name}" (${serverType})`);
throw new Error('URL required');
}
}
}

// Create a map of existing servers by name for updating
const existingByName = new Map(mcpServers.map((s) => [s.name, s]));
const processedNames = new Set<string>();
// Create maps of existing servers by ID and name
const existingById = new Map(mcpServers.map((s) => [s.id, s]));
const existingByName = new Map(mcpServers.map((s) => [s.name, s]));
const processedIds = new Set<string>();

// Update or add servers
for (const [name, config] of Object.entries(servers)) {
const serverConfig = config as Record<string, unknown>;
const serverType = (serverConfig.type as ServerType) || 'stdio';
// Update or add servers
for (const config of serversArray) {
const serverConfig = config as Record<string, unknown>;
const id = serverConfig.id as string | undefined;
const name = serverConfig.name as string;
const serverType = (serverConfig.type as ServerType) || 'stdio';

const serverData: Omit<MCPServerConfig, 'id'> = {
name,
type: serverType,
description: (serverConfig.description as string) || undefined,
enabled: serverConfig.enabled !== false,
};
const serverData: Omit<MCPServerConfig, 'id'> = {
name,
type: serverType,
description: (serverConfig.description as string) || undefined,
enabled: serverConfig.enabled !== false,
};

if (serverType === 'stdio') {
serverData.command = serverConfig.command as string;
if (Array.isArray(serverConfig.args)) {
serverData.args = serverConfig.args as string[];
}
if (typeof serverConfig.env === 'object' && serverConfig.env !== null) {
serverData.env = serverConfig.env as Record<string, string>;
}
} else {
serverData.url = serverConfig.url as string;
if (typeof serverConfig.headers === 'object' && serverConfig.headers !== null) {
serverData.headers = serverConfig.headers as Record<string, string>;
}
if (serverType === 'stdio') {
serverData.command = serverConfig.command as string;
if (Array.isArray(serverConfig.args)) {
serverData.args = serverConfig.args as string[];
}
if (typeof serverConfig.env === 'object' && serverConfig.env !== null) {
serverData.env = serverConfig.env as Record<string, string>;
}
} else {
serverData.url = serverConfig.url as string;
if (typeof serverConfig.headers === 'object' && serverConfig.headers !== null) {
serverData.headers = serverConfig.headers as Record<string, string>;
}
}

const existing = existingByName.get(name);
if (existing) {
updateMCPServer(existing.id, serverData);
} else {
addMCPServer(serverData);
// Try to match by ID first, then by name
const existing = id ? existingById.get(id) : existingByName.get(name);
if (existing) {
updateMCPServer(existing.id, serverData);
processedIds.add(existing.id);
} else {
addMCPServer(serverData);
}
}

// Remove servers that are no longer in the JSON
for (const server of mcpServers) {
if (!processedIds.has(server.id)) {
removeMCPServer(server.id);
}
}
Comment on lines +739 to +754
Copy link

@coderabbitai coderabbitai bot Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, get the file and check its size
wc -l "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts"

Repository: AutoMaker-Org/automaker

Length of output: 150

🏁 Script executed:

#!/bin/bash
# View the specific function mentioned in the review (lines 675-755+)
sed -n '675,760p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 3979

🏁 Script executed:

#!/bin/bash
# Find where addMCPServer is defined/called
rg "addMCPServer\s*=" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -A 5

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Check how mcpServers state is declared and updated
rg "mcpServers" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -B 2 -A 2 | head -100

Repository: AutoMaker-Org/automaker

Length of output: 2958

🏁 Script executed:

#!/bin/bash
# Find the hook or context providing these functions
rg "addMCPServer.*function|addMCPServer.*=.*\(" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -B 3 -A 10 | head -80

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Search for where these functions are returned from the hook
sed -n '800,891p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 2906

🏁 Script executed:

#!/bin/bash
# Look for the functions being called within the component - are they state setters or external mutations?
rg "const.*MCPServer.*useContext|const.*MCPServer.*useState|const.*MCPServer.*use" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -A 2

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Search for where addMCPServer, updateMCPServer, removeMCPServer are actually defined
rg "const addMCPServer|const updateMCPServer|const removeMCPServer" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -B 2 -A 8

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Look at the beginning of the hook to find where these come from
head -100 "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 4568

🏁 Script executed:

#!/bin/bash
# Find the app store implementation
find . -name "app-store.*" -type f | head -5

Repository: AutoMaker-Org/automaker

Length of output: 99

🏁 Script executed:

#!/bin/bash
# Search for addMCPServer in the app store
rg "addMCPServer" --type ts --type tsx -B 3 -A 10 | grep -A 10 "addMCPServer.*=" | head -60

Repository: AutoMaker-Org/automaker

Length of output: 94

🏁 Script executed:

#!/bin/bash
# Search for addMCPServer in the app store
rg "addMCPServer" "apps/ui/src/store/app-store.ts" -B 3 -A 10

Repository: AutoMaker-Org/automaker

Length of output: 996

🏁 Script executed:

#!/bin/bash
# Get the app store file size and view relevant sections
wc -l "apps/ui/src/store/app-store.ts"

Repository: AutoMaker-Org/automaker

Length of output: 102

🏁 Script executed:

#!/bin/bash
# View the removeMCPServer implementation to understand state updates
rg "removeMCPServer" "apps/ui/src/store/app-store.ts" -A 5 | head -20

Repository: AutoMaker-Org/automaker

Length of output: 537

🏁 Script executed:

#!/bin/bash
# Verify whether addMCPServer is synchronous by checking if it returns anything and how state updates work in this store
sed -n '1,50p' "apps/ui/src/store/app-store.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 1284

🏁 Script executed:

#!/bin/bash
# Let me verify the exact issue by tracing through the logic
# Check if addMCPServer generates an ID and returns it
sed -n '675,800p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | grep -A 50 "handleSaveGlobalJsonArray"

Repository: AutoMaker-Org/automaker

Length of output: 2110

🏁 Script executed:

#!/bin/bash
# Check if there's any missing code that should track the new server ID
rg "addMCPServer" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -B 5 -A 5

Repository: AutoMaker-Org/automaker

Length of output: 1679

🏁 Script executed:

#!/bin/bash
# Verify the Zustand store behavior - check if set() is synchronous
sed -n '1,100p' "apps/ui/src/store/app-store.ts" | grep -A 3 "create("

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Double-check the exact sequence in handleSaveGlobalJsonArray
# Extract lines 675-755 to see the full function logic
sed -n '675,755p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 3700

🏁 Script executed:

#!/bin/bash
# Verify Zustand's set() synchronicity and how it affects state
grep -A 20 "create(" "apps/ui/src/store/app-store.ts" | head -30

Repository: AutoMaker-Org/automaker

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Check if there's any await or callback used with addMCPServer in the function
sed -n '700,760p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | grep -E "addMCPServer|await|then"

Repository: AutoMaker-Org/automaker

Length of output: 148

🏁 Script executed:

#!/bin/bash
# Verify the full context - is there any awaiting or syncing after the loop?
sed -n '740,780p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | cat -n

Repository: AutoMaker-Org/automaker

Length of output: 1852

🏁 Script executed:

#!/bin/bash
# Check the full function signature and whether handleSaveGlobalJsonArray gets called
rg "handleSaveGlobalJsonArray" "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" -B 2 -A 2

Repository: AutoMaker-Org/automaker

Length of output: 519

🏁 Script executed:

#!/bin/bash
# Verify by looking at the handleSaveGlobalJsonEdit function to see how handleSaveGlobalJsonArray is invoked
sed -n '600,750p' "apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts" | grep -B 30 "handleSaveGlobalJsonArray"

Repository: AutoMaker-Org/automaker

Length of output: 1912

Critical: Newly added servers are not tracked in processedIds, creating a fragile deletion race condition.

When addMCPServer(serverData) is called at line 71, it generates an ID internally (via Zustand's store), but that ID is not added to processedIds. The cleanup loop (lines 76-80) iterates over mcpServers using the closure-captured state from the current render, which hasn't yet reflected the Zustand store update. This accidentally prevents deletion of new servers in the current cycle, but the logic is fragile: if state updates become synchronous or the code structure changes to await addMCPServer calls, newly added servers would appear in mcpServers but remain absent from processedIds, causing them to be immediately deleted.

The fix requires either: (1) having addMCPServer return the generated ID so it can be tracked, or (2) refactoring to match existing servers by name before removal, or (3) separating add/update logic from cleanup logic.

🤖 Prompt for AI Agents 
In
apps/ui/src/components/views/settings-view/mcp-servers/hooks/use-mcp-servers.ts
around lines 739 to 754, newly added servers aren't added to processedIds
because addMCPServer generates IDs internally and the local mcpServers closure
isn't updated yet, causing a deletion race; fix by changing addMCPServer to
return the created server ID and, when adding, push that returned ID into
processedIds (or alternatively resolve the add call to get the new ID), or
refactor logic so you match by name before the removal loop (e.g., build a set
of names from the incoming JSON and skip removals for any server whose name
exists), ensuring every added or updated server is recorded in processedIds
before the cleanup loop runs.

};

// Helper: Process object format (legacy Claude Desktop format)
const handleSaveGlobalJsonObject = async (servers: Record<string, unknown>) => {
// Validate all servers first
for (const [name, config] of Object.entries(servers)) {
if (typeof config !== 'object' || config === null) {
toast.error(`Invalid config for "${name}"`);
throw new Error('Invalid config');
}

const serverConfig = config as Record<string, unknown>;
const serverType = (serverConfig.type as string) || 'stdio';

if (serverType === 'stdio') {
if (!serverConfig.command || typeof serverConfig.command !== 'string') {
toast.error(`Command is required for "${name}" (stdio)`);
throw new Error('Command required');
}
} else if (serverType === 'sse' || serverType === 'http') {
if (!serverConfig.url || typeof serverConfig.url !== 'string') {
toast.error(`URL is required for "${name}" (${serverType})`);
throw new Error('URL required');
}
processedNames.add(name);
}
}

// Create a map of existing servers by name for updating
const existingByName = new Map(mcpServers.map((s) => [s.name, s]));
const processedNames = new Set<string>();

// Remove servers that are no longer in the JSON
for (const server of mcpServers) {
if (!processedNames.has(server.name)) {
removeMCPServer(server.id);
// Update or add servers
for (const [name, config] of Object.entries(servers)) {
const serverConfig = config as Record<string, unknown>;
const serverType = (serverConfig.type as ServerType) || 'stdio';

const serverData: Omit<MCPServerConfig, 'id'> = {
name,
type: serverType,
description: (serverConfig.description as string) || undefined,
enabled: serverConfig.enabled !== false,
};

if (serverType === 'stdio') {
serverData.command = serverConfig.command as string;
if (Array.isArray(serverConfig.args)) {
serverData.args = serverConfig.args as string[];
}
if (typeof serverConfig.env === 'object' && serverConfig.env !== null) {
serverData.env = serverConfig.env as Record<string, string>;
}
} else {
serverData.url = serverConfig.url as string;
if (typeof serverConfig.headers === 'object' && serverConfig.headers !== null) {
serverData.headers = serverConfig.headers as Record<string, string>;
}
}

await syncSettingsToServer();
const existing = existingByName.get(name);
if (existing) {
updateMCPServer(existing.id, serverData);
} else {
addMCPServer(serverData);
}
processedNames.add(name);
}

toast.success('MCP servers configuration updated');
setIsGlobalJsonEditOpen(false);
setGlobalJsonValue('');
} catch (error) {
toast.error('Invalid JSON: ' + (error instanceof Error ? error.message : 'Parse error'));
// Remove servers that are no longer in the JSON
for (const server of mcpServers) {
if (!processedNames.has(server.name)) {
removeMCPServer(server.id);
}
}
};

Expand Down
4 changes: 4 additions & 0 deletions apps/ui/src/components/views/spec-view/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ export const FEATURE_COUNT_OPTIONS: {
value: FeatureCount;
label: string;
warning?: string;
isCustom?: boolean;
}[] = [
{ value: 20, label: '20' },
{ value: 50, label: '50', warning: 'May take up to 5 minutes' },
{ value: 100, label: '100', warning: 'May take up to 5 minutes' },
{ value: 200, label: '200', warning: 'May take up to 10 minutes' },
{ value: 500, label: '500', warning: 'May take up to 15 minutes' },
{ value: -1, label: 'Custom', isCustom: true },
];

// Phase display labels for UI
Expand Down
Loading