Skip to content

Commit 7ddf51a

Browse files
MayorFajMayorFaj
committed
resolve annotated tags to their target commit in get_file_blame
1 parent 4d2b21c commit 7ddf51a

2 files changed

Lines changed: 136 additions & 50 deletions

File tree

‎pkg/github/repositories.go‎

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,6 +2281,30 @@ type BlameResult struct {
22812281
Truncated bool `json:"truncated,omitempty"`
22822282
}
22832283

2284+
// blameCommitFragment is the GraphQL selection for a Commit's blame data.
2285+
type blameCommitFragment struct {
2286+
Blame struct {
2287+
Ranges []struct {
2288+
StartingLine githubv4.Int
2289+
EndingLine githubv4.Int
2290+
Age githubv4.Int
2291+
Commit struct {
2292+
OID githubv4.String
2293+
Message githubv4.String
2294+
CommittedDate githubv4.DateTime
2295+
Author struct {
2296+
Name githubv4.String
2297+
Email githubv4.String
2298+
User *struct {
2299+
Login githubv4.String
2300+
URL githubv4.String
2301+
}
2302+
}
2303+
}
2304+
}
2305+
} `graphql:"blame(path: $path)"`
2306+
}
2307+
22842308
// validateBlamePath rejects empty, leading-slash, traversal-laden, or
22852309
// control-character paths before any network call is made.
22862310
func validateBlamePath(p string) error {
@@ -2430,29 +2454,16 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
24302454
Name githubv4.String
24312455
}
24322456
Object struct {
2433-
Typename githubv4.String `graphql:"__typename"`
2434-
Commit struct {
2435-
Blame struct {
2436-
Ranges []struct {
2437-
StartingLine githubv4.Int
2438-
EndingLine githubv4.Int
2439-
Age githubv4.Int
2440-
Commit struct {
2441-
OID githubv4.String
2442-
Message githubv4.String
2443-
CommittedDate githubv4.DateTime
2444-
Author struct {
2445-
Name githubv4.String
2446-
Email githubv4.String
2447-
User *struct {
2448-
Login githubv4.String
2449-
URL githubv4.String
2450-
}
2451-
}
2452-
}
2453-
}
2454-
} `graphql:"blame(path: $path)"`
2455-
} `graphql:"... on Commit"`
2457+
Typename githubv4.String `graphql:"__typename"`
2458+
Commit blameCommitFragment `graphql:"... on Commit"`
2459+
// Annotated tag targets are followed one level. Tag-of-tag
2460+
// chains are not followed and will return an error.
2461+
Tag struct {
2462+
Target struct {
2463+
Typename githubv4.String `graphql:"__typename"`
2464+
Commit blameCommitFragment `graphql:"... on Commit"`
2465+
}
2466+
} `graphql:"... on Tag"`
24562467
} `graphql:"object(expression: $ref)"`
24572468
} `graphql:"repository(owner: $owner, name: $repo)"`
24582469
}
@@ -2471,15 +2482,29 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
24712482
), nil, nil
24722483
}
24732484

2474-
// The ref must resolve to a commit. Empty typename means not found;
2475-
// any other type is unsupported for blame.
2485+
// GitHub's Commit.blame field accepts only path, and Blame.ranges is
2486+
// not a connection, so cursor pagination is applied locally below.
2487+
// The ref must resolve to a commit, either directly or via an annotated tag.
24762488
objectTypename := string(blameQuery.Repository.Object.Typename)
24772489
if objectTypename == "" {
24782490
return utils.NewToolResultError(
24792491
fmt.Sprintf("ref %q was not found in %s/%s", refExpression, owner, repo),
24802492
), nil, nil
24812493
}
2482-
if objectTypename != "Commit" {
2494+
blameCommit := &blameQuery.Repository.Object.Commit
2495+
if objectTypename == "Tag" {
2496+
targetTypename := string(blameQuery.Repository.Object.Tag.Target.Typename)
2497+
if targetTypename != "Commit" {
2498+
if targetTypename == "" {
2499+
targetTypename = "unknown"
2500+
}
2501+
return utils.NewToolResultError(
2502+
fmt.Sprintf("ref %q resolved to a tag in %s/%s, but the tag target did not resolve to a commit (resolved to %s)",
2503+
refExpression, owner, repo, targetTypename),
2504+
), nil, nil
2505+
}
2506+
blameCommit = &blameQuery.Repository.Object.Tag.Target.Commit
2507+
} else if objectTypename != "Commit" {
24832508
return utils.NewToolResultError(
24842509
fmt.Sprintf("ref %q did not resolve to a commit in %s/%s (resolved to %s)",
24852510
refExpression, owner, repo, objectTypename),
@@ -2496,7 +2521,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
24962521
}
24972522
}
24982523

2499-
rawRanges := blameQuery.Repository.Object.Commit.Blame.Ranges
2524+
rawRanges := blameCommit.Blame.Ranges
25002525
pageRanges := make([]BlameRange, 0, pagination.PerPage)
25012526
commits := make(map[string]BlameCommit)
25022527
totalRanges := 0

‎pkg/github/repositories_test.go‎

Lines changed: 84 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4397,29 +4397,14 @@ func Test_GetFileBlame(t *testing.T) {
43974397
Name githubv4.String
43984398
}
43994399
Object struct {
4400-
Typename githubv4.String `graphql:"__typename"`
4401-
Commit struct {
4402-
Blame struct {
4403-
Ranges []struct {
4404-
StartingLine githubv4.Int
4405-
EndingLine githubv4.Int
4406-
Age githubv4.Int
4407-
Commit struct {
4408-
OID githubv4.String
4409-
Message githubv4.String
4410-
CommittedDate githubv4.DateTime
4411-
Author struct {
4412-
Name githubv4.String
4413-
Email githubv4.String
4414-
User *struct {
4415-
Login githubv4.String
4416-
URL githubv4.String
4417-
}
4418-
}
4419-
}
4420-
}
4421-
} `graphql:"blame(path: $path)"`
4422-
} `graphql:"... on Commit"`
4400+
Typename githubv4.String `graphql:"__typename"`
4401+
Commit blameCommitFragment `graphql:"... on Commit"`
4402+
Tag struct {
4403+
Target struct {
4404+
Typename githubv4.String `graphql:"__typename"`
4405+
Commit blameCommitFragment `graphql:"... on Commit"`
4406+
}
4407+
} `graphql:"... on Tag"`
44234408
} `graphql:"object(expression: $ref)"`
44244409
} `graphql:"repository(owner: $owner, name: $repo)"`
44254410
}
@@ -4575,6 +4560,56 @@ func Test_GetFileBlame(t *testing.T) {
45754560
assert.Nil(t, br.Commits["xyz789abc123"].Author.Login, "anonymous author has no login")
45764561
},
45774562
},
4563+
{
4564+
name: "successful blame with annotated tag ref",
4565+
mockedClient: githubv4mock.NewMockedHTTPClient(
4566+
githubv4mock.NewQueryMatcher(
4567+
blameQueryShape{},
4568+
makeBlameVars("testowner", "testrepo", "v1.0.0", "src/tagged.go"),
4569+
githubv4mock.DataResponse(map[string]any{
4570+
"repository": map[string]any{
4571+
"defaultBranchRef": map[string]any{"name": "main"},
4572+
"object": map[string]any{
4573+
"__typename": "Tag",
4574+
"target": map[string]any{
4575+
"__typename": "Commit",
4576+
"blame": map[string]any{
4577+
"ranges": []map[string]any{
4578+
{
4579+
"startingLine": 1, "endingLine": 2, "age": 1,
4580+
"commit": map[string]any{
4581+
"oid": "taggedcommit123",
4582+
"message": "Tagged release commit",
4583+
"committedDate": "2024-01-04T10:00:00Z",
4584+
"author": map[string]any{"name": "Tag Author", "email": "tag@example.com", "user": nil},
4585+
},
4586+
},
4587+
},
4588+
},
4589+
},
4590+
},
4591+
},
4592+
}),
4593+
),
4594+
),
4595+
requestArgs: map[string]any{
4596+
"owner": "testowner",
4597+
"repo": "testrepo",
4598+
"path": "src/tagged.go",
4599+
"ref": "v1.0.0",
4600+
},
4601+
validateResponse: func(t *testing.T, result string) {
4602+
var br BlameResult
4603+
require.NoError(t, json.Unmarshal([]byte(result), &br))
4604+
assert.Equal(t, "v1.0.0", br.Ref, "explicit annotated tag ref echoed back")
4605+
require.Len(t, br.Ranges, 1)
4606+
assert.Equal(t, "taggedcommit123", br.Ranges[0].CommitSHA)
4607+
require.Contains(t, br.Commits, "taggedcommit123")
4608+
assert.Equal(t, "Tagged release commit", br.Commits["taggedcommit123"].MessageHeadline,
4609+
"commit metadata threads through the Tag.Target.Commit path")
4610+
assert.Equal(t, "Tag Author", br.Commits["taggedcommit123"].Author.Name)
4611+
},
4612+
},
45784613
{
45794614
name: "empty blame ranges",
45804615
mockedClient: githubv4mock.NewMockedHTTPClient(
@@ -4659,6 +4694,32 @@ func Test_GetFileBlame(t *testing.T) {
46594694
expectError: true,
46604695
expectedErrMsg: "was not found",
46614696
},
4697+
{
4698+
name: "annotated tag target is not commit",
4699+
mockedClient: githubv4mock.NewMockedHTTPClient(
4700+
githubv4mock.NewQueryMatcher(
4701+
blameQueryShape{},
4702+
makeBlameVars("testowner", "testrepo", "tree-tag", "README.md"),
4703+
githubv4mock.DataResponse(map[string]any{
4704+
"repository": map[string]any{
4705+
"defaultBranchRef": map[string]any{"name": "main"},
4706+
"object": map[string]any{
4707+
"__typename": "Tag",
4708+
"target": map[string]any{"__typename": "Tree"},
4709+
},
4710+
},
4711+
}),
4712+
),
4713+
),
4714+
requestArgs: map[string]any{
4715+
"owner": "testowner",
4716+
"repo": "testrepo",
4717+
"path": "README.md",
4718+
"ref": "tree-tag",
4719+
},
4720+
expectError: true,
4721+
expectedErrMsg: "tag target did not resolve to a commit",
4722+
},
46624723
{
46634724
name: "line-range filter clamps and drops ranges",
46644725
mockedClient: githubv4mock.NewMockedHTTPClient(

0 commit comments

Comments
 (0)