From 0651ac60c9a6956fa9df31475845f46df8742f4f Mon Sep 17 00:00:00 2001 From: Reidho Satria Date: Thu, 13 Aug 2026 16:25:21 +0700 Subject: [PATCH] fix builtin MCP server startup --- internal/hub/catalog/mcp.json | 52 ++++++--------- internal/mcp/client.go | 116 +++++++++++++++++++++++++++++----- internal/mcp/client_test.go | 51 +++++++++++++++ internal/mcp/manager.go | 42 ++++++++++++ 4 files changed, 213 insertions(+), 48 deletions(-) diff --git a/internal/hub/catalog/mcp.json b/internal/hub/catalog/mcp.json index a4cb311..e72bff0 100644 --- a/internal/hub/catalog/mcp.json +++ b/internal/hub/catalog/mcp.json @@ -7,7 +7,7 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-filesystem", "${HOME}"], + "args": ["-y", "@modelcontextprotocol/server-filesystem@2026.7.10", "${HOME}"], "setup": "Edit the last argument to the directory you want exposed. Node is required." }, { @@ -18,8 +18,8 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/fetch", "author": "Model Context Protocol", "command": "uvx", - "args": ["mcp-server-fetch"], - "setup": "Needs uv (https://docs.astral.sh/uv/)." + "args": ["--from", "mcp-server-fetch==2026.7.10", "--with", "mcp==1.9.4", "mcp-server-fetch"], + "setup": "Needs uv (https://docs.astral.sh/uv/). Versions are pinned because newer MCP SDK releases break this server." }, { "id": "git", @@ -29,8 +29,8 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/git", "author": "Model Context Protocol", "command": "uvx", - "args": ["mcp-server-git", "--repository", "."], - "setup": "Point --repository at the checkout you want. Needs uv." + "args": ["--from", "mcp-server-git==2026.7.10", "--with", "mcp==1.9.4", "mcp-server-git", "--repository", "."], + "setup": "Point --repository at the checkout you want. Needs uv. Versions are pinned because newer MCP SDK releases break this server." }, { "id": "github", @@ -53,7 +53,7 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/postgres", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-postgres", "postgresql://localhost/mydb"], + "args": ["-y", "@modelcontextprotocol/server-postgres@0.6.2", "postgresql://localhost/mydb"], "setup": "Replace the connection string with your own." }, { @@ -64,8 +64,8 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/sqlite", "author": "Model Context Protocol", "command": "uvx", - "args": ["mcp-server-sqlite", "--db-path", "./data.db"], - "setup": "Point --db-path at your database. Needs uv." + "args": ["--from", "mcp-server-sqlite==2025.4.25", "--with", "mcp==1.9.4", "mcp-server-sqlite", "--db-path", "./data.db"], + "setup": "Point --db-path at your database. Needs uv. Versions are pinned because newer MCP SDK releases break this server." }, { "id": "memory", @@ -75,7 +75,7 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/memory", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-memory"] + "args": ["-y", "@modelcontextprotocol/server-memory@2026.7.4"] }, { "id": "sequential-thinking", @@ -85,7 +85,7 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/sequentialthinking", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-sequential-thinking"] + "args": ["-y", "@modelcontextprotocol/server-sequential-thinking@2026.7.4"] }, { "id": "time", @@ -95,8 +95,8 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/time", "author": "Model Context Protocol", "command": "uvx", - "args": ["mcp-server-time"], - "setup": "Needs uv." + "args": ["--from", "mcp-server-time==2026.7.10", "--with", "mcp==1.23.0", "mcp-server-time"], + "setup": "Needs uv. Versions are pinned because newer MCP SDK releases break older server packages." }, { "id": "slack", @@ -106,7 +106,7 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/slack", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-slack"], + "args": ["-y", "@modelcontextprotocol/server-slack@2025.4.25"], "env": { "SLACK_BOT_TOKEN": "", "SLACK_TEAM_ID": "" }, "needs_keys": ["SLACK_BOT_TOKEN", "SLACK_TEAM_ID"], "setup": "Create a Slack app, add bot scopes, and install it to the workspace." @@ -119,21 +119,11 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/brave-search", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-brave-search"], + "args": ["-y", "@modelcontextprotocol/server-brave-search@0.6.2"], "env": { "BRAVE_API_KEY": "" }, "needs_keys": ["BRAVE_API_KEY"], "setup": "Free tier keys at https://brave.com/search/api/." }, - { - "id": "puppeteer", - "name": "Puppeteer", - "summary": "Browser automation through Puppeteer. Antares has its own browser tool; use this when you want the Puppeteer API instead.", - "tags": ["browser", "web"], - "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/puppeteer", - "author": "Model Context Protocol", - "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-puppeteer"] - }, { "id": "notion", "name": "Notion", @@ -142,7 +132,7 @@ "homepage": "https://github.com/makenotion/notion-mcp-server", "author": "Notion", "command": "npx", - "args": ["-y", "@notionhq/notion-mcp-server"], + "args": ["-y", "@notionhq/notion-mcp-server@2.5.1"], "env": { "NOTION_TOKEN": "" }, "needs_keys": ["NOTION_TOKEN"], "setup": "Create an internal integration at https://www.notion.so/my-integrations and share the pages with it." @@ -154,8 +144,8 @@ "tags": ["issues", "productivity"], "homepage": "https://linear.app/docs/mcp", "author": "Linear", - "url": "https://mcp.linear.app/sse", - "setup": "A hosted server — it will ask you to authorise on first use." + "url": "https://mcp.linear.app/mcp", + "setup": "Hosted Streamable HTTP server. OAuth is required; bearer-token headers can be configured manually." }, { "id": "sentry", @@ -164,8 +154,8 @@ "tags": ["observability"], "homepage": "https://docs.sentry.io/product/sentry-mcp/", "author": "Sentry", - "url": "https://mcp.sentry.dev/sse", - "setup": "A hosted server — it will ask you to authorise on first use." + "url": "https://mcp.sentry.dev/mcp", + "setup": "Hosted Streamable HTTP server. OAuth is required; bearer-token headers can be configured manually." }, { "id": "playwright", @@ -175,7 +165,7 @@ "homepage": "https://github.com/microsoft/playwright-mcp", "author": "Microsoft", "command": "npx", - "args": ["-y", "@playwright/mcp@latest"] + "args": ["-y", "@playwright/mcp@0.0.79"] }, { "id": "everything", @@ -185,6 +175,6 @@ "homepage": "https://github.com/modelcontextprotocol/servers/tree/main/src/everything", "author": "Model Context Protocol", "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-everything"] + "args": ["-y", "@modelcontextprotocol/server-everything@2026.7.4"] } ] diff --git a/internal/mcp/client.go b/internal/mcp/client.go index 0cfaba6..fa54ca6 100644 --- a/internal/mcp/client.go +++ b/internal/mcp/client.go @@ -7,12 +7,14 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "log/slog" "net/http" "os" "os/exec" + "path/filepath" "strings" "sync" "time" @@ -55,10 +57,11 @@ type rpcRequest struct { // rpcResponse is a JSON-RPC 2.0 response. type rpcResponse struct { - JSONRPC string `json:"jsonrpc"` - ID *int64 `json:"id"` - Result json.RawMessage `json:"result"` - Error *rpcError `json:"error"` + JSONRPC string `json:"jsonrpc"` + ID *int64 `json:"id"` + Result json.RawMessage `json:"result"` + Error *rpcError `json:"error"` + transportErr error } type rpcError struct { @@ -386,7 +389,34 @@ type stdioTransport struct { // otherwise make every future call wait the full timeout forever. After // maxConsecutiveTimeouts the transport self-closes so the next caller fails // fast and the process is reaped. Any successful reply resets it to zero. - timeouts int + timeouts int + stderr stderrCapture + stderrDone chan struct{} + readerErr error +} + +type stderrCapture struct { + mu sync.Mutex + buf []byte +} + +const maxStderrBytes = 16 << 10 + +func (c *stderrCapture) append(line string) { + c.mu.Lock() + defer c.mu.Unlock() + c.buf = append(c.buf, line...) + c.buf = append(c.buf, '\n') + if len(c.buf) > maxStderrBytes { + copy(c.buf, c.buf[len(c.buf)-maxStderrBytes:]) + c.buf = c.buf[:maxStderrBytes] + } +} + +func (c *stderrCapture) String() string { + c.mu.Lock() + defer c.mu.Unlock() + return strings.TrimSpace(string(c.buf)) } // maxConsecutiveTimeouts is how many back-to-back ctx.Done timeouts a stdio @@ -397,7 +427,7 @@ func newStdioTransport(cfg ServerConfig) (transport, error) { if strings.TrimSpace(cfg.Command) == "" { return nil, fmt.Errorf("stdio transport needs a command") } - cmd := exec.Command(cfg.Command, cfg.Args...) + cmd := exec.Command(cfg.Command, expandArgs(cfg.Args)...) cmd.Env = os.Environ() for k, v := range cfg.Env { cmd.Env = append(cmd.Env, k+"="+v) @@ -419,26 +449,48 @@ func newStdioTransport(cfg ServerConfig) (transport, error) { if err := cmd.Start(); err != nil { return nil, fmt.Errorf("start %s: %w", cfg.Command, err) } - go func() { - sc := bufio.NewScanner(stderr) - for sc.Scan() { - slog.Debug("mcp server stderr", "command", cfg.Command, "line", sc.Text()) - } - }() - t := &stdioTransport{ cmd: cmd, stdin: stdin, stdout: bufio.NewReaderSize(stdout, 1<<20), pending: map[int64]chan *rpcResponse{}, readerDone: make(chan struct{}), + stderrDone: make(chan struct{}), } + go func() { + defer close(t.stderrDone) + sc := bufio.NewScanner(stderr) + for sc.Scan() { + t.stderr.append(sc.Text()) + slog.Debug("mcp server stderr", "command", cfg.Command, "line", sc.Text()) + } + }() // Reap the child if it exits on its own so it never sits as a zombie until // the next Close/Refresh. Wait is idempotent via waitOnce. go func() { _ = t.reap() }() return t, nil } +func expandArgs(args []string) []string { + home, _ := os.UserHomeDir() + out := make([]string, len(args)) + for i, arg := range args { + arg = os.Expand(arg, func(key string) string { + if key == "HOME" && os.Getenv(key) == "" { + return home + } + return os.Getenv(key) + }) + if arg == "~" { + arg = home + } else if strings.HasPrefix(arg, "~/") || strings.HasPrefix(arg, `~\`) { + arg = filepath.Join(home, arg[2:]) + } + out[i] = arg + } + return out +} + func (t *stdioTransport) reap() error { t.waitOnce.Do(func() { if t.cmd != nil { @@ -496,14 +548,16 @@ func (t *stdioTransport) startReader() { } // failPending delivers an error to every waiting caller and clears the map. -func (t *stdioTransport) failPending(err error) { +func (t *stdioTransport) failPending(readErr error) { + err := t.processError(readErr) t.pendingMu.Lock() + t.readerErr = err for id, ch := range t.pending { // Non-blocking: the caller may have already returned on ctx.Done and // stopped reading. The channel is buffered(1), so a live caller still // receives this; an abandoned one must not wedge the reader goroutine. select { - case ch <- &rpcResponse{Error: &rpcError{Message: err.Error()}}: + case ch <- &rpcResponse{transportErr: err}: default: } delete(t.pending, id) @@ -511,6 +565,24 @@ func (t *stdioTransport) failPending(err error) { t.pendingMu.Unlock() } +func (t *stdioTransport) processError(readErr error) error { + waitErr := t.reap() + <-t.stderrDone + stderr := t.stderr.String() + switch { + case waitErr != nil && stderr != "": + return fmt.Errorf("MCP server exited (%v): %s", waitErr, stderr) + case waitErr != nil: + return fmt.Errorf("MCP server exited: %w", waitErr) + case stderr != "": + return fmt.Errorf("MCP server closed stdout: %s", stderr) + case readErr != nil && !errors.Is(readErr, io.EOF): + return fmt.Errorf("MCP server output failed: %w", readErr) + default: + return errors.New("MCP server exited before replying") + } +} + func (t *stdioTransport) send(ctx context.Context, req rpcRequest) (*rpcResponse, error) { // One in-flight request at a time — required for line-delimited stdio. t.sendMu.Lock() @@ -563,9 +635,19 @@ func (t *stdioTransport) send(ctx context.Context, req rpcRequest) (*rpcResponse } return nil, ctx.Err() case <-t.readerDone: - // The background reader exited (EOF, child died). Surface the failure. - return nil, fmt.Errorf("mcp connection lost") + // The background reader exited (EOF, child died). Surface its exit status + // and bounded stderr tail instead of reducing every startup crash to EOF. + t.pendingMu.Lock() + err := t.readerErr + t.pendingMu.Unlock() + if err == nil { + err = errors.New("MCP server connection lost") + } + return nil, err case r := <-ch: + if r.transportErr != nil { + return nil, r.transportErr + } t.pendingMu.Lock() t.timeouts = 0 t.pendingMu.Unlock() diff --git a/internal/mcp/client_test.go b/internal/mcp/client_test.go index b3f9f1f..32b344a 100644 --- a/internal/mcp/client_test.go +++ b/internal/mcp/client_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "os/exec" + "strings" "testing" "time" @@ -189,6 +190,28 @@ func TestUnknownTransport(t *testing.T) { } } +func TestStdioStartupErrorIncludesChildStderr(t *testing.T) { + _, err := Connect(context.Background(), "broken", ServerConfig{ + Transport: "stdio", + Command: os.Args[0], + Args: []string{"-test.run=TestHelperServer"}, + Env: map[string]string{ + "ANTARES_MCP_HELPER": "broken", + }, + }) + if err == nil || !strings.Contains(err.Error(), "MCP server exited") { + t.Fatalf("error = %v, want child exit diagnostics", err) + } +} + +func TestExpandArgsExpandsHomeAndEnvironment(t *testing.T) { + t.Setenv("MCP_TEST_PATH", "/tmp/mcp-test") + got := expandArgs([]string{"${MCP_TEST_PATH}", "${HOME}/data", "~/cache"}) + if got[0] != "/tmp/mcp-test" || !strings.HasSuffix(got[1], "/data") || !strings.HasSuffix(got[2], "/cache") { + t.Fatalf("expanded args = %#v", got) + } +} + func TestToolNameNamespacing(t *testing.T) { got := mcpToolName("my server", "read/file") if got != "mcp__my_server__read_file" { @@ -196,9 +219,37 @@ func TestToolNameNamespacing(t *testing.T) { } } +func TestUpgradeBuiltinServersReplacesOnlyStaleCommands(t *testing.T) { + cfg := config.Default() + cfg.MCP.Servers["fetch"] = config.MCPServer{Command: "uvx", Args: []string{"mcp-server-fetch"}, Enabled: true} + cfg.MCP.Servers["memory"] = config.MCPServer{Command: "npx", Args: []string{"-y", "@modelcontextprotocol/server-memory"}, Enabled: true} + cfg.MCP.Servers["linear"] = config.MCPServer{Transport: "http", URL: "https://mcp.linear.app/sse", Enabled: true} + cfg.MCP.Servers["git"] = config.MCPServer{Command: "custom-git", Args: []string{"mcp-server-git"}, Enabled: true} + + upgradeBuiltinServers(cfg) + + fetch := cfg.MCP.Servers["fetch"] + if len(fetch.Args) < 5 || fetch.Args[0] != "--from" || fetch.Args[3] != "mcp==1.9.4" { + t.Fatalf("fetch args were not upgraded: %#v", fetch.Args) + } + if memory := cfg.MCP.Servers["memory"]; len(memory.Args) != 2 || memory.Args[1] != "@modelcontextprotocol/server-memory@2026.7.4" { + t.Fatalf("memory args were not upgraded: %#v", memory.Args) + } + if linear := cfg.MCP.Servers["linear"]; linear.URL != "https://mcp.linear.app/mcp" { + t.Fatalf("linear URL was not upgraded: %q", linear.URL) + } + if git := cfg.MCP.Servers["git"]; git.Command != "custom-git" || len(git.Args) != 1 { + t.Fatalf("custom git config was changed: %+v", git) + } +} + // TestHelperServer is not a real test: when ANTARES_MCP_HELPER is set it acts // as a minimal MCP server speaking newline-delimited JSON-RPC on stdio. func TestHelperServer(t *testing.T) { + if os.Getenv("ANTARES_MCP_HELPER") == "broken" { + os.Stderr.WriteString("synthetic startup failure\n") + os.Exit(2) + } if os.Getenv("ANTARES_MCP_HELPER") != "1" { t.Skip("helper process") } diff --git a/internal/mcp/manager.go b/internal/mcp/manager.go index ee1865e..e5f3414 100644 --- a/internal/mcp/manager.go +++ b/internal/mcp/manager.go @@ -10,6 +10,8 @@ import ( "sync" "time" + "github.com/enowdev/antares/internal/hub" + "github.com/enowdev/antares/internal/config" "github.com/enowdev/antares/internal/tools" ) @@ -50,6 +52,7 @@ func connectAll(ctx context.Context, cfg *config.Config) (map[string]*Client, ma names = append(names, name) } sort.Strings(names) + upgradeBuiltinServers(cfg) var wg sync.WaitGroup var mu sync.Mutex @@ -82,6 +85,45 @@ func connectAll(ctx context.Context, cfg *config.Config) (map[string]*Client, ma return clients, errs } +func upgradeBuiltinServers(cfg *config.Config) { + for name, server := range cfg.MCP.Servers { + entry, ok := hub.LookupMCP(name) + if !ok || !isStaleBuiltin(server, entry) { + continue + } + server.Command = entry.Command + server.Args = append([]string(nil), entry.Args...) + server.URL = entry.URL + if entry.URL != "" { + server.Transport = "http" + } + cfg.MCP.Servers[name] = server + } +} + +func isStaleBuiltin(server config.MCPServer, entry hub.Entry) bool { + if entry.URL != "" { + return server.Command == "" && server.URL != entry.URL + } + if server.Command != entry.Command || len(server.Args) == 0 || len(entry.Args) == 0 { + return false + } + // Catalogue entries now pin every package. Upgrade only an exact unpinned + // package reference; custom commands remain intact. + if server.Command == "npx" { + return len(server.Args) > 1 && server.Args[0] == "-y" && len(entry.Args) > 1 && + strings.HasPrefix(entry.Args[1], server.Args[1]+"@") + } + if server.Command == "uvx" && !strings.HasPrefix(server.Args[0], "-") { + for _, arg := range entry.Args { + if arg == server.Args[0] { + return true + } + } + } + return false +} + // Close shuts every server down and removes its tools from the registry. func (m *Manager) Close() { m.refreshMu.Lock()