From 3297d58f4a84b2f7e16974e883fafcf361390af4 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 14:58:31 +0100 Subject: [PATCH 1/7] refactor: extract commitFromGitlabCommit seam (server#5479) Pure extraction of the GitLab commit mapping so it can be unit-tested directly. Behaviour unchanged. Co-Authored-By: Claude Opus 4.8 --- internal/gitlab/gitlab.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 281e80acd..4ba6a6ad0 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -174,15 +174,20 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t return commits, err } for _, commit := range glCommits { - commits = append(commits, types.Commit{ - SHA: commit.ID, - Message: commit.Message, - // TODO(server#5479): author is sourced from the committer; switch to AuthorName/AuthorEmail. - Author: fmt.Sprintf("%s <%s>", commit.CommitterName, commit.CommitterEmail), - Timestamp: commit.CreatedAt.Unix(), - Branch: mr.SourceBranch, - URL: commit.WebURL, - }) + commits = append(commits, commitFromGitlabCommit(commit, mr.SourceBranch)) } return commits, nil } + +// commitFromGitlabCommit maps a GitLab API commit to a types.Commit. +func commitFromGitlabCommit(commit *gitlab.Commit, branch string) types.Commit { + return types.Commit{ + SHA: commit.ID, + Message: commit.Message, + // TODO(server#5479): author is sourced from the committer; switch to AuthorName/AuthorEmail. + Author: fmt.Sprintf("%s <%s>", commit.CommitterName, commit.CommitterEmail), + Timestamp: commit.CreatedAt.Unix(), + Branch: branch, + URL: commit.WebURL, + } +} From 2cca9e823632fd967f9609924509b02e4f9dede7 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 14:59:11 +0100 Subject: [PATCH 2/7] fix: record git author (not committer) in GitLab MR commits (server#5479) GetMergeRequestCommits mapped CommitterName/CommitterEmail into the author field. Map AuthorName/AuthorEmail instead. Red-first test with an author != committer fixture. Co-Authored-By: Claude Opus 4.8 --- internal/gitlab/commit_mapping_test.go | 31 ++++++++++++++++++++++++++ internal/gitlab/gitlab.go | 7 +++--- 2 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 internal/gitlab/commit_mapping_test.go diff --git a/internal/gitlab/commit_mapping_test.go b/internal/gitlab/commit_mapping_test.go new file mode 100644 index 000000000..6f0d54491 --- /dev/null +++ b/internal/gitlab/commit_mapping_test.go @@ -0,0 +1,31 @@ +package gitlab + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + gitlab "gitlab.com/gitlab-org/api/client-go" +) + +// TestCommitFromGitlabCommit_RecordsAuthorNotCommitter is a regression test for +// server#5479. PR commit attestations were recording the git committer in the +// author field. They must record the git author. +func TestCommitFromGitlabCommit_RecordsAuthorNotCommitter(t *testing.T) { + created := time.Unix(1772635812, 0) + commit := &gitlab.Commit{ + ID: "abc1230000000000000000000000000000000000", + Message: "Apply suggestions from code review", + AuthorName: "Steve Tooke", + AuthorEmail: "tooky@kosli.com", + CommitterName: "GitHub", + CommitterEmail: "noreply@github.com", + CreatedAt: &created, + WebURL: "https://gitlab.com/kosli/x/-/commit/abc123", + } + + c := commitFromGitlabCommit(commit, "my-branch") + + require.Equal(t, "Steve Tooke ", c.Author, + "author must be the git author, not the committer") +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 4ba6a6ad0..7f990b77a 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -182,10 +182,9 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t // commitFromGitlabCommit maps a GitLab API commit to a types.Commit. func commitFromGitlabCommit(commit *gitlab.Commit, branch string) types.Commit { return types.Commit{ - SHA: commit.ID, - Message: commit.Message, - // TODO(server#5479): author is sourced from the committer; switch to AuthorName/AuthorEmail. - Author: fmt.Sprintf("%s <%s>", commit.CommitterName, commit.CommitterEmail), + SHA: commit.ID, + Message: commit.Message, + Author: fmt.Sprintf("%s <%s>", commit.AuthorName, commit.AuthorEmail), Timestamp: commit.CreatedAt.Unix(), Branch: branch, URL: commit.WebURL, From c30fbcf3374a9309e48b39c32558a23321aa3159 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 14:59:59 +0100 Subject: [PATCH 3/7] refactor: extract commitFromAzureCommit seam (server#5479) Pure extraction of the Azure commit mapping for direct unit testing. Behaviour unchanged. Co-Authored-By: Claude Opus 4.8 --- internal/azure/azure.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/internal/azure/azure.go b/internal/azure/azure.go index a7d57c028..b15dafdac 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -167,21 +167,26 @@ func (c *AzureConfig) GetPullRequestCommits(pr git.GitPullRequest) ([]types.Comm } for _, commit := range prCommitsResponse.Value { - commits = append(commits, types.Commit{ - SHA: *commit.CommitId, - Message: *commit.Comment, - Author: *commit.Author.Name, - Timestamp: commit.Committer.Date.Time.Unix(), - URL: *commit.Url, - Branch: *pr.SourceRefName, - // TODO(server#5479): AuthorUsername is sourced from the committer; should be the author. - AuthorUsername: *commit.Committer.Name, - }) + commits = append(commits, commitFromAzureCommit(commit, *pr.SourceRefName)) } return commits, nil } +// commitFromAzureCommit maps an Azure DevOps API commit to a types.Commit. +func commitFromAzureCommit(commit git.GitCommitRef, branch string) types.Commit { + return types.Commit{ + SHA: *commit.CommitId, + Message: *commit.Comment, + Author: *commit.Author.Name, + Timestamp: commit.Committer.Date.Time.Unix(), + URL: *commit.Url, + Branch: branch, + // TODO(server#5479): AuthorUsername is sourced from the committer; should be the author. + AuthorUsername: *commit.Committer.Name, + } +} + // PullRequestsForCommit returns a list of pull requests for a specific commit func (c *AzureConfig) PullRequestsForCommit(commit string) ([]git.GitPullRequest, error) { ctx := context.Background() From 2eaee34f1b1faf85f7ae1db31c85db2892de8484 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:01:06 +0100 Subject: [PATCH 4/7] fix: record git author identity in Azure PR commits (server#5479) Azure recorded the author display name only and populated author_username from the committer. Record the author as "Name " (consistent with the other providers) and drop author_username: Azure commits carry no login, so the only honest options are the committer name (wrong) or nothing. Red-first test asserts the format and the dropped field. Co-Authored-By: Claude Opus 4.8 --- internal/azure/azure.go | 4 +-- internal/azure/commit_mapping_test.go | 45 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 internal/azure/commit_mapping_test.go diff --git a/internal/azure/azure.go b/internal/azure/azure.go index b15dafdac..53cf64549 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -178,12 +178,10 @@ func commitFromAzureCommit(commit git.GitCommitRef, branch string) types.Commit return types.Commit{ SHA: *commit.CommitId, Message: *commit.Comment, - Author: *commit.Author.Name, + Author: fmt.Sprintf("%s <%s>", *commit.Author.Name, *commit.Author.Email), Timestamp: commit.Committer.Date.Time.Unix(), URL: *commit.Url, Branch: branch, - // TODO(server#5479): AuthorUsername is sourced from the committer; should be the author. - AuthorUsername: *commit.Committer.Name, } } diff --git a/internal/azure/commit_mapping_test.go b/internal/azure/commit_mapping_test.go new file mode 100644 index 000000000..7300a688e --- /dev/null +++ b/internal/azure/commit_mapping_test.go @@ -0,0 +1,45 @@ +package azure + +import ( + "testing" + "time" + + "github.com/microsoft/azure-devops-go-api/azuredevops" + "github.com/microsoft/azure-devops-go-api/azuredevops/git" + "github.com/stretchr/testify/require" +) + +func azStr(s string) *string { return &s } + +func azTestCommit() git.GitCommitRef { + authorDate := azuredevops.Time{Time: time.Unix(1772630000, 0)} + committerDate := azuredevops.Time{Time: time.Unix(1772635812, 0)} + return git.GitCommitRef{ + CommitId: azStr("abc1230000000000000000000000000000000000"), + Comment: azStr("Apply suggestions from code review"), + Url: azStr("https://dev.azure.com/kosli/_git/x/commit/abc123"), + Author: &git.GitUserDate{ + Name: azStr("Steve Tooke"), + Email: azStr("tooky@kosli.com"), + Date: &authorDate, + }, + Committer: &git.GitUserDate{ + Name: azStr("GitHub"), + Email: azStr("noreply@github.com"), + Date: &committerDate, + }, + } +} + +// TestCommitFromAzureCommit_RecordsAuthorIdentity is a regression test for +// server#5479. Azure recorded the author display name only, and populated +// author_username from the committer. Record the author as "Name ", +// and drop author_username (Azure commits carry no login). +func TestCommitFromAzureCommit_RecordsAuthorIdentity(t *testing.T) { + c := commitFromAzureCommit(azTestCommit(), "my-branch") + + require.Equal(t, "Steve Tooke ", c.Author, + "author must be name of the git author") + require.Empty(t, c.AuthorUsername, + "Azure commits carry no login; author_username must be omitted, not the committer name") +} From 3714f8985ab0ab2f73c2a882f6b94a8b466231f0 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:02:20 +0100 Subject: [PATCH 5/7] fix: use authored date for GitHub PR commit timestamp (server#5479) Fetch authoredDate in graphqlCommitNode and use it for the commit timestamp, falling back to committedDate when absent. Matches the recorded author identity; for rebased/applied-suggestion commits the two dates differ. Red-first test plus a fallback test. Co-Authored-By: Claude Opus 4.8 --- internal/github/build_pr_evidence_test.go | 55 +++++++++++++++++++++++ internal/github/github.go | 11 +++-- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index d5f01608d..e27d06d2f 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -2,6 +2,7 @@ package github import ( "testing" + "time" "github.com/shurcooL/graphql" "github.com/stretchr/testify/require" @@ -53,3 +54,57 @@ func TestBuildPREvidence_RecordsAuthorNotCommitter(t *testing.T) { require.Equal(t, "tooky", c.AuthorUsername, "author_username must be the author's GitHub login, not the committer's (absent) one") } + +// TestBuildPREvidence_UsesAuthoredDate is a regression test for server#5479. +// Now that the recorded identity is the author, the timestamp should be the +// author date too. For rebased / applied-suggestion commits the authored and +// committed dates differ. +func TestBuildPREvidence_UsesAuthoredDate(t *testing.T) { + node := graphqlCommitNode{} + node.Commit.Oid = "0e723254516c841126e81f76100be57258ff1386" + node.Commit.MessageHeadline = "Apply suggestions from code review" + node.Commit.AuthoredDate = "2026-03-01T10:00:00Z" + node.Commit.CommittedDate = "2026-03-01T12:00:00Z" + node.Commit.Author.Name = "Steve Tooke" + node.Commit.Author.Email = "tooky@kosli.com" + + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", "tooky", "2026-03-01T09:00:00Z", "", + "Introduce kosli evaluate", "introduce-kosli-evaluate", + []graphqlCommitNode{node}, nil, + ) + require.NoError(t, err) + require.Len(t, evidence.Commits, 1) + + wantAuthored, _ := time.Parse(time.RFC3339, "2026-03-01T10:00:00Z") + require.Equal(t, wantAuthored.Unix(), evidence.Commits[0].Timestamp, + "timestamp must be the author date, not the committer date") +} + +// TestBuildPREvidence_FallsBackToCommittedDate ensures the timestamp falls back +// to the committed date when the GraphQL response omits the authored date. +func TestBuildPREvidence_FallsBackToCommittedDate(t *testing.T) { + node := graphqlCommitNode{} + node.Commit.Oid = "0e723254516c841126e81f76100be57258ff1386" + node.Commit.MessageHeadline = "msg" + node.Commit.AuthoredDate = "" + node.Commit.CommittedDate = "2026-03-01T12:00:00Z" + node.Commit.Author.Name = "Steve Tooke" + node.Commit.Author.Email = "tooky@kosli.com" + + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", "tooky", "2026-03-01T09:00:00Z", "", + "title", "branch", + []graphqlCommitNode{node}, nil, + ) + require.NoError(t, err) + require.Len(t, evidence.Commits, 1) + + wantCommitted, _ := time.Parse(time.RFC3339, "2026-03-01T12:00:00Z") + require.Equal(t, wantCommitted.Unix(), evidence.Commits[0].Timestamp, + "timestamp must fall back to the committed date when authored date is absent") +} diff --git a/internal/github/github.go b/internal/github/github.go index 886a781a0..cb9b4fac8 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -263,6 +263,7 @@ type graphqlCommitNode struct { Oid graphql.String MessageHeadline graphql.String CommittedDate graphql.String + AuthoredDate graphql.String URL graphql.String Committer struct { Name graphql.String @@ -326,9 +327,13 @@ func buildPREvidence( } for _, n := range commitNodes { - // TODO(server#5479): timestamp uses the committer date; consider authoredDate for - // author-consistency (cross-provider: also Azure commit.Committer.Date, GitLab CreatedAt). - timestamp, err := time.Parse(time.RFC3339, string(n.Commit.CommittedDate)) + // Use the author date to match the recorded author identity; fall back to + // the committed date if the API omits it (server#5479). + dateStr := string(n.Commit.AuthoredDate) + if dateStr == "" { + dateStr = string(n.Commit.CommittedDate) + } + timestamp, err := time.Parse(time.RFC3339, dateStr) if err != nil { return nil, err } From 52f7aba164b25a704056e72bd8365fe89e0eed44 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:02:55 +0100 Subject: [PATCH 6/7] fix: use author date for Azure PR commit timestamp (server#5479) Timestamp now sourced from commit.Author.Date rather than the committer date, matching the recorded author identity. Red-first test. Co-Authored-By: Claude Opus 4.8 --- internal/azure/azure.go | 2 +- internal/azure/commit_mapping_test.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/azure/azure.go b/internal/azure/azure.go index 53cf64549..b83cf56ab 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -179,7 +179,7 @@ func commitFromAzureCommit(commit git.GitCommitRef, branch string) types.Commit SHA: *commit.CommitId, Message: *commit.Comment, Author: fmt.Sprintf("%s <%s>", *commit.Author.Name, *commit.Author.Email), - Timestamp: commit.Committer.Date.Time.Unix(), + Timestamp: commit.Author.Date.Time.Unix(), URL: *commit.Url, Branch: branch, } diff --git a/internal/azure/commit_mapping_test.go b/internal/azure/commit_mapping_test.go index 7300a688e..5343334cd 100644 --- a/internal/azure/commit_mapping_test.go +++ b/internal/azure/commit_mapping_test.go @@ -43,3 +43,12 @@ func TestCommitFromAzureCommit_RecordsAuthorIdentity(t *testing.T) { require.Empty(t, c.AuthorUsername, "Azure commits carry no login; author_username must be omitted, not the committer name") } + +// TestCommitFromAzureCommit_UsesAuthorDate is a regression test for server#5479: +// the timestamp must match the recorded author identity. +func TestCommitFromAzureCommit_UsesAuthorDate(t *testing.T) { + c := commitFromAzureCommit(azTestCommit(), "my-branch") + + require.Equal(t, int64(1772630000), c.Timestamp, + "timestamp must be the author date, not the committer date") +} From d075b7e6fbcadac7ab6d5eba9600f2d68f0bad6f Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:03:41 +0100 Subject: [PATCH 7/7] fix: use authored date for GitLab MR commit timestamp (server#5479) Prefer AuthoredDate, falling back to CreatedAt when nil. Matches the recorded author identity. Red-first test plus a fallback test. Co-Authored-By: Claude Opus 4.8 --- internal/gitlab/commit_mapping_test.go | 41 ++++++++++++++++++++++++++ internal/gitlab/gitlab.go | 8 ++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/gitlab/commit_mapping_test.go b/internal/gitlab/commit_mapping_test.go index 6f0d54491..15554426a 100644 --- a/internal/gitlab/commit_mapping_test.go +++ b/internal/gitlab/commit_mapping_test.go @@ -29,3 +29,44 @@ func TestCommitFromGitlabCommit_RecordsAuthorNotCommitter(t *testing.T) { require.Equal(t, "Steve Tooke ", c.Author, "author must be the git author, not the committer") } + +// TestCommitFromGitlabCommit_UsesAuthoredDate is a regression test for +// server#5479: the timestamp must match the recorded author identity. +func TestCommitFromGitlabCommit_UsesAuthoredDate(t *testing.T) { + authored := time.Unix(1772630000, 0) + created := time.Unix(1772635812, 0) + commit := &gitlab.Commit{ + ID: "abc1230000000000000000000000000000000000", + Message: "msg", + AuthorName: "Steve Tooke", + AuthorEmail: "tooky@kosli.com", + AuthoredDate: &authored, + CreatedAt: &created, + WebURL: "https://gitlab.com/kosli/x/-/commit/abc123", + } + + c := commitFromGitlabCommit(commit, "my-branch") + + require.Equal(t, int64(1772630000), c.Timestamp, + "timestamp must be the authored date, not created_at") +} + +// TestCommitFromGitlabCommit_FallsBackToCreatedAt ensures the timestamp falls +// back to created_at when the API omits the authored date. +func TestCommitFromGitlabCommit_FallsBackToCreatedAt(t *testing.T) { + created := time.Unix(1772635812, 0) + commit := &gitlab.Commit{ + ID: "abc1230000000000000000000000000000000000", + Message: "msg", + AuthorName: "Steve Tooke", + AuthorEmail: "tooky@kosli.com", + AuthoredDate: nil, + CreatedAt: &created, + WebURL: "https://gitlab.com/kosli/x/-/commit/abc123", + } + + c := commitFromGitlabCommit(commit, "my-branch") + + require.Equal(t, int64(1772635812), c.Timestamp, + "timestamp must fall back to created_at when authored date is absent") +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 7f990b77a..bb901ac67 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -181,11 +181,17 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t // commitFromGitlabCommit maps a GitLab API commit to a types.Commit. func commitFromGitlabCommit(commit *gitlab.Commit, branch string) types.Commit { + // Use the authored date to match the recorded author identity; fall back to + // created_at if the API omits it (server#5479). + timestamp := commit.CreatedAt.Unix() + if commit.AuthoredDate != nil { + timestamp = commit.AuthoredDate.Unix() + } return types.Commit{ SHA: commit.ID, Message: commit.Message, Author: fmt.Sprintf("%s <%s>", commit.AuthorName, commit.AuthorEmail), - Timestamp: commit.CreatedAt.Unix(), + Timestamp: timestamp, Branch: branch, URL: commit.WebURL, }