From 0ec3ec6864675664d74381b9cff32cc388057e66 Mon Sep 17 00:00:00 2001 From: sneharathod7 Date: Mon, 18 May 2026 09:45:36 +0530 Subject: [PATCH] refactor(mcp): call pkg/functions directly for create and delete tools --- cmd/mcp.go | 15 +++++--- pkg/mcp/mcp.go | 16 ++++++++ pkg/mcp/tools_create.go | 72 ++++++++++++++++++++++++++++++++++++ pkg/mcp/tools_create_test.go | 40 ++++++++++++++++++++ pkg/mcp/tools_delete.go | 68 +++++++++++++++++++++++++++++++++- pkg/mcp/tools_delete_test.go | 56 ++++++++++++++++++++++++++++ 6 files changed, 261 insertions(+), 6 deletions(-) diff --git a/cmd/mcp.go b/cmd/mcp.go index 821cf874d4..91987bcf5d 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -110,11 +110,16 @@ func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) err rootCmd := cmd.Root() cmdPrefix := rootCmd.Use - // Instantiate - client, done := newClient(ClientConfig{}, - fn.WithMCPServer(mcp.New( - mcp.WithPrefix(cmdPrefix), - mcp.WithReadonly(!writeEnabled)))) + // Instantiate using a closure to resolve the circular dependency + // between the client and the MCP server. + var client *fn.Client + mcpServer := mcp.New( + mcp.WithPrefix(cmdPrefix), + mcp.WithReadonly(!writeEnabled), + mcp.WithClientProvider(func() *fn.Client { return client }), + ) + var done func() + client, done = newClient(ClientConfig{}, fn.WithMCPServer(mcpServer)) defer done() // Start diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index fe912028fe..5c707246ed 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -8,6 +8,7 @@ import ( "sync/atomic" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) const ( @@ -28,6 +29,11 @@ type Server struct { executor executor transport mcp.Transport // Transport to use (defaults to StdioTransport) impl *mcp.Server // implements the protocol + + // clientProvider returns a *fn.Client for direct library calls. + // Set via WithClientProvider; nil for tool handlers that still shell out. + // Tracks the migration in https://github.com/knative/func/issues/3771. + clientProvider func() *fn.Client } type executor interface { @@ -75,6 +81,16 @@ func WithReadonly(readonly bool) Option { } } +// WithClientProvider sets a provider that returns the functions Client used +// for direct library calls (replacing the executor subprocess path). +// The provider is called lazily per tool invocation so it can capture a +// client that is constructed after the MCP server. +func WithClientProvider(p func() *fn.Client) Option { + return func(s *Server) { + s.clientProvider = p + } +} + // New MCP Server func New(options ...Option) *Server { s := &Server{ diff --git a/pkg/mcp/tools_create.go b/pkg/mcp/tools_create.go index 28e4b7a03e..01fa5566be 100644 --- a/pkg/mcp/tools_create.go +++ b/pkg/mcp/tools_create.go @@ -3,8 +3,11 @@ package mcp import ( "context" "fmt" + "os" + "path/filepath" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) var createTool = &mcp.Tool{ @@ -20,6 +23,56 @@ var createTool = &mcp.Tool{ } func (s *Server) createHandler(ctx context.Context, r *mcp.CallToolRequest, input CreateInput) (result *mcp.CallToolResult, output CreateOutput, err error) { + if s.clientProvider != nil { + client := s.clientProvider() + if client == nil { + err = fmt.Errorf("create tool: client provider returned nil") + return + } + + // Derive function name and absolute path + derivedName, absolutePath, errVal := deriveNameAndPath(input.Path) + if errVal != nil { + err = errVal + return + } + + templateVal := fn.DefaultTemplate + if input.Template != nil { + templateVal = *input.Template + } + + var customClient *fn.Client + if input.Repository != nil && *input.Repository != "" { + // Construct a custom client with the provided repository + customClient = fn.New( + fn.WithVerbose(input.Verbose != nil && *input.Verbose), + fn.WithRepository(*input.Repository), + ) + } else { + customClient = client + } + + // Direct call to pkg/functions client.Init + _, err = customClient.Init(fn.Function{ + Name: derivedName, + Root: absolutePath, + Runtime: input.Language, + Template: templateVal, + }) + if err != nil { + return + } + + output = CreateOutput{ + Runtime: input.Language, + Template: &templateVal, + Message: fmt.Sprintf("Created %s function in %s", input.Language, absolutePath), + } + return + } + + // Fallback to executor for backward compatibility and testing out, err := s.executor.Execute(ctx, "create", input.Args()...) if err != nil { err = fmt.Errorf("%w\n%s", err, string(out)) @@ -56,3 +109,22 @@ type CreateOutput struct { Template *string `json:"template" jsonschema:"Template used"` Message string `json:"message,omitempty" jsonschema:"Output message"` } + +// deriveNameAndPath returns resolved function name and absolute path +// to the function project root. +func deriveNameAndPath(path string) (string, string, error) { + if path == "" { + var err error + path, err = os.Getwd() + if err != nil { + return "", "", err + } + } + + absPath, err := filepath.Abs(path) + if err != nil { + return "", "", err + } + + return filepath.Base(absPath), absPath, nil +} diff --git a/pkg/mcp/tools_create_test.go b/pkg/mcp/tools_create_test.go index 737102b8a3..f49a8d4721 100644 --- a/pkg/mcp/tools_create_test.go +++ b/pkg/mcp/tools_create_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/mcp/mock" ) @@ -123,3 +124,42 @@ func TestTool_Create_BinaryFailure(t *testing.T) { t.Errorf("expected error to include binary output, got: %s", resultToString(result)) } } + +// TestTool_Create_DirectClient validates direct pkg/functions client invocation +func TestTool_Create_DirectClient(t *testing.T) { + tempDir := t.TempDir() + + fnClient := fn.New() + client, _, err := newTestPair(t, WithClientProvider(func() *fn.Client { + return fnClient + })) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{ + "language": "go", + "path": tempDir, + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", resultToString(result)) + } + + // Verify the function was successfully created + f, err := fn.NewFunction(tempDir) + if err != nil { + t.Fatalf("failed to load created function: %v", err) + } + if !f.Initialized() { + t.Fatal("expected function to be initialized") + } + if f.Runtime != "go" { + t.Fatalf("expected runtime 'go', got %q", f.Runtime) + } +} diff --git a/pkg/mcp/tools_delete.go b/pkg/mcp/tools_delete.go index 27760f864d..25a04992bb 100644 --- a/pkg/mcp/tools_delete.go +++ b/pkg/mcp/tools_delete.go @@ -5,6 +5,8 @@ import ( "fmt" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" ) var deleteTool = &mcp.Tool{ @@ -31,7 +33,53 @@ func (s *Server) deleteHandler(ctx context.Context, r *mcp.CallToolRequest, inpu return } - // Execute + if s.clientProvider != nil { + client := s.clientProvider() + if client == nil { + err = fmt.Errorf("delete tool: client provider returned nil") + return + } + + allVal := true + if input.All != nil { + allVal = *input.All + } + + if input.Name != nil { + ns := getNamespace(input.Namespace, fn.Function{}) + err = client.Remove(ctx, *input.Name, ns, fn.Function{}, allVal) + if err != nil { + return + } + output = DeleteOutput{ + Message: fmt.Sprintf("Removed function %s from namespace %s", *input.Name, ns), + } + return + } else { + // Delete by path + _, absolutePath, errVal := deriveNameAndPath(*input.Path) + if errVal != nil { + err = errVal + return + } + f, errVal := fn.NewFunction(absolutePath) + if errVal != nil { + err = errVal + return + } + ns := getNamespace(input.Namespace, f) + err = client.Remove(ctx, f.Name, ns, f, allVal) + if err != nil { + return + } + output = DeleteOutput{ + Message: fmt.Sprintf("Removed function at path %s from namespace %s", absolutePath, ns), + } + return + } + } + + // Fallback to executor for backward compatibility and testing out, err := s.executor.Execute(ctx, "delete", input.Args()...) if err != nil { err = fmt.Errorf("%w\n%s", err, string(out)) @@ -73,3 +121,21 @@ func (i DeleteInput) Args() []string { type DeleteOutput struct { Message string `json:"message" jsonschema:"Output message"` } + +// getNamespace resolves the target namespace based on parameters, function spec, active kubernetes config, or standard defaults +func getNamespace(nsParam *string, f fn.Function) string { + if nsParam != nil && *nsParam != "" { + return *nsParam + } + if f.Deploy.Namespace != "" { + return f.Deploy.Namespace + } + if f.Namespace != "" { + return f.Namespace + } + ns, err := k8s.GetDefaultNamespace() + if err == nil && ns != "" { + return ns + } + return "default" +} diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index 58f5769821..d375cded84 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/mcp/mock" ) @@ -95,3 +96,58 @@ func TestTool_Delete_Readonly(t *testing.T) { t.Fatal("expected delete to be rejected in readonly mode") } } + +// TestTool_Delete_DirectClient validates direct pkg/functions client invocation for delete +func TestTool_Delete_DirectClient(t *testing.T) { + tempDir := t.TempDir() + + fnClient := fn.New() + + // Create a dummy function first so there is something initialized to delete by path + _, err := fnClient.Init(fn.Function{ + Name: "my-func", + Root: tempDir, + Runtime: "go", + Template: "http", + }) + if err != nil { + t.Fatal(err) + } + + client, server, err := newTestPair(t, WithClientProvider(func() *fn.Client { + return fnClient + })) + if err != nil { + t.Fatal(err) + } + server.readonly.Store(false) + + // Call delete with path + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{ + "path": tempDir, + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", resultToString(result)) + } + + // Call delete with name + result, err = client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{ + "name": "my-func", + "namespace": "default", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", resultToString(result)) + } +}