fix(mcp): MCP servers cannot block interaction in interactive mode

internal/agent/coordinator.go

@@ -225,13 +225,24 @@ func (c *coordinator) run(ctx context.Context, accept *AcceptedRun, sessionID st
 		return nil, err
 	}
 
-	// Wait for MCP initialization to complete before building the tool list.
-	// Without this, slow-to-start MCP servers (e.g. stdio Python via uv) may
-	// not have registered their tools yet when buildTools reads the registry,
-	// so their tools silently never appear in the LLM tool palette — even
-	// though crush_info reports them as connected.
-	if err := mcp.WaitForInit(ctx); err != nil {
-		return nil, fmt.Errorf("failed to wait for MCP initialization: %w", err)
+	// MCP servers connect asynchronously (see mcp.Initialize).
+	//
+	// Interactive runs never wait for that to finish: the tool list below
+	// is built from whatever is registered right now, servers still
+	// connecting are simply absent from this run's palette, and they are
+	// picked up by later runs once they register and publish
+	// EventToolsListChanged. Blocking here froze the TUI for the duration
+	// of the slowest server's connect timeout whenever a prompt was sent
+	// before initialization finished — most visibly on the first message.
+	//
+	// Non-interactive runs get a single shot at the tool palette, so they
+	// do wait for initialization to settle. The wait is bounded by each
+	// server's own connect timeout, so a hung server cannot stall the run
+	// indefinitely.
+	if !c.interactive {
+		if err := mcp.WaitForInit(ctx); err != nil {
+			return nil, fmt.Errorf("failed to wait for MCP initialization: %w", err)
+		}
 	}
 
 	// refresh models before each run
@@ -633,21 +644,15 @@ func (c *coordinator) buildAgent(ctx context.Context, prompt *prompt.Prompt, age
 	})
 
 	// The readiness goroutines below perform one-time setup — building the
-	// system prompt and the (MCP-gated) tool list — whose results the
+	// system prompt and the initial tool list — whose results the
 	// coordinator needs for its whole lifetime, so they must survive the
 	// caller's context being canceled. Several entry points build an agent
 	// from a short-lived HTTP request context: the server's
 	// InitAgent/UpdateAgent handlers, and UpdateModels -> buildTools ->
-	// agentTool -> buildAgent for the sub-agent. Because mcp.WaitForInit
-	// blocks until MCP initialization finishes, a slow MCP server keeps one
-	// of these goroutines parked past the request; when the handler returns
-	// and cancels its context, WaitForInit would observe the cancellation,
-	// the errgroup would record context.Canceled, and every later run would
-	// fail at readyWg.Wait() before emitting anything — the client/server
-	// session hangs with no visible response. WithoutCancel drops
-	// cancellation while keeping context values; the work is bounded
-	// (WaitForInit by MCP init timeouts, the rest is local) so it always
-	// completes.
+	// agentTool -> buildAgent for the sub-agent. The tool-list build reads
+	// the MCP registry as it stands; servers still connecting are picked up
+	// by later runs. WithoutCancel drops cancellation while keeping context
+	// values; the work is local and always completes.
 	initCtx := context.WithoutCancel(ctx)
 
 	c.readyWg.Go(func() error {
@@ -660,13 +665,6 @@ func (c *coordinator) buildAgent(ctx context.Context, prompt *prompt.Prompt, age
 	})
 
 	c.readyWg.Go(func() error {
-		// Wait for MCP servers to finish registering their tools before
-		// building the initial tool list. This ensures the first tool set
-		// (used if anything reads it before run() rebuilds) includes all
-		// MCP tools, not just fast-to-init ones.
-		if err := mcp.WaitForInit(initCtx); err != nil {
-			return err
-		}
 		tools, err := c.buildTools(initCtx, agent, isSubAgent)
 		if err != nil {
 			return err

internal/agent/coordinator_mcp_gate_test.go

@@ -0,0 +1,115 @@
+package agent
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/charmbracelet/crush/internal/agent/prompt"
+	"github.com/charmbracelet/crush/internal/agent/tools/mcp"
+	"github.com/charmbracelet/crush/internal/config"
+	"github.com/stretchr/testify/require"
+)
+
+// newGateTestCoordinator builds a minimal coordinator against a hermetic
+// config: one openai-typed provider pointed at a closed port, with large and
+// small models selected so model resolution and the system-prompt build both
+// succeed without any network access.
+func newGateTestCoordinator(t *testing.T, interactive bool) *coordinator {
+	t.Helper()
+
+	env := testEnv(t)
+
+	crushJSON := `{
+  "options": {"disable_default_providers": true, "disable_provider_auto_update": true},
+  "providers": {"mock": {"id": "mock", "name": "Mock", "type": "openai",
+    "base_url": "http://127.0.0.1:9/v1", "api_key": "test-key",
+    "models": [{"id": "mock-model", "name": "Mock", "context_window": 8192, "default_max_tokens": 128}]}},
+  "models": {"large": {"provider": "mock", "model": "mock-model"},
+             "small": {"provider": "mock", "model": "mock-model"}}
+}`
+	require.NoError(t, os.WriteFile(filepath.Join(env.workingDir, "crush.json"), []byte(crushJSON), 0o644))
+
+	cfg, err := config.Init(env.workingDir, "", false)
+	require.NoError(t, err)
+	cfg.SetupAgents()
+
+	coord := &coordinator{
+		cfg:         cfg,
+		sessions:    env.sessions,
+		messages:    env.messages,
+		permissions: env.permissions,
+		history:     env.history,
+		filetracker: *env.filetracker,
+		agents:      make(map[string]SessionAgent),
+		interactive: interactive,
+	}
+
+	p, err := coderPrompt(prompt.WithWorkingDir(env.workingDir))
+	require.NoError(t, err)
+	agentCfg := cfg.Config().Agents[config.AgentCoder]
+
+	agent, err := coord.buildAgent(context.Background(), p, agentCfg, false)
+	require.NoError(t, err)
+	coord.currentAgent = agent
+	coord.agents[config.AgentCoder] = agent
+
+	return coord
+}
+
+// TestRunWaitsForMCPOnlyWhenNonInteractive pins the split behavior for
+// in-flight MCP initialization.
+//
+// MCP servers connect asynchronously. Interactive runs must not wait for them:
+// blocking the send path meant a slow stdio server (e.g. Python via uv) froze
+// the TUI for the length of its connect timeout, most visibly on the first
+// message of a session. Tools from late servers simply miss that run's palette
+// and show up on the next one.
+//
+// Non-interactive runs (`crush run`, both local and client/server) get a single
+// shot at the palette, so they still wait for initialization to settle.
+func TestRunWaitsForMCPOnlyWhenNonInteractive(t *testing.T) {
+	t.Run("non-interactive waits", func(t *testing.T) {
+		coord := newGateTestCoordinator(t, false)
+
+		// Arm the gate and never complete initialization, standing in for an
+		// MCP server that is still connecting.
+		mcp.ArmInit()
+		t.Cleanup(mcp.DisarmInit)
+
+		ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
+		defer cancel()
+
+		_, err := coord.run(ctx, nil, "test-session", "hello")
+		require.ErrorContains(t, err, "MCP initialization",
+			"non-interactive run must block on MCP initialization")
+	})
+
+	t.Run("interactive does not wait", func(t *testing.T) {
+		coord := newGateTestCoordinator(t, true)
+
+		mcp.ArmInit()
+		t.Cleanup(mcp.DisarmInit)
+
+		done := make(chan error, 1)
+		go func() {
+			_, err := coord.run(context.Background(), nil, "test-session", "hello")
+			done <- err
+		}()
+
+		select {
+		case err := <-done:
+			// The run fails for unrelated reasons (no such session, closed
+			// provider port); all that matters is that it got past the gate
+			// instead of parking on MCP initialization.
+			if err != nil {
+				require.NotContains(t, err.Error(), "MCP initialization",
+					"interactive run must not block on MCP initialization")
+			}
+		case <-time.After(10 * time.Second):
+			t.Fatal("interactive run blocked; it must not wait for MCP initialization")
+		}
+	})
+}

internal/agent/coordinator_readiness_test.go

@@ -16,29 +16,30 @@ import (
 // TestBuildAgentReadinessSurvivesCallerCancellation is a regression test for
 // the CRUSH_CLIENT_SERVER=1 "new session hangs" bug.
 //
-// buildAgent starts readiness goroutines that run mcp.WaitForInit before
-// building the tool list. Several server entry points build an agent from a
+// buildAgent starts readiness goroutines that build the system prompt and the
+// initial tool list. Several server entry points build an agent from a
 // short-lived HTTP request context — the InitAgent/UpdateAgent handlers, and
 // the sub-agent build reached through UpdateModels -> buildTools -> agentTool.
-// When a slow MCP server kept initialization in flight, that request context
-// was canceled the moment the handler returned; WaitForInit then observed the
-// cancellation, the readyWg errgroup recorded context.Canceled, and every
-// later coordinator.run failed at readyWg.Wait() before emitting anything —
-// the session hung with no visible LLM response.
+// When that request context was canceled the moment the handler returned, the
+// readyWg errgroup recorded context.Canceled and every later coordinator.run
+// failed at readyWg.Wait() before emitting anything — the session hung with
+// no visible LLM response. (This was made worse while the tool-list goroutine
+// also blocked in mcp.WaitForInit, which kept it parked long enough to
+// observe the cancellation; the readiness work no longer waits on MCP init —
+// see coordinator.run — but the cancellation detachment still matters.)
 //
 // The fix detaches the readiness work from the caller context via
 // context.WithoutCancel, so canceling the context that triggered the build no
-// longer poisons readyWg. Here we arm MCP init so WaitForInit blocks, build an
-// agent with a cancelable context, cancel it, and require that readyWg keeps
-// waiting for init instead of failing with context.Canceled.
+// longer poisons readyWg. Here we build an agent with a cancelable context,
+// cancel it, and require that readyWg still completes cleanly.
 func TestBuildAgentReadinessSurvivesCallerCancellation(t *testing.T) {
 	env := testEnv(t)
 
 	// Minimal hermetic config: one openai-typed provider with selected large
 	// and small models so buildAgentModels and the system-prompt build both
 	// succeed. No MCP servers are configured, so initialization would complete
-	// instantly if we let it — we deliberately do not, so WaitForInit stays
-	// blocked for the duration of the assertion.
+	// instantly if we let it — we arm the gate anyway to prove the readiness
+	// goroutines no longer block on it.
 	crushJSON := `{
   "options": {"disable_default_providers": true, "disable_provider_auto_update": true},
   "providers": {"mock": {"id": "mock", "name": "Mock", "type": "openai",
@@ -62,11 +63,11 @@ func TestBuildAgentReadinessSurvivesCallerCancellation(t *testing.T) {
 		filetracker: *env.filetracker,
 	}
 
-	// Arm the MCP init gate so buildAgent's readiness goroutine blocks in
-	// WaitForInit. We never complete init, so the goroutine stays parked; the
-	// agent package's TestMain does not enforce goleak and no other test in it
-	// builds a coordinator, so the parked goroutine is harmless.
+	// Arm the MCP init gate. We never complete init; the readiness goroutines
+	// must not care, since they build the tool list from the registry as it
+	// stands rather than waiting for initialization to finish.
 	mcp.ArmInit()
+	t.Cleanup(mcp.DisarmInit)
 
 	p, err := coderPrompt(prompt.WithWorkingDir(env.workingDir))
 	require.NoError(t, err)
@@ -77,23 +78,20 @@ func TestBuildAgentReadinessSurvivesCallerCancellation(t *testing.T) {
 	require.NoError(t, err)
 
 	// The caller goes away, mirroring an HTTP handler returning and canceling
-	// its request context while MCP init is still in flight.
+	// its request context.
 	cancel()
 
 	done := make(chan error, 1)
 	go func() { done <- coord.readyWg.Wait() }()
 
 	select {
 	case err := <-done:
-		// readyWg finished early. context.Canceled is the regression: the
-		// caller's cancellation leaked into the readiness work and poisoned the
-		// errgroup. Any other early return means this minimal setup failed to
-		// build, which the NoError check surfaces distinctly.
+		// context.Canceled is the regression: the caller's cancellation
+		// leaked into the readiness work and poisoned the errgroup.
 		require.NotErrorIs(t, err, context.Canceled,
 			"readyWg was poisoned by caller cancellation (client/server new-session hang regression)")
 		require.NoError(t, err, "unexpected buildAgent readiness error")
-	case <-time.After(250 * time.Millisecond):
-		// readyWg is still waiting on MCP init despite the canceled caller
-		// context. This is the fixed behavior.
+	case <-time.After(2 * time.Second):
+		t.Fatal("readyWg did not complete; the readiness goroutines must not block on MCP init")
 	}
 }

internal/agent/tools/mcp/init.go

@@ -113,6 +113,16 @@ func ArmInit() {
 	initMu.Unlock()
 }
 
+// DisarmInit undoes ArmInit so WaitForInit stops blocking and returns
+// immediately. It exists for tests in other packages that arm the gate
+// without ever running Initialize and must not leak a permanently-blocking
+// gate into the rest of the test binary. Production code never needs it.
+func DisarmInit() {
+	initMu.Lock()
+	initStarted = false
+	initMu.Unlock()
+}
+
 // renewLock returns the per-server mutex used to serialize session renewals,
 // creating it on first use.
 func renewLock(name string) *sync.Mutex {
@@ -282,6 +292,7 @@ func Close(ctx context.Context) error {
 func Initialize(ctx context.Context, permissions permission.Service, cfg *config.ConfigStore) {
 	ArmInit()
 	slog.Info("Initializing MCP clients")
+	start := time.Now()
 
 	var wg sync.WaitGroup
 	// Initialize states for all configured MCPs
@@ -298,6 +309,10 @@ func Initialize(ctx context.Context, permissions permission.Service, cfg *config
 	}
 	wg.Wait()
 	initOnce.Do(func() { close(initDone) })
+	// Non-interactive runs wait for this to finish before sending a prompt, so
+	// the total is the floor on their startup latency. Interactive runs do not
+	// wait, but the total still explains when late-arriving tools show up.
+	slog.Debug("Finished initializing MCP clients", "duration", time.Since(start).Truncate(time.Millisecond).String())
 }
 
 // WaitForInit blocks until MCP initialization is complete, i.e. until
@@ -609,9 +624,13 @@ func goInitClient(ctx context.Context, cfg *config.ConfigStore, name string, m c
 				slog.Error("Panic in MCP client initialization", "error", err, "name", name)
 			}
 		}()
-		if err := initClient(ctx, cfg, name, m, gen, cfg.Resolver()); err != nil {
-			slog.Debug("Failed to initialize MCP client", "name", name, "error", err)
-		}
+		start := time.Now()
+		err := initClient(ctx, cfg, name, m, gen, cfg.Resolver())
+		slog.Debug("MCP client initialization finished",
+			"name", name,
+			"duration", time.Since(start).Truncate(time.Millisecond).String(),
+			"error", err,
+		)
 	}()
 }
 

internal/agent/tools/mcp/waitforinit_test.go

@@ -34,11 +34,14 @@ func swapInitGate(t *testing.T) chan struct{} {
 	return initDone
 }
 
-// TestWaitForInit_BlocksUntilInitCompletes pins the contract the coordinator
-// relies on: WaitForInit blocks while MCP initialization is still in flight and
-// returns once it completes. The coordinator calls it before reading the tool
-// registry so slow-to-start servers (e.g. stdio Python via uv) have registered
-// their tools first.
+// TestWaitForInit_BlocksUntilInitCompletes pins the contract the
+// non-interactive path relies on: WaitForInit blocks while MCP initialization
+// is still in flight and returns once it completes. Non-interactive runs
+// (`crush run`) wait on it before reading the tool registry so slow-to-start
+// servers (e.g. stdio Python via uv) have registered their tools first.
+// Interactive runs deliberately do not gate on it (a slow server froze the
+// TUI's first prompt); they build the tool palette from whatever is registered
+// at send time and pick up late servers on later runs. See coordinator.run.
 func TestWaitForInit_BlocksUntilInitCompletes(t *testing.T) {
 	gate := swapInitGate(t)
 
@@ -54,11 +57,11 @@ func TestWaitForInit_BlocksUntilInitCompletes(t *testing.T) {
 		"WaitForInit must return once initialization has completed")
 }
 
-// TestWaitForInit_ReturnsWhenNotArmed is the regression test for coordinators
-// built outside app startup. Those paths never call mcp.Initialize (which is
+// TestWaitForInit_ReturnsWhenNotArmed is the regression test for callers
+// outside app startup. Those paths never call mcp.Initialize (which is
 // what arms the gate), so WaitForInit must return immediately instead of
-// blocking on a channel that will never close. Before the fix it blocked until
-// ctx was cancelled, hanging coordinator.run's readyWg forever.
+// blocking on a channel that will never close. Before the fix it blocked
+// until ctx was cancelled, hanging RunNonInteractive's gate forever.
 func TestWaitForInit_ReturnsWhenNotArmed(t *testing.T) {
 	// Ensure the gate looks unarmed regardless of test ordering.
 	initMu.Lock()
@@ -77,12 +80,12 @@ func TestWaitForInit_ReturnsWhenNotArmed(t *testing.T) {
 		"WaitForInit must return immediately when initialization was never armed")
 }
 
-// TestWaitForInit_ToolsVisibleAfterInit is the regression test for the bug the
-// coordinator fix addresses: buildTools read allTools concurrently with MCP
-// initialization, so a slow server's tools were silently missing from the LLM's
-// palette even though crush_info later reported the server as connected. Gating
-// on WaitForInit fixes it — any tool registered before initialization completes
-// must be visible once WaitForInit returns.
+// TestWaitForInit_ToolsVisibleAfterInit pins the visibility guarantee
+// WaitForInit gives the non-interactive path: any tool registered before
+// initialization completes must be visible once WaitForInit returns. The
+// interactive coordinator deliberately no longer relies on this (it reads the
+// registry ungated and picks up late tools on subsequent runs); this test
+// keeps the guarantee for non-interactive runs, which still wait.
 func TestWaitForInit_ToolsVisibleAfterInit(t *testing.T) {
 	const name = "test-waitforinit-tools"
 	t.Cleanup(func() {

internal/app/app.go

@@ -311,7 +311,11 @@ func (app *App) RunNonInteractive(ctx context.Context, output io.Writer, prompt,
 		}
 	}
 
-	// Wait for MCP initialization to complete before reading MCP tools.
+	// Non-interactive runs get a single shot at the tool palette, so wait for
+	// MCP initialization to settle before reading MCP tools. The coordinator
+	// waits again for the same reason (it is the gate the client/server path
+	// goes through); doing it here too surfaces the failure before we create a
+	// session, and lets the UpdateModels below see every MCP tool.
 	if err := mcp.WaitForInit(ctx); err != nil {
 		return fmt.Errorf("failed to wait for MCP initialization: %w", err)
 	}