rearchitect tools

apply rearchitecture to remaining tools

Author: lkingland

.gitignore

@@ -14,6 +14,7 @@
 /pkg/functions/testdata/default_home/go
 /pkg/functions/testdata/default_home/.cache
 /pkg/functions/testdata/migrations/*/.gitignore
+/pkg/creds/auth.json
 
 # Go
 /templates/go/cloudevents/go.sum

cmd/mcp.go

@@ -177,7 +177,7 @@ For detailed instructions, see: docs/mcp-integration/integration.md`
 	cmdPrefix := rootCmd.Use
 
 	s := mcp.New(mcp.WithPrefix(cmdPrefix))
-	if err := s.Start(); err != nil {
+	if err := s.Start(cmd.Context()); err != nil {
 		log.Fatalf("Server error: %v", err)
 		return err
 	}

pkg/mcp/mcp.go

@@ -21,7 +21,7 @@ type Server struct {
 
 type tool interface {
 	desc() *mcp.Tool
-	handle(context.Context, *mcp.CallToolRequest, string, Executor) (*mcp.CallToolResult, error)
+	handle(context.Context, toolRequestInterface, string, Executor) (*mcp.CallToolResult, error)
 }
 
 type resource interface {
@@ -34,24 +34,20 @@ type prompt interface {
 	handler(prefix string) mcp.PromptHandler
 }
 
-// Option is a functional option for configuring a Server
 type Option func(*Server)
 
-// WithPrefix sets the command prefix (e.g., "func" or "kn func")
 func WithPrefix(prefix string) Option {
 	return func(s *Server) {
 		s.prefix = prefix
 	}
 }
 
-// WithExecutor sets the command executor for the server
 func WithExecutor(executor Executor) Option {
 	return func(s *Server) {
 		s.executor = executor
 	}
 }
 
-// DefaultCommand is the default function command to use when executing.
 const DefaultCommand = "func"
 
 func New(options ...Option) *Server {
@@ -64,14 +60,14 @@ func New(options ...Option) *Server {
 		}, nil),
 		tools: []tool{
 			healthCheck{},
-			create{},
-			deploy{},
-			list{},
-			build{},
-			del{},
-			configVolumes{},
-			configLabels{},
-			configEnvs{},
+			createTool{},
+			deployTool{},
+			listTool{},
+			buildTool{},
+			deleteTool{},
+			configVolumesTool{},
+			configLabelsTool{},
+			configEnvsTool{},
 		},
 		resources: []resource{
 			rootHelpResource{},
@@ -95,7 +91,6 @@ func New(options ...Option) *Server {
 		},
 	}
 
-	// Apply functional options
 	for _, o := range options {
 		o(s)
 	}
@@ -115,6 +110,13 @@ func New(options ...Option) *Server {
 	return s
 }
 
-func (s *Server) Start() error {
-	return s.impl.Run(context.Background(), &mcp.StdioTransport{})
+// Start the server in blocking mode using default STDIO transport.
+func (s *Server) Start(ctx context.Context) error {
+	return s.impl.Run(ctx, &mcp.StdioTransport{})
+}
+
+// Connect starts the server with a custom context and transport (for testing)
+// Non-blocking mode for testing.
+func (s *Server) Connect(ctx context.Context, transport mcp.Transport) (*mcp.ServerSession, error) {
+	return s.impl.Connect(ctx, transport, nil)
 }

pkg/mcp/mcp_test.go

@@ -4,19 +4,20 @@ import (
 	"context"
 	"testing"
 
-	mcpmock "knative.dev/func/pkg/mcp/mock"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	"knative.dev/func/pkg/mcp/mock"
 )
 
-// TestServerCreation verifies that the MCP server can be instantiated with custom options
-func TestServerCreation(t *testing.T) {
+// TestNew ensures that the MCP server can be instantiated with custom options
+func TestNew(t *testing.T) {
 	// Define a mock executor
-	mock := mcpmock.NewExecutor()
-	mock.ExecuteFn = func(ctx context.Context, dir string, name string, args ...string) ([]byte, error) {
+	executor := mock.NewExecutor()
+	executor.ExecuteFn = func(ctx context.Context, dir string, name string, args ...string) ([]byte, error) {
 		return []byte("success"), nil
 	}
 
 	// Instantiate the MCP Server with mock executor and custom prefix
-	server := New(WithPrefix("kn func"), WithExecutor(mock))
+	server := New(WithPrefix("kn func"), WithExecutor(executor))
 
 	// Verify server was created
 	if server == nil {
@@ -28,23 +29,101 @@ func TestServerCreation(t *testing.T) {
 		t.Errorf("expected prefix 'kn func', got '%s'", server.prefix)
 	}
 
-	// Verify executor was set correctly
-	if server.executor != mock {
+	// Verify executor was set
+	if server.executor != executor {
 		t.Error("expected executor to be the mock")
 	}
+}
+
+// TestStart ensures the server starts and a client can retrieve server metadata
+func TestStart(t *testing.T) {
+	var (
+		ctx, cancel          = context.WithCancel(context.Background())
+		serverTpt, clientTpt = mcp.NewInMemoryTransports()
+		client               = mcp.NewClient(&mcp.Implementation{
+			Name:    "test-client",
+			Version: "1.0.0",
+		}, nil)
+	)
+	defer cancel()
+
+	// Create server with mock executor
+	executor := mock.NewExecutor()
+	server := New(WithExecutor(executor))
+
+	// Connect Server
+	serverSession, err := server.Connect(ctx, serverTpt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer serverSession.Close()
+
+	// Connect Client
+	clientSession, err := client.Connect(ctx, clientTpt, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer clientSession.Close()
+
+	// Verify initialization result contains server info
+	initResult := clientSession.InitializeResult()
+	if initResult == nil {
+		t.Fatal("expected non-nil initialization result")
+	}
+	if initResult.ServerInfo.Name != "func-mcp" {
+		t.Errorf("expected server name 'func-mcp', got %q", initResult.ServerInfo.Name)
+	}
+	if initResult.ServerInfo.Version != "1.0.0" {
+		t.Errorf("expected server version '1.0.0', got %q", initResult.ServerInfo.Version)
+	}
 
-	// Verify tools were registered
-	if len(server.tools) != 9 {
-		t.Errorf("expected 9 tools, got %d", len(server.tools))
+	// List tools - should have all 9 registered tools
+	toolsResult, err := clientSession.ListTools(ctx, &mcp.ListToolsParams{})
+	if err != nil {
+		t.Fatalf("failed to list tools: %v", err)
+	}
+	if len(toolsResult.Tools) != 9 {
+		t.Errorf("expected 9 tools, got %d", len(toolsResult.Tools))
 	}
 
-	// Verify resources were registered
-	if len(server.resources) != 13 {
-		t.Errorf("expected 13 resources, got %d", len(server.resources))
+	// Verify expected tool names are present
+	expectedTools := map[string]bool{
+		"healthcheck":    false,
+		"create":         false,
+		"build":          false,
+		"deploy":         false,
+		"delete":         false,
+		"list":           false,
+		"config_volumes": false,
+		"config_labels":  false,
+		"config_envs":    false,
+	}
+	for _, tool := range toolsResult.Tools {
+		if _, ok := expectedTools[tool.Name]; ok {
+			expectedTools[tool.Name] = true
+		}
+	}
+	for name, found := range expectedTools {
+		if !found {
+			t.Errorf("expected tool %q not found in tools list", name)
+		}
 	}
 
-	// Verify prompts were registered
-	if len(server.prompts) != 3 {
-		t.Errorf("expected 3 prompts, got %d", len(server.prompts))
+	// List resources - should have all 13 registered resources
+	resourcesResult, err := clientSession.ListResources(ctx, &mcp.ListResourcesParams{})
+	if err != nil {
+		t.Fatalf("failed to list resources: %v", err)
+	}
+	if len(resourcesResult.Resources) != 13 {
+		t.Errorf("expected 13 resources, got %d", len(resourcesResult.Resources))
+	}
+
+	// List prompts - should have all 3 registered prompts
+	promptsResult, err := clientSession.ListPrompts(ctx, &mcp.ListPromptsParams{})
+	if err != nil {
+		t.Fatalf("failed to list prompts: %v", err)
+	}
+	if len(promptsResult.Prompts) != 3 {
+		t.Errorf("expected 3 prompts, got %d", len(promptsResult.Prompts))
 	}
 }