Skip to content

Commit 4269157

Browse files
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 Repository field on MinimalCommit so cross-repo commit search results identify the repo each commit came from (the API returns it; we were trimming it away). - 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 4269157

5 files changed

Lines changed: 126 additions & 97 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: 82 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,23 @@ type MinimalPRFile struct {
121121

122122
// MinimalCommit is the trimmed output type for commit objects.
123123
type MinimalCommit struct {
124-
SHA string `json:"sha"`
125-
HTMLURL string `json:"html_url"`
126-
Commit *MinimalCommitInfo `json:"commit,omitempty"`
127-
Author *MinimalUser `json:"author,omitempty"`
128-
Committer *MinimalUser `json:"committer,omitempty"`
129-
Stats *MinimalCommitStats `json:"stats,omitempty"`
130-
Files []MinimalCommitFile `json:"files,omitempty"`
124+
SHA string `json:"sha"`
125+
HTMLURL string `json:"html_url"`
126+
Commit *MinimalCommitInfo `json:"commit,omitempty"`
127+
Author *MinimalUser `json:"author,omitempty"`
128+
Committer *MinimalUser `json:"committer,omitempty"`
129+
Repository *MinimalRepoRef `json:"repository,omitempty"`
130+
Stats *MinimalCommitStats `json:"stats,omitempty"`
131+
Files []MinimalCommitFile `json:"files,omitempty"`
132+
}
133+
134+
// MinimalRepoRef is a lightweight reference to a repository, used when a
135+
// result needs to identify which repository it belongs to (for example, in
136+
// cross-repo commit search results).
137+
type MinimalRepoRef struct {
138+
FullName string `json:"full_name"`
139+
HTMLURL string `json:"html_url,omitempty"`
140+
Private bool `json:"private,omitempty"`
131141
}
132142

133143
// MinimalRelease is the trimmed output type for release objects.
@@ -700,57 +710,73 @@ func convertToMinimalUser(user *github.User) *MinimalUser {
700710
}
701711
}
702712

703-
// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit
704-
func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit {
713+
// newMinimalCommitFromCore builds a MinimalCommit from the fields that are
714+
// shared between *github.RepositoryCommit and *github.CommitResult. Caller
715+
// is responsible for setting any type-specific extras (stats/files for
716+
// RepositoryCommit, repository for CommitResult).
717+
func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author, committer *github.User) MinimalCommit {
705718
minimalCommit := MinimalCommit{
706-
SHA: commit.GetSHA(),
707-
HTMLURL: commit.GetHTMLURL(),
719+
SHA: sha,
720+
HTMLURL: htmlURL,
708721
}
709722

710-
if commit.Commit != nil {
723+
if commit != nil {
711724
minimalCommit.Commit = &MinimalCommitInfo{
712-
Message: commit.Commit.GetMessage(),
725+
Message: commit.GetMessage(),
713726
}
714727

715-
if commit.Commit.Author != nil {
728+
if commit.Author != nil {
716729
minimalCommit.Commit.Author = &MinimalCommitAuthor{
717-
Name: commit.Commit.Author.GetName(),
718-
Email: commit.Commit.Author.GetEmail(),
730+
Name: commit.Author.GetName(),
731+
Email: commit.Author.GetEmail(),
719732
}
720-
if commit.Commit.Author.Date != nil {
721-
minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format(time.RFC3339)
733+
if commit.Author.Date != nil {
734+
minimalCommit.Commit.Author.Date = commit.Author.Date.Format(time.RFC3339)
722735
}
723736
}
724737

725-
if commit.Commit.Committer != nil {
738+
if commit.Committer != nil {
726739
minimalCommit.Commit.Committer = &MinimalCommitAuthor{
727-
Name: commit.Commit.Committer.GetName(),
728-
Email: commit.Commit.Committer.GetEmail(),
740+
Name: commit.Committer.GetName(),
741+
Email: commit.Committer.GetEmail(),
729742
}
730-
if commit.Commit.Committer.Date != nil {
731-
minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format(time.RFC3339)
743+
if commit.Committer.Date != nil {
744+
minimalCommit.Commit.Committer.Date = commit.Committer.Date.Format(time.RFC3339)
732745
}
733746
}
734747
}
735748

736-
if commit.Author != nil {
749+
if author != nil {
737750
minimalCommit.Author = &MinimalUser{
738-
Login: commit.Author.GetLogin(),
739-
ID: commit.Author.GetID(),
740-
ProfileURL: commit.Author.GetHTMLURL(),
741-
AvatarURL: commit.Author.GetAvatarURL(),
751+
Login: author.GetLogin(),
752+
ID: author.GetID(),
753+
ProfileURL: author.GetHTMLURL(),
754+
AvatarURL: author.GetAvatarURL(),
742755
}
743756
}
744757

745-
if commit.Committer != nil {
758+
if committer != nil {
746759
minimalCommit.Committer = &MinimalUser{
747-
Login: commit.Committer.GetLogin(),
748-
ID: commit.Committer.GetID(),
749-
ProfileURL: commit.Committer.GetHTMLURL(),
750-
AvatarURL: commit.Committer.GetAvatarURL(),
760+
Login: committer.GetLogin(),
761+
ID: committer.GetID(),
762+
ProfileURL: committer.GetHTMLURL(),
763+
AvatarURL: committer.GetAvatarURL(),
751764
}
752765
}
753766

767+
return minimalCommit
768+
}
769+
770+
// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit
771+
func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit {
772+
minimalCommit := newMinimalCommitFromCore(
773+
commit.GetSHA(),
774+
commit.GetHTMLURL(),
775+
commit.Commit,
776+
commit.Author,
777+
commit.Committer,
778+
)
779+
754780
// Only include stats and files if includeDiffs is true
755781
if includeDiffs {
756782
if commit.Stats != nil {
@@ -779,6 +805,29 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
779805
return minimalCommit
780806
}
781807

808+
// convertCommitResultToMinimalCommit converts a GitHub API commit search
809+
// result to a MinimalCommit, attaching the containing repository so the
810+
// caller can tell which repo each result came from.
811+
func convertCommitResultToMinimalCommit(commit *github.CommitResult) MinimalCommit {
812+
minimalCommit := newMinimalCommitFromCore(
813+
commit.GetSHA(),
814+
commit.GetHTMLURL(),
815+
commit.Commit,
816+
commit.Author,
817+
commit.Committer,
818+
)
819+
820+
if commit.Repository != nil {
821+
minimalCommit.Repository = &MinimalRepoRef{
822+
FullName: commit.Repository.GetFullName(),
823+
HTMLURL: commit.Repository.GetHTMLURL(),
824+
Private: commit.Repository.GetPrivate(),
825+
}
826+
}
827+
828+
return minimalCommit
829+
}
830+
782831
// MinimalPageInfo contains pagination cursor information.
783832
type MinimalPageInfo struct {
784833
HasNextPage bool `json:"hasNextPage"`

pkg/github/search.go

Lines changed: 3 additions & 57 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,59 +564,6 @@ 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-
621567
minimalCommits := make([]MinimalCommit, 0, len(result.Commits))
622568
for _, commit := range result.Commits {
623569
minimalCommits = append(minimalCommits, convertCommitResultToMinimalCommit(commit))

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)