feat(mcp): integrate tools with functions.Client API instead of CLI shell-out

Replace MCP tool handlers that exec'd the func binary with a native service
layer backed by functions.Client. This enables structured JSON outputs,
correct path handling, and readonly enforcement at the API level.

Adds describe, invoke, run, and logs MCP tools. Help resources continue to
use CLI subprocess for --help text.

Author: vishwas-droid

cmd/mcp.go

@@ -6,11 +6,13 @@ import (
 	"strconv"
 
 	"github.com/spf13/cobra"
-	"knative.dev/func/pkg/mcp"
-
 	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/mcp"
 )
 
+// testMCPOptions is set by tests to inject MCP server options (e.g. in-memory transport).
+var testMCPOptions []mcp.Option
+
 func NewMCPCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "mcp",
@@ -89,14 +91,10 @@ DESCRIPTION
 			return runMCPStart(cmd, args, newClient)
 		},
 	}
-	// no flags at this time; future enhancements may be to allow configuring
-	// HTTP Stream vs stdio, single vs multiuser modes, etc.  For now
-	// we just use a simple gathering of options in runMCPStart.
 	return cmd
 }
 
 func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) error {
-	// Configure write mode
 	writeEnabled := false
 	if val := os.Getenv("FUNC_ENABLE_MCP_WRITE"); val != "" {
 		parsed, err := strconv.ParseBool(val)
@@ -106,18 +104,23 @@ func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) err
 		writeEnabled = parsed
 	}
 
-	// Configure 'func' or 'kn func'?
-	rootCmd := cmd.Root()
-	cmdPrefix := rootCmd.Use
+	cmdPrefix := cmd.Root().Use
 
-	// Instantiate
-	client, done := newClient(ClientConfig{},
-		fn.WithMCPServer(mcp.New(
-			mcp.WithPrefix(cmdPrefix),
-			mcp.WithReadonly(!writeEnabled))))
-	defer done()
+	factory := func(cfg mcp.ClientConfig, options ...fn.Option) (*fn.Client, func()) {
+		return newClient(ClientConfig{
+			Verbose:            cfg.Verbose,
+			InsecureSkipVerify: cfg.InsecureSkipVerify,
+		}, options...)
+	}
+
+	opts := []mcp.Option{
+		mcp.WithPrefix(cmdPrefix),
+		mcp.WithReadonly(!writeEnabled),
+		mcp.WithClientFactory(factory),
+	}
+	opts = append(opts, testMCPOptions...)
 
-	// Start
-	return client.StartMCPServer(cmd.Context())
+	server := mcp.New(opts...)
 
+	return server.Start(cmd.Context())
 }

cmd/mcp_test.go

@@ -1,60 +1,77 @@
 package cmd
 
 import (
+	"context"
 	"testing"
 
-	fn "knative.dev/func/pkg/functions"
-	"knative.dev/func/pkg/mock"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	mcpkg "knative.dev/func/pkg/mcp"
 	. "knative.dev/func/pkg/testing"
 )
 
-// TestMCP_Start ensures the "mcp start" command starts the MCP server.
 func TestMCP_Start(t *testing.T) {
 	_ = FromTempDirectory(t)
 
-	server := mock.NewMCPServer()
+	serverTpt, clientTpt := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt)}
+	t.Cleanup(func() { testMCPOptions = nil })
 
-	cmd := NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	cmd := NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
 
-	if !server.StartInvoked {
-		// Indicates a failure of the command to correctly map the request
-		// for "mcp start" to an actual invocation of the client's
-		// StartMCPServer method, or something more fundamental like failure
-		// to register the subcommand, etc.
-		t.Fatal("MCP server's start method not invoked")
+	ctx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+
+	go func() { _ = cmd.ExecuteContext(ctx) }()
+
+	client := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session, err := client.Connect(t.Context(), clientTpt, nil)
+	if err != nil {
+		cancel()
+		t.Fatal(err)
 	}
+	session.Close()
+	cancel()
 }
 
-// TestMCP_StartWriteable ensures that the FUNC_ENABLE_MCP_WRITE environment
-// variable is correctly parsed and the server starts in both default
-// (readonly) and write-enabled modes.
 func TestMCP_StartWriteable(t *testing.T) {
 	_ = FromTempDirectory(t)
 
-	// Ensure it defaults to readonly (no env var set).
-	server := mock.NewMCPServer()
-	cmd := NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	// Readonly mode
+	serverTpt, clientTpt := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt)}
+	t.Cleanup(func() { testMCPOptions = nil })
+
+	cmd := NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
+	ctx, cancel := context.WithCancel(t.Context())
+	go func() { _ = cmd.ExecuteContext(ctx) }()
+
+	client := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session, err := client.Connect(t.Context(), clientTpt, nil)
+	if err != nil {
+		cancel()
 		t.Fatal(err)
 	}
-	if !server.StartInvoked {
-		t.Fatal("MCP server was not started in default mode")
-	}
+	session.Close()
+	cancel()
 
-	// Ensure it starts successfully with write mode enabled.
+	// Write mode
 	t.Setenv("FUNC_ENABLE_MCP_WRITE", "true")
-	server = mock.NewMCPServer()
-	cmd = NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	serverTpt2, clientTpt2 := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt2)}
+
+	cmd = NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
+	ctx2, cancel2 := context.WithCancel(t.Context())
+	go func() { _ = cmd.ExecuteContext(ctx2) }()
+
+	client2 := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session2, err := client2.Connect(t.Context(), clientTpt2, nil)
+	if err != nil {
+		cancel2()
 		t.Fatal(err)
 	}
-	if !server.StartInvoked {
-		t.Fatal("MCP server was not started with write mode enabled")
-	}
+	session2.Close()
+	cancel2()
 }

pkg/mcp/client_options.go

@@ -0,0 +1,175 @@
+package mcp
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/google/go-containerregistry/pkg/name"
+	"knative.dev/func/pkg/builders"
+	"knative.dev/func/pkg/buildpacks"
+	"knative.dev/func/pkg/config"
+	"knative.dev/func/pkg/creds"
+	"knative.dev/func/pkg/docker"
+	fn "knative.dev/func/pkg/functions"
+	fnhttp "knative.dev/func/pkg/http"
+	"knative.dev/func/pkg/k8s"
+	"knative.dev/func/pkg/keda"
+	"knative.dev/func/pkg/knative"
+	"knative.dev/func/pkg/oci"
+	"knative.dev/func/pkg/pipelines/tekton"
+	"knative.dev/func/pkg/s2i"
+)
+
+// builderClientOptions returns client options for the given builder name.
+func builderClientOptions(builder string, cfg ClientConfig, withTimestamp bool, registryAuthfile string) ([]fn.Option, error) {
+	if builder == "" {
+		builder = builders.Pack
+	}
+
+	o := []fn.Option{
+		fn.WithVerbose(cfg.Verbose),
+	}
+
+	t := fnhttp.NewRoundTripper(
+		fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify),
+		fnhttp.WithOpenShiftServiceCA(),
+	)
+	credsProvider := newCredentialsProvider(config.Dir(), t, registryAuthfile, cfg.InsecureSkipVerify)
+
+	switch builder {
+	case builders.Host:
+		o = append(o,
+			fn.WithScaffolder(oci.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(oci.NewBuilder(builders.Host, cfg.Verbose)),
+			fn.WithPusher(oci.NewPusher(cfg.InsecureSkipVerify, false, cfg.Verbose,
+				oci.WithTransport(fnhttp.NewRoundTripper(fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify), fnhttp.WithOpenShiftServiceCA())),
+				oci.WithCredentialsProvider(credsProvider),
+				oci.WithVerbose(cfg.Verbose))),
+		)
+	case builders.Pack:
+		o = append(o,
+			fn.WithScaffolder(buildpacks.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(buildpacks.NewBuilder(
+				buildpacks.WithName(builders.Pack),
+				buildpacks.WithTimestamp(withTimestamp),
+				buildpacks.WithVerbose(cfg.Verbose))),
+			fn.WithPusher(docker.NewPusher(
+				docker.WithCredentialsProvider(credsProvider),
+				docker.WithTransport(t),
+				docker.WithVerbose(cfg.Verbose),
+				docker.WithInsecure(cfg.InsecureSkipVerify))),
+		)
+	case builders.S2I:
+		o = append(o,
+			fn.WithScaffolder(s2i.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(s2i.NewBuilder(
+				s2i.WithName(builders.S2I),
+				s2i.WithVerbose(cfg.Verbose))),
+			fn.WithPusher(docker.NewPusher(
+				docker.WithCredentialsProvider(credsProvider),
+				docker.WithTransport(t),
+				docker.WithVerbose(cfg.Verbose),
+				docker.WithInsecure(cfg.InsecureSkipVerify))),
+		)
+	default:
+		return o, builders.ErrUnknownBuilder{Name: builder, Known: builders.All()}
+	}
+	return o, nil
+}
+
+// deployClientOptions returns client options for a deploy operation.
+func deployClientOptions(builder, deployer string, cfg ClientConfig, withTimestamp bool) ([]fn.Option, error) {
+	o, err := builderClientOptions(builder, cfg, withTimestamp, "")
+	if err != nil {
+		return nil, err
+	}
+
+	t := fnhttp.NewRoundTripper(
+		fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify),
+		fnhttp.WithOpenShiftServiceCA(),
+	)
+	credsProvider := newCredentialsProvider(config.Dir(), t, "", cfg.InsecureSkipVerify)
+
+	o = append(o, fn.WithPipelinesProvider(tekton.NewPipelinesProvider(
+		tekton.WithCredentialsProvider(credsProvider),
+		tekton.WithVerbose(cfg.Verbose),
+		tekton.WithTransport(t),
+	)))
+
+	if deployer == "" {
+		deployer = knative.KnativeDeployerName
+	}
+	switch deployer {
+	case knative.KnativeDeployerName:
+		o = append(o, fn.WithDeployer(knative.NewDeployer(knative.WithDeployerVerbose(cfg.Verbose))))
+	case k8s.KubernetesDeployerName:
+		o = append(o, fn.WithDeployer(k8s.NewDeployer(k8s.WithDeployerVerbose(cfg.Verbose))))
+	case keda.KedaDeployerName:
+		o = append(o, fn.WithDeployer(keda.NewDeployer(keda.WithDeployerVerbose(cfg.Verbose))))
+	default:
+		return nil, fmt.Errorf("unsupported deployer: %s (supported: %s, %s, %s)",
+			deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName)
+	}
+	return o, nil
+}
+
+func platformBuildOptions(platform string) ([]fn.BuildOption, error) {
+	if platform == "" {
+		return nil, nil
+	}
+	parts := strings.Split(platform, "/")
+	if len(parts) != 2 {
+		return nil, fmt.Errorf("platform must be in the form OS/Architecture (e.g. linux/amd64)")
+	}
+	return []fn.BuildOption{fn.BuildWithPlatforms([]fn.Platform{{OS: parts[0], Architecture: parts[1]}})}, nil
+}
+
+func shouldBuild(buildFlag string, f fn.Function) (bool, error) {
+	if buildFlag == "" || buildFlag == "auto" {
+		if f.Built() {
+			return false, nil
+		}
+		return true, nil
+	}
+	build, err := parseBoolFlag(buildFlag)
+	if err != nil {
+		return false, fmt.Errorf("invalid build flag %q: must be 'auto' or a boolean", buildFlag)
+	}
+	return build, nil
+}
+
+func parseBoolFlag(v string) (bool, error) {
+	switch strings.ToLower(v) {
+	case "true", "1", "t", "yes":
+		return true, nil
+	case "false", "0", "f", "no":
+		return false, nil
+	default:
+		return false, fmt.Errorf("invalid boolean value %q", v)
+	}
+}
+
+func isDigestedImage(v string) (bool, error) {
+	ref, err := name.ParseReference(v)
+	if err != nil {
+		return false, err
+	}
+	_, ok := ref.(name.Digest)
+	return ok, nil
+}
+
+func newCredentialsProvider(configPath string, t fnhttp.RoundTripCloser, authFilePath string, insecure bool) oci.CredentialsProvider {
+	additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...)
+	additionalLoaders = append(additionalLoaders, k8s.GetECRCredentialLoader()...)
+	additionalLoaders = append(additionalLoaders, k8s.GetACRCredentialLoader()...)
+
+	options := []creds.Opt{
+		creds.WithTransport(t),
+		creds.WithInsecure(insecure),
+		creds.WithAdditionalCredentialLoaders(additionalLoaders...),
+	}
+	if authFilePath != "" {
+		options = append(options, creds.WithAuthFilePath(authFilePath))
+	}
+	return creds.NewCredentialsProvider(configPath, options...)
+}