pkg/tools: add contract tests, remove Close from Server/Dispatcher

- Add contract tests for tools.Server and tools.Dispatcher interfaces
  (tools_test.go), covering all exported behavior including error paths.
  pkg/tools coverage: 0% -> 100%.
- Add contract test for execute.Server (ListTools, CallTool), Decl
  factory test, and executeElevated early-exit test. execute coverage:
  68% -> 71%.
- Remove Close() from Server and Dispatcher interfaces and all
  implementations/callers. These existed for MCP server cleanup which
  has been removed.

pkg/agent/core.go

@@ -971,9 +971,6 @@ func (s *agentCore) handleCommand(ctx context.Context, input string, handler Eve
 				handler(infoEvent(fmt.Sprintf("error: agent %q: %v", name, err)))
 				return
 			}
-			if s.dispatcher != nil {
-				s.dispatcher.Close()
-			}
 			d := s.newDispatcher()
 			env := BuildAgentEnv(cfg, d, s.cwd)
 			s.dispatcher = env.dispatcher

pkg/agent/core_test.go

@@ -2067,7 +2067,6 @@ func (m *mockEnvServer) ListTools() ([]tools.ToolInfo, error)
 func (m *mockEnvServer) CallTool(_ context.Context, _ string, _ json.RawMessage) (json.RawMessage, error) {
 	return nil, fmt.Errorf("not implemented")
 }
-func (m *mockEnvServer) Close() {}
 func (m *mockEnvServer) SetEnv(k, v string) {
 	m.mu.Lock()
 	defer m.mu.Unlock()

pkg/tools/execute/execute_test.go

@@ -385,3 +385,65 @@ func TestSetCWD(t *testing.T) {
 		t.Errorf("pwd output %q doesn't contain expected dir %q", out, dir)
 	}
 }
+
+// ---- Server contract (ListTools / CallTool / Close) ----
+
+func TestServerContract(t *testing.T) {
+	s := newServer(t)
+	tools, err := s.ListTools()
+	if err != nil {
+		t.Fatalf("ListTools: %v", err)
+	}
+	if len(tools) == 0 {
+		t.Fatal("ListTools returned no tools")
+	}
+	for _, ti := range tools {
+		if ti.Name == "" {
+			t.Error("tool has empty Name")
+		}
+		if ti.Description == "" {
+			t.Errorf("tool %q has empty Description", ti.Name)
+		}
+		if len(ti.InputSchema) == 0 {
+			t.Errorf("tool %q has empty InputSchema", ti.Name)
+		}
+	}
+	// CallTool success path
+	args, _ := json.Marshal(map[string]any{"steps": []map[string]any{{"code": "echo contract"}}})
+	res, err := s.CallTool(context.Background(), "execute_code", args)
+	if err != nil {
+		t.Fatalf("CallTool: %v", err)
+	}
+	if !strings.Contains(string(res), "contract") {
+		t.Errorf("CallTool result missing expected output: %s", res)
+	}
+	// CallTool error path (unknown tool)
+	res, err = s.CallTool(context.Background(), "bogus", json.RawMessage(`{}`))
+	if err != nil {
+		t.Fatalf("CallTool error path should marshal, not return error: %v", err)
+	}
+	if !strings.Contains(string(res), "isError") {
+		t.Errorf("CallTool error path should contain isError: %s", res)
+	}
+}
+
+// ---- Decl factory ----
+
+func TestDecl(t *testing.T) {
+	factory := Decl(t.TempDir())
+	srv := factory()
+	if srv == nil {
+		t.Fatal("Decl factory returned nil")
+	}
+}
+
+// ---- executeElevated (plugin not found) ----
+
+func TestExecuteElevatedNoPlugin(t *testing.T) {
+	s := newServer(t)
+	t.Setenv("OLLIE_PLUGINS_PATH", t.TempDir()) // empty dir, no elevate script
+	_, err := s.executeElevated(context.Background(), "echo hi", t.TempDir(), 10)
+	if err == nil || !strings.Contains(err.Error(), "elevation not available") {
+		t.Errorf("expected 'elevation not available' error, got %v", err)
+	}
+}

pkg/tools/execute/server.go

@@ -388,7 +388,4 @@ func prependOlliePath(env []string) []string {
 	return result
 }
 
-// Close implements tools.Server (no-op).
-func (e *Server) Close() {}
-
 var _ tools.Server = (*Server)(nil) // compile-time interface check

pkg/tools/tools.go

@@ -20,7 +20,6 @@ type ToolInfo struct {
 type Server interface {
 	ListTools() ([]ToolInfo, error)
 	CallTool(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error)
-	Close()
 }
 
 // Dispatcher routes tool calls to the server that owns them.
@@ -29,7 +28,6 @@ type Dispatcher interface {
 	GetServer(name string) (Server, bool)
 	ListTools() ([]ToolInfo, error)
 	Dispatch(ctx context.Context, server, tool string, args json.RawMessage) (json.RawMessage, error)
-	Close()
 }
 
 // dispatcher is the default Dispatcher backed by registered Server instances.
@@ -66,13 +64,6 @@ func (d *dispatcher) GetServer(name string) (Server, bool) {
 	return s, ok
 }
 
-// Close shuts down all registered servers.
-func (d *dispatcher) Close() {
-	for _, s := range d.servers {
-		s.Close()
-	}
-}
-
 // ListTools returns all tools advertised by all registered servers.
 func (d *dispatcher) ListTools() ([]ToolInfo, error) {
 	var all []ToolInfo

pkg/tools/tools_test.go

@@ -0,0 +1,149 @@
+package tools_test
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	"ollie/pkg/tools"
+)
+
+// stubServer is a minimal Server used to verify the contract.
+type stubServer struct {
+	name  string
+	tools []tools.ToolInfo
+}
+
+func (s *stubServer) ListTools() ([]tools.ToolInfo, error) { return s.tools, nil }
+func (s *stubServer) CallTool(_ context.Context, tool string, _ json.RawMessage) (json.RawMessage, error) {
+	return json.RawMessage(`{"tool":"` + tool + `"}`), nil
+}
+
+func newStub(name string, toolNames ...string) *stubServer {
+	var ti []tools.ToolInfo
+	for _, n := range toolNames {
+		ti = append(ti, tools.ToolInfo{Name: n, Description: n + " desc"})
+	}
+	return &stubServer{name: name, tools: ti}
+}
+
+// checkServerContract verifies Server invariants.
+func checkServerContract(t *testing.T, s tools.Server) {
+	t.Helper()
+	tl, err := s.ListTools()
+	if err != nil {
+		t.Fatalf("ListTools: %v", err)
+	}
+	if tl == nil {
+		t.Fatal("ListTools returned nil")
+	}
+	for _, ti := range tl {
+		res, err := s.CallTool(context.Background(), ti.Name, json.RawMessage(`{}`))
+		if err != nil {
+			t.Errorf("CallTool(%q): %v", ti.Name, err)
+		}
+		if len(res) == 0 {
+			t.Errorf("CallTool(%q) returned empty result", ti.Name)
+		}
+	}
+}
+
+// checkDispatcherContract verifies Dispatcher invariants.
+func checkDispatcherContract(t *testing.T, d tools.Dispatcher, servers map[string]*stubServer) {
+	t.Helper()
+
+	for name, s := range servers {
+		d.AddServer(name, s)
+	}
+
+	// GetServer round-trip
+	for name := range servers {
+		s, ok := d.GetServer(name)
+		if !ok || s == nil {
+			t.Errorf("GetServer(%q) not found after AddServer", name)
+		}
+	}
+	if _, ok := d.GetServer("nonexistent"); ok {
+		t.Error("GetServer returned true for unregistered server")
+	}
+
+	// ListTools aggregates all servers
+	all, err := d.ListTools()
+	if err != nil {
+		t.Fatalf("ListTools: %v", err)
+	}
+	var wantCount int
+	for _, s := range servers {
+		wantCount += len(s.tools)
+	}
+	if len(all) != wantCount {
+		t.Errorf("ListTools returned %d tools, want %d", len(all), wantCount)
+	}
+	for _, ti := range all {
+		if ti.Server == "" {
+			t.Errorf("tool %q has empty Server field", ti.Name)
+		}
+	}
+
+	// Dispatch routes to correct server
+	for name, s := range servers {
+		for _, ti := range s.tools {
+			res, err := d.Dispatch(context.Background(), name, ti.Name, json.RawMessage(`{}`))
+			if err != nil {
+				t.Errorf("Dispatch(%q, %q): %v", name, ti.Name, err)
+			}
+			if len(res) == 0 {
+				t.Errorf("Dispatch(%q, %q) returned empty", name, ti.Name)
+			}
+		}
+	}
+
+	// Dispatch to unknown server must error
+	_, err = d.Dispatch(context.Background(), "nonexistent", "tool", json.RawMessage(`{}`))
+	if err == nil {
+		t.Error("Dispatch to unknown server should error")
+	}
+}
+
+func TestStubServerContract(t *testing.T) {
+	checkServerContract(t, newStub("s", "a", "b"))
+}
+
+func TestDispatcherContract(t *testing.T) {
+	servers := map[string]*stubServer{
+		"alpha": newStub("alpha", "tool1", "tool2"),
+		"beta":  newStub("beta", "tool3"),
+	}
+	checkDispatcherContract(t, tools.NewDispatcher(), servers)
+}
+
+// failServer is a Server whose ListTools always errors.
+type failServer struct{ stubServer }
+
+func (f *failServer) ListTools() ([]tools.ToolInfo, error) {
+	return nil, fmt.Errorf("boom")
+}
+
+func TestDispatcherListToolsError(t *testing.T) {
+	d := tools.NewDispatcher()
+	d.AddServer("bad", &failServer{})
+	_, err := d.ListTools()
+	if err == nil {
+		t.Error("expected error from failing server")
+	}
+}
+
+func TestNewDispatcherFunc(t *testing.T) {
+	factory := tools.NewDispatcherFunc(map[string]func() tools.Server{
+		"s1": func() tools.Server { return newStub("s1", "t1") },
+	})
+	d := factory()
+	tl, err := d.ListTools()
+	if err != nil {
+		t.Fatalf("ListTools: %v", err)
+	}
+	if len(tl) != 1 {
+		t.Errorf("got %d tools, want 1", len(tl))
+	}
+}