Skip to content
23 changes: 13 additions & 10 deletions internal/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,24 @@ 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: fmt.Sprintf("%s <%s>", *commit.Author.Name, *commit.Author.Email),
Timestamp: commit.Author.Date.Time.Unix(),
Comment thread
AlexKantor87 marked this conversation as resolved.
URL: *commit.Url,
Branch: branch,
}
}

// PullRequestsForCommit returns a list of pull requests for a specific commit
func (c *AzureConfig) PullRequestsForCommit(commit string) ([]git.GitPullRequest, error) {
ctx := context.Background()
Expand Down
54 changes: 54 additions & 0 deletions internal/azure/commit_mapping_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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 <email>",
// 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 <tooky@kosli.com>", c.Author,
"author must be name <email> of the git author")
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")
}
55 changes: 55 additions & 0 deletions internal/github/build_pr_evidence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package github

import (
"testing"
"time"

"github.com/shurcooL/graphql"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -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")
}
11 changes: 8 additions & 3 deletions internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
72 changes: 72 additions & 0 deletions internal/gitlab/commit_mapping_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
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 <tooky@kosli.com>", 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")
}
28 changes: 19 additions & 9 deletions internal/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,25 @@ 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 {
// 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: timestamp,
Branch: branch,
URL: commit.WebURL,
}
}
Loading