From eec414a54b87b8689382f1c18e6189aaf032c452 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 8 May 2026 03:51:27 +0530 Subject: [PATCH 1/4] test: add structured output tests for build and deploy tools - Introduced TestTool_Build_StructuredOutput to verify that the Image field is correctly populated from the .func/built-image file after a successful build. - Added TestTool_Build_StructuredOutput_NoFuncYaml to ensure that a missing func.yaml does not cause failures, resulting in an empty Image field. - Implemented TestParseDeployedURL to validate URL extraction from various deploy output formats. - Created TestTool_Deploy_StructuredOutput to confirm that the URL is correctly populated from parsed output after deployment. These tests enhance coverage for the build and deploy functionalities, ensuring proper handling of structured outputs. --- pkg/mcp/tools_build.go | 4 ++ pkg/mcp/tools_build_test.go | 86 ++++++++++++++++++++++++++++++++++++ pkg/mcp/tools_deploy.go | 42 ++++++++++++++++++ pkg/mcp/tools_deploy_test.go | 86 ++++++++++++++++++++++++++++++++++++ 4 files changed, 218 insertions(+) diff --git a/pkg/mcp/tools_build.go b/pkg/mcp/tools_build.go index 52576187aa..7c68b1af17 100644 --- a/pkg/mcp/tools_build.go +++ b/pkg/mcp/tools_build.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) var buildTool = &mcp.Tool{ @@ -28,6 +29,9 @@ func (s *Server) buildHandler(ctx context.Context, r *mcp.CallToolRequest, input output = BuildOutput{ Message: string(out), } + if f, ferr := fn.NewFunction(input.Path); ferr == nil { + output.Image = f.Build.Image + } return } diff --git a/pkg/mcp/tools_build_test.go b/pkg/mcp/tools_build_test.go index 310fce45e0..8401aaa128 100644 --- a/pkg/mcp/tools_build_test.go +++ b/pkg/mcp/tools_build_test.go @@ -2,6 +2,9 @@ package mcp import ( "context" + "encoding/json" + "os" + "path/filepath" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -67,3 +70,86 @@ func TestTool_Build_Args(t *testing.T) { t.Fatal("executor was not invoked") } } + +// TestTool_Build_StructuredOutput verifies that the Image field is populated +// from the .func/built-image file written by the func CLI after a successful build. +func TestTool_Build_StructuredOutput(t *testing.T) { + const wantImage = "ghcr.io/user/my-func:latest" + + // Create a minimal function directory that fn.NewFunction can read. + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "func.yaml"), []byte("name: my-func\nruntime: go\n"), 0644); err != nil { + t.Fatal(err) + } + funcDir := filepath.Join(root, ".func") + if err := os.MkdirAll(funcDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(funcDir, "built-image"), []byte(wantImage), 0644); err != nil { + t.Fatal(err) + } + + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + return []byte("Build successful\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "build", + Arguments: map[string]any{"path": root}, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + + raw := resultToString(result) + var output BuildOutput + if err := json.Unmarshal([]byte(raw), &output); err != nil { + t.Fatalf("failed to unmarshal output: %v\nraw: %s", err, raw) + } + if output.Image != wantImage { + t.Errorf("Image = %q, want %q", output.Image, wantImage) + } +} + +// TestTool_Build_StructuredOutput_NoFuncYaml verifies that a missing func.yaml +// (e.g. an invalid path) does not cause the handler to fail — Image is just empty. +func TestTool_Build_StructuredOutput_NoFuncYaml(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + return []byte("Build successful\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "build", + Arguments: map[string]any{"path": t.TempDir()}, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + + raw := resultToString(result) + var output BuildOutput + if err := json.Unmarshal([]byte(raw), &output); err != nil { + t.Fatalf("failed to unmarshal output: %v\nraw: %s", err, raw) + } + if output.Image != "" { + t.Errorf("expected empty Image when func.yaml absent, got %q", output.Image) + } +} diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index 518052130f..a4d45697fc 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -1,10 +1,14 @@ package mcp import ( + "bufio" + "bytes" "context" "fmt" + "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) var deployTool = &mcp.Tool{ @@ -31,10 +35,48 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu } output = DeployOutput{ Message: string(out), + URL: parseDeployedURL(out), + } + if f, ferr := fn.NewFunction(input.Path); ferr == nil { + output.Image = f.Deploy.Image } return } +// parseDeployedURL extracts the deployed function URL from combined command +// output. It handles two formats produced by the func CLI: +// +// - Local deploy (written to stderr by the functions client): +// "✅ Function deployed/updated in namespace "ns" and exposed at URL: \n " +// +// - Remote pipeline deploy (written to stdout by cmd/deploy.go): +// "Function Deployed at " +func parseDeployedURL(out []byte) string { + scanner := bufio.NewScanner(bytes.NewReader(out)) + urlNext := false + for scanner.Scan() { + line := scanner.Text() + if urlNext { + if u := strings.TrimSpace(line); u != "" { + return u + } + } + // Local deploy: URL follows on the next non-empty line after this marker. + if strings.Contains(line, "exposed at URL:") { + urlNext = true + continue + } + // Remote pipeline deploy: URL is on the same line after the prefix. + const remotePrefix = "Function Deployed at " + if idx := strings.Index(line, remotePrefix); idx >= 0 { + if u := strings.TrimSpace(line[idx+len(remotePrefix):]); u != "" { + return u + } + } + } + return "" +} + // DeployInput defines the input parameters for the deploy tool. type DeployInput struct { Path string `json:"path" jsonschema:"required,Path to the function project directory"` diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index 4b8d2e83c4..ccdea20c55 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -2,6 +2,7 @@ package mcp import ( "context" + "encoding/json" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -79,3 +80,88 @@ func TestTool_Deploy_Args(t *testing.T) { t.Fatal("executor was not invoked") } } + +// TestParseDeployedURL verifies URL extraction from both local and remote deploy output. +func TestParseDeployedURL(t *testing.T) { + tests := []struct { + name string + out string + want string + }{ + { + name: "local deploy", + out: "✅ Function deployed in namespace \"default\" and exposed at URL: \n https://my-func.default.example.com\n", + want: "https://my-func.default.example.com", + }, + { + name: "local update", + out: "✅ Function updated in namespace \"prod\" and exposed at URL: \n https://my-func.prod.example.com\n", + want: "https://my-func.prod.example.com", + }, + { + name: "remote pipeline deploy", + out: "Function Deployed at https://my-func.remote.example.com\n", + want: "https://my-func.remote.example.com", + }, + { + name: "remote pipeline deploy with surrounding output", + out: "Building...\nPushing...\nFunction Deployed at https://my-func.remote.example.com\nDone.\n", + want: "https://my-func.remote.example.com", + }, + { + name: "no url in output", + out: "function up-to-date. Force rebuild with --build\n", + want: "", + }, + { + name: "empty output", + out: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseDeployedURL([]byte(tt.out)) + if got != tt.want { + t.Errorf("parseDeployedURL(%q) = %q, want %q", tt.out, got, tt.want) + } + }) + } +} + +// TestTool_Deploy_StructuredOutput verifies that URL is populated from parsed output. +func TestTool_Deploy_StructuredOutput(t *testing.T) { + const wantURL = "https://my-func.default.example.com" + + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + out := "✅ Function deployed in namespace \"default\" and exposed at URL: \n " + wantURL + "\n" + return []byte(out), nil + } + + client, server, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + server.readonly = false + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "deploy", + Arguments: map[string]any{"path": t.TempDir()}, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + + raw := resultToString(result) + var output DeployOutput + if err := json.Unmarshal([]byte(raw), &output); err != nil { + t.Fatalf("failed to unmarshal output: %v\nraw: %s", err, raw) + } + if output.URL != wantURL { + t.Errorf("URL = %q, want %q", output.URL, wantURL) + } +} From 14834e4d538855bc4c5dbfd8e3aecfa07bb85aac Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Mon, 11 May 2026 18:15:35 +0530 Subject: [PATCH 2/4] fix(mcp): surface NewFunction and scanner errors in build/deploy output - parseDeployedURL returns (string, error) and checks scanner.Err(); buffer increased to 1MB to avoid silent truncation on large deploy output - buildHandler and deployHandler append warning to Message when NewFunction fails, so agents see structured output failures instead of silent empty fields --- pkg/mcp/tools_build.go | 2 ++ pkg/mcp/tools_deploy.go | 17 ++++++++++++----- pkg/mcp/tools_deploy_test.go | 5 ++++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/mcp/tools_build.go b/pkg/mcp/tools_build.go index 7c68b1af17..0104dfc18b 100644 --- a/pkg/mcp/tools_build.go +++ b/pkg/mcp/tools_build.go @@ -31,6 +31,8 @@ func (s *Server) buildHandler(ctx context.Context, r *mcp.CallToolRequest, input } if f, ferr := fn.NewFunction(input.Path); ferr == nil { output.Image = f.Build.Image + } else { + output.Message += fmt.Sprintf("\n(warning: could not read built image from func.yaml: %v)", ferr) } return } diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index a4d45697fc..c14c0cc7ae 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -33,12 +33,18 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu err = fmt.Errorf("%w\n%s", err, string(out)) return } + url, urlErr := parseDeployedURL(out) output = DeployOutput{ Message: string(out), - URL: parseDeployedURL(out), + URL: url, + } + if urlErr != nil { + output.Message += fmt.Sprintf("\n(warning: could not parse deployed URL from output: %v)", urlErr) } if f, ferr := fn.NewFunction(input.Path); ferr == nil { output.Image = f.Deploy.Image + } else { + output.Message += fmt.Sprintf("\n(warning: could not read deployed image from func.yaml: %v)", ferr) } return } @@ -51,14 +57,15 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu // // - Remote pipeline deploy (written to stdout by cmd/deploy.go): // "Function Deployed at " -func parseDeployedURL(out []byte) string { +func parseDeployedURL(out []byte) (string, error) { scanner := bufio.NewScanner(bytes.NewReader(out)) + scanner.Buffer(make([]byte, 1024*1024), 1024*1024) // handle large/verbose deploy output urlNext := false for scanner.Scan() { line := scanner.Text() if urlNext { if u := strings.TrimSpace(line); u != "" { - return u + return u, nil } } // Local deploy: URL follows on the next non-empty line after this marker. @@ -70,11 +77,11 @@ func parseDeployedURL(out []byte) string { const remotePrefix = "Function Deployed at " if idx := strings.Index(line, remotePrefix); idx >= 0 { if u := strings.TrimSpace(line[idx+len(remotePrefix):]); u != "" { - return u + return u, nil } } } - return "" + return "", scanner.Err() } // DeployInput defines the input parameters for the deploy tool. diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index ccdea20c55..09e52d3085 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -121,7 +121,10 @@ func TestParseDeployedURL(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := parseDeployedURL([]byte(tt.out)) + got, err := parseDeployedURL([]byte(tt.out)) + if err != nil { + t.Fatalf("parseDeployedURL(%q) unexpected error: %v", tt.out, err) + } if got != tt.want { t.Errorf("parseDeployedURL(%q) = %q, want %q", tt.out, got, tt.want) } From dbbcff9f670c17ca0115f754cab7512080cf3699 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Mon, 11 May 2026 18:18:19 +0530 Subject: [PATCH 3/4] fix(mcp): simplify parseDeployedURL comments --- pkg/mcp/tools_deploy.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index c14c0cc7ae..5ab2de8e6e 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -49,17 +49,12 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu return } -// parseDeployedURL extracts the deployed function URL from combined command -// output. It handles two formats produced by the func CLI: -// -// - Local deploy (written to stderr by the functions client): -// "✅ Function deployed/updated in namespace "ns" and exposed at URL: \n " -// -// - Remote pipeline deploy (written to stdout by cmd/deploy.go): -// "Function Deployed at " +// parseDeployedURL extracts the deployed URL from deploy output. +// Handles two formats: local deploy prints the URL on the line after +// "exposed at URL:", remote pipeline prints it inline after "Function Deployed at ". func parseDeployedURL(out []byte) (string, error) { scanner := bufio.NewScanner(bytes.NewReader(out)) - scanner.Buffer(make([]byte, 1024*1024), 1024*1024) // handle large/verbose deploy output + scanner.Buffer(make([]byte, 1024*1024), 1024*1024) urlNext := false for scanner.Scan() { line := scanner.Text() @@ -68,12 +63,10 @@ func parseDeployedURL(out []byte) (string, error) { return u, nil } } - // Local deploy: URL follows on the next non-empty line after this marker. if strings.Contains(line, "exposed at URL:") { urlNext = true continue } - // Remote pipeline deploy: URL is on the same line after the prefix. const remotePrefix = "Function Deployed at " if idx := strings.Index(line, remotePrefix); idx >= 0 { if u := strings.TrimSpace(line[idx+len(remotePrefix):]); u != "" { From 75462f9ece0283c722ecf94bf1be4b3a3b4218cb Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Mon, 11 May 2026 18:19:48 +0530 Subject: [PATCH 4/4] Revert "fix(mcp): simplify parseDeployedURL comments" This reverts commit dbbcff9f670c17ca0115f754cab7512080cf3699. --- pkg/mcp/tools_deploy.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index 5ab2de8e6e..c14c0cc7ae 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -49,12 +49,17 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu return } -// parseDeployedURL extracts the deployed URL from deploy output. -// Handles two formats: local deploy prints the URL on the line after -// "exposed at URL:", remote pipeline prints it inline after "Function Deployed at ". +// parseDeployedURL extracts the deployed function URL from combined command +// output. It handles two formats produced by the func CLI: +// +// - Local deploy (written to stderr by the functions client): +// "✅ Function deployed/updated in namespace "ns" and exposed at URL: \n " +// +// - Remote pipeline deploy (written to stdout by cmd/deploy.go): +// "Function Deployed at " func parseDeployedURL(out []byte) (string, error) { scanner := bufio.NewScanner(bytes.NewReader(out)) - scanner.Buffer(make([]byte, 1024*1024), 1024*1024) + scanner.Buffer(make([]byte, 1024*1024), 1024*1024) // handle large/verbose deploy output urlNext := false for scanner.Scan() { line := scanner.Text() @@ -63,10 +68,12 @@ func parseDeployedURL(out []byte) (string, error) { return u, nil } } + // Local deploy: URL follows on the next non-empty line after this marker. if strings.Contains(line, "exposed at URL:") { urlNext = true continue } + // Remote pipeline deploy: URL is on the same line after the prefix. const remotePrefix = "Function Deployed at " if idx := strings.Index(line, remotePrefix); idx >= 0 { if u := strings.TrimSpace(line[idx+len(remotePrefix):]); u != "" {