improved mcp reliability

mcp.go

@@ -2,7 +2,12 @@ package sidekick
 
 import (
   "context"
+  "encoding/json"
   "fmt"
+  "os"
+  "os/exec"
+  "strings"
+  "time"
 
   "github.com/mark3labs/mcp-go/client"
   "github.com/mark3labs/mcp-go/client/transport"
@@ -12,6 +17,7 @@ import (
 // MCPClientInterface represents a scoped connection to an MCP server
 type MCPClientInterface interface {
   CallTool(ctx context.Context, name string, args map[string]interface{}) (string, error)
+  ListTools(ctx context.Context) ([]ToolDefinition, error)
   Close() error
 }
 
@@ -46,8 +52,35 @@ func (w *mcpGoClientWrapper) CallTool(ctx context.Context, name string,
   return output, nil
 }
 
+func (w *mcpGoClientWrapper) ListTools(ctx context.Context) ([]ToolDefinition, error) {
+  req := mcp.ListToolsRequest{}
+  res, err := w.client.ListTools(ctx, req)
+  if err != nil {
+    return nil, err
+  }
+
+  var tools []ToolDefinition
+  for _, t := range res.Tools {
+    var params map[string]interface{}
+    b, err := json.Marshal(t.InputSchema)
+    if err == nil {
+      _ = json.Unmarshal(b, &params)
+    }
+
+    tools = append(tools, ToolDefinition{
+      Name:        t.Name,
+      Description: t.Description,
+      Parameters:  params,
+      Internal:    false,
+    })
+  }
+  return tools, nil
+}
+
 func (w *mcpGoClientWrapper) Close() error {
-  // Client does not expose a close method directly in all transports, or we close transport
+  if w.client != nil {
+    return w.client.Close()
+  }
   return nil
 }
 
@@ -62,6 +95,20 @@ func InitMCPServers(servers map[string]MCPServerConfig) {
   mcpServers = servers
 }
 
+type sidekickLogger struct{}
+
+func (l *sidekickLogger) Infof(format string, v ...any) {
+  if GlobalLogger != nil {
+    GlobalLogger(format, v...)
+  }
+}
+
+func (l *sidekickLogger) Errorf(format string, v ...any) {
+  if GlobalLogger != nil {
+    GlobalLogger("ERROR: "+format, v...)
+  }
+}
+
 // defaultMCPFactory creates a real MCP client based on the global configuration
 var defaultMCPFactory MCPClientFactory = func(toolset string) (MCPClientInterface, error) {
   cfg, ok := mcpServers[toolset]
@@ -78,7 +125,56 @@ var defaultMCPFactory MCPClientFactory = func(toolset string) (MCPClientInterfac
 
   switch cfg.Transport {
   case "stdio":
-    c, err = client.NewStdioMCPClient(cfg.Command, cfg.Env, cfg.Args...)
+    if GlobalLogger != nil {
+      GlobalLogger("Running stdio MCP: %s %v (env: %d vars)", cfg.Command, cfg.Args, len(cfg.Env))
+    }
+
+    command := cfg.Command
+    args := cfg.Args
+
+    // If using npx, try to be quiet to avoid stdout pollution
+    if command == "npx" {
+      quietFound := false
+      for _, arg := range args {
+        if arg == "--quiet" || arg == "-q" {
+          quietFound = true
+          break
+        }
+      }
+      if !quietFound {
+        // Prepend --quiet to args
+        args = append([]string{"--quiet"}, args...)
+      }
+    }
+
+    env := cfg.Env
+    if len(env) > 0 {
+      foundPath := false
+      for _, e := range env {
+        if strings.HasPrefix(strings.ToUpper(e), "PATH=") {
+          foundPath = true
+          break
+        }
+      }
+      if !foundPath {
+        if path := os.Getenv("PATH"); path != "" {
+          env = append(env, "PATH="+path)
+        }
+      }
+    } else {
+      env = os.Environ()
+    }
+
+    // Use WithCommandFunc and redirect Stderr directly to os.Stderr
+    // This provides the most reliable way to see subprocess errors
+    c, err = client.NewStdioMCPClientWithOptions(command, env, args,
+      transport.WithCommandFunc(func(ctx context.Context, command string, env []string, args []string) (*exec.Cmd, error) {
+        cmd := exec.CommandContext(ctx, command, args...)
+        cmd.Env = env
+        cmd.Stderr = os.Stderr
+        return cmd, nil
+      }),
+    )
   case "sse":
     var opts []transport.ClientOption
     headers := make(map[string]string)
@@ -121,8 +217,11 @@ var defaultMCPFactory MCPClientFactory = func(toolset string) (MCPClientInterfac
     Version: "1.0.0",
   }
 
-  _, err = c.Initialize(context.Background(), initReq)
+  initCtx, initCancel := context.WithTimeout(context.Background(), 20*time.Second)
+  defer initCancel()
+  _, err = c.Initialize(initCtx, initReq)
   if err != nil {
+    _ = c.Close()
     return nil, fmt.Errorf("failed to initialize MCP client: %w", err)
   }
 
@@ -137,6 +236,42 @@ func SetMCPClientFactory(factory MCPClientFactory) {
   defaultMCPFactory = factory
 }
 
+// LoadExternalTools connects to all registered MCP servers and fetches their tools
+func LoadExternalTools(ctx context.Context) []ToolDefinition {
+  var extTools []ToolDefinition
+  for toolset := range mcpServers {
+    c, err := defaultMCPFactory(toolset)
+    if err != nil {
+      if GlobalLogger != nil {
+        GlobalLogger("Failed to connect to MCP server %s: %v", toolset, err)
+      }
+      continue
+    }
+
+    listCtx, listCancel := context.WithTimeout(ctx, 10*time.Second)
+    tools, err := c.ListTools(listCtx)
+    listCancel()
+    c.Close()
+    if err != nil {
+      if GlobalLogger != nil {
+        GlobalLogger("Failed to list tools from MCP server %s: %v", toolset, err)
+      }
+      continue
+    }
+
+    // Assign toolset to each tool
+    for i := range tools {
+      tools[i].Toolset = toolset
+    }
+
+    if GlobalLogger != nil {
+      GlobalLogger("Loaded %d tools from MCP server %s", len(tools), toolset)
+    }
+    extTools = append(extTools, tools...)
+  }
+  return extTools
+}
+
 // CallExternalTool creates a scoped MCP connection, calls the tool, and cleans up
 func CallExternalTool(ctx context.Context, toolset string, toolName string,
   args map[string]interface{}) (string, error) {