From b9c23da81687ba8d6a53470a0104bf8ea85543a8 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 12:08:22 +0100 Subject: [PATCH 01/13] refactor: rename Commit.Committer to Author (server#5479) The types.Commit Go field was named Committer but tagged json:"author", the naming mismatch at the root of the author/committer conflation in PR attestations. Rename the field (and CommitterUsername -> AuthorUsername) so the Go name matches the wire contract. Pure rename: the json tags are unchanged, so wire output is identical and behaviour is preserved across all four providers. Each provider's data source (which currently still feeds committer data into some of these fields) is corrected in follow-up commits, now visible at the construction sites. Co-Authored-By: Claude Fable 5 --- internal/azure/azure.go | 14 +++++++------- internal/bitbucket/bitbucket.go | 2 +- internal/github/github.go | 14 +++++++------- internal/gitlab/gitlab.go | 2 +- internal/types/types.go | 14 +++++++------- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/internal/azure/azure.go b/internal/azure/azure.go index 6f6628484..6d6a7c0a2 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -168,13 +168,13 @@ 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, - Committer: *commit.Author.Name, - Timestamp: commit.Committer.Date.Time.Unix(), - URL: *commit.Url, - Branch: *pr.SourceRefName, - CommitterUsername: *commit.Committer.Name, + SHA: *commit.CommitId, + Message: *commit.Comment, + Author: *commit.Author.Name, + Timestamp: commit.Committer.Date.Time.Unix(), + URL: *commit.Url, + Branch: *pr.SourceRefName, + AuthorUsername: *commit.Committer.Name, }) } diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index 40cb9cc7a..e535d0010 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -243,7 +243,7 @@ func (c *Config) getPullRequestCommitsFromBitbucket(prID int) ([]types.Commit, e allCommits = append(allCommits, types.Commit{ SHA: commit["hash"].(string), Message: commit["message"].(string), - Committer: commit["author"].(map[string]any)["raw"].(string), + Author: commit["author"].(map[string]any)["raw"].(string), Timestamp: timestamp, URL: commit["links"].(map[string]any)["html"].(map[string]any)["href"].(string), }) diff --git a/internal/github/github.go b/internal/github/github.go index b9f295420..6f334a9a8 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -328,13 +328,13 @@ func buildPREvidence( committerUsername = string(n.Commit.Committer.User.Login) } evidence.Commits = append(evidence.Commits, types.Commit{ - SHA: string(n.Commit.Oid), - Message: string(n.Commit.MessageHeadline), - Committer: fmt.Sprintf("%s <%s>", string(n.Commit.Committer.Name), string(n.Commit.Committer.Email)), - CommitterUsername: committerUsername, - Timestamp: timestamp.Unix(), - Branch: headRef, - URL: string(n.Commit.URL), + SHA: string(n.Commit.Oid), + Message: string(n.Commit.MessageHeadline), + Author: fmt.Sprintf("%s <%s>", string(n.Commit.Committer.Name), string(n.Commit.Committer.Email)), + AuthorUsername: committerUsername, + Timestamp: timestamp.Unix(), + Branch: headRef, + URL: string(n.Commit.URL), }) } diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index a58226763..0d1ff972c 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -177,7 +177,7 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t commits = append(commits, types.Commit{ SHA: commit.ID, Message: commit.Message, - Committer: fmt.Sprintf("%s <%s>", commit.CommitterName, commit.CommitterEmail), + Author: fmt.Sprintf("%s <%s>", commit.CommitterName, commit.CommitterEmail), Timestamp: commit.CreatedAt.Unix(), Branch: mr.SourceBranch, URL: commit.WebURL, diff --git a/internal/types/types.go b/internal/types/types.go index cc84f2173..84d42f9f3 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -20,13 +20,13 @@ type PRApprovals struct { } type Commit struct { - SHA string `json:"sha1"` - Message string `json:"message"` - Committer string `json:"author"` - CommitterUsername string `json:"author_username,omitempty"` - Timestamp int64 `json:"timestamp"` - Branch string `json:"branch"` - URL string `json:"url,omitempty"` + SHA string `json:"sha1"` + Message string `json:"message"` + Author string `json:"author"` + AuthorUsername string `json:"author_username,omitempty"` + Timestamp int64 `json:"timestamp"` + Branch string `json:"branch"` + URL string `json:"url,omitempty"` } type PRRetriever interface { From 77fba59ac23c5ce43b5c1c1625b0cf9c33699adf Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 12:10:17 +0100 Subject: [PATCH 02/13] fix: record git author (not committer) in GitHub PR commits (server#5479) The GitHub GraphQL queries only fetched each commit's committer, and buildPREvidence mapped that into the author field. For web-flow commits (applied suggestions, bot commits) the committer is "GitHub ", so the real author was lost and author_username dropped (the committer has no associated GitHub user). Add author { name email user { login } } to the shared commit node (which both PR-by-number and PR-by-commit queries use) and map it into Author / AuthorUsername. Adds a red-first unit test on buildPREvidence with a web-flow commit where author != committer. GitLab and Azure populate the same fields from committer data via separate code paths and are corrected in follow-up commits. Co-Authored-By: Claude Fable 5 --- internal/github/build_pr_evidence_test.go | 55 +++++++++++++++++++++++ internal/github/github.go | 17 ++++--- 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 internal/github/build_pr_evidence_test.go diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go new file mode 100644 index 000000000..d5f01608d --- /dev/null +++ b/internal/github/build_pr_evidence_test.go @@ -0,0 +1,55 @@ +package github + +import ( + "testing" + + "github.com/shurcooL/graphql" + "github.com/stretchr/testify/require" +) + +// TestBuildPREvidence_RecordsAuthorNotCommitter is a regression test for +// server#5479. PR commit attestations were recording the git committer in the +// "author" field. For GitHub web-flow commits (applied suggestions, bot +// commits) the committer is "GitHub ", distinct from the +// real author — so the true author was being lost and the author_username +// dropped entirely (the committer has no associated GitHub user). +func TestBuildPREvidence_RecordsAuthorNotCommitter(t *testing.T) { + node := graphqlCommitNode{} + node.Commit.Oid = "0e723254516c841126e81f76100be57258ff1386" + node.Commit.MessageHeadline = "Apply suggestions from code review" + node.Commit.CommittedDate = "2026-03-01T12:00:00Z" + node.Commit.URL = "https://github.com/kosli-dev/cli/commit/0e723254516c841126e81f76100be57258ff1386" + + // Committer is GitHub's web-flow identity, with no associated user. + node.Commit.Committer.Name = "GitHub" + node.Commit.Committer.Email = "noreply@github.com" + node.Commit.Committer.User = nil + + // Author is the real person who wrote the change. + node.Commit.Author.Name = "Steve Tooke" + node.Commit.Author.Email = "tooky@kosli.com" + node.Commit.Author.User = &struct { + Login graphql.String + }{Login: "tooky"} + + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", + "tooky", + "2026-03-01T11:00:00Z", + "", + "Introduce kosli evaluate", + "introduce-kosli-evaluate", + []graphqlCommitNode{node}, + nil, + ) + require.NoError(t, err) + require.Len(t, evidence.Commits, 1) + + c := evidence.Commits[0] + require.Equal(t, "Steve Tooke ", c.Author, + "the commit author (wire field author) must be the git author, not the committer") + require.Equal(t, "tooky", c.AuthorUsername, + "author_username must be the author's GitHub login, not the committer's (absent) one") +} diff --git a/internal/github/github.go b/internal/github/github.go index 6f334a9a8..bb9b8939c 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -272,6 +272,13 @@ type graphqlCommitNode struct { Login graphql.String } } + Author struct { + Name graphql.String + Email graphql.String + User *struct { + Login graphql.String + } + } } } @@ -323,15 +330,15 @@ func buildPREvidence( if err != nil { return nil, err } - committerUsername := "" - if n.Commit.Committer.User != nil { - committerUsername = string(n.Commit.Committer.User.Login) + authorUsername := "" + if n.Commit.Author.User != nil { + authorUsername = string(n.Commit.Author.User.Login) } evidence.Commits = append(evidence.Commits, types.Commit{ SHA: string(n.Commit.Oid), Message: string(n.Commit.MessageHeadline), - Author: fmt.Sprintf("%s <%s>", string(n.Commit.Committer.Name), string(n.Commit.Committer.Email)), - AuthorUsername: committerUsername, + Author: fmt.Sprintf("%s <%s>", string(n.Commit.Author.Name), string(n.Commit.Author.Email)), + AuthorUsername: authorUsername, Timestamp: timestamp.Unix(), Branch: headRef, URL: string(n.Commit.URL), From fd736142133de99c1e354a7654145c0aac511555 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 13:14:42 +0100 Subject: [PATCH 03/13] chore: exclude PR-evidence test fixture from Snyk Code (server#5479) Snyk Code's NoHardcodedCredentials rule flags the GitHub username in build_pr_evidence_test.go (a graphql Login field set to "tooky") as a hardcoded credential. It is a public identifier in test data, not a secret. Add the test file to the existing .snyk exclude.global list, matching how the repo already suppresses Snyk Code false positives. Co-Authored-By: Claude Fable 5 --- .snyk | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.snyk b/.snyk index c481b7339..212a26fe5 100644 --- a/.snyk +++ b/.snyk @@ -13,3 +13,7 @@ exclude: global: - internal/azure/azure_apps.go - cmd/kosli/root.go + # False positive: Snyk Code flags the GitHub username in this test fixture + # (a graphql `Login` field) as a hardcoded credential. It is a public + # identifier in test data, not a secret. See kosli-dev/server#5479. + - internal/github/build_pr_evidence_test.go From 4c3dbb95a76034f4dee74f0bfb1d3584267878c3 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 14:44:57 +0100 Subject: [PATCH 04/13] chore: mark deferred author/committer follow-ups with TODOs (server#5479) Address review feedback on cli#957. Add TODO(server#5479) markers at the known-deferred spots so the follow-up slices are easy to find: - GitLab commit author still sourced from CommitterName/CommitterEmail - Azure AuthorUsername still sourced from the committer - GitHub commit timestamp uses the committer date; author-date alignment (authoredDate) is tracked as a separate cross-provider slice No behaviour change. Co-Authored-By: Claude Fable 5 --- internal/azure/azure.go | 13 +++++++------ internal/github/github.go | 2 ++ internal/gitlab/gitlab.go | 5 +++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/azure/azure.go b/internal/azure/azure.go index 6d6a7c0a2..a7d57c028 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -168,12 +168,13 @@ 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, + 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, }) } diff --git a/internal/github/github.go b/internal/github/github.go index bb9b8939c..886a781a0 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -326,6 +326,8 @@ 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)) if err != nil { return nil, err diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index 0d1ff972c..281e80acd 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -175,8 +175,9 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t } for _, commit := range glCommits { commits = append(commits, types.Commit{ - SHA: commit.ID, - Message: commit.Message, + 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, From 3297d58f4a84b2f7e16974e883fafcf361390af4 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 14:58:31 +0100 Subject: [PATCH 05/13] 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 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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, } From faee235a68298c92fdc055dfc8833ba35f25aeae Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:58:45 +0100 Subject: [PATCH 12/13] refactor: drop unused committer sub-selection from GitHub commit query (server#5479) buildPREvidence reads only Author/AuthoredDate (CommittedDate as fallback), so the Committer block in graphqlCommitNode was fetched on every PR query and never consumed. Remove it. The deferred committer-capture work will re-add what it needs. Co-Authored-By: Claude Opus 4.8 --- internal/github/github.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/github/github.go b/internal/github/github.go index cb9b4fac8..973b24a41 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -265,15 +265,7 @@ type graphqlCommitNode struct { CommittedDate graphql.String AuthoredDate graphql.String URL graphql.String - Committer struct { - Name graphql.String - Email graphql.String - Date graphql.String - User *struct { - Login graphql.String - } - } - Author struct { + Author struct { Name graphql.String Email graphql.String User *struct { From ce2f085cab044b8b0b1dd17441678fa64c236aff Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 12 Jun 2026 15:59:39 +0100 Subject: [PATCH 13/13] test: drop removed committer fields from buildPREvidence test (server#5479) Follow-up to dropping the Committer sub-selection: the regression test no longer sets node.Commit.Committer (the field is gone). The query no longer fetches the committer, so the test sets only the author. Fixes the build broken by faee235a. Co-Authored-By: Claude Opus 4.8 --- internal/github/build_pr_evidence_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index e27d06d2f..b15dd7f63 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -21,12 +21,9 @@ func TestBuildPREvidence_RecordsAuthorNotCommitter(t *testing.T) { node.Commit.CommittedDate = "2026-03-01T12:00:00Z" node.Commit.URL = "https://github.com/kosli-dev/cli/commit/0e723254516c841126e81f76100be57258ff1386" - // Committer is GitHub's web-flow identity, with no associated user. - node.Commit.Committer.Name = "GitHub" - node.Commit.Committer.Email = "noreply@github.com" - node.Commit.Committer.User = nil - - // Author is the real person who wrote the change. + // Author is the real person who wrote the change. The query no longer + // fetches the committer at all (it would be GitHub's web-flow identity for + // applied-suggestion / bot commits), so only the author is recorded. node.Commit.Author.Name = "Steve Tooke" node.Commit.Author.Email = "tooky@kosli.com" node.Commit.Author.User = &struct {