Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ The following sets of tools are available:

- **get_commit** - Get commit details
- **Required OAuth Scopes**: `repo`
- `include_diff`: Whether to include file diffs and stats in the response. Default is true. (boolean, optional)
- `detail`: Level of detail to include for changed files. "none" omits stats and files entirely. "stats" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. "full_patch" additionally includes the unified diff content for each file and can be very large. (string, 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)
Expand Down
13 changes: 9 additions & 4 deletions pkg/github/__toolsnaps__/get_commit.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@
"description": "Get details for a commit from a GitHub repository",
"inputSchema": {
"properties": {
"include_diff": {
"default": true,
"description": "Whether to include file diffs and stats in the response. Default is true.",
"type": "boolean"
"detail": {
"default": "stats",
"description": "Level of detail to include for changed files. \"none\" omits stats and files entirely. \"stats\" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. \"full_patch\" additionally includes the unified diff content for each file and can be very large.",
"enum": [
"none",
"stats",
"full_patch"
],
"type": "string"
},
"owner": {
"description": "Repository owner",
Expand Down
73 changes: 52 additions & 21 deletions pkg/github/minimal_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ type MinimalCommitFile struct {
Additions int `json:"additions,omitempty"`
Deletions int `json:"deletions,omitempty"`
Changes int `json:"changes,omitempty"`
Patch string `json:"patch,omitempty"`
}

// MinimalPRFile represents a file changed in a pull request.
Expand Down Expand Up @@ -1463,8 +1464,34 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author
return minimalCommit
}

// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit
func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit {
// commitDetail controls how much per-file information convertToMinimalCommit
// includes in its output.
type commitDetail string

const (
// commitDetailNone omits Stats and Files entirely.
commitDetailNone commitDetail = "none"
// commitDetailStats includes Stats and Files with metadata only
// (filename, status, additions, deletions, changes) but no patch text.
commitDetailStats commitDetail = "stats"
// commitDetailFullPatch additionally includes the unified diff for each file.
commitDetailFullPatch commitDetail = "full_patch"
)

// parseCommitDetail validates the user-supplied detail value and returns the
// default (stats) when the value is empty.
func parseCommitDetail(s string) (commitDetail, error) {
switch s {
case "":
return commitDetailStats, nil
case string(commitDetailNone), string(commitDetailStats), string(commitDetailFullPatch):
return commitDetail(s), nil
default:
return "", fmt.Errorf("invalid detail %q: must be one of \"none\", \"stats\", \"full_patch\"", s)
}
}

func convertToMinimalCommit(commit *github.RepositoryCommit, detail commitDetail) MinimalCommit {
minimalCommit := newMinimalCommitFromCore(
commit.GetSHA(),
commit.GetHTMLURL(),
Expand All @@ -1473,28 +1500,32 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
commit.Committer,
)

// 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 detail == commitDetailNone {
return minimalCommit
}

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)
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(),
}
if detail == commitDetailFullPatch {
minimalFile.Patch = file.GetPatch()
}
minimalCommit.Files = append(minimalCommit.Files, minimalFile)
}
}

Expand Down
19 changes: 12 additions & 7 deletions pkg/github/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool {
Type: "string",
Description: "Commit SHA, branch name, or tag name",
},
"include_diff": {
Type: "boolean",
Description: "Whether to include file diffs and stats in the response. Default is true.",
Default: json.RawMessage(`true`),
"detail": {
Type: "string",
Enum: []any{"none", "stats", "full_patch"},
Description: "Level of detail to include for changed files. \"none\" omits stats and files entirely. \"stats\" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. \"full_patch\" additionally includes the unified diff content for each file and can be very large.",
Default: json.RawMessage(`"stats"`),
},
},
Required: []string{"owner", "repo", "sha"},
Expand All @@ -69,7 +70,11 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool {
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
includeDiff, err := OptionalBoolParamWithDefault(args, "include_diff", true)
detailRaw, err := OptionalParam[string](args, "detail")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
detail, err := parseCommitDetail(detailRaw)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
Expand Down Expand Up @@ -106,7 +111,7 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool {
}

// Convert to minimal commit
minimalCommit := convertToMinimalCommit(commit, includeDiff)
minimalCommit := convertToMinimalCommit(commit, detail)

r, err := json.Marshal(minimalCommit)
if err != nil {
Expand Down Expand Up @@ -252,7 +257,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
// Convert to minimal commits
minimalCommits := make([]MinimalCommit, len(commits))
for i, commit := range commits {
minimalCommits[i] = convertToMinimalCommit(commit, false)
minimalCommits[i] = convertToMinimalCommit(commit, commitDetailNone)
}

r, err := json.Marshal(minimalCommits)
Expand Down
114 changes: 114 additions & 0 deletions pkg/github/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,120 @@ func Test_GetCommit(t *testing.T) {
}
}

func Test_GetCommit_Detail(t *testing.T) {
mockCommit := &github.RepositoryCommit{
SHA: github.Ptr("abc123def456"),
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"),
Commit: &github.Commit{
Message: github.Ptr("First commit"),
},
Stats: &github.CommitStats{
Additions: github.Ptr(10),
Deletions: github.Ptr(2),
Total: github.Ptr(12),
},
Files: []*github.CommitFile{
{
Filename: github.Ptr("file1.go"),
Status: github.Ptr("modified"),
Additions: github.Ptr(10),
Deletions: github.Ptr(2),
Changes: github.Ptr(12),
Patch: github.Ptr("@@ -1,2 +1,10 @@\n+new line"),
},
},
}

cases := []struct {
name string
args map[string]any
expectFiles bool
expectStats bool
expectPatch bool
expectError string
}{
{
name: "default returns stats",
args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456"},
expectFiles: true,
expectStats: true,
expectPatch: false,
},
{
name: "detail=none omits stats and files",
args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "none"},
expectFiles: false,
expectStats: false,
expectPatch: false,
},
{
name: "detail=stats returns metadata without patch",
args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "stats"},
expectFiles: true,
expectStats: true,
expectPatch: false,
},
{
name: "detail=full_patch includes patch text",
args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "full_patch"},
expectFiles: true,
expectStats: true,
expectPatch: true,
},
{
name: "invalid detail value is rejected",
args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "everything"},
expectError: `invalid detail "everything"`,
},
}

serverTool := GetCommit(translations.NullTranslationHelper)

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposCommitsByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockCommit),
})
client := mustNewGHClient(t, mockedClient)
deps := BaseDeps{Client: client}
handler := serverTool.Handler(deps)

request := createMCPRequest(tc.args)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

if tc.expectError != "" {
require.True(t, result.IsError)
assert.Contains(t, getErrorResult(t, result).Text, tc.expectError)
return
}
require.False(t, result.IsError)

var returned MinimalCommit
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned))

if tc.expectStats {
require.NotNil(t, returned.Stats)
assert.Equal(t, 12, returned.Stats.Total)
} else {
assert.Nil(t, returned.Stats)
}

if tc.expectFiles {
require.Len(t, returned.Files, 1)
assert.Equal(t, "file1.go", returned.Files[0].Filename)
if tc.expectPatch {
assert.Equal(t, "@@ -1,2 +1,10 @@\n+new line", returned.Files[0].Patch)
} else {
assert.Empty(t, returned.Files[0].Patch)
}
} else {
assert.Empty(t, returned.Files)
}
})
}
}

func Test_ListCommits(t *testing.T) {
// Verify tool definition once
serverTool := ListCommits(translations.NullTranslationHelper)
Expand Down
Loading