fix(acp): make configured MCP tools callable

internal/acp/agent.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Gitlawb/zero/internal/providercatalog"
 	"github.com/Gitlawb/zero/internal/providermodelcatalog"
 	"github.com/Gitlawb/zero/internal/providermodeldiscovery"
+	"github.com/Gitlawb/zero/internal/redaction"
 	"github.com/Gitlawb/zero/internal/sandbox"
 	"github.com/Gitlawb/zero/internal/sessions"
 	"github.com/Gitlawb/zero/internal/tools"
@@ -30,17 +31,40 @@ type Deps struct {
 	DiscoverModels func(context.Context, config.ProviderProfile) ([]providermodeldiscovery.Model, error)
 	NewProvider    func(profile config.ProviderProfile) (zeroruntime.Provider, error)
 	RunAgent       func(ctx context.Context, prompt string, provider zeroruntime.Provider, opts agent.Options) (agent.Result, error)
-	// BuildWorkspace builds the SCOPED tool registry and the sandbox engine for a
-	// validated workspace root, so ACP shell tools (bash/exec_command) are confined
-	// exactly like the exec surface — never run unconfined on the host.
-	BuildWorkspace func(workspaceRoot string, resolved config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error)
+	// BuildWorkspace creates the per-turn scoped tool workspace for a validated
+	// workspace root. Its Close method releases any resources the registry owns
+	// (notably MCP server connections) after the turn completes or is cancelled.
+	// Shell tools remain confined exactly like the exec surface — never unconfined
+	// on the host.
+	BuildWorkspace func(ctx context.Context, workspaceRoot string, resolved config.ResolvedConfig, mode agent.PermissionMode) (*Workspace, error)
 	// ResolveWorkspaceRoot validates + normalizes a client-supplied cwd (must be an
 	// existing directory; never the bare root). It is the file-tool confinement root.
 	ResolveWorkspaceRoot func(cwd string) (string, error)
 	Store                *sessions.Store
 	AgentInfo            Implementation
 }
 
+// Workspace is the per-turn execution environment passed to the agent. ACP does
+// not retain it across turns because MCP connections belong to the registry that
+// advertised their tools; keeping a stale registry after a cancelled turn would
+// leak its server process and its permission state into the next turn.
+type Workspace struct {
+	Registry       *tools.Registry
+	Sandbox        *sandbox.Engine
+	DeferThreshold int
+	Notices        []string
+	Cleanup        func() error
+}
+
+// Close releases resources created with the workspace. A nil cleanup is valid
+// for core-only workspaces.
+func (w *Workspace) Close() error {
+	if w == nil || w.Cleanup == nil {
+		return nil
+	}
+	return w.Cleanup()
+}
+
 // Agent is the ACP agent server bound to one JSON-RPC connection (one editor).
 type Agent struct {
 	conn *Conn
@@ -241,13 +265,34 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
 	if err != nil {
 		return "", RPCError(codeInternalError, "provider: "+err.Error())
 	}
+	mode := sess.currentMode()
 	// Build the SCOPED registry + sandbox engine for this session's workspace so
 	// shell/file tools are confined to the workspace exactly like the exec surface.
-	registry, sandboxEngine, err := a.deps.BuildWorkspace(sess.cwd, resolved)
+	// The workspace can also own MCP connections, which must be closed after every
+	// turn even if the agent run returns an error or the client cancels it.
+	workspace, err := a.deps.BuildWorkspace(ctx, sess.cwd, resolved, mode)
 	if err != nil {
 		return "", RPCError(codeInternalError, "workspace: "+err.Error())
 	}
+	if workspace == nil || workspace.Registry == nil {
+		if workspace != nil {
+			_ = workspace.Close()
+		}
+		return "", RPCError(codeInternalError, "workspace: missing tool registry")
+	}
+	defer func() {
+		if err := workspace.Close(); err != nil {
+			log.Printf("acp: close workspace: %v", err)
+		}
+	}()
+	registry := workspace.Registry
+	sandboxEngine := workspace.Sandbox
 	note := &notifier{conn: a.conn, sessionID: sess.id}
+	for _, notice := range workspace.Notices {
+		if text := strings.TrimSpace(redaction.RedactString(notice, redaction.Options{})); text != "" {
+			note.text("\n\n[zero warning] " + text + "\n")
+		}
+	}
 
 	opts := agent.Options{
 		Cwd:            sess.cwd,
@@ -256,7 +301,8 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
 		Model:          resolved.Provider.Model,
 		Registry:       registry,
 		Sandbox:        sandboxEngine,
-		PermissionMode: sess.currentMode(),
+		PermissionMode: mode,
+		DeferThreshold: workspace.DeferThreshold,
 		MaxTurns:       resolved.MaxTurns,
 		Images:         images,
 		OnText:         note.text,

internal/acp/agent_test.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -35,6 +36,35 @@ func (f fakeProvider) StreamCompletion(_ context.Context, _ zeroruntime.Completi
 	return ch, nil
 }
 
+type acpDeferredPromptTool struct{ name string }
+
+func (t acpDeferredPromptTool) Name() string           { return t.name }
+func (acpDeferredPromptTool) Description() string      { return "deferred MCP test tool" }
+func (acpDeferredPromptTool) Parameters() tools.Schema { return tools.Schema{Type: "object"} }
+func (acpDeferredPromptTool) Deferred() bool           { return true }
+func (acpDeferredPromptTool) Run(context.Context, map[string]any) tools.Result {
+	return tools.Result{Status: tools.StatusOK, Output: "ok"}
+}
+func (acpDeferredPromptTool) Safety() tools.Safety {
+	return tools.Safety{
+		SideEffect: tools.SideEffectNetwork, Permission: tools.PermissionPrompt,
+		AdvertiseInAuto: true,
+	}
+}
+
+type captureToolsProvider struct {
+	requests chan zeroruntime.CompletionRequest
+}
+
+func (p captureToolsProvider) StreamCompletion(_ context.Context, request zeroruntime.CompletionRequest) (<-chan zeroruntime.StreamEvent, error) {
+	p.requests <- request
+	ch := make(chan zeroruntime.StreamEvent, 2)
+	ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventText, Content: "done"}
+	ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone}
+	close(ch)
+	return ch, nil
+}
+
 func testDeps(t *testing.T) Deps {
 	t.Helper()
 	store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
@@ -53,10 +83,10 @@ func testDeps(t *testing.T) Deps {
 			return fakeProvider{text: "Hello from ZERO"}, nil
 		},
 		RunAgent: agent.Run,
-		BuildWorkspace: func(string, config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
+		BuildWorkspace: func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
 			r := tools.NewRegistry()
 			r.Register(tools.NewUpdatePlanTool())
-			return r, nil, nil
+			return &Workspace{Registry: r}, nil
 		},
 		ResolveWorkspaceRoot: func(cwd string) (string, error) { return cwd, nil },
 		Store:                store,
@@ -468,8 +498,8 @@ func TestACPRunTurnWiresSandboxAndScopedRegistry(t *testing.T) {
 	reg := tools.NewRegistry()
 	reg.Register(tools.NewUpdatePlanTool())
 	engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir()})
-	deps.BuildWorkspace = func(string, config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
-		return reg, engine, nil
+	deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
+		return &Workspace{Registry: reg, Sandbox: engine}, nil
 	}
 	var captured agent.Options
 	deps.RunAgent = func(_ context.Context, _ string, _ zeroruntime.Provider, opts agent.Options) (agent.Result, error) {
@@ -497,6 +527,124 @@ func TestACPRunTurnWiresSandboxAndScopedRegistry(t *testing.T) {
 	}
 }
 
+func TestACPRunTurnUsesWorkspaceDeferThresholdAtProviderBoundary(t *testing.T) {
+	for _, tc := range []struct {
+		name      string
+		threshold int
+		want      []string
+	}{
+		{name: "explicit zero disables", threshold: 0, want: []string{"mcp_docs_alpha", "mcp_docs_beta"}},
+		{name: "below threshold stays eager", threshold: 3, want: []string{"mcp_docs_alpha", "mcp_docs_beta"}},
+		{name: "at threshold defers", threshold: 2, want: []string{tools.ToolSearchToolName}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			requests := make(chan zeroruntime.CompletionRequest, 4)
+			deps := testDeps(t)
+			deps.NewProvider = func(config.ProviderProfile) (zeroruntime.Provider, error) {
+				return captureToolsProvider{requests: requests}, nil
+			}
+			deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
+				registry := tools.NewRegistry()
+				registry.Register(acpDeferredPromptTool{name: "mcp_docs_alpha"})
+				registry.Register(acpDeferredPromptTool{name: "mcp_docs_beta"})
+				registry.Register(tools.NewToolSearchTool(registry))
+				return &Workspace{Registry: registry, DeferThreshold: tc.threshold}, nil
+			}
+
+			h := newHarness(t, deps)
+			defer h.stop()
+			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+			defer cancel()
+			var created NewSessionResult
+			if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
+				t.Fatal(err)
+			}
+			if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hi")}}, &PromptResult{}); err != nil {
+				t.Fatal(err)
+			}
+			request := <-requests
+			got := make([]string, 0, len(request.Tools))
+			for _, definition := range request.Tools {
+				got = append(got, definition.Name)
+			}
+			if !slices.Equal(got, tc.want) {
+				t.Fatalf("provider tools = %#v, want %#v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestACPRunTurnEmitsWorkspaceSetupNotices(t *testing.T) {
+	deps := testDeps(t)
+	deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
+		registry := tools.NewRegistry()
+		registry.Register(tools.NewUpdatePlanTool())
+		return &Workspace{Registry: registry, Notices: []string{"MCP server docs unavailable, skipped: timeout"}}, nil
+	}
+	h := newHarness(t, deps)
+	defer h.stop()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	var created NewSessionResult
+	if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
+		t.Fatal(err)
+	}
+	if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hi")}}, &PromptResult{}); err != nil {
+		t.Fatal(err)
+	}
+	got := drainText(t, h.updates)
+	if !strings.Contains(got, "[zero warning] MCP server docs unavailable, skipped: timeout") || !strings.Contains(got, "Hello from ZERO") {
+		t.Fatalf("streamed text = %q", got)
+	}
+}
+
+// TestACPRunTurnClosesWorkspace proves a per-turn workspace never outlives the
+// agent run. This is particularly important for MCP: its registry owns live
+// client connections and, for stdio servers, child processes.
+func TestACPRunTurnClosesWorkspaceAfterSuccessAndFailure(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		runErr error
+	}{
+		{name: "success"},
+		{name: "agent failure", runErr: errors.New("provider interrupted")},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			deps := testDeps(t)
+			closed := 0
+			deps.BuildWorkspace = func(context.Context, string, config.ResolvedConfig, agent.PermissionMode) (*Workspace, error) {
+				registry := tools.NewRegistry()
+				registry.Register(tools.NewUpdatePlanTool())
+				return &Workspace{Registry: registry, Cleanup: func() error {
+					closed++
+					return nil
+				}}, nil
+			}
+			deps.RunAgent = func(context.Context, string, zeroruntime.Provider, agent.Options) (agent.Result, error) {
+				return agent.Result{FinalAnswer: "ok"}, tc.runErr
+			}
+			h := newHarness(t, deps)
+			defer h.stop()
+			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+			defer cancel()
+			var created NewSessionResult
+			if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir()}, &created); err != nil {
+				t.Fatalf("session/new: %v", err)
+			}
+			err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("hello")}}, &PromptResult{})
+			if tc.runErr == nil && err != nil {
+				t.Fatalf("session/prompt: %v", err)
+			}
+			if tc.runErr != nil && err == nil {
+				t.Fatal("session/prompt succeeded after agent failure")
+			}
+			if closed != 1 {
+				t.Fatalf("workspace cleanup calls = %d, want 1", closed)
+			}
+		})
+	}
+}
+
 // TestACPRejectsInvalidCwd confirms session/new fails when the workspace root
 // resolver rejects the client cwd (e.g. filesystem root).
 func TestACPRejectsInvalidCwd(t *testing.T) {