Fix VS Code MCP server initialization crash and multi-agent architecture

CRITICAL FIX:
- Removed auto-registration during initialization that was causing crashes
- Fixed pattern match error with Inbox.start_link (already_started case)
- Changed architecture to require explicit agent registration with unique IDs

MULTI-AGENT SUPPORT:
- Agents must now register themselves with unique identifiers (e.g., 'Green Platypus')
- register_agent tool now works without requiring prior agent_id
- All other tools require agent_id parameter to identify the calling agent
- Proper error handling for missing agent_id in tool calls

ARCHITECTURE CHANGE:
- One MCP server instance serves multiple agents (as per VS Code design)
- Removed auto-registration of 'GitHub Copilot' agent during initialization
- Each agent must explicitly call register_agent before using other tools

This fixes the VS Code connection error:
MatchError: no match of right hand side value: {:error, {:already_started, #PID<0.215.0>}}

.github/instructions/no-duplicate-files.instructions.md

@@ -8,7 +8,7 @@ applyTo: '**'
 
 **NEVER** create files with adjectives or verbs that duplicate existing functionality:
 - ❌ `enhanced_mcp_server.ex` when `mcp_server.ex` exists
-- ❌ `unified_mcp_server.ex` when `mcp_server.ex` exists  
+- ❌ `unified_mcp_server.ex` when `mcp_server.ex` exists
 - ❌ `mcp_server_manager.ex` when `mcp_server.ex` exists
 - ❌ `new_config.ex` when `config.ex` exists
 - ❌ `improved_task_registry.ex` when `task_registry.ex` exists

lib/agent_coordinator/mcp_server.ex

@@ -1,7 +1,7 @@
 defmodule AgentCoordinator.MCPServer do
   @moduledoc """
   Unified MCP (Model Context Protocol) server for agent coordination.
-  
+
   This server provides:
   - Agent coordination tools for task management and communication
   - External MCP server management and unified tool access
@@ -16,7 +16,7 @@ defmodule AgentCoordinator.MCPServer do
   # State for tracking external servers and agent sessions
   defstruct [
     :external_servers,
-    :server_processes, 
+    :server_processes,
     :tool_registry,
     :agent_sessions,
     :session_monitors,
@@ -26,7 +26,7 @@ defmodule AgentCoordinator.MCPServer do
   @mcp_tools [
     %{
       "name" => "register_agent",
-      "description" => "Register a new agent with the coordination system",
+      "description" => "Register a new agent with the coordination system. Each agent must choose a unique identifier (e.g., 'Green Platypus', 'Blue Koala') and include their agent_id in all subsequent tool calls to identify themselves.",
       "inputSchema" => %{
         "type" => "object",
         "properties" => %{
@@ -347,7 +347,7 @@ defmodule AgentCoordinator.MCPServer do
       session_monitors: %{},
       server_config: load_server_config()
     }
-    
+
     # Start external MCP servers
     {:ok, state, {:continue, :start_external_servers}}
   end
@@ -381,36 +381,57 @@ defmodule AgentCoordinator.MCPServer do
 
   def handle_call({:mcp_request, request}, from, state) do
     # Extract agent context for automatic heartbeat management
-    agent_context = extract_agent_context(request, from, state)
+    case extract_agent_context(request, from, state) do
-    
+      {:error, error_message} ->
-    # Send pre-operation heartbeat if we have agent context
+        # Return error if agent context extraction fails (unless this is register_agent)
-    if agent_context[:agent_id] do
+        method = Map.get(request, "method")
-      TaskRegistry.heartbeat_agent(agent_context[:agent_id])
+        if method == "tools/call" and 
-      update_session_activity(agent_context[:agent_id])
+           Map.get(request, "params", %{}) |> Map.get("name") == "register_agent" do
-    end
+          # Allow register_agent to proceed without agent_id
-    
+          response = process_mcp_request(request)
-    # Process the request
+          {:reply, response, state}
-    response = process_mcp_request(request)
+        else
-    
+          error_response = %{
-    # Send post-operation heartbeat and update session activity
+            "jsonrpc" => "2.0",
-    if agent_context[:agent_id] do
+            "id" => Map.get(request, "id"),
-      TaskRegistry.heartbeat_agent(agent_context[:agent_id])
+            "error" => %{
-      update_session_activity(agent_context[:agent_id])
+              "code" => -32602,
-      
+              "message" => error_message
-      # Add heartbeat metadata to successful responses
+            }
-      enhanced_response = case response do
+          }
-        %{"result" => _} = success ->
+          {:reply, error_response, state}
-          Map.put(success, "_heartbeat_metadata", %{
+        end
-            agent_id: agent_context[:agent_id],
+
-            timestamp: DateTime.utc_now()
+      agent_context ->
-          })
+        # Send pre-operation heartbeat if we have agent context
-        error_result ->
+        if agent_context[:agent_id] do
-          error_result
+          TaskRegistry.heartbeat_agent(agent_context[:agent_id])
-      end
+          update_session_activity(agent_context[:agent_id])
-      
+        end
-      {:reply, enhanced_response, state}
+
-    else
+        # Process the request
-      {:reply, response, state}
+        response = process_mcp_request(request)
+
+        # Send post-operation heartbeat and update session activity
+        if agent_context[:agent_id] do
+          TaskRegistry.heartbeat_agent(agent_context[:agent_id])
+          update_session_activity(agent_context[:agent_id])
+
+          # Add heartbeat metadata to successful responses
+          enhanced_response = case response do
+            %{"result" => _} = success ->
+              Map.put(success, "_heartbeat_metadata", %{
+                agent_id: agent_context[:agent_id],
+                timestamp: DateTime.utc_now()
+              })
+            error_result ->
+              error_result
+          end
+
+          {:reply, enhanced_response, state}
+        else
+          {:reply, response, state}
+        end
     end
   end
 
@@ -464,7 +485,7 @@ defmodule AgentCoordinator.MCPServer do
          } = request
        ) do
     id = Map.get(request, "id", nil)
-    
+
     # Determine if this is a coordinator tool or external tool
     result = route_tool_call(tool_name, args)
 
@@ -515,12 +536,18 @@ defmodule AgentCoordinator.MCPServer do
         # Add agent to codebase registry
         CodebaseRegistry.add_agent_to_codebase(agent.codebase_id, agent.id)
 
-        # Start inbox for the agent
+        # Start inbox for the agent (handle already started case)
-        {:ok, _pid} = Inbox.start_link(agent.id)
+        case Inbox.start_link(agent.id) do
-        
+          {:ok, _pid} -> :ok
+          {:error, {:already_started, _pid}} -> :ok
+          {:error, reason} -> 
+            Logger.warning("Failed to start inbox for agent #{agent.id}: #{inspect(reason)}")
+            :ok
+        end
+
         # Track the session if we have caller info
         track_agent_session(agent.id, name, capabilities)
-        
+
         {:ok, %{agent_id: agent.id, codebase_id: agent.codebase_id, status: "registered"}}
 
       {:error, reason} ->
@@ -994,7 +1021,7 @@ defmodule AgentCoordinator.MCPServer do
   end
 
   # External MCP server management functions
-  
+
   defp start_external_server(name, %{type: :stdio} = config) do
     case start_stdio_external_server(name, config) do
       {:ok, os_pid, port, pid_file_path} ->
@@ -1045,7 +1072,7 @@ defmodule AgentCoordinator.MCPServer do
       pid_file_path: nil,
       config: config
     }
-    
+
     Logger.info("Registering HTTP server: #{name} at #{Map.get(config, :url)}")
     {:ok, server_info}
   end
@@ -1279,21 +1306,21 @@ defmodule AgentCoordinator.MCPServer do
   defp get_all_unified_tools do
     # Combine coordinator tools with external server tools
     coordinator_tools = @mcp_tools
-    
+
     # Get external tools from the current process state
-    external_tools = 
+    external_tools =
       case Process.get(:external_tool_registry) do
         nil -> []
         registry -> Map.values(registry) |> List.flatten()
       end
-    
+
     coordinator_tools ++ external_tools
   end
 
   defp route_tool_call(tool_name, args) do
     # Check if it's a coordinator tool first
     coordinator_tool_names = Enum.map(@mcp_tools, & &1["name"])
-    
+
     if tool_name in coordinator_tool_names do
       handle_coordinator_tool(tool_name, args)
     else
@@ -1340,7 +1367,7 @@ defmodule AgentCoordinator.MCPServer do
       registered_at: DateTime.utc_now(),
       last_activity: DateTime.utc_now()
     }
-    
+
     # Store in process dictionary for now (could be enhanced to track caller PID)
     Process.put({:agent_session, agent_id}, session_info)
   end
@@ -1364,40 +1391,15 @@ defmodule AgentCoordinator.MCPServer do
         agent_id = request["params"]["arguments"]["agent_id"]
         %{agent_id: agent_id}
 
-      # If no explicit agent_id, try to auto-register a default agent
+      # If no explicit agent_id, return error - agents must register first
       true ->
-        default_agent_context()
+        {:error, "Missing agent_id. Agents must register themselves using register_agent before calling other tools."}
     end
   end
 
-  defp default_agent_context do
-    # Create or use a default agent session for GitHub Copilot
-    default_agent_id = "github_copilot_session"
-
-    # Check if we already have this agent in our session tracking
-    case Process.get({:agent_session, default_agent_id}) do
-      nil ->
-        # Auto-register GitHub Copilot as an agent
-        case register_agent(%{
-               "name" => "GitHub Copilot",
-               "capabilities" => ["coding", "analysis", "review", "documentation"]
-             }) do
-          {:ok, %{agent_id: agent_id}} ->
-            %{agent_id: agent_id}
-
-          _ ->
-            # Fallback to default ID even if registration fails
-            %{agent_id: default_agent_id}
-        end
-
-      _session_info ->
-        %{agent_id: default_agent_id}
-    end
-  end
-  
   defp load_server_config do
     config_file = System.get_env("MCP_CONFIG_FILE", "mcp_servers.json")
-    
+
     if File.exists?(config_file) do
       try do
         case Jason.decode!(File.read!(config_file)) do
@@ -1417,12 +1419,12 @@ defmodule AgentCoordinator.MCPServer do
       get_default_server_config()
     end
   end
-  
+
   defp normalize_server_config(config) do
     config
     |> Map.update("type", :stdio, fn
       "stdio" -> :stdio
-      "http" -> :http  
+      "http" -> :http
       type when is_atom(type) -> type
       type -> String.to_existing_atom(type)
     end)
@@ -1431,7 +1433,7 @@ defmodule AgentCoordinator.MCPServer do
       {key, value} -> {String.to_atom(key), value}
     end)
   end
-  
+
   defp get_default_server_config do
     %{
       servers: %{
@@ -1444,7 +1446,7 @@ defmodule AgentCoordinator.MCPServer do
         },
         "mcp_memory" => %{
           type: :stdio,
-          command: "bunx", 
+          command: "bunx",
           args: ["-y", "@modelcontextprotocol/server-memory"],
           auto_restart: true,
           description: "Memory and knowledge graph server"

mcp_servers.json

@@ -10,12 +10,6 @@
             "auto_restart": true,
             "description": "Context7 library documentation server"
         },
-        "mcp_figma": {
-            "url": "http://127.0.0.1:3845/mcp",
-			"type": "http",
-            "auto_restart": true,
-            "description": "Figma design integration server"
-        },
         "mcp_filesystem": {
             "type": "stdio",
             "command": "bunx",