From 29329a779d4a0ce71b23b1c359fdeb688f08d58b Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 15 May 2026 05:24:16 +0530 Subject: [PATCH 1/3] feat: add describe tool for function details This commit introduces a new 'describe' tool to the MCP package, allowing users to retrieve detailed information about deployed functions, including name, image, namespace, routes, and event subscriptions. The tool accepts either a local path or a function name as input. Additionally, unit tests have been added to ensure the correct functionality of the describe tool, covering various scenarios such as input validation and expected outputs. - Added describeTool definition in tools_describe.go - Implemented describeHandler to process requests - Created tools_describe_test.go with comprehensive tests for the describe tool This enhancement improves the usability of the MCP by providing a straightforward way to obtain function details. --- pkg/mcp/mcp.go | 1 + pkg/mcp/tools_describe.go | 73 ++++++++++++++++ pkg/mcp/tools_describe_test.go | 154 +++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+) create mode 100644 pkg/mcp/tools_describe.go create mode 100644 pkg/mcp/tools_describe_test.go diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index b43715ee34..ac13d64deb 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -95,6 +95,7 @@ func New(options ...Option) *Server { mcp.AddTool(i, buildTool, s.buildHandler) mcp.AddTool(i, deployTool, s.deployHandler) mcp.AddTool(i, listTool, s.listHandler) + mcp.AddTool(i, describeTool, s.describeHandler) mcp.AddTool(i, deleteTool, s.deleteHandler) mcp.AddTool(i, configVolumesListTool, s.configVolumesListHandler) mcp.AddTool(i, configVolumesAddTool, s.configVolumesAddHandler) diff --git a/pkg/mcp/tools_describe.go b/pkg/mcp/tools_describe.go new file mode 100644 index 0000000000..4319452726 --- /dev/null +++ b/pkg/mcp/tools_describe.go @@ -0,0 +1,73 @@ +package mcp + +import ( + "context" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +var describeTool = &mcp.Tool{ + Name: "describe", + Title: "Describe Function", + Description: "Print the name, image, namespace, routes, and event subscriptions for a deployed function. Accepts either a local path (--path) or a function name.", + Annotations: &mcp.ToolAnnotations{ + Title: "Describe Function", + ReadOnlyHint: true, + IdempotentHint: true, // Describing the same function multiple times returns the same result at any point in time. + }, +} + +func (s *Server) describeHandler(ctx context.Context, r *mcp.CallToolRequest, input DescribeInput) (result *mcp.CallToolResult, output DescribeOutput, err error) { + if input.Path != nil && input.Name != nil { + err = fmt.Errorf("'path' and 'name' are mutually exclusive: provide one or the other") + return + } + if input.Namespace != nil && input.Name == nil { + err = fmt.Errorf("'namespace' requires 'name' to also be provided") + return + } + + out, err := s.executor.Execute(ctx, "describe", input.Args()...) + if err != nil { + err = fmt.Errorf("%w\n%s", err, string(out)) + return + } + output = DescribeOutput{ + Message: string(out), + } + return +} + +// DescribeInput defines the input parameters for the describe tool. +// Exactly one of Path or Name should be provided; if neither is given, the +// function in the current working directory is described. +type DescribeInput struct { + // Path and Name are mutually exclusive. Namespace is only valid with Name. + Path *string `json:"path,omitempty" jsonschema:"Path to the function project directory (mutually exclusive with name)"` + Name *string `json:"name,omitempty" jsonschema:"Name of the function to describe (mutually exclusive with path)"` + Namespace *string `json:"namespace,omitempty" jsonschema:"Kubernetes namespace (only used together with name)"` + Output *string `json:"output,omitempty" jsonschema:"Output format: human, plain, json, xml, yaml, or url"` + Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` +} + +func (i DescribeInput) Args() []string { + args := []string{} + + // Name is a positional argument; path is a flag. + if i.Name != nil { + args = append(args, *i.Name) + } else if i.Path != nil { + args = append(args, "--path", *i.Path) + } + + args = appendStringFlag(args, "--namespace", i.Namespace) + args = appendStringFlag(args, "--output", i.Output) + args = appendBoolFlag(args, "--verbose", i.Verbose) + return args +} + +// DescribeOutput defines the structured output returned by the describe tool. +type DescribeOutput struct { + Message string `json:"message" jsonschema:"Output message from func describe"` +} diff --git a/pkg/mcp/tools_describe_test.go b/pkg/mcp/tools_describe_test.go new file mode 100644 index 0000000000..e688e4973d --- /dev/null +++ b/pkg/mcp/tools_describe_test.go @@ -0,0 +1,154 @@ +package mcp + +import ( + "context" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "knative.dev/func/pkg/mcp/mock" +) + +// TestTool_Describe_ByPath ensures the describe tool passes --path and optional flags correctly. +func TestTool_Describe_ByPath(t *testing.T) { + stringFlags := map[string]struct { + jsonKey string + flag string + value string + }{ + "path": {"path", "--path", "./my-func"}, + "output": {"output", "--output", "json"}, + } + + boolFlags := map[string]string{ + "verbose": "--verbose", + } + + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "describe" { + t.Fatalf("expected subcommand 'describe', got %q", subcommand) + } + validateArgLength(t, args, len(stringFlags), len(boolFlags)) + validateStringFlags(t, args, stringFlags) + validateBoolFlags(t, args, boolFlags) + return []byte("Function name:\n my-func\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + inputArgs := buildInputArgs(stringFlags, boolFlags) + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe", + Arguments: inputArgs, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_Describe_ByName ensures the describe tool passes the function name as a positional argument. +func TestTool_Describe_ByName(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "describe" { + t.Fatalf("expected subcommand 'describe', got %q", subcommand) + } + // expect: ["my-func", "--namespace", "prod"] + if len(args) != 3 { + t.Fatalf("expected 3 args, got %d: %v", len(args), args) + } + if args[0] != "my-func" { + t.Fatalf("expected positional name 'my-func', got %q", args[0]) + } + argsMap := argsToMap(args[1:]) + if ns, ok := argsMap["--namespace"]; !ok || ns != "prod" { + t.Fatalf("expected --namespace prod, got map: %v", argsMap) + } + return []byte("Function name:\n my-func\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe", + Arguments: map[string]any{ + "name": "my-func", + "namespace": "prod", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_Describe_PathAndNameConflict ensures an error is returned when both path and name are provided. +func TestTool_Describe_PathAndNameConflict(t *testing.T) { + executor := mock.NewExecutor() + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe", + Arguments: map[string]any{ + "path": "./my-func", + "name": "my-func", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when both path and name are provided") + } + if executor.ExecuteInvoked { + t.Fatal("executor should not have been invoked") + } +} + +// TestTool_Describe_NamespaceWithoutName ensures an error is returned when namespace is set without a name. +func TestTool_Describe_NamespaceWithoutName(t *testing.T) { + executor := mock.NewExecutor() + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe", + Arguments: map[string]any{ + "namespace": "prod", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when namespace is provided without name") + } + if executor.ExecuteInvoked { + t.Fatal("executor should not have been invoked") + } +} From 6fc2cd2199dfa778249d6cb14b849f9c900e775c Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 15 May 2026 05:27:58 +0530 Subject: [PATCH 2/3] test: enhance describe tool tests and improve input validation This commit expands the unit tests for the describe tool, adding checks for both string and boolean flags, and ensuring correct argument validation. The tests now verify that the tool handles positional arguments and flags appropriately, including scenarios with no arguments. Additionally, the describe tool's input validation logic has been refined to clarify the mutual exclusivity of the 'path' and 'name' parameters, and to ensure that the namespace is only valid when a name is provided. - Updated TestTool_Describe_ByName to validate input arguments - Added TestTool_Describe_NoArgs to check behavior with no arguments - Improved input validation in describeHandler for clarity and correctness --- pkg/mcp/mcp.go | 2 +- pkg/mcp/tools_describe.go | 24 ++++++----- pkg/mcp/tools_describe_test.go | 75 ++++++++++++++++++++++++++++------ 3 files changed, 77 insertions(+), 24 deletions(-) diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index ac13d64deb..637135ac56 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -93,7 +93,7 @@ func New(options ...Option) *Server { mcp.AddTool(i, healthCheckTool, s.healthcheckHandler) mcp.AddTool(i, createTool, s.createHandler) mcp.AddTool(i, buildTool, s.buildHandler) - mcp.AddTool(i, deployTool, s.deployHandler) + mcp.AddTool(i, deployTool, s.deployHandler) mcp.AddTool(i, listTool, s.listHandler) mcp.AddTool(i, describeTool, s.describeHandler) mcp.AddTool(i, deleteTool, s.deleteHandler) diff --git a/pkg/mcp/tools_describe.go b/pkg/mcp/tools_describe.go index 4319452726..60185a991b 100644 --- a/pkg/mcp/tools_describe.go +++ b/pkg/mcp/tools_describe.go @@ -10,20 +10,24 @@ import ( var describeTool = &mcp.Tool{ Name: "describe", Title: "Describe Function", - Description: "Print the name, image, namespace, routes, and event subscriptions for a deployed function. Accepts either a local path (--path) or a function name.", + Description: "Print the name, image, namespace, routes, and event subscriptions for a deployed function. Accepts either a local directory path or a function name.", Annotations: &mcp.ToolAnnotations{ - Title: "Describe Function", - ReadOnlyHint: true, - IdempotentHint: true, // Describing the same function multiple times returns the same result at any point in time. + Title: "Describe Function", + ReadOnlyHint: true, + DestructiveHint: ptr(false), + IdempotentHint: true, // Describe has no side effects regardless of how many times it is called. }, } func (s *Server) describeHandler(ctx context.Context, r *mcp.CallToolRequest, input DescribeInput) (result *mcp.CallToolResult, output DescribeOutput, err error) { - if input.Path != nil && input.Name != nil { + pathSet := input.Path != nil && *input.Path != "" + nameSet := input.Name != nil && *input.Name != "" + + if pathSet && nameSet { err = fmt.Errorf("'path' and 'name' are mutually exclusive: provide one or the other") return } - if input.Namespace != nil && input.Name == nil { + if input.Namespace != nil && *input.Namespace != "" && !nameSet { err = fmt.Errorf("'namespace' requires 'name' to also be provided") return } @@ -40,14 +44,14 @@ func (s *Server) describeHandler(ctx context.Context, r *mcp.CallToolRequest, in } // DescribeInput defines the input parameters for the describe tool. -// Exactly one of Path or Name should be provided; if neither is given, the +// At most one of Path or Name should be provided; if neither is given, the // function in the current working directory is described. type DescribeInput struct { // Path and Name are mutually exclusive. Namespace is only valid with Name. Path *string `json:"path,omitempty" jsonschema:"Path to the function project directory (mutually exclusive with name)"` Name *string `json:"name,omitempty" jsonschema:"Name of the function to describe (mutually exclusive with path)"` Namespace *string `json:"namespace,omitempty" jsonschema:"Kubernetes namespace (only used together with name)"` - Output *string `json:"output,omitempty" jsonschema:"Output format: human, plain, json, xml, yaml, or url"` + Output *string `json:"output,omitempty" jsonschema:"Output format: human, plain, json, yaml, or url"` Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` } @@ -55,9 +59,9 @@ func (i DescribeInput) Args() []string { args := []string{} // Name is a positional argument; path is a flag. - if i.Name != nil { + if i.Name != nil && *i.Name != "" { args = append(args, *i.Name) - } else if i.Path != nil { + } else if i.Path != nil && *i.Path != "" { args = append(args, "--path", *i.Path) } diff --git a/pkg/mcp/tools_describe_test.go b/pkg/mcp/tools_describe_test.go index e688e4973d..ce0a478056 100644 --- a/pkg/mcp/tools_describe_test.go +++ b/pkg/mcp/tools_describe_test.go @@ -58,21 +58,73 @@ func TestTool_Describe_ByPath(t *testing.T) { // TestTool_Describe_ByName ensures the describe tool passes the function name as a positional argument. func TestTool_Describe_ByName(t *testing.T) { + stringFlags := map[string]struct { + jsonKey string + flag string + value string + }{ + "namespace": {"namespace", "--namespace", "prod"}, + } + + boolFlags := map[string]string{} + + name := "my-func" + executor := mock.NewExecutor() executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { if subcommand != "describe" { t.Fatalf("expected subcommand 'describe', got %q", subcommand) } - // expect: ["my-func", "--namespace", "prod"] - if len(args) != 3 { - t.Fatalf("expected 3 args, got %d: %v", len(args), args) + + // Expected: 1 positional + 1 string flag * 2 + 0 bool flags = 3 args + if len(args) != 1+len(stringFlags)*2+len(boolFlags) { + t.Fatalf("expected %d args, got %d: %v", 1+len(stringFlags)*2+len(boolFlags), len(args), args) + } + + // Validate positional name argument comes first + if args[0] != name { + t.Fatalf("expected positional arg %q, got %q", name, args[0]) } - if args[0] != "my-func" { - t.Fatalf("expected positional name 'my-func', got %q", args[0]) + + validateStringFlags(t, args[1:], stringFlags) + validateBoolFlags(t, args[1:], boolFlags) + + return []byte("Function name:\n my-func\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + inputArgs := buildInputArgs(stringFlags, boolFlags) + inputArgs["name"] = name + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "describe", + Arguments: inputArgs, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_Describe_NoArgs ensures the describe tool works with no arguments, +// falling back to describing the function in the current working directory. +func TestTool_Describe_NoArgs(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "describe" { + t.Fatalf("expected subcommand 'describe', got %q", subcommand) } - argsMap := argsToMap(args[1:]) - if ns, ok := argsMap["--namespace"]; !ok || ns != "prod" { - t.Fatalf("expected --namespace prod, got map: %v", argsMap) + if len(args) != 0 { + t.Fatalf("expected no args for current-directory describe, got %d: %v", len(args), args) } return []byte("Function name:\n my-func\n"), nil } @@ -83,11 +135,8 @@ func TestTool_Describe_ByName(t *testing.T) { } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ - Name: "describe", - Arguments: map[string]any{ - "name": "my-func", - "namespace": "prod", - }, + Name: "describe", + Arguments: map[string]any{}, }) if err != nil { t.Fatal(err) From 1f6358c022c5046df0cba23172e0ff58aebb5708 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 15 May 2026 07:36:58 +0530 Subject: [PATCH 3/3] fix: remove trailing whitespace in MCP tool registration This commit cleans up the MCP tool registration code by removing unnecessary trailing whitespace from the line that adds the deploy tool. This minor adjustment improves code readability and adheres to standard formatting practices. --- pkg/mcp/mcp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index 637135ac56..ac13d64deb 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -93,7 +93,7 @@ func New(options ...Option) *Server { mcp.AddTool(i, healthCheckTool, s.healthcheckHandler) mcp.AddTool(i, createTool, s.createHandler) mcp.AddTool(i, buildTool, s.buildHandler) - mcp.AddTool(i, deployTool, s.deployHandler) + mcp.AddTool(i, deployTool, s.deployHandler) mcp.AddTool(i, listTool, s.listHandler) mcp.AddTool(i, describeTool, s.describeHandler) mcp.AddTool(i, deleteTool, s.deleteHandler)