Content-Length: 15943 | pFad | http://github.com/github/github-mcp-server/pull/486.patch

thub.com From f31698175ae5b04f64528e3f853fda0968dfebab Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 5 Jun 2025 23:05:43 +0200 Subject: [PATCH] cleanup search_users response --- pkg/github/__toolsnaps__/search_users.snap | 8 +- pkg/github/search.go | 207 ++++++++++++--------- pkg/github/search_test.go | 134 ++++++++++++- pkg/github/tools.go | 5 + 4 files changed, 261 insertions(+), 93 deletions(-) diff --git a/pkg/github/__toolsnaps__/search_users.snap b/pkg/github/__toolsnaps__/search_users.snap index aad2970b6..5cf9796f2 100644 --- a/pkg/github/__toolsnaps__/search_users.snap +++ b/pkg/github/__toolsnaps__/search_users.snap @@ -3,7 +3,7 @@ "title": "Search users", "readOnlyHint": true }, - "description": "Search for GitHub users", + "description": "Search for GitHub users exclusively", "inputSchema": { "properties": { "order": { @@ -25,8 +25,8 @@ "minimum": 1, "type": "number" }, - "q": { - "description": "Search query using GitHub users search syntax", + "query": { + "description": "Search query using GitHub users search syntax scoped to type:user", "type": "string" }, "sort": { @@ -40,7 +40,7 @@ } }, "required": [ - "q" + "query" ], "type": "object" }, diff --git a/pkg/github/search.go b/pkg/github/search.go index 13d017129..5106b84d8 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -168,100 +168,139 @@ 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, "query") + 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 ghErrors.NewGitHubAPIErrorResponse(ctx, + fmt.Sprintf("failed to search %ss with query '%s'", accountType, query), + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + body, err := io.ReadAll(resp.Body) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - fmt.Sprintf("failed to search users with query '%s'", query), - resp, - err, - ), nil + 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("query", + 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("query", + 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 f206ebb44..bfd014993 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -328,12 +328,12 @@ func Test_SearchUsers(t *testing.T) { assert.Equal(t, "search_users", tool.Name) assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "q") + assert.Contains(t, tool.InputSchema.Properties, "query") 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"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results mockSearchResult := &github.UsersSearchResult{ @@ -381,7 +381,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "location:finland language:go", + "query": "location:finland language:go", "sort": "followers", "order": "desc", "page": float64(1), @@ -405,7 +405,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "location:finland language:go", + "query": "location:finland language:go", }, expectError: false, expectedResult: mockSearchResult, @@ -422,7 +422,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "invalid:query", + "query": "invalid:query", }, expectError: true, expectedErrMsg: "failed to search users", @@ -474,3 +474,127 @@ 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, "query") + 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{"query"}) + + // 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{}{ + "query": "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{}{ + "query": "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.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, 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 5b970698c..06088a36b 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -64,6 +64,10 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG 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)), @@ -143,6 +147,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG tsg.AddToolset(contextTools) tsg.AddToolset(repos) tsg.AddToolset(issues) + tsg.AddToolset(orgs) tsg.AddToolset(users) tsg.AddToolset(pullRequests) tsg.AddToolset(actions)

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!