Skip to content

Commit 59745b8

Browse files
SamMorrowDrumsCopilot
SamMorrowDrums
and
Copilot
committed
refactor(search_commits): share commit conversion, surface repo, tighten query docs
- Extract newMinimalCommitFromCore to share field mapping between convertToMinimalCommit (RepositoryCommit) and the new convertCommitResultToMinimalCommit (CommitResult), removing ~50 lines of duplicated logic from the search_commits handler. - Add MinimalRepoRef and a search-only MinimalCommitSearchItem type (embedding MinimalCommit) so cross-repo commit search results identify the repo each commit came from. Keeping the field off MinimalCommit avoids paying for a never-populated field on the get_commit/list_commits output types. - Rewrite the query description to teach the model the actual commit-search qualifier surface (repo:/org:/user: scoping, author/ committer/date qualifiers, hash/tree/parent, merge:, is:public) and reword the sort description to drop redundancy with the enum. - Extend tests to assert the repository field is surfaced and to cover commits with no resolved GitHub user (nil Author/Committer). - Refresh README and toolsnap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fc260e0 commit 59745b8

5 files changed

Lines changed: 132 additions & 94 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,8 +1307,8 @@ The following sets of tools are available:
13071307
- `order`: Sort order (string, optional)
13081308
- `page`: Page number for pagination (min 1) (number, optional)
13091309
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
1310-
- `query`: Commit search query. Examples: 'repo:owner/repo fix bug', 'author:defunkt', 'committer-date:>2024-01-01'. Supports advanced search syntax. (string, required)
1311-
- `sort`: Sort field ('author-date' or 'committer-date') (string, optional)
1310+
- `query`: Commit search query (GitHub commit search REST). Searches commit messages on the default branch only. Scope the search with `repo:owner/repo`, `org:`, or `user:` (queries without a scope qualifier match across all of GitHub and are usually not what you want). Other qualifiers: `author:`, `committer:`, `author-name:`, `committer-name:`, `author-email:`, `committer-email:`, `author-date:`, `committer-date:` (supports `>`, `<`, `>=`, `<=`, and `YYYY-MM-DD..YYYY-MM-DD` ranges), `merge:true|false`, `hash:`, `tree:`, `parent:`, `is:public`. Examples: `repo:owner/repo fix panic`; `org:github author:defunkt committer-date:>=2024-01-01`; `"refactor cache" repo:o/r`; `hash:abc1234 repo:o/r`. (string, required)
1311+
- `sort`: Sort by author or committer date (defaults to best match) (string, optional)
13121312

13131313
- **search_repositories** - Search repositories
13141314
- **Required OAuth Scopes**: `repo`

‎pkg/github/__toolsnaps__/search_commits.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"readOnlyHint": true,
44
"title": "Search commits"
55
},
6-
"description": "Search for commits across GitHub repositories using specialized commit search syntax. Great for finding specific changes, authors, or messages.",
6+
"description": "Search for commits across GitHub repositories using GitHub's commit search syntax. Useful for finding specific changes, authors, or messages across one or many repositories. Searches the default branch only.",
77
"inputSchema": {
88
"properties": {
99
"order": {
@@ -26,11 +26,11 @@
2626
"type": "number"
2727
},
2828
"query": {
29-
"description": "Commit search query. Examples: 'repo:owner/repo fix bug', 'author:defunkt', 'committer-date:\u003e2024-01-01'. Supports advanced search syntax.",
29+
"description": "Commit search query (GitHub commit search REST). Searches commit messages on the default branch only. Scope the search with `repo:owner/repo`, `org:`, or `user:` (queries without a scope qualifier match across all of GitHub and are usually not what you want). Other qualifiers: `author:`, `committer:`, `author-name:`, `committer-name:`, `author-email:`, `committer-email:`, `author-date:`, `committer-date:` (supports `\u003e`, `\u003c`, `\u003e=`, `\u003c=`, and `YYYY-MM-DD..YYYY-MM-DD` ranges), `merge:true|false`, `hash:`, `tree:`, `parent:`, `is:public`. Examples: `repo:owner/repo fix panic`; `org:github author:defunkt committer-date:\u003e=2024-01-01`; `\"refactor cache\" repo:o/r`; `hash:abc1234 repo:o/r`.",
3030
"type": "string"
3131
},
3232
"sort": {
33-
"description": "Sort field ('author-date' or 'committer-date')",
33+
"description": "Sort by author or committer date (defaults to best match)",
3434
"enum": [
3535
"author-date",
3636
"committer-date"

‎pkg/github/minimal_types.go‎

Lines changed: 87 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,23 @@ type MinimalCommit struct {
130130
Files []MinimalCommitFile `json:"files,omitempty"`
131131
}
132132

133+
// MinimalRepoRef is a lightweight reference to a repository, used when a
134+
// result needs to identify which repository it belongs to (for example, in
135+
// cross-repo commit search results).
136+
type MinimalRepoRef struct {
137+
FullName string `json:"full_name"`
138+
HTMLURL string `json:"html_url,omitempty"`
139+
Private bool `json:"private,omitempty"`
140+
}
141+
142+
// MinimalCommitSearchItem extends MinimalCommit with the containing
143+
// repository, since commit search spans repositories and callers need to
144+
// know which repo each result came from.
145+
type MinimalCommitSearchItem struct {
146+
MinimalCommit
147+
Repository *MinimalRepoRef `json:"repository,omitempty"`
148+
}
149+
133150
// MinimalRelease is the trimmed output type for release objects.
134151
type MinimalRelease struct {
135152
ID int64 `json:"id"`
@@ -256,9 +273,9 @@ type MinimalIssueComment struct {
256273

257274
// MinimalSearchCommitsResult is the trimmed output type for commit search results.
258275
type MinimalSearchCommitsResult struct {
259-
TotalCount int `json:"total_count"`
260-
IncompleteResults bool `json:"incomplete_results"`
261-
Items []MinimalCommit `json:"items"`
276+
TotalCount int `json:"total_count"`
277+
IncompleteResults bool `json:"incomplete_results"`
278+
Items []MinimalCommitSearchItem `json:"items"`
262279
}
263280

264281
// MinimalFileContentResponse is the trimmed output type for create/update/delete file responses.
@@ -700,57 +717,73 @@ func convertToMinimalUser(user *github.User) *MinimalUser {
700717
}
701718
}
702719

703-
// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit
704-
func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit {
720+
// newMinimalCommitFromCore builds a MinimalCommit from the fields that are
721+
// shared between *github.RepositoryCommit and *github.CommitResult. Caller
722+
// is responsible for setting any type-specific extras (stats/files for
723+
// RepositoryCommit, repository for CommitResult).
724+
func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author, committer *github.User) MinimalCommit {
705725
minimalCommit := MinimalCommit{
706-
SHA: commit.GetSHA(),
707-
HTMLURL: commit.GetHTMLURL(),
726+
SHA: sha,
727+
HTMLURL: htmlURL,
708728
}
709729

710-
if commit.Commit != nil {
730+
if commit != nil {
711731
minimalCommit.Commit = &MinimalCommitInfo{
712-
Message: commit.Commit.GetMessage(),
732+
Message: commit.GetMessage(),
713733
}
714734

715-
if commit.Commit.Author != nil {
735+
if commit.Author != nil {
716736
minimalCommit.Commit.Author = &MinimalCommitAuthor{
717-
Name: commit.Commit.Author.GetName(),
718-
Email: commit.Commit.Author.GetEmail(),
737+
Name: commit.Author.GetName(),
738+
Email: commit.Author.GetEmail(),
719739
}
720-
if commit.Commit.Author.Date != nil {
721-
minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format(time.RFC3339)
740+
if commit.Author.Date != nil {
741+
minimalCommit.Commit.Author.Date = commit.Author.Date.Format(time.RFC3339)
722742
}
723743
}
724744

725-
if commit.Commit.Committer != nil {
745+
if commit.Committer != nil {
726746
minimalCommit.Commit.Committer = &MinimalCommitAuthor{
727-
Name: commit.Commit.Committer.GetName(),
728-
Email: commit.Commit.Committer.GetEmail(),
747+
Name: commit.Committer.GetName(),
748+
Email: commit.Committer.GetEmail(),
729749
}
730-
if commit.Commit.Committer.Date != nil {
731-
minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format(time.RFC3339)
750+
if commit.Committer.Date != nil {
751+
minimalCommit.Commit.Committer.Date = commit.Committer.Date.Format(time.RFC3339)
732752
}
733753
}
734754
}
735755

736-
if commit.Author != nil {
756+
if author != nil {
737757
minimalCommit.Author = &MinimalUser{
738-
Login: commit.Author.GetLogin(),
739-
ID: commit.Author.GetID(),
740-
ProfileURL: commit.Author.GetHTMLURL(),
741-
AvatarURL: commit.Author.GetAvatarURL(),
758+
Login: author.GetLogin(),
759+
ID: author.GetID(),
760+
ProfileURL: author.GetHTMLURL(),
761+
AvatarURL: author.GetAvatarURL(),
742762
}
743763
}
744764

745-
if commit.Committer != nil {
765+
if committer != nil {
746766
minimalCommit.Committer = &MinimalUser{
747-
Login: commit.Committer.GetLogin(),
748-
ID: commit.Committer.GetID(),
749-
ProfileURL: commit.Committer.GetHTMLURL(),
750-
AvatarURL: commit.Committer.GetAvatarURL(),
767+
Login: committer.GetLogin(),
768+
ID: committer.GetID(),
769+
ProfileURL: committer.GetHTMLURL(),
770+
AvatarURL: committer.GetAvatarURL(),
751771
}
752772
}
753773

774+
return minimalCommit
775+
}
776+
777+
// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit
778+
func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit {
779+
minimalCommit := newMinimalCommitFromCore(
780+
commit.GetSHA(),
781+
commit.GetHTMLURL(),
782+
commit.Commit,
783+
commit.Author,
784+
commit.Committer,
785+
)
786+
754787
// Only include stats and files if includeDiffs is true
755788
if includeDiffs {
756789
if commit.Stats != nil {
@@ -779,6 +812,31 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
779812
return minimalCommit
780813
}
781814

815+
// convertCommitResultToMinimalCommit converts a GitHub API commit search
816+
// result, attaching the containing repository so the caller can tell which
817+
// repo each result came from.
818+
func convertCommitResultToMinimalCommit(commit *github.CommitResult) MinimalCommitSearchItem {
819+
item := MinimalCommitSearchItem{
820+
MinimalCommit: newMinimalCommitFromCore(
821+
commit.GetSHA(),
822+
commit.GetHTMLURL(),
823+
commit.Commit,
824+
commit.Author,
825+
commit.Committer,
826+
),
827+
}
828+
829+
if commit.Repository != nil {
830+
item.Repository = &MinimalRepoRef{
831+
FullName: commit.Repository.GetFullName(),
832+
HTMLURL: commit.Repository.GetHTMLURL(),
833+
Private: commit.Repository.GetPrivate(),
834+
}
835+
}
836+
837+
return item
838+
}
839+
782840
// MinimalPageInfo contains pagination cursor information.
783841
type MinimalPageInfo struct {
784842
HasNextPage bool `json:"hasNextPage"`

‎pkg/github/search.go‎

Lines changed: 4 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9-
"time"
109

1110
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1211
"github.com/github/github-mcp-server/pkg/ifc"
@@ -487,11 +486,11 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
487486
Properties: map[string]*jsonschema.Schema{
488487
"query": {
489488
Type: "string",
490-
Description: "Commit search query. Examples: 'repo:owner/repo fix bug', 'author:defunkt', 'committer-date:>2024-01-01'. Supports advanced search syntax.",
489+
Description: "Commit search query (GitHub commit search REST). Searches commit messages on the default branch only. Scope the search with `repo:owner/repo`, `org:`, or `user:` (queries without a scope qualifier match across all of GitHub and are usually not what you want). Other qualifiers: `author:`, `committer:`, `author-name:`, `committer-name:`, `author-email:`, `committer-email:`, `author-date:`, `committer-date:` (supports `>`, `<`, `>=`, `<=`, and `YYYY-MM-DD..YYYY-MM-DD` ranges), `merge:true|false`, `hash:`, `tree:`, `parent:`, `is:public`. Examples: `repo:owner/repo fix panic`; `org:github author:defunkt committer-date:>=2024-01-01`; `\"refactor cache\" repo:o/r`; `hash:abc1234 repo:o/r`.",
491490
},
492491
"sort": {
493492
Type: "string",
494-
Description: "Sort field ('author-date' or 'committer-date')",
493+
Description: "Sort by author or committer date (defaults to best match)",
495494
Enum: []any{"author-date", "committer-date"},
496495
},
497496
"order": {
@@ -508,7 +507,7 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
508507
ToolsetMetadataRepos,
509508
mcp.Tool{
510509
Name: "search_commits",
511-
Description: t("TOOL_SEARCH_COMMITS_DESCRIPTION", "Search for commits across GitHub repositories using specialized commit search syntax. Great for finding specific changes, authors, or messages."),
510+
Description: t("TOOL_SEARCH_COMMITS_DESCRIPTION", "Search for commits across GitHub repositories using GitHub's commit search syntax. Useful for finding specific changes, authors, or messages across one or many repositories. Searches the default branch only."),
512511
Annotations: &mcp.ToolAnnotations{
513512
Title: t("TOOL_SEARCH_COMMITS_USER_TITLE", "Search commits"),
514513
ReadOnlyHint: true,
@@ -565,60 +564,7 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
565564
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search commits", resp, body), nil, nil
566565
}
567566

568-
convertCommitResultToMinimalCommit := func(commit *github.CommitResult) MinimalCommit {
569-
minimalCommit := MinimalCommit{
570-
SHA: commit.GetSHA(),
571-
HTMLURL: commit.GetHTMLURL(),
572-
}
573-
574-
if commit.Commit != nil {
575-
minimalCommit.Commit = &MinimalCommitInfo{
576-
Message: commit.Commit.GetMessage(),
577-
}
578-
579-
if commit.Commit.Author != nil {
580-
minimalCommit.Commit.Author = &MinimalCommitAuthor{
581-
Name: commit.Commit.Author.GetName(),
582-
Email: commit.Commit.Author.GetEmail(),
583-
}
584-
if commit.Commit.Author.Date != nil {
585-
minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format(time.RFC3339)
586-
}
587-
}
588-
589-
if commit.Commit.Committer != nil {
590-
minimalCommit.Commit.Committer = &MinimalCommitAuthor{
591-
Name: commit.Commit.Committer.GetName(),
592-
Email: commit.Commit.Committer.GetEmail(),
593-
}
594-
if commit.Commit.Committer.Date != nil {
595-
minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format(time.RFC3339)
596-
}
597-
}
598-
}
599-
600-
if commit.Author != nil {
601-
minimalCommit.Author = &MinimalUser{
602-
Login: commit.Author.GetLogin(),
603-
ID: commit.Author.GetID(),
604-
ProfileURL: commit.Author.GetHTMLURL(),
605-
AvatarURL: commit.Author.GetAvatarURL(),
606-
}
607-
}
608-
609-
if commit.Committer != nil {
610-
minimalCommit.Committer = &MinimalUser{
611-
Login: commit.Committer.GetLogin(),
612-
ID: commit.Committer.GetID(),
613-
ProfileURL: commit.Committer.GetHTMLURL(),
614-
AvatarURL: commit.Committer.GetAvatarURL(),
615-
}
616-
}
617-
618-
return minimalCommit
619-
}
620-
621-
minimalCommits := make([]MinimalCommit, 0, len(result.Commits))
567+
minimalCommits := make([]MinimalCommitSearchItem, 0, len(result.Commits))
622568
for _, commit := range result.Commits {
623569
minimalCommits = append(minimalCommits, convertCommitResultToMinimalCommit(commit))
624570
}

‎pkg/github/search_test.go‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,7 @@ func Test_SearchCommits(t *testing.T) {
875875

876876
now := time.Now().Truncate(time.Second)
877877
mockSearchResult := &github.CommitsSearchResult{
878-
Total: github.Ptr(1),
878+
Total: github.Ptr(2),
879879
IncompleteResults: github.Ptr(false),
880880
Commits: []*github.CommitResult{
881881
{
@@ -894,6 +894,23 @@ func Test_SearchCommits(t *testing.T) {
894894
ID: github.Ptr(int64(1)),
895895
HTMLURL: github.Ptr("https://github.com/author"),
896896
},
897+
Repository: &github.Repository{
898+
FullName: github.Ptr("owner/repo"),
899+
HTMLURL: github.Ptr("https://github.com/owner/repo"),
900+
Private: github.Ptr(false),
901+
},
902+
},
903+
{
904+
// Commit with no resolved GitHub user for author or committer
905+
// (common when the commit email isn't linked to an account).
906+
SHA: github.Ptr("def456commit"),
907+
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456commit"),
908+
Commit: &github.Commit{
909+
Message: github.Ptr("Unlinked author"),
910+
},
911+
Repository: &github.Repository{
912+
FullName: github.Ptr("owner/repo"),
913+
},
897914
},
898915
},
899916
}
@@ -945,7 +962,7 @@ func Test_SearchCommits(t *testing.T) {
945962

946963
for _, tc := range tests {
947964
t.Run(tc.name, func(t *testing.T) {
948-
client := github.NewClient(tc.mockedClient)
965+
client := mustNewGHClient(t, tc.mockedClient)
949966
deps := BaseDeps{
950967
Client: client,
951968
}
@@ -977,6 +994,23 @@ func Test_SearchCommits(t *testing.T) {
977994
assert.Equal(t, *tc.expectedResult.Commits[0].Commit.Author.Name, returnedResult.Items[0].Commit.Author.Name)
978995
assert.Equal(t, now.Format(time.RFC3339), returnedResult.Items[0].Commit.Author.Date)
979996
assert.Equal(t, *tc.expectedResult.Commits[0].Author.Login, returnedResult.Items[0].Author.Login)
997+
998+
// Repository info is required so callers can identify which repo
999+
// each cross-repo search result belongs to.
1000+
require.NotNil(t, returnedResult.Items[0].Repository)
1001+
assert.Equal(t, "owner/repo", returnedResult.Items[0].Repository.FullName)
1002+
assert.Equal(t, "https://github.com/owner/repo", returnedResult.Items[0].Repository.HTMLURL)
1003+
1004+
// Second commit has no resolved GitHub user for author/committer
1005+
// and no commit-level author block — the handler must not panic
1006+
// and must omit those fields cleanly.
1007+
require.Len(t, returnedResult.Items, 2)
1008+
assert.Equal(t, "def456commit", returnedResult.Items[1].SHA)
1009+
assert.Nil(t, returnedResult.Items[1].Author)
1010+
assert.Nil(t, returnedResult.Items[1].Committer)
1011+
require.NotNil(t, returnedResult.Items[1].Commit)
1012+
assert.Nil(t, returnedResult.Items[1].Commit.Author)
1013+
assert.Nil(t, returnedResult.Items[1].Commit.Committer)
9801014
})
9811015
}
9821016
}

0 commit comments

Comments
 (0)