Use arrays rather than comma separated lists

Author: williammartin

README.md

@@ -129,8 +129,8 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
   - `repo`: Repository name (string, required)
   - `title`: Issue title (string, required)
   - `body`: Issue body content (string, optional)
-  - `assignees`: Comma-separated list of usernames to assign to this issue (string, optional)
-  - `labels`: Comma-separated list of labels to apply to this issue (string, optional)
+  - `assignees`: Usernames to assign to this issue (string[], optional)
+  - `labels`: Labels to apply to this issue (string[], optional)
 
 - **add_issue_comment** - Add a comment to an issue
 
@@ -144,7 +144,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
   - `owner`: Repository owner (string, required)
   - `repo`: Repository name (string, required)
   - `state`: Filter by state ('open', 'closed', 'all') (string, optional)
-  - `labels`: Comma-separated list of labels to filter by (string, optional)
+  - `labels`: Labels to filter by (string[], optional)
   - `sort`: Sort by ('created', 'updated', 'comments') (string, optional)
   - `direction`: Sort direction ('asc', 'desc') (string, optional)
   - `since`: Filter by date (ISO 8601 timestamp) (string, optional)
@@ -159,8 +159,8 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
   - `title`: New title (string, optional)
   - `body`: New description (string, optional)
   - `state`: New state ('open' or 'closed') (string, optional)
-  - `labels`: Comma-separated list of new labels (string, optional)
-  - `assignees`: Comma-separated list of new assignees (string, optional)
+  - `labels`: New labels (string[], optional)
+  - `assignees`: New assignees (string[], optional)
   - `milestone`: New milestone number (number, optional)
 
 - **search_issues** - Search for issues and pull requests

pkg/github/issues.go

@@ -228,11 +228,21 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t
 			mcp.WithString("body",
 				mcp.Description("Issue body content"),
 			),
-			mcp.WithString("assignees",
-				mcp.Description("Comma-separate list of usernames to assign to this issue"),
-			),
-			mcp.WithString("labels",
-				mcp.Description("Comma-separate list of labels to apply to this issue"),
+			mcp.WithArray("assignees",
+				mcp.Description("Usernames to assign to this issue"),
+				mcp.Items(
+					map[string]interface{}{
+						"type": "string",
+					},
+				),
+			),
+			mcp.WithArray("labels",
+				mcp.Description("Labels to apply to this issue"),
+				mcp.Items(
+					map[string]interface{}{
+						"type": "string",
+					},
+				),
 			),
 		),
 		func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
@@ -256,12 +266,13 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t
 			}
 
 			// Get assignees
-			assignees, err := optionalCommaSeparatedListParam(request, "assignees")
+			assignees, err := optionalParam[[]string](request, "assignees")
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}
+
 			// Get labels
-			labels, err := optionalCommaSeparatedListParam(request, "labels")
+			labels, err := optionalParam[[]string](request, "labels")
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}
@@ -311,15 +322,23 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
 			),
 			mcp.WithString("state",
 				mcp.Description("Filter by state ('open', 'closed', 'all')"),
+				mcp.Enum("open", "closed", "all"),
 			),
-			mcp.WithString("labels",
-				mcp.Description("Comma-separated list of labels to filter by"),
+			mcp.WithArray("labels",
+				mcp.Description("Filter by labels"),
+				mcp.Items(
+					map[string]interface{}{
+						"type": "string",
+					},
+				),
 			),
 			mcp.WithString("sort",
 				mcp.Description("Sort by ('created', 'updated', 'comments')"),
+				mcp.Enum("created", "updated", "comments"),
 			),
 			mcp.WithString("direction",
 				mcp.Description("Sort direction ('asc', 'desc')"),
+				mcp.Enum("asc", "desc"),
 			),
 			mcp.WithString("since",
 				mcp.Description("Filter by date (ISO 8601 timestamp)"),
@@ -349,10 +368,12 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			opts.Labels, err = optionalCommaSeparatedListParam(request, "labels")
+			// Get labels
+			labels, err := optionalParam[[]string](request, "labels")
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}
+			opts.Labels = labels
 
 			opts.Sort, err = optionalParam[string](request, "sort")
 			if err != nil {
@@ -431,12 +452,23 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
 			),
 			mcp.WithString("state",
 				mcp.Description("New state ('open' or 'closed')"),
-			),
-			mcp.WithString("labels",
-				mcp.Description("Comma-separated list of new labels"),
-			),
-			mcp.WithString("assignees",
-				mcp.Description("Comma-separated list of new assignees"),
+				mcp.Enum("open", "closed"),
+			),
+			mcp.WithArray("labels",
+				mcp.Description("New labels"),
+				mcp.Items(
+					map[string]interface{}{
+						"type": "string",
+					},
+				),
+			),
+			mcp.WithArray("assignees",
+				mcp.Description("New assignees"),
+				mcp.Items(
+					map[string]interface{}{
+						"type": "string",
+					},
+				),
 			),
 			mcp.WithNumber("milestone",
 				mcp.Description("New milestone number"),
@@ -484,15 +516,17 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
 				issueRequest.State = github.Ptr(state)
 			}
 
-			labels, err := optionalCommaSeparatedListParam(request, "labels")
+			// Get labels
+			labels, err := optionalParam[[]string](request, "labels")
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 			if len(labels) > 0 {
 				issueRequest.Labels = &labels
 			}
 
-			assignees, err := optionalCommaSeparatedListParam(request, "assignees")
+			// Get assignees
+			assignees, err := optionalParam[[]string](request, "assignees")
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}

pkg/github/issues_test.go

@@ -426,8 +426,8 @@ func Test_CreateIssue(t *testing.T) {
 				"repo":      "repo",
 				"title":     "Test Issue",
 				"body":      "This is a test issue",
-				"assignees": "user1, user2",
-				"labels":    "bug, help wanted",
+				"assignees": []string{"user1, user2"},
+				"labels":    []string{"bug", "help wanted"},
 			},
 			expectError:   false,
 			expectedIssue: mockIssue,
@@ -615,7 +615,7 @@ func Test_ListIssues(t *testing.T) {
 				"owner":     "owner",
 				"repo":      "repo",
 				"state":     "open",
-				"labels":    "bug,enhancement",
+				"labels":    []string{"bug", "enhancement"},
 				"sort":      "created",
 				"direction": "desc",
 				"since":     "2023-01-01T00:00:00Z",
@@ -760,8 +760,8 @@ func Test_UpdateIssue(t *testing.T) {
 				"title":        "Updated Issue Title",
 				"body":         "Updated issue description",
 				"state":        "closed",
-				"labels":       "bug,priority",
-				"assignees":    "assignee1,assignee2",
+				"labels":       []string{"bug", "priority"},
+				"assignees":    []string{"assignee1", "assignee2"},
 				"milestone":    float64(5),
 			},
 			expectError:   false,

pkg/github/server.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
 
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/google/go-github/v69/github"
@@ -119,25 +118,6 @@ func isAcceptedError(err error) bool {
 	return errors.As(err, &acceptedError)
 }
 
-// parseCommaSeparatedList is a helper function that parses a comma-separated list of strings from the input string.
-func parseCommaSeparatedList(input string) []string {
-	if input == "" {
-		return nil
-	}
-
-	parts := strings.Split(input, ",")
-	result := make([]string, 0, len(parts))
-
-	for _, part := range parts {
-		trimmed := strings.TrimSpace(part)
-		if trimmed != "" {
-			result = append(result, trimmed)
-		}
-	}
-
-	return result
-}
-
 // requiredParam is a helper function that can be used to fetch a requested parameter from the request.
 // It does the following checks:
 // 1. Checks if the parameter is present in the request.
@@ -221,20 +201,3 @@ func optionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, e
 	}
 	return v, nil
 }
-
-// optionalCommaSeparatedListParam is a helper function that can be used to fetch a requested parameter from the request.
-// It does the following:
-//  1. Checks if the parameter is present in the request, if not, it returns an empty list
-//  2. If it is present, it checks if the parameter is of the expected type and uses parseCommaSeparatedList to parse it
-//     and return the list of strings
-func optionalCommaSeparatedListParam(r mcp.CallToolRequest, p string) ([]string, error) {
-	v, err := optionalParam[string](r, p)
-	if err != nil {
-		return []string{}, err
-	}
-	l := parseCommaSeparatedList(v)
-	if len(l) == 0 {
-		return []string{}, nil
-	}
-	return l, nil
-}

pkg/github/server_test.go

@@ -168,67 +168,6 @@ func Test_IsAcceptedError(t *testing.T) {
 	}
 }
 
-func Test_ParseCommaSeparatedList(t *testing.T) {
-	tests := []struct {
-		name     string
-		input    string
-		expected []string
-	}{
-		{
-			name:     "simple comma separated values",
-			input:    "one,two,three",
-			expected: []string{"one", "two", "three"},
-		},
-		{
-			name:     "values with spaces",
-			input:    "one, two, three",
-			expected: []string{"one", "two", "three"},
-		},
-		{
-			name:     "values with extra spaces",
-			input:    "  one  ,  two  ,  three  ",
-			expected: []string{"one", "two", "three"},
-		},
-		{
-			name:     "empty values in between",
-			input:    "one,,three",
-			expected: []string{"one", "three"},
-		},
-		{
-			name:     "only spaces",
-			input:    " , , ",
-			expected: []string{},
-		},
-		{
-			name:     "empty string",
-			input:    "",
-			expected: nil,
-		},
-		{
-			name:     "single value",
-			input:    "one",
-			expected: []string{"one"},
-		},
-		{
-			name:     "trailing comma",
-			input:    "one,two,",
-			expected: []string{"one", "two"},
-		},
-		{
-			name:     "leading comma",
-			input:    ",one,two",
-			expected: []string{"one", "two"},
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			result := parseCommaSeparatedList(tc.input)
-			assert.Equal(t, tc.expected, result)
-		})
-	}
-}
-
 func Test_RequiredStringParam(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -492,59 +431,6 @@ func Test_OptionalNumberParamWithDefault(t *testing.T) {
 	}
 }
 
-func Test_OptionalCommaSeparatedListParam(t *testing.T) {
-	tests := []struct {
-		name        string
-		params      map[string]interface{}
-		paramName   string
-		expected    []string
-		expectError bool
-	}{
-		{
-			name:        "valid comma-separated list",
-			params:      map[string]interface{}{"tags": "one,two,three"},
-			paramName:   "tags",
-			expected:    []string{"one", "two", "three"},
-			expectError: false,
-		},
-		{
-			name:        "empty list",
-			params:      map[string]interface{}{"tags": ""},
-			paramName:   "tags",
-			expected:    []string{},
-			expectError: false,
-		},
-		{
-			name:        "missing parameter",
-			params:      map[string]interface{}{},
-			paramName:   "tags",
-			expected:    []string{},
-			expectError: false,
-		},
-		{
-			name:        "wrong type parameter",
-			params:      map[string]interface{}{"tags": 123},
-			paramName:   "tags",
-			expected:    nil,
-			expectError: true,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			request := createMCPRequest(tc.params)
-			result, err := optionalCommaSeparatedListParam(request, tc.paramName)
-
-			if tc.expectError {
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, tc.expected, result)
-			}
-		})
-	}
-}
-
 func Test_OptionalBooleanParam(t *testing.T) {
 	tests := []struct {
 		name        string