Merge pull request #2413 from simon-agent-go-expert/fix/mcp-stdio-retry-on-unavailable

fix: retry stdio MCP toolset when binary is unavailable at startup

Author: dgageot

pkg/teamloader/registry.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"time"
@@ -268,10 +269,15 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon
 
 	// STDIO MCP Server from shell command
 	case toolset.Command != "":
-		// Auto-install missing command binary if needed
+		// Auto-install missing command binary if needed.
+		// If EnsureCommand fails (binary not on PATH, no aqua package, etc.),
+		// treat as transient: create the toolset with the original command
+		// and let mcp.Toolset.Start() retry on each conversation turn.
 		resolvedCommand, err := toolinstall.EnsureCommand(ctx, toolset.Command, toolset.Version)
 		if err != nil {
-			return nil, fmt.Errorf("resolving command %q: %w", toolset.Command, err)
+			slog.Warn("MCP command not yet available, will retry on next turn",
+				"command", toolset.Command, "error", err)
+			resolvedCommand = toolset.Command
 		}
 
 		env, err := environment.ExpandAll(ctx, environment.ToValues(toolset.Env), envProvider)

pkg/teamloader/registry_test.go

@@ -3,11 +3,13 @@ package teamloader
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/docker/docker-agent/pkg/config"
 	"github.com/docker/docker-agent/pkg/config/latest"
 	"github.com/docker/docker-agent/pkg/environment"
+	"github.com/docker/docker-agent/pkg/tools"
 )
 
 func TestCreateShellTool(t *testing.T) {
@@ -26,3 +28,45 @@ func TestCreateShellTool(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, tool)
 }
+
+func TestCreateMCPTool_CommandNotFound_CreatesToolsetAnyway(t *testing.T) {
+	t.Setenv("DOCKER_AGENT_TOOLS_DIR", t.TempDir())
+
+	toolset := latest.Toolset{
+		Type:    "mcp",
+		Command: "./bin/nonexistent-mcp-server",
+	}
+
+	registry := NewDefaultToolsetRegistry()
+
+	runConfig := &config.RuntimeConfig{
+		Config:              config.Config{WorkingDir: t.TempDir()},
+		EnvProviderForTests: environment.NewOsEnvProvider(),
+	}
+
+	tool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent")
+	require.NoError(t, err)
+	require.NotNil(t, tool)
+	assert.Equal(t, "mcp(stdio cmd=./bin/nonexistent-mcp-server)", tools.DescribeToolSet(tool))
+}
+
+func TestCreateMCPTool_BareCommandNotFound_CreatesToolsetAnyway(t *testing.T) {
+	t.Setenv("DOCKER_AGENT_TOOLS_DIR", t.TempDir())
+
+	toolset := latest.Toolset{
+		Type:    "mcp",
+		Command: "some-nonexistent-mcp-binary",
+	}
+
+	registry := NewDefaultToolsetRegistry()
+
+	runConfig := &config.RuntimeConfig{
+		Config:              config.Config{WorkingDir: t.TempDir()},
+		EnvProviderForTests: environment.NewOsEnvProvider(),
+	}
+
+	tool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent")
+	require.NoError(t, err)
+	require.NotNil(t, tool)
+	assert.Equal(t, "mcp(stdio cmd=some-nonexistent-mcp-binary)", tools.DescribeToolSet(tool))
+}

pkg/tools/mcp/mcp.go

@@ -12,6 +12,8 @@ import (
 	"log/slog"
 	"net"
 	"net/url"
+	"os"
+	"os/exec"
 	"strings"
 	"sync"
 	"time"
@@ -118,9 +120,9 @@ func NewRemoteToolset(name, urlString, transport string, headers map[string]stri
 }
 
 // errServerUnavailable is returned by doStart when the MCP server could not be
-// reached but the error is non-fatal (e.g. EOF). The toolset is considered
-// "started" so the agent can proceed, but watchConnection must not be spawned
-// because there is no live connection to monitor.
+// reached but the error is non-fatal (e.g. EOF, binary not found).
+// Start() propagates this so started remains false, and the agent runtime
+// retries via ensureToolSetsAreStarted on the next conversation turn.
 var errServerUnavailable = errors.New("MCP server unavailable")
 
 // Describe returns a short, user-visible description of this toolset instance.
@@ -155,16 +157,11 @@ func (ts *Toolset) Start(ctx context.Context) error {
 		return nil
 	}
 
-	ts.restarted = make(chan struct{})
+	if ts.restarted == nil {
+		ts.restarted = make(chan struct{})
+	}
 
 	if err := ts.doStart(ctx); err != nil {
-		if errors.Is(err, errServerUnavailable) {
-			// The server is unreachable but the error is non-fatal.
-			// Mark as started so the agent can proceed; tools will simply
-			// be empty. Don't spawn a watcher — there's nothing to watch.
-			ts.started = true
-			return nil
-		}
 		return err
 	}
 
@@ -240,10 +237,11 @@ func (ts *Toolset) doStart(ctx context.Context) error {
 		//
 		// Only retry when initialization fails due to sending the initialized notification.
 		if !isInitNotificationSendError(err) {
-			if errors.Is(err, io.EOF) {
+			if isServerUnavailableError(err) {
 				slog.Debug(
-					"MCP client unavailable (EOF), skipping MCP toolset",
+					"MCP client unavailable, will retry on next conversation turn",
 					"server", ts.logID,
+					"error", err,
 				)
 				return errServerUnavailable
 			}
@@ -548,6 +546,15 @@ func isInitNotificationSendError(err error) bool {
 	return false
 }
 
+// isServerUnavailableError returns true if err indicates the MCP server process
+// could not be reached — binary missing/not-found, or process exited immediately
+// before completing the MCP handshake (io.EOF). These are retryable conditions.
+func isServerUnavailableError(err error) bool {
+	return errors.Is(err, io.EOF) ||
+		errors.Is(err, exec.ErrNotFound) ||
+		errors.Is(err, os.ErrNotExist)
+}
+
 func processMCPContent(toolResult *mcp.CallToolResult) *tools.ToolCallResult {
 	var text strings.Builder
 	var images, audios []tools.MediaContent