From 558be5c7e64aa7a7699d65add71af8506a8e5f2d Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Thu, 11 Jun 2026 16:27:34 -0400 Subject: [PATCH] feat: reduce pull request search responses by default --- README.md | 1 + .../__toolsnaps__/search_pull_requests.snap | 5 + pkg/github/minimal_types.go | 84 ++++++++++++ pkg/github/pullrequests.go | 51 ++++++- pkg/github/pullrequests_test.go | 128 ++++++++++++++---- 5 files changed, 240 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index dc063f22ce..63cf6471d9 100644 --- a/README.md +++ b/README.md @@ -1147,6 +1147,7 @@ The following sets of tools are available: - **search_pull_requests** - Search pull requests - **Required OAuth Scopes**: `repo` + - `minimal_output`: Return minimal pull request search results (default: true). When false, returns the full GitHub API search payload. (boolean, optional) - `order`: Sort order (string, optional) - `owner`: Optional repository owner. If provided with repo, only pull requests for this repository are listed. (string, optional) - `page`: Page number for pagination (min 1) (number, optional) diff --git a/pkg/github/__toolsnaps__/search_pull_requests.snap b/pkg/github/__toolsnaps__/search_pull_requests.snap index 05376c0065..519af55d2e 100644 --- a/pkg/github/__toolsnaps__/search_pull_requests.snap +++ b/pkg/github/__toolsnaps__/search_pull_requests.snap @@ -6,6 +6,11 @@ "description": "Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr", "inputSchema": { "properties": { + "minimal_output": { + "default": true, + "description": "Return minimal pull request search results (default: true). When false, returns the full GitHub API search payload.", + "type": "boolean" + }, "order": { "description": "Sort order", "enum": [ diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index eff6edc133..7501874d4f 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -370,6 +370,38 @@ type MinimalSearchCommitsResult struct { Items []MinimalCommitSearchItem `json:"items"` } +// MinimalSearchPullRequestsResult is the trimmed output type for pull request search results. +type MinimalSearchPullRequestsResult struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []MinimalSearchPullRequestItem `json:"items"` +} + +// MinimalSearchPullRequestItem is the trimmed output type for a single pull request search hit. +type MinimalSearchPullRequestItem struct { + Number int `json:"number"` + Title string `json:"title"` + State string `json:"state"` + Draft bool `json:"draft,omitempty"` + User string `json:"user,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + Repository string `json:"repository,omitempty"` + RepositoryURL string `json:"repository_url,omitempty"` + Labels []string `json:"labels,omitempty"` + Comments int `json:"comments,omitempty"` + PullRequest *MinimalSearchPullRequestLinks `json:"pull_request,omitempty"` +} + +// MinimalSearchPullRequestLinks contains the PR-specific URLs already present on an issue search hit. +type MinimalSearchPullRequestLinks struct { + URL string `json:"url,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + DiffURL string `json:"diff_url,omitempty"` + PatchURL string `json:"patch_url,omitempty"` +} + // MinimalFileContentResponse is the trimmed output type for create/update/delete file responses. type MinimalFileContentResponse struct { Content *MinimalFileContent `json:"content,omitempty"` @@ -1565,6 +1597,58 @@ func convertCommitResultToMinimalCommit(commit *github.CommitResult) MinimalComm return item } +func convertToMinimalSearchPullRequestsResult(result *github.IssuesSearchResult) MinimalSearchPullRequestsResult { + minimal := MinimalSearchPullRequestsResult{} + if result == nil { + return minimal + } + + minimal.TotalCount = result.GetTotal() + minimal.IncompleteResults = result.GetIncompleteResults() + minimal.Items = make([]MinimalSearchPullRequestItem, 0, len(result.Issues)) + for _, issue := range result.Issues { + if issue == nil { + continue + } + minimal.Items = append(minimal.Items, convertToMinimalSearchPullRequestItem(issue)) + } + + return minimal +} + +func convertToMinimalSearchPullRequestItem(issue *github.Issue) MinimalSearchPullRequestItem { + minimal := MinimalSearchPullRequestItem{ + Number: issue.GetNumber(), + Title: issue.GetTitle(), + State: issue.GetState(), + Draft: issue.GetDraft(), + User: issue.GetUser().GetLogin(), + CreatedAt: formatProjectTimestamp(issue.CreatedAt), + UpdatedAt: formatProjectTimestamp(issue.UpdatedAt), + HTMLURL: issue.GetHTMLURL(), + Repository: issueRepositoryFullName(issue), + RepositoryURL: issue.GetRepositoryURL(), + Comments: issue.GetComments(), + } + + for _, label := range issue.Labels { + if label != nil { + minimal.Labels = append(minimal.Labels, label.GetName()) + } + } + + if links := issue.GetPullRequestLinks(); links != nil { + minimal.PullRequest = &MinimalSearchPullRequestLinks{ + URL: links.GetURL(), + HTMLURL: links.GetHTMLURL(), + DiffURL: links.GetDiffURL(), + PatchURL: links.GetPatchURL(), + } + } + + return minimal +} + // MinimalPageInfo contains pagination cursor information. type MinimalPageInfo struct { HasNextPage bool `json:"hasNextPage"` diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index ae7d04331d..44cf7a916e 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1475,6 +1475,11 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo Description: "Sort order", Enum: []any{"asc", "desc"}, }, + "minimal_output": { + Type: "boolean", + Description: "Return minimal pull request search results (default: true). When false, returns the full GitHub API search payload.", + Default: json.RawMessage(`true`), + }, }, Required: []string{"query"}, } @@ -1493,8 +1498,50 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo }, []scopes.Scope{scopes.Repo}, func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - result, err := searchHandler(ctx, deps.GetClient, args, "pr", "failed to search pull requests", ifcSearchPostProcessOption(ctx, deps)) - return result, nil, err + minimalOutput, err := OptionalBoolParamWithDefault(args, "minimal_output", true) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if !minimalOutput { + result, err := searchHandler(ctx, deps.GetClient, args, "pr", "failed to search pull requests", ifcSearchPostProcessOption(ctx, deps)) + return result, nil, err + } + + query, opts, err := prepareSearchArgs(args, "pr") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to search pull requests: failed to get GitHub client", err), nil, nil + } + result, resp, err := client.Search.Issues(ctx, query, opts) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to search pull requests", err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to search pull requests: failed to read response body", err), nil, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search pull requests", resp, body), nil, nil + } + + r, err := json.Marshal(convertToMinimalSearchPullRequestsResult(result)) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to search pull requests: failed to marshal response", err), nil, nil + } + + callResult := utils.NewToolResultText(string(r)) + cfg := searchConfig{} + ifcSearchPostProcessOption(ctx, deps)(&cfg) + if cfg.postProcess != nil { + cfg.postProcess(ctx, result, callResult) + } + return callResult, nil, nil }) } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 2b911636a9..93186f223b 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -829,23 +829,45 @@ func Test_SearchPullRequests(t *testing.T) { assert.Contains(t, schema.Properties, "repo") assert.Contains(t, schema.Properties, "sort") assert.Contains(t, schema.Properties, "order") + assert.Contains(t, schema.Properties, "minimal_output") assert.Contains(t, schema.Properties, "perPage") assert.Contains(t, schema.Properties, "page") assert.ElementsMatch(t, schema.Required, []string{"query"}) + createdAt := &github.Timestamp{Time: time.Date(2026, 6, 10, 12, 0, 0, 0, time.UTC)} + updatedAt := &github.Timestamp{Time: time.Date(2026, 6, 11, 15, 30, 0, 0, time.UTC)} mockSearchResult := &github.IssuesSearchResult{ Total: github.Ptr(2), IncompleteResults: github.Ptr(false), Issues: []*github.Issue{ { - Number: github.Ptr(42), - Title: github.Ptr("Test PR 1"), - Body: github.Ptr("Updated tests."), - State: github.Ptr("open"), - HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"), - Comments: github.Ptr(5), + Number: github.Ptr(42), + Title: github.Ptr("Test PR 1"), + Body: github.Ptr("Updated tests."), + State: github.Ptr("open"), + Draft: github.Ptr(true), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"), + RepositoryURL: github.Ptr("https://api.github.com/repos/owner/repo"), + Comments: github.Ptr(5), + CreatedAt: createdAt, + UpdatedAt: updatedAt, User: &github.User{ - Login: github.Ptr("user1"), + Login: github.Ptr("user1"), + URL: github.Ptr("https://api.github.com/users/user1"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/1?v=4"), + }, + Repository: &github.Repository{ + FullName: github.Ptr("owner/repo"), + }, + Labels: []*github.Label{ + {Name: github.Ptr("ci")}, + {Name: github.Ptr("backend")}, + }, + PullRequestLinks: &github.PullRequestLinks{ + URL: github.Ptr("https://api.github.com/repos/owner/repo/pulls/1"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"), + DiffURL: github.Ptr("https://github.com/owner/repo/pull/1.diff"), + PatchURL: github.Ptr("https://github.com/owner/repo/pull/1.patch"), }, }, { @@ -863,12 +885,14 @@ func Test_SearchPullRequests(t *testing.T) { } tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]any - expectError bool - expectedResult *github.IssuesSearchResult - expectedErrMsg string + name string + mockedClient *http.Client + requestArgs map[string]any + expectError bool + expectRaw bool + expectedResult *github.IssuesSearchResult + expectedErrMsg string + expectedJSONLack []string }{ { name: "successful pull request search with all parameters", @@ -972,8 +996,9 @@ func Test_SearchPullRequests(t *testing.T) { requestArgs: map[string]any{ "query": "is:pr repo:owner/repo is:open", }, - expectError: false, - expectedResult: mockSearchResult, + expectError: false, + expectedResult: mockSearchResult, + expectedJSONLack: []string{"avatar_url", "\"url\":\"https://api.github.com/users/user1\""}, }, { name: "query with existing is:pr filter - no duplication", @@ -1051,6 +1076,18 @@ func Test_SearchPullRequests(t *testing.T) { expectError: true, expectedErrMsg: "failed to search pull requests", }, + { + name: "pull request search with minimal_output false returns raw result", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetSearchIssues: mockResponse(t, http.StatusOK, mockSearchResult), + }), + requestArgs: map[string]any{ + "query": "is:pr repo:owner/repo is:open", + "minimal_output": false, + }, + expectRaw: true, + expectedResult: mockSearchResult, + }, } for _, tc := range tests { @@ -1084,20 +1121,57 @@ func Test_SearchPullRequests(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedResult github.IssuesSearchResult + if tc.expectRaw { + var returnedResult github.IssuesSearchResult + 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.Issues, len(tc.expectedResult.Issues)) + for i, issue := range returnedResult.Issues { + assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number) + assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title) + assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State) + assert.Equal(t, *tc.expectedResult.Issues[i].HTMLURL, *issue.HTMLURL) + assert.Equal(t, *tc.expectedResult.Issues[i].User.Login, *issue.User.Login) + } + assert.Contains(t, textContent.Text, "\"avatar_url\":\"https://avatars.githubusercontent.com/u/1?v=4\"") + return + } + + for _, missingJSON := range tc.expectedJSONLack { + assert.NotContains(t, textContent.Text, missingJSON) + } + + var returnedResult MinimalSearchPullRequestsResult 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.Issues, len(tc.expectedResult.Issues)) - for i, issue := range returnedResult.Issues { - assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number) - assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title) - assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State) - assert.Equal(t, *tc.expectedResult.Issues[i].HTMLURL, *issue.HTMLURL) - assert.Equal(t, *tc.expectedResult.Issues[i].User.Login, *issue.User.Login) - } + assert.Equal(t, tc.expectedResult.GetTotal(), returnedResult.TotalCount) + assert.Equal(t, tc.expectedResult.GetIncompleteResults(), returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Items, len(tc.expectedResult.Issues)) + + first := returnedResult.Items[0] + assert.Equal(t, 42, first.Number) + assert.Equal(t, "Test PR 1", first.Title) + assert.Equal(t, "open", first.State) + assert.True(t, first.Draft) + assert.Equal(t, "user1", first.User) + assert.Equal(t, createdAt.Format(time.RFC3339), first.CreatedAt) + assert.Equal(t, updatedAt.Format(time.RFC3339), first.UpdatedAt) + assert.Equal(t, "https://github.com/owner/repo/pull/1", first.HTMLURL) + assert.Equal(t, "owner/repo", first.Repository) + assert.Equal(t, "https://api.github.com/repos/owner/repo", first.RepositoryURL) + assert.Equal(t, []string{"ci", "backend"}, first.Labels) + assert.Equal(t, 5, first.Comments) + require.NotNil(t, first.PullRequest) + assert.Equal(t, "https://api.github.com/repos/owner/repo/pulls/1", first.PullRequest.URL) + assert.Equal(t, "https://github.com/owner/repo/pull/1", first.PullRequest.HTMLURL) + assert.Equal(t, "https://github.com/owner/repo/pull/1.diff", first.PullRequest.DiffURL) + assert.Equal(t, "https://github.com/owner/repo/pull/1.patch", first.PullRequest.PatchURL) + + second := returnedResult.Items[1] + assert.Equal(t, 43, second.Number) + assert.Equal(t, "user2", second.User) }) }