From ba785d2ad85ed9fba26b17dacfd3bd521fad9570 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 27 Aug 2025 15:41:51 +0100 Subject: [PATCH 01/21] add comprehensive minimal response where appropriate --- README.md | 3 + pkg/github/__toolsnaps__/get_commit.snap | 4 + pkg/github/__toolsnaps__/list_commits.snap | 4 + .../__toolsnaps__/search_repositories.snap | 5 + pkg/github/gists.go | 17 +- pkg/github/gists_test.go | 37 +---- pkg/github/issues.go | 19 ++- pkg/github/issues_test.go | 78 ++------- pkg/github/minimal_types.go | 156 ++++++++++++++++++ pkg/github/pullrequests.go | 19 ++- pkg/github/pullrequests_test.go | 70 ++------ pkg/github/repositories.go | 143 +++++++++++++++- pkg/github/repositories_test.go | 94 +++++++++-- pkg/github/search.go | 79 +++++++-- pkg/github/search_test.go | 81 ++++++++- 15 files changed, 607 insertions(+), 202 deletions(-) create mode 100644 pkg/github/minimal_types.go diff --git a/README.md b/README.md index a6e740e66..e77cd1605 100644 --- a/README.md +++ b/README.md @@ -830,6 +830,7 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - **get_commit** - Get commit details + - `include_diff`: Whether to include file diffs and stats in the response. Default is true for single commit queries. (boolean, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -865,6 +866,7 @@ The following sets of tools are available (all are on by default): - **list_commits** - List commits - `author`: Author username or email address to filter commits by (string, optional) + - `include_diffs`: Whether to include file diffs and stats in the response. Default is false for faster responses. (boolean, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) @@ -898,6 +900,7 @@ The following sets of tools are available (all are on by default): - `sort`: Sort field ('indexed' only) (string, optional) - **search_repositories** - Search repositories + - `minimal_output`: Return minimal repository information (default: true). When false, returns full GitHub API repository objects. (boolean, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `query`: Repository search query. Examples: 'machine learning in:name stars:>1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering. (string, required) diff --git a/pkg/github/__toolsnaps__/get_commit.snap b/pkg/github/__toolsnaps__/get_commit.snap index af0038110..a3791a9b8 100644 --- a/pkg/github/__toolsnaps__/get_commit.snap +++ b/pkg/github/__toolsnaps__/get_commit.snap @@ -6,6 +6,10 @@ "description": "Get details for a commit from a GitHub repository", "inputSchema": { "properties": { + "include_diff": { + "description": "Whether to include file diffs and stats in the response. Default is true for single commit queries.", + "type": "boolean" + }, "owner": { "description": "Repository owner", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_commits.snap b/pkg/github/__toolsnaps__/list_commits.snap index a802436c2..271eed5b5 100644 --- a/pkg/github/__toolsnaps__/list_commits.snap +++ b/pkg/github/__toolsnaps__/list_commits.snap @@ -10,6 +10,10 @@ "description": "Author username or email address to filter commits by", "type": "string" }, + "include_diffs": { + "description": "Whether to include file diffs and stats in the response. Default is false for faster responses.", + "type": "boolean" + }, "owner": { "description": "Repository owner", "type": "string" diff --git a/pkg/github/__toolsnaps__/search_repositories.snap b/pkg/github/__toolsnaps__/search_repositories.snap index d283a2cc0..f350c8e2b 100644 --- a/pkg/github/__toolsnaps__/search_repositories.snap +++ b/pkg/github/__toolsnaps__/search_repositories.snap @@ -6,6 +6,11 @@ "description": "Find GitHub repositories by name, description, readme, topics, or other metadata. Perfect for discovering projects, finding examples, or locating specific repositories across GitHub.", "inputSchema": { "properties": { + "minimal_output": { + "default": true, + "description": "Return minimal repository information (default: true). When false, returns full GitHub API repository objects.", + "type": "boolean" + }, "page": { "description": "Page number for pagination (min 1)", "minimum": 1, diff --git a/pkg/github/gists.go b/pkg/github/gists.go index fce34f6a8..c33d5b8da 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -165,7 +165,14 @@ func CreateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to create gist: %s", string(body))), nil } - r, err := json.Marshal(createdGist) + minimalResponse := MinimalGistResponse{ + URL: createdGist.GetHTMLURL(), + ID: createdGist.GetID(), + Description: createdGist.GetDescription(), + Public: createdGist.GetPublic(), + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -249,7 +256,13 @@ func UpdateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to update gist: %s", string(body))), nil } - r, err := json.Marshal(updatedGist) + minimalResponse := MinimalUpdateResponse{ + URL: updatedGist.GetHTMLURL(), + Updated: true, + Message: "Gist updated successfully", + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/gists_test.go b/pkg/github/gists_test.go index 49d63a252..acf54263f 100644 --- a/pkg/github/gists_test.go +++ b/pkg/github/gists_test.go @@ -321,23 +321,13 @@ func Test_CreateGist(t *testing.T) { // Parse the result and get the text content textContent := getTextResult(t, result) - // Unmarshal and verify the result - var gist *github.Gist + // Unmarshal and verify the minimal result + var gist MinimalGistResponse err = json.Unmarshal([]byte(textContent.Text), &gist) require.NoError(t, err) - assert.Equal(t, *tc.expectedGist.ID, *gist.ID) - assert.Equal(t, *tc.expectedGist.Description, *gist.Description) - assert.Equal(t, *tc.expectedGist.HTMLURL, *gist.HTMLURL) - assert.Equal(t, *tc.expectedGist.Public, *gist.Public) - - // Verify file content - for filename, expectedFile := range tc.expectedGist.Files { - actualFile, exists := gist.Files[filename] - assert.True(t, exists) - assert.Equal(t, *expectedFile.Filename, *actualFile.Filename) - assert.Equal(t, *expectedFile.Content, *actualFile.Content) - } + assert.Equal(t, tc.expectedGist.GetID(), gist.ID) + assert.Equal(t, tc.expectedGist.GetHTMLURL(), gist.URL) }) } } @@ -486,22 +476,13 @@ func Test_UpdateGist(t *testing.T) { // Parse the result and get the text content textContent := getTextResult(t, result) - // Unmarshal and verify the result - var gist *github.Gist - err = json.Unmarshal([]byte(textContent.Text), &gist) + // Unmarshal and verify the minimal result + var updateResp MinimalUpdateResponse + err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) - assert.Equal(t, *tc.expectedGist.ID, *gist.ID) - assert.Equal(t, *tc.expectedGist.Description, *gist.Description) - assert.Equal(t, *tc.expectedGist.HTMLURL, *gist.HTMLURL) - - // Verify file content - for filename, expectedFile := range tc.expectedGist.Files { - actualFile, exists := gist.Files[filename] - assert.True(t, exists) - assert.Equal(t, *expectedFile.Filename, *actualFile.Filename) - assert.Equal(t, *expectedFile.Content, *actualFile.Content) - } + assert.Equal(t, tc.expectedGist.GetHTMLURL(), updateResp.URL) + assert.Equal(t, true, updateResp.Updated) }) } } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 89375ae90..f2f3822ff 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -872,7 +872,15 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(fmt.Sprintf("failed to create issue: %s", string(body))), nil } - r, err := json.Marshal(issue) + // Return minimal response with just essential information + minimalResponse := MinimalIssueResponse{ + URL: issue.GetHTMLURL(), + Number: issue.GetNumber(), + State: issue.GetState(), + Title: issue.GetTitle(), + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -1242,7 +1250,14 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil } - r, err := json.Marshal(updatedIssue) + // Return minimal response with just essential information + minimalResponse := MinimalUpdateResponse{ + URL: updatedIssue.GetHTMLURL(), + Updated: true, + Message: "Issue updated successfully", + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7c4983c64..b13ea2ef6 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -712,39 +712,15 @@ func Test_CreateIssue(t *testing.T) { require.NoError(t, err) textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedIssue github.Issue + // Unmarshal and verify the minimal result + var returnedIssue MinimalIssueResponse err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) - assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) - assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) - assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) - - if tc.expectedIssue.Body != nil { - assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) - } - - if tc.expectedIssue.Type != nil { - assert.Equal(t, *tc.expectedIssue.Type.Name, *returnedIssue.Type.Name) - } - - // Check assignees if expected - if len(tc.expectedIssue.Assignees) > 0 { - assert.Equal(t, len(tc.expectedIssue.Assignees), len(returnedIssue.Assignees)) - for i, assignee := range returnedIssue.Assignees { - assert.Equal(t, *tc.expectedIssue.Assignees[i].Login, *assignee.Login) - } - } - - // Check labels if expected - if len(tc.expectedIssue.Labels) > 0 { - assert.Equal(t, len(tc.expectedIssue.Labels), len(returnedIssue.Labels)) - for i, label := range returnedIssue.Labels { - assert.Equal(t, *tc.expectedIssue.Labels[i].Name, *label.Name) - } - } + assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.Number) + assert.Equal(t, tc.expectedIssue.GetTitle(), returnedIssue.Title) + assert.Equal(t, tc.expectedIssue.GetState(), returnedIssue.State) + assert.Equal(t, tc.expectedIssue.GetHTMLURL(), returnedIssue.URL) }) } } @@ -1233,45 +1209,13 @@ func Test_UpdateIssue(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedIssue github.Issue - err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + // Unmarshal and verify the minimal result + var updateResp MinimalUpdateResponse + err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) - assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) - assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) - assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) - - if tc.expectedIssue.Body != nil { - assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) - } - - if tc.expectedIssue.Type != nil { - assert.Equal(t, *tc.expectedIssue.Type.Name, *returnedIssue.Type.Name) - } - - // Check assignees if expected - if len(tc.expectedIssue.Assignees) > 0 { - assert.Len(t, returnedIssue.Assignees, len(tc.expectedIssue.Assignees)) - for i, assignee := range returnedIssue.Assignees { - assert.Equal(t, *tc.expectedIssue.Assignees[i].Login, *assignee.Login) - } - } - - // Check labels if expected - if len(tc.expectedIssue.Labels) > 0 { - assert.Len(t, returnedIssue.Labels, len(tc.expectedIssue.Labels)) - for i, label := range returnedIssue.Labels { - assert.Equal(t, *tc.expectedIssue.Labels[i].Name, *label.Name) - } - } - - // Check milestone if expected - if tc.expectedIssue.Milestone != nil { - assert.NotNil(t, returnedIssue.Milestone) - assert.Equal(t, *tc.expectedIssue.Milestone.Number, *returnedIssue.Milestone.Number) - } + assert.Equal(t, tc.expectedIssue.GetHTMLURL(), updateResp.URL) + assert.Equal(t, true, updateResp.Updated) }) } } diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go new file mode 100644 index 000000000..d83e1f608 --- /dev/null +++ b/pkg/github/minimal_types.go @@ -0,0 +1,156 @@ +package github + +// MinimalUser is the output type for user and organization search results. +type MinimalUser struct { + Login string `json:"login"` + ID int64 `json:"id,omitempty"` + ProfileURL string `json:"profile_url,omitempty"` + AvatarURL string `json:"avatar_url,omitempty"` + Details *UserDetails `json:"details,omitempty"` // Optional field for additional user details +} + +// MinimalSearchUsersResult is the trimmed output type for user search results. +type MinimalSearchUsersResult struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []MinimalUser `json:"items"` +} + +// MinimalRepository is the trimmed output type for repository objects to reduce verbosity. +type MinimalRepository struct { + ID int64 `json:"id"` + Name string `json:"name"` + FullName string `json:"full_name"` + Description string `json:"description,omitempty"` + HTMLURL string `json:"html_url"` + CloneURL string `json:"clone_url,omitempty"` + Language string `json:"language,omitempty"` + Stars int `json:"stargazers_count"` + Forks int `json:"forks_count"` + OpenIssues int `json:"open_issues_count"` + UpdatedAt string `json:"updated_at,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + Topics []string `json:"topics,omitempty"` + Private bool `json:"private"` + Fork bool `json:"fork"` + Archived bool `json:"archived"` + DefaultBranch string `json:"default_branch,omitempty"` +} + +// MinimalSearchRepositoriesResult is the trimmed output type for repository search results. +type MinimalSearchRepositoriesResult struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []MinimalRepository `json:"items"` +} + +// MinimalCommitAuthor represents commit author information. +type MinimalCommitAuthor struct { + Name string `json:"name,omitempty"` + Email string `json:"email,omitempty"` + Date string `json:"date,omitempty"` +} + +// MinimalCommitInfo represents core commit information. +type MinimalCommitInfo struct { + Message string `json:"message"` + Author *MinimalCommitAuthor `json:"author,omitempty"` + Committer *MinimalCommitAuthor `json:"committer,omitempty"` +} + +// MinimalCommitStats represents commit statistics. +type MinimalCommitStats struct { + Additions int `json:"additions,omitempty"` + Deletions int `json:"deletions,omitempty"` + Total int `json:"total,omitempty"` +} + +// MinimalCommitFile represents a file changed in a commit. +type MinimalCommitFile struct { + Filename string `json:"filename"` + Status string `json:"status,omitempty"` + Additions int `json:"additions,omitempty"` + Deletions int `json:"deletions,omitempty"` + Changes int `json:"changes,omitempty"` +} + +// MinimalCommit is the trimmed output type for commit objects. +type MinimalCommit struct { + SHA string `json:"sha"` + HTMLURL string `json:"html_url"` + Commit *MinimalCommitInfo `json:"commit,omitempty"` + Author *MinimalUser `json:"author,omitempty"` + Committer *MinimalUser `json:"committer,omitempty"` + Stats *MinimalCommitStats `json:"stats,omitempty"` + Files []MinimalCommitFile `json:"files,omitempty"` +} + +// MinimalRelease is the trimmed output type for release objects. +type MinimalRelease struct { + ID int64 `json:"id"` + TagName string `json:"tag_name"` + Name string `json:"name,omitempty"` + Body string `json:"body,omitempty"` + HTMLURL string `json:"html_url"` + PublishedAt string `json:"published_at,omitempty"` + Prerelease bool `json:"prerelease"` + Draft bool `json:"draft"` + Author *MinimalUser `json:"author,omitempty"` +} + +// MinimalBranch is the trimmed output type for branch objects. +type MinimalBranch struct { + Name string `json:"name"` + SHA string `json:"sha"` + Protected bool `json:"protected"` +} + +// Minimal response types for create/edit operations + +// MinimalCreateResponse represents a minimal response for resource creation operations. +type MinimalCreateResponse struct { + URL string `json:"url"` + ID int64 `json:"id,omitempty"` + Number int `json:"number,omitempty"` + Name string `json:"name,omitempty"` + State string `json:"state,omitempty"` +} + +// MinimalPullRequestResponse represents a minimal response for pull request operations. +type MinimalPullRequestResponse struct { + URL string `json:"url"` + Number int `json:"number"` + State string `json:"state"` + Title string `json:"title,omitempty"` +} + +// MinimalRepositoryResponse represents a minimal response for repository operations. +type MinimalRepositoryResponse struct { + URL string `json:"url"` + CloneURL string `json:"clone_url"` + Name string `json:"name"` + FullName string `json:"full_name"` +} + +// MinimalIssueResponse represents a minimal response for issue operations. +type MinimalIssueResponse struct { + URL string `json:"url"` + Number int `json:"number"` + State string `json:"state"` + Title string `json:"title,omitempty"` +} + +// MinimalUpdateResponse represents a minimal response for update operations. +type MinimalUpdateResponse struct { + URL string `json:"url"` + Updated bool `json:"updated"` + Message string `json:"message,omitempty"` +} + +// MinimalGistResponse represents a minimal response for gist operations. +type MinimalGistResponse struct { + URL string `json:"url"` + ID string `json:"id"` + Description string `json:"description,omitempty"` + Public bool `json:"public"` +} diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 63c5594d3..89676815a 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -193,7 +193,15 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultError(fmt.Sprintf("failed to create pull request: %s", string(body))), nil } - r, err := json.Marshal(pr) + // Return minimal response with just essential information + minimalResponse := MinimalPullRequestResponse{ + URL: pr.GetHTMLURL(), + Number: pr.GetNumber(), + State: pr.GetState(), + Title: pr.GetTitle(), + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -464,7 +472,14 @@ func UpdatePullRequest(getClient GetClientFn, getGQLClient GetGQLClientFn, t tra } }() - r, err := json.Marshal(finalPR) + // Return minimal response with just essential information + minimalResponse := MinimalUpdateResponse{ + URL: finalPR.GetHTMLURL(), + Updated: true, + Message: "Pull request updated successfully", + } + + r, err := json.Marshal(minimalResponse) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("Failed to marshal response: %v", err)), nil } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index ed6921477..f97fda300 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -381,47 +381,12 @@ func Test_UpdatePullRequest(t *testing.T) { // Parse the result and get the text content textContent := getTextResult(t, result) - // Unmarshal and verify the successful result - var returnedPR github.PullRequest - err = json.Unmarshal([]byte(textContent.Text), &returnedPR) + // Unmarshal and verify the minimal result + var updateResp MinimalUpdateResponse + err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) - assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) - if tc.expectedPR.Title != nil { - assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) - } - if tc.expectedPR.Body != nil { - assert.Equal(t, *tc.expectedPR.Body, *returnedPR.Body) - } - if tc.expectedPR.State != nil { - assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) - } - if tc.expectedPR.Base != nil && tc.expectedPR.Base.Ref != nil { - assert.NotNil(t, returnedPR.Base) - assert.Equal(t, *tc.expectedPR.Base.Ref, *returnedPR.Base.Ref) - } - if tc.expectedPR.MaintainerCanModify != nil { - assert.Equal(t, *tc.expectedPR.MaintainerCanModify, *returnedPR.MaintainerCanModify) - } - - // Check reviewers if they exist in the expected PR - if len(tc.expectedPR.RequestedReviewers) > 0 { - assert.NotNil(t, returnedPR.RequestedReviewers) - assert.Equal(t, len(tc.expectedPR.RequestedReviewers), len(returnedPR.RequestedReviewers)) - - // Create maps of reviewer logins for easy comparison - expectedReviewers := make(map[string]bool) - for _, reviewer := range tc.expectedPR.RequestedReviewers { - expectedReviewers[*reviewer.Login] = true - } - - actualReviewers := make(map[string]bool) - for _, reviewer := range returnedPR.RequestedReviewers { - actualReviewers[*reviewer.Login] = true - } - - // Compare the maps - assert.Equal(t, expectedReviewers, actualReviewers) - } + assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL) + assert.Equal(t, true, updateResp.Updated) }) } } @@ -599,11 +564,12 @@ func Test_UpdatePullRequest_Draft(t *testing.T) { textContent := getTextResult(t, result) - // Unmarshal and verify the successful result - var returnedPR github.PullRequest - err = json.Unmarshal([]byte(textContent.Text), &returnedPR) + // Unmarshal and verify the minimal result + var updateResp MinimalUpdateResponse + err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) - assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) + assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL) + assert.Equal(t, true, updateResp.Updated) }) } } @@ -1988,18 +1954,14 @@ func Test_CreatePullRequest(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedPR github.PullRequest + // Unmarshal and verify the minimal result + var returnedPR MinimalPullRequestResponse err = json.Unmarshal([]byte(textContent.Text), &returnedPR) require.NoError(t, err) - assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) - assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) - assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) - assert.Equal(t, *tc.expectedPR.HTMLURL, *returnedPR.HTMLURL) - assert.Equal(t, *tc.expectedPR.Head.SHA, *returnedPR.Head.SHA) - assert.Equal(t, *tc.expectedPR.Base.Ref, *returnedPR.Base.Ref) - assert.Equal(t, *tc.expectedPR.Body, *returnedPR.Body) - assert.Equal(t, *tc.expectedPR.User.Login, *returnedPR.User.Login) + assert.Equal(t, tc.expectedPR.GetNumber(), returnedPR.Number) + assert.Equal(t, tc.expectedPR.GetTitle(), returnedPR.Title) + assert.Equal(t, tc.expectedPR.GetState(), returnedPR.State) + assert.Equal(t, tc.expectedPR.GetHTMLURL(), returnedPR.URL) }) } } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index de2c6d01f..04ab1e92b 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -18,6 +18,94 @@ import ( "github.com/mark3labs/mcp-go/server" ) +// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit +func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit { + minimalCommit := MinimalCommit{ + SHA: commit.GetSHA(), + HTMLURL: commit.GetHTMLURL(), + } + + if commit.Commit != nil { + minimalCommit.Commit = &MinimalCommitInfo{ + Message: commit.Commit.GetMessage(), + } + + if commit.Commit.Author != nil { + minimalCommit.Commit.Author = &MinimalCommitAuthor{ + Name: commit.Commit.Author.GetName(), + Email: commit.Commit.Author.GetEmail(), + } + if commit.Commit.Author.Date != nil { + minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format("2006-01-02T15:04:05Z") + } + } + + if commit.Commit.Committer != nil { + minimalCommit.Commit.Committer = &MinimalCommitAuthor{ + Name: commit.Commit.Committer.GetName(), + Email: commit.Commit.Committer.GetEmail(), + } + if commit.Commit.Committer.Date != nil { + minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format("2006-01-02T15:04:05Z") + } + } + } + + if commit.Author != nil { + minimalCommit.Author = &MinimalUser{ + Login: commit.Author.GetLogin(), + ID: commit.Author.GetID(), + ProfileURL: commit.Author.GetHTMLURL(), + AvatarURL: commit.Author.GetAvatarURL(), + } + } + + if commit.Committer != nil { + minimalCommit.Committer = &MinimalUser{ + Login: commit.Committer.GetLogin(), + ID: commit.Committer.GetID(), + ProfileURL: commit.Committer.GetHTMLURL(), + AvatarURL: commit.Committer.GetAvatarURL(), + } + } + + // Only include stats and files if includeDiffs is true + if includeDiffs { + if commit.Stats != nil { + minimalCommit.Stats = &MinimalCommitStats{ + Additions: commit.Stats.GetAdditions(), + Deletions: commit.Stats.GetDeletions(), + Total: commit.Stats.GetTotal(), + } + } + + if len(commit.Files) > 0 { + minimalCommit.Files = make([]MinimalCommitFile, 0, len(commit.Files)) + for _, file := range commit.Files { + minimalFile := MinimalCommitFile{ + Filename: file.GetFilename(), + Status: file.GetStatus(), + Additions: file.GetAdditions(), + Deletions: file.GetDeletions(), + Changes: file.GetChanges(), + } + minimalCommit.Files = append(minimalCommit.Files, minimalFile) + } + } + } + + return minimalCommit +} + +// convertToMinimalBranch converts a GitHub API Branch to MinimalBranch +func convertToMinimalBranch(branch *github.Branch) MinimalBranch { + return MinimalBranch{ + Name: branch.GetName(), + SHA: branch.GetCommit().GetSHA(), + Protected: branch.GetProtected(), + } +} + func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_commit", mcp.WithDescription(t("TOOL_GET_COMMITS_DESCRIPTION", "Get details for a commit from a GitHub repository")), @@ -37,6 +125,9 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too mcp.Required(), mcp.Description("Commit SHA, branch name, or tag name"), ), + mcp.WithBoolean("include_diff", + mcp.Description("Whether to include file diffs and stats in the response. Default is true for single commit queries."), + ), WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -52,6 +143,10 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too if err != nil { return mcp.NewToolResultError(err.Error()), nil } + includeDiff, err := OptionalParam[bool](request, "include_diff") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -84,7 +179,10 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil } - r, err := json.Marshal(commit) + // Convert to minimal commit + minimalCommit := convertToMinimalCommit(commit, includeDiff) + + r, err := json.Marshal(minimalCommit) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -115,6 +213,9 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t mcp.WithString("author", mcp.Description("Author username or email address to filter commits by"), ), + mcp.WithBoolean("include_diffs", + mcp.Description("Whether to include file diffs and stats in the response. Default is false for faster responses."), + ), WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -134,6 +235,10 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } + includeDiffs, err := OptionalParam[bool](request, "include_diffs") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -174,7 +279,13 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(fmt.Sprintf("failed to list commits: %s", string(body))), nil } - r, err := json.Marshal(commits) + // Convert to minimal commits + minimalCommits := make([]MinimalCommit, 0, len(commits)) + for _, commit := range commits { + minimalCommits = append(minimalCommits, convertToMinimalCommit(commit, includeDiffs)) + } + + r, err := json.Marshal(minimalCommits) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -245,7 +356,13 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( return mcp.NewToolResultError(fmt.Sprintf("failed to list branches: %s", string(body))), nil } - r, err := json.Marshal(branches) + // Convert to minimal branches + minimalBranches := make([]MinimalBranch, 0, len(branches)) + for _, branch := range branches { + minimalBranches = append(minimalBranches, convertToMinimalBranch(branch)) + } + + r, err := json.Marshal(minimalBranches) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -436,7 +553,15 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(fmt.Sprintf("failed to create repository: %s", string(body))), nil } - r, err := json.Marshal(createdRepo) + // Return minimal response with just essential information + minimalResponse := MinimalRepositoryResponse{ + URL: createdRepo.GetHTMLURL(), + CloneURL: createdRepo.GetCloneURL(), + Name: createdRepo.GetName(), + FullName: createdRepo.GetFullName(), + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -707,7 +832,15 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) return mcp.NewToolResultError(fmt.Sprintf("failed to fork repository: %s", string(body))), nil } - r, err := json.Marshal(forkedRepo) + // Return minimal response with just essential information + minimalResponse := MinimalRepositoryResponse{ + URL: forkedRepo.GetHTMLURL(), + CloneURL: forkedRepo.GetCloneURL(), + Name: forkedRepo.GetName(), + FullName: forkedRepo.GetFullName(), + } + + r, err := json.Marshal(minimalResponse) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index f5ebfd32b..7ea343f87 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -720,6 +720,7 @@ func Test_ListCommits(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sha") assert.Contains(t, tool.InputSchema.Properties, "author") + assert.Contains(t, tool.InputSchema.Properties, "include_diffs") assert.Contains(t, tool.InputSchema.Properties, "page") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) @@ -737,9 +738,33 @@ func Test_ListCommits(t *testing.T) { }, }, Author: &github.User{ - Login: github.Ptr("testuser"), + Login: github.Ptr("testuser"), + ID: github.Ptr(int64(12345)), + HTMLURL: github.Ptr("https://github.com/testuser"), + AvatarURL: github.Ptr("https://github.com/testuser.png"), }, HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"), + Stats: &github.CommitStats{ + Additions: github.Ptr(10), + Deletions: github.Ptr(5), + Total: github.Ptr(15), + }, + Files: []*github.CommitFile{ + { + Filename: github.Ptr("src/main.go"), + Status: github.Ptr("modified"), + Additions: github.Ptr(8), + Deletions: github.Ptr(3), + Changes: github.Ptr(11), + }, + { + Filename: github.Ptr("README.md"), + Status: github.Ptr("added"), + Additions: github.Ptr(2), + Deletions: github.Ptr(2), + Changes: github.Ptr(4), + }, + }, }, { SHA: github.Ptr("def456abc789"), @@ -752,9 +777,26 @@ func Test_ListCommits(t *testing.T) { }, }, Author: &github.User{ - Login: github.Ptr("anotheruser"), + Login: github.Ptr("anotheruser"), + ID: github.Ptr(int64(67890)), + HTMLURL: github.Ptr("https://github.com/anotheruser"), + AvatarURL: github.Ptr("https://github.com/anotheruser.png"), }, HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456abc789"), + Stats: &github.CommitStats{ + Additions: github.Ptr(20), + Deletions: github.Ptr(10), + Total: github.Ptr(30), + }, + Files: []*github.CommitFile{ + { + Filename: github.Ptr("src/utils.go"), + Status: github.Ptr("added"), + Additions: github.Ptr(20), + Deletions: github.Ptr(10), + Changes: github.Ptr(30), + }, + }, }, } @@ -875,16 +917,39 @@ func Test_ListCommits(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result - var returnedCommits []*github.RepositoryCommit + var returnedCommits []MinimalCommit err = json.Unmarshal([]byte(textContent.Text), &returnedCommits) require.NoError(t, err) assert.Len(t, returnedCommits, len(tc.expectedCommits)) for i, commit := range returnedCommits { - assert.Equal(t, *tc.expectedCommits[i].Author, *commit.Author) - assert.Equal(t, *tc.expectedCommits[i].SHA, *commit.SHA) - assert.Equal(t, *tc.expectedCommits[i].Commit.Message, *commit.Commit.Message) - assert.Equal(t, *tc.expectedCommits[i].Author.Login, *commit.Author.Login) - assert.Equal(t, *tc.expectedCommits[i].HTMLURL, *commit.HTMLURL) + assert.Equal(t, tc.expectedCommits[i].GetSHA(), commit.SHA) + assert.Equal(t, tc.expectedCommits[i].GetHTMLURL(), commit.HTMLURL) + if tc.expectedCommits[i].Commit != nil { + assert.Equal(t, tc.expectedCommits[i].Commit.GetMessage(), commit.Commit.Message) + } + if tc.expectedCommits[i].Author != nil { + assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit.Author.Login) + } + + // Check if diffs are included based on include_diffs parameter + includeDiffs, exists := tc.requestArgs["include_diffs"] + if exists && includeDiffs == true { + // When include_diffs=true, files and stats should be present + if len(tc.expectedCommits[i].Files) > 0 { + assert.NotNil(t, commit.Files) + assert.Len(t, commit.Files, len(tc.expectedCommits[i].Files)) + } + if tc.expectedCommits[i].Stats != nil { + assert.NotNil(t, commit.Stats) + assert.Equal(t, tc.expectedCommits[i].Stats.GetAdditions(), commit.Stats.Additions) + assert.Equal(t, tc.expectedCommits[i].Stats.GetDeletions(), commit.Stats.Deletions) + assert.Equal(t, tc.expectedCommits[i].Stats.GetTotal(), commit.Stats.Total) + } + } else { + // When include_diffs=false or not specified (default is false for performance), files and stats should not be present + assert.Nil(t, commit.Files) + assert.Nil(t, commit.Stats) + } } }) } @@ -1192,17 +1257,16 @@ func Test_CreateRepository(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedRepo github.Repository + // Unmarshal and verify the minimal result + var returnedRepo MinimalRepositoryResponse err = json.Unmarshal([]byte(textContent.Text), &returnedRepo) assert.NoError(t, err) // Verify repository details - assert.Equal(t, *tc.expectedRepo.Name, *returnedRepo.Name) - assert.Equal(t, *tc.expectedRepo.Description, *returnedRepo.Description) - assert.Equal(t, *tc.expectedRepo.Private, *returnedRepo.Private) - assert.Equal(t, *tc.expectedRepo.HTMLURL, *returnedRepo.HTMLURL) - assert.Equal(t, *tc.expectedRepo.Owner.Login, *returnedRepo.Owner.Login) + assert.Equal(t, tc.expectedRepo.GetName(), returnedRepo.Name) + assert.Equal(t, tc.expectedRepo.GetHTMLURL(), returnedRepo.URL) + assert.Equal(t, tc.expectedRepo.GetCloneURL(), returnedRepo.CloneURL) + assert.Equal(t, tc.expectedRepo.GetFullName(), returnedRepo.FullName) }) } } diff --git a/pkg/github/search.go b/pkg/github/search.go index 248f17e17..e0634af6f 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -26,6 +26,10 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF mcp.Required(), mcp.Description("Repository search query. Examples: 'machine learning in:name stars:>1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering."), ), + mcp.WithBoolean("minimal_output", + mcp.Description("Return minimal repository information (default: true). When false, returns full GitHub API repository objects."), + mcp.DefaultBool(true), + ), WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -37,6 +41,10 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF if err != nil { return mcp.NewToolResultError(err.Error()), nil } + minimalOutput, err := OptionalParam[bool](request, "minimal_output") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } opts := &github.SearchOptions{ ListOptions: github.ListOptions{ @@ -67,9 +75,59 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF return mcp.NewToolResultError(fmt.Sprintf("failed to search repositories: %s", string(body))), nil } - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Return either minimal or full response based on parameter + var r []byte + if minimalOutput { + minimalRepos := make([]MinimalRepository, 0, len(result.Repositories)) + for _, repo := range result.Repositories { + minimalRepo := MinimalRepository{ + ID: repo.GetID(), + Name: repo.GetName(), + FullName: repo.GetFullName(), + Description: repo.GetDescription(), + HTMLURL: repo.GetHTMLURL(), + CloneURL: repo.GetCloneURL(), + Language: repo.GetLanguage(), + Stars: repo.GetStargazersCount(), + Forks: repo.GetForksCount(), + OpenIssues: repo.GetOpenIssuesCount(), + Private: repo.GetPrivate(), + Fork: repo.GetFork(), + Archived: repo.GetArchived(), + DefaultBranch: repo.GetDefaultBranch(), + } + + if repo.UpdatedAt != nil { + minimalRepo.UpdatedAt = repo.UpdatedAt.Format("2006-01-02T15:04:05Z") + } + if repo.CreatedAt != nil { + minimalRepo.CreatedAt = repo.CreatedAt.Format("2006-01-02T15:04:05Z") + } + if repo.Topics != nil { + minimalRepo.Topics = repo.Topics + } + + minimalRepos = append(minimalRepos, minimalRepo) + } + + minimalResult := &MinimalSearchRepositoriesResult{ + TotalCount: result.GetTotal(), + IncompleteResults: result.GetIncompleteResults(), + Items: minimalRepos, + } + + var err error + r, err = json.Marshal(minimalResult) + if err != nil { + return nil, fmt.Errorf("failed to marshal minimal response: %w", err) + } + } else { + // Return full GitHub API response + var err error + r, err = json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal full response: %w", err) + } } return mcp.NewToolResultText(string(r)), nil @@ -156,21 +214,6 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to } } -// MinimalUser is the output type for user and organization search results. -type MinimalUser struct { - Login string `json:"login"` - ID int64 `json:"id,omitempty"` - ProfileURL string `json:"profile_url,omitempty"` - AvatarURL string `json:"avatar_url,omitempty"` - Details *UserDetails `json:"details,omitempty"` // Optional field for additional user details -} - -type MinimalSearchUsersResult struct { - TotalCount int `json:"total_count"` - IncompleteResults bool `json:"incomplete_results"` - Items []MinimalUser `json:"items"` -} - 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") diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index cfc87c02b..337dbf8e5 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -148,23 +148,86 @@ func Test_SearchRepositories(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result - var returnedResult github.RepositoriesSearchResult + var returnedResult MinimalSearchRepositoriesResult 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.Repositories, len(tc.expectedResult.Repositories)) - for i, repo := range returnedResult.Repositories { - assert.Equal(t, *tc.expectedResult.Repositories[i].ID, *repo.ID) - assert.Equal(t, *tc.expectedResult.Repositories[i].Name, *repo.Name) - assert.Equal(t, *tc.expectedResult.Repositories[i].FullName, *repo.FullName) - assert.Equal(t, *tc.expectedResult.Repositories[i].HTMLURL, *repo.HTMLURL) + assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) + assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Items, len(tc.expectedResult.Repositories)) + for i, repo := range returnedResult.Items { + assert.Equal(t, *tc.expectedResult.Repositories[i].ID, repo.ID) + assert.Equal(t, *tc.expectedResult.Repositories[i].Name, repo.Name) + assert.Equal(t, *tc.expectedResult.Repositories[i].FullName, repo.FullName) + assert.Equal(t, *tc.expectedResult.Repositories[i].HTMLURL, repo.HTMLURL) } }) } } +func Test_SearchRepositories_FullOutput(t *testing.T) { + // Test the minimal_output=false functionality + + // Setup mock search results + mockSearchResult := &github.RepositoriesSearchResult{ + Total: github.Ptr(1), + IncompleteResults: github.Ptr(false), + Repositories: []*github.Repository{ + { + ID: github.Ptr(int64(12345)), + Name: github.Ptr("test-repo"), + FullName: github.Ptr("owner/test-repo"), + HTMLURL: github.Ptr("https://github.com/owner/test-repo"), + Description: github.Ptr("Test repository"), + StargazersCount: github.Ptr(100), + }, + }, + } + + mockedClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchRepositories, + expectQueryParams(t, map[string]string{ + "q": "golang test", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ) + + // Setup client with mock + client := github.NewClient(mockedClient) + _, handlerTest := SearchRepositories(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(map[string]interface{}{ + "query": "golang test", + "minimal_output": false, + }) + + // Call handler + result, err := handlerTest(context.Background(), request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + + // Unmarshal as full GitHub API response + var returnedResult github.RepositoriesSearchResult + err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + require.NoError(t, err) + + // Verify it's the full API response, not minimal + assert.Equal(t, *mockSearchResult.Total, *returnedResult.Total) + assert.Equal(t, *mockSearchResult.IncompleteResults, *returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Repositories, 1) + assert.Equal(t, *mockSearchResult.Repositories[0].ID, *returnedResult.Repositories[0].ID) + assert.Equal(t, *mockSearchResult.Repositories[0].Name, *returnedResult.Repositories[0].Name) +} + func Test_SearchCode(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) From bf3e789f7880d4c13c7ba2d50b7ad095c43d5a77 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 27 Aug 2025 15:49:14 +0100 Subject: [PATCH 02/21] remove unneeded comments --- pkg/github/search_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 337dbf8e5..91ca45af5 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -166,9 +166,6 @@ func Test_SearchRepositories(t *testing.T) { } func Test_SearchRepositories_FullOutput(t *testing.T) { - // Test the minimal_output=false functionality - - // Setup mock search results mockSearchResult := &github.RepositoriesSearchResult{ Total: github.Ptr(1), IncompleteResults: github.Ptr(false), @@ -197,17 +194,14 @@ func Test_SearchRepositories_FullOutput(t *testing.T) { ), ) - // Setup client with mock client := github.NewClient(mockedClient) _, handlerTest := SearchRepositories(stubGetClientFn(client), translations.NullTranslationHelper) - // Create call request request := createMCPRequest(map[string]interface{}{ "query": "golang test", "minimal_output": false, }) - // Call handler result, err := handlerTest(context.Background(), request) require.NoError(t, err) From 278b94e109cdd1a0708e2a9af2730c6fc463f816 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 27 Aug 2025 16:42:17 +0100 Subject: [PATCH 03/21] remove incorrect diff param --- pkg/github/__toolsnaps__/list_commits.snap | 4 ---- pkg/github/repositories.go | 9 +-------- pkg/github/repositories_test.go | 23 +++------------------- 3 files changed, 4 insertions(+), 32 deletions(-) diff --git a/pkg/github/__toolsnaps__/list_commits.snap b/pkg/github/__toolsnaps__/list_commits.snap index 271eed5b5..a802436c2 100644 --- a/pkg/github/__toolsnaps__/list_commits.snap +++ b/pkg/github/__toolsnaps__/list_commits.snap @@ -10,10 +10,6 @@ "description": "Author username or email address to filter commits by", "type": "string" }, - "include_diffs": { - "description": "Whether to include file diffs and stats in the response. Default is false for faster responses.", - "type": "boolean" - }, "owner": { "description": "Repository owner", "type": "string" diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 04ab1e92b..6f8a026cd 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -213,9 +213,6 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t mcp.WithString("author", mcp.Description("Author username or email address to filter commits by"), ), - mcp.WithBoolean("include_diffs", - mcp.Description("Whether to include file diffs and stats in the response. Default is false for faster responses."), - ), WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -235,10 +232,6 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } - includeDiffs, err := OptionalParam[bool](request, "include_diffs") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -282,7 +275,7 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t // Convert to minimal commits minimalCommits := make([]MinimalCommit, 0, len(commits)) for _, commit := range commits { - minimalCommits = append(minimalCommits, convertToMinimalCommit(commit, includeDiffs)) + minimalCommits = append(minimalCommits, convertToMinimalCommit(commit, false)) } r, err := json.Marshal(minimalCommits) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 7ea343f87..4836a0dee 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -720,7 +720,6 @@ func Test_ListCommits(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sha") assert.Contains(t, tool.InputSchema.Properties, "author") - assert.Contains(t, tool.InputSchema.Properties, "include_diffs") assert.Contains(t, tool.InputSchema.Properties, "page") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) @@ -931,25 +930,9 @@ func Test_ListCommits(t *testing.T) { assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit.Author.Login) } - // Check if diffs are included based on include_diffs parameter - includeDiffs, exists := tc.requestArgs["include_diffs"] - if exists && includeDiffs == true { - // When include_diffs=true, files and stats should be present - if len(tc.expectedCommits[i].Files) > 0 { - assert.NotNil(t, commit.Files) - assert.Len(t, commit.Files, len(tc.expectedCommits[i].Files)) - } - if tc.expectedCommits[i].Stats != nil { - assert.NotNil(t, commit.Stats) - assert.Equal(t, tc.expectedCommits[i].Stats.GetAdditions(), commit.Stats.Additions) - assert.Equal(t, tc.expectedCommits[i].Stats.GetDeletions(), commit.Stats.Deletions) - assert.Equal(t, tc.expectedCommits[i].Stats.GetTotal(), commit.Stats.Total) - } - } else { - // When include_diffs=false or not specified (default is false for performance), files and stats should not be present - assert.Nil(t, commit.Files) - assert.Nil(t, commit.Stats) - } + // Files and stats are never included in list_commits (only available in individual commit queries) + assert.Nil(t, commit.Files) + assert.Nil(t, commit.Stats) } }) } From 0ebdf30cd43f1a1f9b542e5471d6e4f9a4c4ca40 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 27 Aug 2025 16:45:52 +0100 Subject: [PATCH 04/21] update docs --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index e77cd1605..cdd1c1298 100644 --- a/README.md +++ b/README.md @@ -866,7 +866,6 @@ The following sets of tools are available (all are on by default): - **list_commits** - List commits - `author`: Author username or email address to filter commits by (string, optional) - - `include_diffs`: Whether to include file diffs and stats in the response. Default is false for faster responses. (boolean, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) From 26ebb0d66cda924e7132fe9d1009073c4be5fcdb Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 10:07:07 +0100 Subject: [PATCH 05/21] rm comment --- pkg/github/minimal_types.go | 2 -- pkg/github/search.go | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index d83e1f608..b840d35a2 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -105,8 +105,6 @@ type MinimalBranch struct { Protected bool `json:"protected"` } -// Minimal response types for create/edit operations - // MinimalCreateResponse represents a minimal response for resource creation operations. type MinimalCreateResponse struct { URL string `json:"url"` diff --git a/pkg/github/search.go b/pkg/github/search.go index e0634af6f..8f4135cec 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -45,7 +45,9 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF if err != nil { return mcp.NewToolResultError(err.Error()), nil } - + if _, ok := request.GetArguments()["minimal_output"]; !ok { + minimalOutput = true + } opts := &github.SearchOptions{ ListOptions: github.ListOptions{ Page: pagination.Page, From 4a32e08e16659391757b9fa8786e120e4229a56f Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 14:16:53 +0100 Subject: [PATCH 06/21] Update pkg/github/repositories.go Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com> --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 6f8a026cd..301e48299 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -126,7 +126,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too mcp.Description("Commit SHA, branch name, or tag name"), ), mcp.WithBoolean("include_diff", - mcp.Description("Whether to include file diffs and stats in the response. Default is true for single commit queries."), + mcp.Description("Whether to include file diffs and stats in the response. Default is true."), ), WithPagination(), ), From ca91a806222428007d26a099c304e566a24cabcf Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 14:19:58 +0100 Subject: [PATCH 07/21] update toolsnaps and docs --- README.md | 2 +- pkg/github/__toolsnaps__/get_commit.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cdd1c1298..b9f31ee48 100644 --- a/README.md +++ b/README.md @@ -830,7 +830,7 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - **get_commit** - Get commit details - - `include_diff`: Whether to include file diffs and stats in the response. Default is true for single commit queries. (boolean, optional) + - `include_diff`: Whether to include file diffs and stats in the response. Default is true. (boolean, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) diff --git a/pkg/github/__toolsnaps__/get_commit.snap b/pkg/github/__toolsnaps__/get_commit.snap index a3791a9b8..7a1ee1281 100644 --- a/pkg/github/__toolsnaps__/get_commit.snap +++ b/pkg/github/__toolsnaps__/get_commit.snap @@ -7,7 +7,7 @@ "inputSchema": { "properties": { "include_diff": { - "description": "Whether to include file diffs and stats in the response. Default is true for single commit queries.", + "description": "Whether to include file diffs and stats in the response. Default is true.", "type": "boolean" }, "owner": { From 9ea0ac92536b4135f5b869ad4b05b93572e8750c Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 15:34:09 +0100 Subject: [PATCH 08/21] change minimal_output to use new OptionalBoolParamWithDefault --- pkg/github/search.go | 5 +---- pkg/github/server.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 8f4135cec..a4f0867ba 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -41,13 +41,10 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF if err != nil { return mcp.NewToolResultError(err.Error()), nil } - minimalOutput, err := OptionalParam[bool](request, "minimal_output") + minimalOutput, err := OptionalBoolParamWithDefault(request, "minimal_output", true) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - if _, ok := request.GetArguments()["minimal_output"]; !ok { - minimalOutput = true - } opts := &github.SearchOptions{ ListOptions: github.ListOptions{ Page: pagination.Page, diff --git a/pkg/github/server.go b/pkg/github/server.go index 80a1bbac6..c543215ef 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -144,6 +144,19 @@ func OptionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, e return v, nil } +// OptionalBoolParamWithDefault is a helper function that can be used to fetch a requested parameter from the request +// similar to optionalBoolParam, but it also takes a default value. +func OptionalBoolParamWithDefault(r mcp.CallToolRequest, p string, d bool) (bool, error) { + v, err := OptionalParam[bool](r, p) + if err != nil { + return false, err + } + if !v { + return d, nil + } + return v, nil +} + // OptionalStringArrayParam 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, if not, it returns its zero-value From 4142f93ea1a8aee2e3c6584bde249d975b70973d Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 15:34:33 +0100 Subject: [PATCH 09/21] Update pkg/github/repositories.go Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com> --- pkg/github/repositories.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 301e48299..6ba49e386 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -273,10 +273,10 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t } // Convert to minimal commits - minimalCommits := make([]MinimalCommit, 0, len(commits)) - for _, commit := range commits { - minimalCommits = append(minimalCommits, convertToMinimalCommit(commit, false)) - } + minimalCommits := make([]MinimalCommit, len(commits)) + for i, commit := range commits { + minimalCommits[i] = convertToMinimalCommit(commit, false) + } r, err := json.Marshal(minimalCommits) if err != nil { From 17c8564db21eceaaf4c5152b84a262358ac13c6e Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 28 Aug 2025 15:38:29 +0100 Subject: [PATCH 10/21] refactor minimal conversion funcs to minimal_types.go --- pkg/github/minimal_types.go | 92 ++++++++++++++++++++++++++++++++++++ pkg/github/repositories.go | 94 ++----------------------------------- 2 files changed, 95 insertions(+), 91 deletions(-) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index b840d35a2..41366d01e 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -1,5 +1,7 @@ package github +import "github.com/google/go-github/v74/github" + // MinimalUser is the output type for user and organization search results. type MinimalUser struct { Login string `json:"login"` @@ -152,3 +154,93 @@ type MinimalGistResponse struct { Description string `json:"description,omitempty"` Public bool `json:"public"` } + +// Helper functions + +// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit +func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit { + minimalCommit := MinimalCommit{ + SHA: commit.GetSHA(), + HTMLURL: commit.GetHTMLURL(), + } + + if commit.Commit != nil { + minimalCommit.Commit = &MinimalCommitInfo{ + Message: commit.Commit.GetMessage(), + } + + if commit.Commit.Author != nil { + minimalCommit.Commit.Author = &MinimalCommitAuthor{ + Name: commit.Commit.Author.GetName(), + Email: commit.Commit.Author.GetEmail(), + } + if commit.Commit.Author.Date != nil { + minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format("2006-01-02T15:04:05Z") + } + } + + if commit.Commit.Committer != nil { + minimalCommit.Commit.Committer = &MinimalCommitAuthor{ + Name: commit.Commit.Committer.GetName(), + Email: commit.Commit.Committer.GetEmail(), + } + if commit.Commit.Committer.Date != nil { + minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format("2006-01-02T15:04:05Z") + } + } + } + + if commit.Author != nil { + minimalCommit.Author = &MinimalUser{ + Login: commit.Author.GetLogin(), + ID: commit.Author.GetID(), + ProfileURL: commit.Author.GetHTMLURL(), + AvatarURL: commit.Author.GetAvatarURL(), + } + } + + if commit.Committer != nil { + minimalCommit.Committer = &MinimalUser{ + Login: commit.Committer.GetLogin(), + ID: commit.Committer.GetID(), + ProfileURL: commit.Committer.GetHTMLURL(), + AvatarURL: commit.Committer.GetAvatarURL(), + } + } + + // Only include stats and files if includeDiffs is true + if includeDiffs { + if commit.Stats != nil { + minimalCommit.Stats = &MinimalCommitStats{ + Additions: commit.Stats.GetAdditions(), + Deletions: commit.Stats.GetDeletions(), + Total: commit.Stats.GetTotal(), + } + } + + if len(commit.Files) > 0 { + minimalCommit.Files = make([]MinimalCommitFile, 0, len(commit.Files)) + for _, file := range commit.Files { + minimalFile := MinimalCommitFile{ + Filename: file.GetFilename(), + Status: file.GetStatus(), + Additions: file.GetAdditions(), + Deletions: file.GetDeletions(), + Changes: file.GetChanges(), + } + minimalCommit.Files = append(minimalCommit.Files, minimalFile) + } + } + } + + return minimalCommit +} + +// convertToMinimalBranch converts a GitHub API Branch to MinimalBranch +func convertToMinimalBranch(branch *github.Branch) MinimalBranch { + return MinimalBranch{ + Name: branch.GetName(), + SHA: branch.GetCommit().GetSHA(), + Protected: branch.GetProtected(), + } +} diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 6ba49e386..aadb1cff8 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -18,94 +18,6 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit -func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit { - minimalCommit := MinimalCommit{ - SHA: commit.GetSHA(), - HTMLURL: commit.GetHTMLURL(), - } - - if commit.Commit != nil { - minimalCommit.Commit = &MinimalCommitInfo{ - Message: commit.Commit.GetMessage(), - } - - if commit.Commit.Author != nil { - minimalCommit.Commit.Author = &MinimalCommitAuthor{ - Name: commit.Commit.Author.GetName(), - Email: commit.Commit.Author.GetEmail(), - } - if commit.Commit.Author.Date != nil { - minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format("2006-01-02T15:04:05Z") - } - } - - if commit.Commit.Committer != nil { - minimalCommit.Commit.Committer = &MinimalCommitAuthor{ - Name: commit.Commit.Committer.GetName(), - Email: commit.Commit.Committer.GetEmail(), - } - if commit.Commit.Committer.Date != nil { - minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format("2006-01-02T15:04:05Z") - } - } - } - - if commit.Author != nil { - minimalCommit.Author = &MinimalUser{ - Login: commit.Author.GetLogin(), - ID: commit.Author.GetID(), - ProfileURL: commit.Author.GetHTMLURL(), - AvatarURL: commit.Author.GetAvatarURL(), - } - } - - if commit.Committer != nil { - minimalCommit.Committer = &MinimalUser{ - Login: commit.Committer.GetLogin(), - ID: commit.Committer.GetID(), - ProfileURL: commit.Committer.GetHTMLURL(), - AvatarURL: commit.Committer.GetAvatarURL(), - } - } - - // Only include stats and files if includeDiffs is true - if includeDiffs { - if commit.Stats != nil { - minimalCommit.Stats = &MinimalCommitStats{ - Additions: commit.Stats.GetAdditions(), - Deletions: commit.Stats.GetDeletions(), - Total: commit.Stats.GetTotal(), - } - } - - if len(commit.Files) > 0 { - minimalCommit.Files = make([]MinimalCommitFile, 0, len(commit.Files)) - for _, file := range commit.Files { - minimalFile := MinimalCommitFile{ - Filename: file.GetFilename(), - Status: file.GetStatus(), - Additions: file.GetAdditions(), - Deletions: file.GetDeletions(), - Changes: file.GetChanges(), - } - minimalCommit.Files = append(minimalCommit.Files, minimalFile) - } - } - } - - return minimalCommit -} - -// convertToMinimalBranch converts a GitHub API Branch to MinimalBranch -func convertToMinimalBranch(branch *github.Branch) MinimalBranch { - return MinimalBranch{ - Name: branch.GetName(), - SHA: branch.GetCommit().GetSHA(), - Protected: branch.GetProtected(), - } -} - func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_commit", mcp.WithDescription(t("TOOL_GET_COMMITS_DESCRIPTION", "Get details for a commit from a GitHub repository")), @@ -274,9 +186,9 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t // Convert to minimal commits minimalCommits := make([]MinimalCommit, len(commits)) - for i, commit := range commits { - minimalCommits[i] = convertToMinimalCommit(commit, false) - } + for i, commit := range commits { + minimalCommits[i] = convertToMinimalCommit(commit, false) + } r, err := json.Marshal(minimalCommits) if err != nil { From c82651fba16a350ff070e52ae139bf4bbcdb663f Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:01:59 +0100 Subject: [PATCH 11/21] consolidate response structs and remove unneeded message field --- pkg/github/gists.go | 1 - pkg/github/issues.go | 3 +-- pkg/github/issues_test.go | 2 +- pkg/github/minimal_types.go | 22 ++-------------------- pkg/github/pullrequests.go | 3 +-- pkg/github/pullrequests_test.go | 2 +- 6 files changed, 6 insertions(+), 27 deletions(-) diff --git a/pkg/github/gists.go b/pkg/github/gists.go index c33d5b8da..303c151c0 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -259,7 +259,6 @@ func UpdateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (to minimalResponse := MinimalUpdateResponse{ URL: updatedGist.GetHTMLURL(), Updated: true, - Message: "Gist updated successfully", } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f2f3822ff..aa7219e6f 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -873,7 +873,7 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } // Return minimal response with just essential information - minimalResponse := MinimalIssueResponse{ + minimalResponse := MinimalResourceResponse{ URL: issue.GetHTMLURL(), Number: issue.GetNumber(), State: issue.GetState(), @@ -1254,7 +1254,6 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t minimalResponse := MinimalUpdateResponse{ URL: updatedIssue.GetHTMLURL(), Updated: true, - Message: "Issue updated successfully", } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index b13ea2ef6..6223481e7 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -713,7 +713,7 @@ func Test_CreateIssue(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var returnedIssue MinimalIssueResponse + var returnedIssue MinimalResourceResponse err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 41366d01e..ad8a087c4 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -107,17 +107,8 @@ type MinimalBranch struct { Protected bool `json:"protected"` } -// MinimalCreateResponse represents a minimal response for resource creation operations. -type MinimalCreateResponse struct { - URL string `json:"url"` - ID int64 `json:"id,omitempty"` - Number int `json:"number,omitempty"` - Name string `json:"name,omitempty"` - State string `json:"state,omitempty"` -} - -// MinimalPullRequestResponse represents a minimal response for pull request operations. -type MinimalPullRequestResponse struct { +// MinimalResourceResponse represents a minimal response for numbered resource operations (issues, pull requests). +type MinimalResourceResponse struct { URL string `json:"url"` Number int `json:"number"` State string `json:"state"` @@ -132,19 +123,10 @@ type MinimalRepositoryResponse struct { FullName string `json:"full_name"` } -// MinimalIssueResponse represents a minimal response for issue operations. -type MinimalIssueResponse struct { - URL string `json:"url"` - Number int `json:"number"` - State string `json:"state"` - Title string `json:"title,omitempty"` -} - // MinimalUpdateResponse represents a minimal response for update operations. type MinimalUpdateResponse struct { URL string `json:"url"` Updated bool `json:"updated"` - Message string `json:"message,omitempty"` } // MinimalGistResponse represents a minimal response for gist operations. diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 89676815a..631701e79 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -194,7 +194,7 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu } // Return minimal response with just essential information - minimalResponse := MinimalPullRequestResponse{ + minimalResponse := MinimalResourceResponse{ URL: pr.GetHTMLURL(), Number: pr.GetNumber(), State: pr.GetState(), @@ -476,7 +476,6 @@ func UpdatePullRequest(getClient GetClientFn, getGQLClient GetGQLClientFn, t tra minimalResponse := MinimalUpdateResponse{ URL: finalPR.GetHTMLURL(), Updated: true, - Message: "Pull request updated successfully", } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index f97fda300..7dae740b9 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1955,7 +1955,7 @@ func Test_CreatePullRequest(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var returnedPR MinimalPullRequestResponse + var returnedPR MinimalResourceResponse err = json.Unmarshal([]byte(textContent.Text), &returnedPR) require.NoError(t, err) assert.Equal(t, tc.expectedPR.GetNumber(), returnedPR.Number) From 60213e66217e9e11141b0c41dc08a7da586f341f Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:13:46 +0100 Subject: [PATCH 12/21] consolidate response further --- pkg/github/gists.go | 12 ++++-------- pkg/github/gists_test.go | 6 ++---- pkg/github/issues.go | 12 ++++-------- pkg/github/issues_test.go | 8 ++------ pkg/github/minimal_types.go | 33 +++++---------------------------- pkg/github/pullrequests.go | 12 ++++-------- pkg/github/pullrequests_test.go | 11 +++-------- pkg/github/repositories.go | 14 ++++---------- pkg/github/repositories_test.go | 5 +---- 9 files changed, 29 insertions(+), 84 deletions(-) diff --git a/pkg/github/gists.go b/pkg/github/gists.go index 303c151c0..3f1645f3e 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -165,11 +165,8 @@ func CreateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to create gist: %s", string(body))), nil } - minimalResponse := MinimalGistResponse{ - URL: createdGist.GetHTMLURL(), - ID: createdGist.GetID(), - Description: createdGist.GetDescription(), - Public: createdGist.GetPublic(), + minimalResponse := MinimalResponse{ + URL: createdGist.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) @@ -256,9 +253,8 @@ func UpdateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to update gist: %s", string(body))), nil } - minimalResponse := MinimalUpdateResponse{ - URL: updatedGist.GetHTMLURL(), - Updated: true, + minimalResponse := MinimalResponse{ + URL: updatedGist.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/gists_test.go b/pkg/github/gists_test.go index acf54263f..9b8b4eb6e 100644 --- a/pkg/github/gists_test.go +++ b/pkg/github/gists_test.go @@ -322,11 +322,10 @@ func Test_CreateGist(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var gist MinimalGistResponse + var gist MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &gist) require.NoError(t, err) - assert.Equal(t, tc.expectedGist.GetID(), gist.ID) assert.Equal(t, tc.expectedGist.GetHTMLURL(), gist.URL) }) } @@ -477,12 +476,11 @@ func Test_UpdateGist(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var updateResp MinimalUpdateResponse + var updateResp MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) assert.Equal(t, tc.expectedGist.GetHTMLURL(), updateResp.URL) - assert.Equal(t, true, updateResp.Updated) }) } } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index aa7219e6f..01ce7b42e 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -873,11 +873,8 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } // Return minimal response with just essential information - minimalResponse := MinimalResourceResponse{ - URL: issue.GetHTMLURL(), - Number: issue.GetNumber(), - State: issue.GetState(), - Title: issue.GetTitle(), + minimalResponse := MinimalResponse{ + URL: issue.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) @@ -1251,9 +1248,8 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } // Return minimal response with just essential information - minimalResponse := MinimalUpdateResponse{ - URL: updatedIssue.GetHTMLURL(), - Updated: true, + minimalResponse := MinimalResponse{ + URL: updatedIssue.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 6223481e7..5a0d409a6 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -713,13 +713,10 @@ func Test_CreateIssue(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var returnedIssue MinimalResourceResponse + var returnedIssue MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) - assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.Number) - assert.Equal(t, tc.expectedIssue.GetTitle(), returnedIssue.Title) - assert.Equal(t, tc.expectedIssue.GetState(), returnedIssue.State) assert.Equal(t, tc.expectedIssue.GetHTMLURL(), returnedIssue.URL) }) } @@ -1210,12 +1207,11 @@ func Test_UpdateIssue(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var updateResp MinimalUpdateResponse + var updateResp MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) assert.Equal(t, tc.expectedIssue.GetHTMLURL(), updateResp.URL) - assert.Equal(t, true, updateResp.Updated) }) } } diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index ad8a087c4..8c44c4a70 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -107,34 +107,11 @@ type MinimalBranch struct { Protected bool `json:"protected"` } -// MinimalResourceResponse represents a minimal response for numbered resource operations (issues, pull requests). -type MinimalResourceResponse struct { - URL string `json:"url"` - Number int `json:"number"` - State string `json:"state"` - Title string `json:"title,omitempty"` -} - -// MinimalRepositoryResponse represents a minimal response for repository operations. -type MinimalRepositoryResponse struct { - URL string `json:"url"` - CloneURL string `json:"clone_url"` - Name string `json:"name"` - FullName string `json:"full_name"` -} - -// MinimalUpdateResponse represents a minimal response for update operations. -type MinimalUpdateResponse struct { - URL string `json:"url"` - Updated bool `json:"updated"` -} - -// MinimalGistResponse represents a minimal response for gist operations. -type MinimalGistResponse struct { - URL string `json:"url"` - ID string `json:"id"` - Description string `json:"description,omitempty"` - Public bool `json:"public"` +// MinimalResponse represents a minimal response for all CRUD operations. +// Success is implicit in the HTTP response status, and all other information +// can be derived from the URL or fetched separately if needed. +type MinimalResponse struct { + URL string `json:"url"` } // Helper functions diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 631701e79..d7547519d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -194,11 +194,8 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu } // Return minimal response with just essential information - minimalResponse := MinimalResourceResponse{ - URL: pr.GetHTMLURL(), - Number: pr.GetNumber(), - State: pr.GetState(), - Title: pr.GetTitle(), + minimalResponse := MinimalResponse{ + URL: pr.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) @@ -473,9 +470,8 @@ func UpdatePullRequest(getClient GetClientFn, getGQLClient GetGQLClientFn, t tra }() // Return minimal response with just essential information - minimalResponse := MinimalUpdateResponse{ - URL: finalPR.GetHTMLURL(), - Updated: true, + minimalResponse := MinimalResponse{ + URL: finalPR.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 7dae740b9..ea2df97f4 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -382,11 +382,10 @@ func Test_UpdatePullRequest(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var updateResp MinimalUpdateResponse + var updateResp MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL) - assert.Equal(t, true, updateResp.Updated) }) } } @@ -565,11 +564,10 @@ func Test_UpdatePullRequest_Draft(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var updateResp MinimalUpdateResponse + var updateResp MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &updateResp) require.NoError(t, err) assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL) - assert.Equal(t, true, updateResp.Updated) }) } } @@ -1955,12 +1953,9 @@ func Test_CreatePullRequest(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var returnedPR MinimalResourceResponse + var returnedPR MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &returnedPR) require.NoError(t, err) - assert.Equal(t, tc.expectedPR.GetNumber(), returnedPR.Number) - assert.Equal(t, tc.expectedPR.GetTitle(), returnedPR.Title) - assert.Equal(t, tc.expectedPR.GetState(), returnedPR.State) assert.Equal(t, tc.expectedPR.GetHTMLURL(), returnedPR.URL) }) } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index aadb1cff8..f7a2c65fc 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -459,11 +459,8 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun } // Return minimal response with just essential information - minimalResponse := MinimalRepositoryResponse{ - URL: createdRepo.GetHTMLURL(), - CloneURL: createdRepo.GetCloneURL(), - Name: createdRepo.GetName(), - FullName: createdRepo.GetFullName(), + minimalResponse := MinimalResponse{ + URL: createdRepo.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) @@ -738,11 +735,8 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) } // Return minimal response with just essential information - minimalResponse := MinimalRepositoryResponse{ - URL: forkedRepo.GetHTMLURL(), - CloneURL: forkedRepo.GetCloneURL(), - Name: forkedRepo.GetName(), - FullName: forkedRepo.GetFullName(), + minimalResponse := MinimalResponse{ + URL: forkedRepo.GetHTMLURL(), } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 4836a0dee..a29aeb380 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1241,15 +1241,12 @@ func Test_CreateRepository(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the minimal result - var returnedRepo MinimalRepositoryResponse + var returnedRepo MinimalResponse err = json.Unmarshal([]byte(textContent.Text), &returnedRepo) assert.NoError(t, err) // Verify repository details - assert.Equal(t, tc.expectedRepo.GetName(), returnedRepo.Name) assert.Equal(t, tc.expectedRepo.GetHTMLURL(), returnedRepo.URL) - assert.Equal(t, tc.expectedRepo.GetCloneURL(), returnedRepo.CloneURL) - assert.Equal(t, tc.expectedRepo.GetFullName(), returnedRepo.FullName) }) } } From c36e4ce3347029a44f49de22bbe882d140fde5ce Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:14:52 +0100 Subject: [PATCH 13/21] remove CloneURL field --- pkg/github/minimal_types.go | 1 - pkg/github/repositories_test.go | 1 - pkg/github/search.go | 1 - 3 files changed, 3 deletions(-) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 8c44c4a70..0c3c220aa 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -25,7 +25,6 @@ type MinimalRepository struct { FullName string `json:"full_name"` Description string `json:"description,omitempty"` HTMLURL string `json:"html_url"` - CloneURL string `json:"clone_url,omitempty"` Language string `json:"language,omitempty"` Stars int `json:"stargazers_count"` Forks int `json:"forks_count"` diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index a29aeb380..6f4693724 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1125,7 +1125,6 @@ func Test_CreateRepository(t *testing.T) { Description: github.Ptr("Test repository"), Private: github.Ptr(true), HTMLURL: github.Ptr("https://github.com/testuser/test-repo"), - CloneURL: github.Ptr("https://github.com/testuser/test-repo.git"), CreatedAt: &github.Timestamp{Time: time.Now()}, Owner: &github.User{ Login: github.Ptr("testuser"), diff --git a/pkg/github/search.go b/pkg/github/search.go index a4f0867ba..38a010814 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -85,7 +85,6 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF FullName: repo.GetFullName(), Description: repo.GetDescription(), HTMLURL: repo.GetHTMLURL(), - CloneURL: repo.GetCloneURL(), Language: repo.GetLanguage(), Stars: repo.GetStargazersCount(), Forks: repo.GetForksCount(), From 8e49c3f09643fc71a213523753b2085eb8baac45 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:33:16 +0100 Subject: [PATCH 14/21] Update pkg/github/repositories.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index f7a2c65fc..2da9100b3 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -55,7 +55,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too if err != nil { return mcp.NewToolResultError(err.Error()), nil } - includeDiff, err := OptionalParam[bool](request, "include_diff") + includeDiff, err := mcp.OptionalBoolParamWithDefault(request, "include_diff", true) if err != nil { return mcp.NewToolResultError(err.Error()), nil } From 7e4616e2894f077cc38bc0255be47e8b346080d5 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:33:28 +0100 Subject: [PATCH 15/21] Update pkg/github/server.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/github/server.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/github/server.go b/pkg/github/server.go index c543215ef..16d28643c 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -147,11 +147,13 @@ func OptionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, e // OptionalBoolParamWithDefault is a helper function that can be used to fetch a requested parameter from the request // similar to optionalBoolParam, but it also takes a default value. func OptionalBoolParamWithDefault(r mcp.CallToolRequest, p string, d bool) (bool, error) { + args := r.GetArguments() + _, ok := args[p] v, err := OptionalParam[bool](r, p) if err != nil { return false, err } - if !v { + if !ok { return d, nil } return v, nil From 093f86f6dd593c1bc28c11ad372679d8eaa92102 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 10:34:28 +0100 Subject: [PATCH 16/21] fix undefined --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 2da9100b3..0758112d3 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -55,7 +55,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too if err != nil { return mcp.NewToolResultError(err.Error()), nil } - includeDiff, err := mcp.OptionalBoolParamWithDefault(request, "include_diff", true) + includeDiff, err := OptionalBoolParamWithDefault(request, "include_diff", true) if err != nil { return mcp.NewToolResultError(err.Error()), nil } From 4fa0729f81e06e56553f64ed2331017decd76602 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 14:47:21 +0100 Subject: [PATCH 17/21] change incorrect comment --- pkg/github/repositories_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 6f4693724..6db069874 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -930,7 +930,7 @@ func Test_ListCommits(t *testing.T) { assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit.Author.Login) } - // Files and stats are never included in list_commits (only available in individual commit queries) + // Files and stats are never included in list_commits assert.Nil(t, commit.Files) assert.Nil(t, commit.Stats) } From cfeed3d5e3095f0de2de2e4916d867a7379f98fb Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Mon, 1 Sep 2025 16:36:30 +0100 Subject: [PATCH 18/21] remove old err var declaration --- pkg/github/search.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 38a010814..55e4cf8b4 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -114,14 +114,11 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF Items: minimalRepos, } - var err error r, err = json.Marshal(minimalResult) if err != nil { return nil, fmt.Errorf("failed to marshal minimal response: %w", err) } } else { - // Return full GitHub API response - var err error r, err = json.Marshal(result) if err != nil { return nil, fmt.Errorf("failed to marshal full response: %w", err) From 586a0d6591a9b408890c39195e36dccccd3d7e40 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Mon, 1 Sep 2025 17:31:51 +0100 Subject: [PATCH 19/21] Update pkg/github/repositories.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 0758112d3..9027c17d4 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -37,8 +37,8 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too mcp.Required(), mcp.Description("Commit SHA, branch name, or tag name"), ), - mcp.WithBoolean("include_diff", mcp.Description("Whether to include file diffs and stats in the response. Default is true."), + mcp.DefaultBool(true), ), WithPagination(), ), From debef3ea5146bb79f7a7cb9be64d2d12327d51bc Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Mon, 1 Sep 2025 17:35:27 +0100 Subject: [PATCH 20/21] fix syntax issue --- pkg/github/repositories.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 9027c17d4..dce8501db 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -37,6 +37,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too mcp.Required(), mcp.Description("Commit SHA, branch name, or tag name"), ), + mcp.WithBoolean("include_diff", mcp.Description("Whether to include file diffs and stats in the response. Default is true."), mcp.DefaultBool(true), ), From e72c1430debb8b0410cbaa9ba0a0cae33adae91a Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Mon, 1 Sep 2025 17:41:18 +0100 Subject: [PATCH 21/21] update toolsnaps --- pkg/github/__toolsnaps__/get_commit.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/github/__toolsnaps__/get_commit.snap b/pkg/github/__toolsnaps__/get_commit.snap index 7a1ee1281..1c2ecc9a3 100644 --- a/pkg/github/__toolsnaps__/get_commit.snap +++ b/pkg/github/__toolsnaps__/get_commit.snap @@ -7,6 +7,7 @@ "inputSchema": { "properties": { "include_diff": { + "default": true, "description": "Whether to include file diffs and stats in the response. Default is true.", "type": "boolean" },