From 8009fe83eb384371c1d3dadfb5cdf724e961789f Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Tue, 9 Dec 2025 17:04:11 +0200 Subject: [PATCH] feat(mcp): only set variables for set flags Previously we had code which tried to detect nil values. But for example if a flag is an integer that wouldn't work. Instead we use a more robust approach of only setting variables if the flag was passed in as an argument. Test Plan: this command actually returns results now. Previously it wouldn't since it would have limit set to 0. go run ./cmd/src -v mcp list-repos -query 'cloud' --- cmd/src/mcp.go | 2 +- internal/mcp/mcp_args.go | 30 +++++++++++------------------- internal/mcp/mcp_args_test.go | 2 +- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/cmd/src/mcp.go b/cmd/src/mcp.go index b6934b466c..93cc5fbf63 100644 --- a/cmd/src/mcp.go +++ b/cmd/src/mcp.go @@ -65,7 +65,7 @@ func mcpMain(args []string) error { if err := flags.Parse(flagArgs); err != nil { return err } - mcp.DerefFlagValues(vars) + mcp.DerefFlagValues(flags, vars) if err := validateToolArgs(tool.InputSchema, args, vars); err != nil { return err diff --git a/internal/mcp/mcp_args.go b/internal/mcp/mcp_args.go index ecfb98e2b0..863f51af9d 100644 --- a/internal/mcp/mcp_args.go +++ b/internal/mcp/mcp_args.go @@ -24,36 +24,28 @@ func (s *strSliceFlag) String() string { return strings.Join(s.vals, ",") } -func DerefFlagValues(vars map[string]any) { +func DerefFlagValues(fs *flag.FlagSet, vars map[string]any) { + setFlags := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { + setFlags[f.Name] = true + }) + for k, v := range vars { + if !setFlags[k] { + delete(vars, k) + continue + } rfl := reflect.ValueOf(v) if rfl.Kind() == reflect.Pointer { vv := rfl.Elem().Interface() if slice, ok := vv.(strSliceFlag); ok { vv = slice.vals } - if isNil(vv) { - delete(vars, k) - } else { - vars[k] = vv - } + vars[k] = vv } } } -func isNil(v any) bool { - if v == nil { - return true - } - rv := reflect.ValueOf(v) - switch rv.Kind() { - case reflect.Slice, reflect.Map, reflect.Pointer, reflect.Interface: - return rv.IsNil() - default: - return false - } -} - func BuildArgFlagSet(tool *ToolDef) (*flag.FlagSet, map[string]any, error) { if tool == nil { return nil, nil, errors.New("cannot build flagset on nil Tool Definition") diff --git a/internal/mcp/mcp_args_test.go b/internal/mcp/mcp_args_test.go index 17d5b466e0..ca40c246ad 100644 --- a/internal/mcp/mcp_args_test.go +++ b/internal/mcp/mcp_args_test.go @@ -63,7 +63,7 @@ func TestFlagSetParse(t *testing.T) { if err := flagSet.Parse(args); err != nil { t.Fatalf("flagset parsing failed: %v", err) } - DerefFlagValues(vars) + DerefFlagValues(flagSet, vars) if v, ok := vars["repos"].([]string); ok { if len(v) != 2 {