Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions pkg/mcp/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func New(options ...Option) *Server {
// Help
// A resource for each command which returns its help
// eg. "config volumes add" -> "func://help/config/volumes/add")
i.AddResource(newHelpResource(s, "Help", "help for the command root", ""))
i.AddResource(newHelpResource(s, "Help", "help for the command root"))
i.AddResource(newHelpResource(s, "Create Help", "help for 'create'", "create"))
i.AddResource(newHelpResource(s, "Build Help", "help for 'build'", "build"))
i.AddResource(newHelpResource(s, "Deploy Help", "help for 'deploy'", "deploy"))
Expand Down Expand Up @@ -151,12 +151,19 @@ type defaultExecutor struct {
}

func (e defaultExecutor) Execute(ctx context.Context, subcommand string, args ...string) ([]byte, error) {
// Parse prefix: "func" or "kn func" -> ["func"] or ["kn", "func"]
cmdParts := strings.Fields(e.s.prefix)
cmdParts = append(cmdParts, subcommand)
cmdParts = append(cmdParts, args...)

cmdParts := buildArgs(e.s.prefix, subcommand, args)
cmd := exec.CommandContext(ctx, cmdParts[0], cmdParts[1:]...)
// cmd.Dir not set - inherits process working directory which is the current working directory
Comment on lines 153 to 156
return cmd.CombinedOutput()
}

// buildArgs constructs the ordered argument list for execution.
// An empty subcommand is omitted so that commands like "func --help" are
// built correctly rather than "func --help" with a spurious empty argument.
func buildArgs(prefix, subcommand string, args []string) []string {
parts := strings.Fields(prefix)
if subcommand != "" {
parts = append(parts, subcommand)
}
return append(parts, args...)
}
62 changes: 62 additions & 0 deletions pkg/mcp/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,65 @@ func newTestPairWithReadonly(t *testing.T, readonly bool) (*mcp.ClientSession, *
func newTestPair(t *testing.T, options ...Option) (session *mcp.ClientSession, server *Server, err error) {
return newTestPairCore(t, false, options...)
}

// TestBuildArgs verifies that buildArgs constructs the command argument list
// correctly, and in particular that an empty subcommand is never injected.
func TestBuildArgs(t *testing.T) {
cases := []struct {
name string
prefix string
subcommand string
args []string
want []string
}{
{
name: "empty subcommand is omitted",
prefix: "func",
subcommand: "",
args: []string{"create", "--help"},
want: []string{"func", "create", "--help"},
},
{
name: "non-empty subcommand is included",
prefix: "func",
subcommand: "deploy",
args: []string{"--verbose"},
want: []string{"func", "deploy", "--verbose"},
},
{
name: "multi-word prefix is split correctly",
prefix: "kn func",
subcommand: "build",
args: []string{"--push"},
want: []string{"kn", "func", "build", "--push"},
},
{
name: "multi-word prefix with empty subcommand",
prefix: "kn func",
subcommand: "",
args: []string{"list", "--help"},
want: []string{"kn", "func", "list", "--help"},
},
{
name: "no args",
prefix: "func",
subcommand: "list",
args: nil,
want: []string{"func", "list"},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := buildArgs(tc.prefix, tc.subcommand, tc.args)
if len(got) != len(tc.want) {
t.Fatalf("buildArgs(%q, %q, %v) = %v, want %v", tc.prefix, tc.subcommand, tc.args, got, tc.want)
}
for i := range got {
if got[i] != tc.want[i] {
t.Errorf("buildArgs result[%d] = %q, want %q (full: %v)", i, got[i], tc.want[i], got)
}
}
})
}
}
119 changes: 119 additions & 0 deletions pkg/mcp/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,125 @@ func TestResource_FunctionState(t *testing.T) {
}
}

// TestResource_Help verifies that every help resource handler invokes the
// executor with an empty subcommand and the correct command arguments ending
// with "--help". This is the regression test for the bug where the empty
// subcommand was unconditionally appended, injecting a spurious "" argument
// into every help command.
Comment on lines +87 to +91
func TestResource_Help(t *testing.T) {
cases := []struct {
name string
uri string
wantArgs []string // arguments expected after the empty subcommand
}{
{
name: "root help",
uri: "func://help",
wantArgs: []string{"--help"},
},
{
name: "create help",
uri: "func://help/create",
wantArgs: []string{"create", "--help"},
},
{
name: "build help",
uri: "func://help/build",
wantArgs: []string{"build", "--help"},
},
{
name: "deploy help",
uri: "func://help/deploy",
wantArgs: []string{"deploy", "--help"},
},
{
name: "list help",
uri: "func://help/list",
wantArgs: []string{"list", "--help"},
},
{
name: "config volumes help",
uri: "func://help/config/volumes",
wantArgs: []string{"config", "volumes", "--help"},
},
{
name: "config volumes add help",
uri: "func://help/config/volumes/add",
wantArgs: []string{"config", "volumes", "add", "--help"},
},
{
name: "config labels help",
uri: "func://help/config/labels",
wantArgs: []string{"config", "labels", "--help"},
},
{
name: "config labels add help",
uri: "func://help/config/labels/add",
wantArgs: []string{"config", "labels", "add", "--help"},
},
{
name: "config envs help",
uri: "func://help/config/envs",
wantArgs: []string{"config", "envs", "--help"},
},
{
name: "config envs add help",
uri: "func://help/config/envs/add",
wantArgs: []string{"config", "envs", "add", "--help"},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
const helpOutput = "Usage: func ...\n"

executor := mock.NewExecutor()
executor.ExecuteFn = func(_ context.Context, subcommand string, args ...string) ([]byte, error) {
// subcommand must always be empty: help resources encode the
// command path entirely in args, not in the subcommand field.
if subcommand != "" {
t.Errorf("expected empty subcommand, got %q", subcommand)
}
// No arg may be an empty string — that was the original bug.
for i, a := range args {
if a == "" {
t.Errorf("args[%d] is an empty string (full args: %v)", i, args)
}
}
if len(args) != len(tc.wantArgs) {
t.Fatalf("expected args %v, got %v", tc.wantArgs, args)
}
for i := range args {
if args[i] != tc.wantArgs[i] {
t.Errorf("args[%d] = %q, want %q", i, args[i], tc.wantArgs[i])
}
}
return []byte(helpOutput), nil
}

client, _, err := newTestPair(t, WithExecutor(executor))
if err != nil {
t.Fatal(err)
}

result, err := client.ReadResource(t.Context(), &mcp.ReadResourceParams{URI: tc.uri})
if err != nil {
t.Fatalf("ReadResource(%q): %v", tc.uri, err)
}

if len(result.Contents) != 1 {
t.Fatalf("expected 1 content, got %d", len(result.Contents))
}
if result.Contents[0].Text != helpOutput {
t.Errorf("expected output %q, got %q", helpOutput, result.Contents[0].Text)
}
if !executor.ExecuteInvoked {
t.Fatal("executor was not invoked")
}
})
}
}

func TestResource_Listings(t *testing.T) {
cases := []struct {
name string
Expand Down
Loading