From 6c70992278add4159373af4a45a690da7e157044 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 5 Jun 2025 23:05:43 +0200 Subject: [PATCH 1/3] cleanup search_users response --- pkg/github/search.go | 36 +++++++++++++++++++++++++++++++++--- pkg/github/search_test.go | 29 +++++++++++------------------ 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 2df39bcd8..8b5e83960 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -146,6 +146,19 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to } } +type MinimalUser struct { + Login string `json:"login"` + ID int64 `json:"id,omitempty"` + ProfileURL string `json:"profile_url,omitempty"` + AvatarURL string `json:"avatar_url,omitempty"` +} + +type MinimalSearchUsersResult struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []MinimalUser `json:"items"` +} + // SearchUsers creates a tool to search for GitHub users. func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_users", @@ -200,7 +213,7 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - result, resp, err := client.Search.Users(ctx, query, opts) + result, resp, err := client.Search.Users(ctx, "type:user "+query, opts) if err != nil { return nil, fmt.Errorf("failed to search users: %w", err) } @@ -214,11 +227,28 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(fmt.Sprintf("failed to search users: %s", string(body))), nil } - r, err := json.Marshal(result) + minimalUsers := make([]MinimalUser, 0, len(result.Users)) + for _, user := range result.Users { + mu := MinimalUser{ + Login: user.GetLogin(), + ID: user.GetID(), + ProfileURL: user.GetHTMLURL(), + AvatarURL: user.GetAvatarURL(), + } + + minimalUsers = append(minimalUsers, mu) + } + + minimalResp := MinimalSearchUsersResult{ + TotalCount: result.GetTotal(), + IncompleteResults: result.GetIncompleteResults(), + Items: minimalUsers, + } + + r, err := json.Marshal(minimalResp) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } - return mcp.NewToolResultText(string(r)), nil } } diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 3cd858de0..62645e91d 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -335,9 +335,6 @@ func Test_SearchUsers(t *testing.T) { ID: github.Ptr(int64(1001)), HTMLURL: github.Ptr("https://github.com/user1"), AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/1001"), - Type: github.Ptr("User"), - Followers: github.Ptr(100), - Following: github.Ptr(50), }, { Login: github.Ptr("user2"), @@ -345,8 +342,6 @@ func Test_SearchUsers(t *testing.T) { HTMLURL: github.Ptr("https://github.com/user2"), AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/1002"), Type: github.Ptr("User"), - Followers: github.Ptr(200), - Following: github.Ptr(75), }, }, } @@ -365,7 +360,7 @@ func Test_SearchUsers(t *testing.T) { mock.WithRequestMatchHandler( mock.GetSearchUsers, expectQueryParams(t, map[string]string{ - "q": "location:finland language:go", + "q": "type:user location:finland language:go", "sort": "followers", "order": "desc", "page": "1", @@ -391,7 +386,7 @@ func Test_SearchUsers(t *testing.T) { mock.WithRequestMatchHandler( mock.GetSearchUsers, expectQueryParams(t, map[string]string{ - "q": "location:finland language:go", + "q": "type:user location:finland language:go", "page": "1", "per_page": "30", }).andThen( @@ -451,19 +446,17 @@ func Test_SearchUsers(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result - var returnedResult github.UsersSearchResult + var returnedResult MinimalSearchUsersResult err = json.Unmarshal([]byte(textContent.Text), &returnedResult) require.NoError(t, err) - assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total) - assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults) - assert.Len(t, returnedResult.Users, len(tc.expectedResult.Users)) - for i, user := range returnedResult.Users { - assert.Equal(t, *tc.expectedResult.Users[i].Login, *user.Login) - assert.Equal(t, *tc.expectedResult.Users[i].ID, *user.ID) - assert.Equal(t, *tc.expectedResult.Users[i].HTMLURL, *user.HTMLURL) - assert.Equal(t, *tc.expectedResult.Users[i].AvatarURL, *user.AvatarURL) - assert.Equal(t, *tc.expectedResult.Users[i].Type, *user.Type) - assert.Equal(t, *tc.expectedResult.Users[i].Followers, *user.Followers) + assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) + assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Items, len(tc.expectedResult.Users)) + for i, user := range returnedResult.Items { + assert.Equal(t, *tc.expectedResult.Users[i].Login, user.Login) + assert.Equal(t, *tc.expectedResult.Users[i].ID, user.ID) + assert.Equal(t, *tc.expectedResult.Users[i].HTMLURL, user.ProfileURL) + assert.Equal(t, *tc.expectedResult.Users[i].AvatarURL, user.AvatarURL) } }) } From 02f0a75b2caacd33347da594b278da0f765ffdb8 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 5 Jun 2025 23:05:43 +0200 Subject: [PATCH 2/3] cleanup search_users response --- pkg/github/search.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 8b5e83960..16e8b56a7 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -244,7 +244,6 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t IncompleteResults: result.GetIncompleteResults(), Items: minimalUsers, } - r, err := json.Marshal(minimalResp) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) From ba7bc51401f517e13860a1da667aa493b6dcde08 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 6 Jun 2025 00:08:04 +0200 Subject: [PATCH 3/3] separate org and user search --- pkg/github/search.go | 198 +++++++++++++++++++++++--------------- pkg/github/search_test.go | 122 +++++++++++++++++++++++ pkg/github/tools.go | 5 + 3 files changed, 246 insertions(+), 79 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 16e8b56a7..6c4df36a2 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -159,95 +159,135 @@ type MinimalSearchUsersResult struct { Items []MinimalUser `json:"items"` } -// SearchUsers creates a tool to search for GitHub users. -func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { - return mcp.NewTool("search_users", - mcp.WithDescription(t("TOOL_SEARCH_USERS_DESCRIPTION", "Search for GitHub users")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_SEARCH_USERS_USER_TITLE", "Search users"), - ReadOnlyHint: toBoolPtr(true), - }), - mcp.WithString("q", - mcp.Required(), - mcp.Description("Search query using GitHub users search syntax"), - ), - mcp.WithString("sort", - mcp.Description("Sort field by category"), - mcp.Enum("followers", "repositories", "joined"), - ), - mcp.WithString("order", - mcp.Description("Sort order"), - mcp.Enum("asc", "desc"), - ), - WithPagination(), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - query, err := requiredParam[string](request, "q") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - sort, err := OptionalParam[string](request, "sort") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - order, err := OptionalParam[string](request, "order") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pagination, err := OptionalPaginationParams(request) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } +func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHandlerFunc { + return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + query, err := requiredParam[string](request, "q") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + sort, err := OptionalParam[string](request, "sort") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + order, err := OptionalParam[string](request, "order") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } - opts := &github.SearchOptions{ - Sort: sort, - Order: order, - ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, - }, - } + opts := &github.SearchOptions{ + Sort: sort, + Order: order, + ListOptions: github.ListOptions{ + PerPage: pagination.perPage, + Page: pagination.page, + }, + } - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } - result, resp, err := client.Search.Users(ctx, "type:user "+query, opts) + searchQuery := "type:" + accountType + " " + query + result, resp, err := client.Search.Users(ctx, searchQuery, opts) + if err != nil { + return nil, fmt.Errorf("failed to search %ss: %w", accountType, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to search users: %w", err) + return nil, fmt.Errorf("failed to read response body: %w", err) } - defer func() { _ = resp.Body.Close() }() + return mcp.NewToolResultError(fmt.Sprintf("failed to search %ss: %s", accountType, string(body))), nil + } - if resp.StatusCode != 200 { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to search users: %s", string(body))), nil - } + minimalUsers := make([]MinimalUser, 0, len(result.Users)) - minimalUsers := make([]MinimalUser, 0, len(result.Users)) - for _, user := range result.Users { - mu := MinimalUser{ - Login: user.GetLogin(), - ID: user.GetID(), - ProfileURL: user.GetHTMLURL(), - AvatarURL: user.GetAvatarURL(), + for _, user := range result.Users { + if user.Login != nil { + mu := MinimalUser{Login: *user.Login} + if user.ID != nil { + mu.ID = *user.ID + } + if user.HTMLURL != nil { + mu.ProfileURL = *user.HTMLURL + } + if user.AvatarURL != nil { + mu.AvatarURL = *user.AvatarURL } - minimalUsers = append(minimalUsers, mu) } + } + minimalResp := &MinimalSearchUsersResult{ + TotalCount: result.GetTotal(), + IncompleteResults: result.GetIncompleteResults(), + Items: minimalUsers, + } + if result.Total != nil { + minimalResp.TotalCount = *result.Total + } + if result.IncompleteResults != nil { + minimalResp.IncompleteResults = *result.IncompleteResults + } - minimalResp := MinimalSearchUsersResult{ - TotalCount: result.GetTotal(), - IncompleteResults: result.GetIncompleteResults(), - Items: minimalUsers, - } - r, err := json.Marshal(minimalResp) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - return mcp.NewToolResultText(string(r)), nil + r, err := json.Marshal(minimalResp) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) } + return mcp.NewToolResultText(string(r)), nil + } +} + +// SearchUsers creates a tool to search for GitHub users. +func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("search_users", + mcp.WithDescription(t("TOOL_SEARCH_USERS_DESCRIPTION", "Search for GitHub users exclusively")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_SEARCH_USERS_USER_TITLE", "Search users"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("q", + mcp.Required(), + mcp.Description("Search query using GitHub users search syntax scoped to type:user"), + ), + mcp.WithString("sort", + mcp.Description("Sort field by category"), + mcp.Enum("followers", "repositories", "joined"), + ), + mcp.WithString("order", + mcp.Description("Sort order"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), userOrOrgHandler("user", getClient) +} + +// SearchOrgs creates a tool to search for GitHub organizations. +func SearchOrgs(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("search_orgs", + mcp.WithDescription(t("TOOL_SEARCH_ORGS_DESCRIPTION", "Search for GitHub organizations exclusively")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_SEARCH_ORGS_USER_TITLE", "Search organizations"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("q", + mcp.Required(), + mcp.Description("Search query using GitHub organizations search syntax scoped to type:org"), + ), + mcp.WithString("sort", + mcp.Description("Sort field by category"), + mcp.Enum("followers", "repositories", "joined"), + ), + mcp.WithString("order", + mcp.Description("Sort order"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), userOrOrgHandler("org", getClient) } diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 62645e91d..a192bb214 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -461,3 +461,125 @@ func Test_SearchUsers(t *testing.T) { }) } } + +func Test_SearchOrgs(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := SearchOrgs(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "search_orgs", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "q") + assert.Contains(t, tool.InputSchema.Properties, "sort") + assert.Contains(t, tool.InputSchema.Properties, "order") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"q"}) + + // Setup mock search results + mockSearchResult := &github.UsersSearchResult{ + Total: github.Ptr(int(2)), + IncompleteResults: github.Ptr(false), + Users: []*github.User{ + { + Login: github.Ptr("org-1"), + ID: github.Ptr(int64(111)), + HTMLURL: github.Ptr("https://github.com/org-1"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/111?v=4"), + }, + { + Login: github.Ptr("org-2"), + ID: github.Ptr(int64(222)), + HTMLURL: github.Ptr("https://github.com/org-2"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/222?v=4"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.UsersSearchResult + expectedErrMsg string + }{ + { + name: "successful org search", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org github", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "q": "github", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "org search fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message": "Validation Failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "q": "invalid:query", + }, + expectError: true, + expectedErrMsg: "failed to search orgs", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := SearchOrgs(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + require.NotNil(t, result) + + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedResult MinimalSearchUsersResult + err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + require.NoError(t, err) + assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) + assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Items, len(tc.expectedResult.Users)) + for i, org := range returnedResult.Items { + assert.Equal(t, *tc.expectedResult.Users[i].Login, org.Login) + assert.Equal(t, *tc.expectedResult.Users[i].ID, org.ID) + assert.Equal(t, *tc.expectedResult.Users[i].HTMLURL, org.ProfileURL) + assert.Equal(t, *tc.expectedResult.Users[i].AvatarURL, org.AvatarURL) + } + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index ab0528174..e2fe588b4 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -57,6 +57,10 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, AddReadTools( toolsets.NewServerTool(SearchUsers(getClient, t)), ) + orgs := toolsets.NewToolset("orgs", "GitHub Organization related tools"). + AddReadTools( + toolsets.NewServerTool(SearchOrgs(getClient, t)), + ) pullRequests := toolsets.NewToolset("pull_requests", "GitHub Pull Request related tools"). AddReadTools( toolsets.NewServerTool(GetPullRequest(getClient, t)), @@ -111,6 +115,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, tsg.AddToolset(repos) tsg.AddToolset(issues) tsg.AddToolset(users) + tsg.AddToolset(orgs) tsg.AddToolset(pullRequests) tsg.AddToolset(codeSecurity) tsg.AddToolset(secretProtection)