diff --git a/context/github-sync-invariants.md b/context/github-sync-invariants.md index ddbc2020f..02ef3c98b 100644 --- a/context/github-sync-invariants.md +++ b/context/github-sync-invariants.md @@ -137,6 +137,7 @@ PR timeline storage is intentionally selective. stored rows may predate parent-time realignment. (`internal/server/pullapi/routes.go::withSyntheticMRLifecycleEvents`) - Keep the existing event families stable: comments, reviews, commits, force pushes, and the currently supported PR system events. +- GitHub commit events use the committer login/name and committed date as their activity actor/time when present, preserve a distinct original author in commit metadata, and fall back independently to author identity/time when committer data is absent. - Review comments are UI-aware but are not part of the stored sync model unless they can be fetched within the supported timeline path. - If bulk sync persists PR system events, detail sync must persist the same diff --git a/frontend/src/lib/components/detail/EventTimeline.test.ts b/frontend/src/lib/components/detail/EventTimeline.test.ts index 457ebef6c..5ae2b8f6c 100644 --- a/frontend/src/lib/components/detail/EventTimeline.test.ts +++ b/frontend/src/lib/components/detail/EventTimeline.test.ts @@ -1573,6 +1573,27 @@ describe("EventTimeline", () => { expect(toggle?.getAttribute("aria-expanded")).toBe("true"); }); + it("renders the normalized committer identity for rebased commits", () => { + const { container } = renderTimeline({ + props: { + activityViewMode: "compact", + events: [ + makeEvent({ + EventType: "commit", + Author: "rebase-committer", + MetadataJSON: '{"commit_author":"original-author"}', + Summary: "abcdef1234567890", + Body: "feat: rewrite commit", + }), + ], + }, + }); + + const row = container.querySelector(".event-card--compact-row"); + expect(row?.textContent).toContain("rebase-committer"); + expect(row?.textContent).not.toContain("original-author"); + }); + it("keeps compact commit details collapsed when commit details are hidden", () => { const { container } = renderTimeline({ props: { diff --git a/internal/github/graphql.go b/internal/github/graphql.go index 80148a620..405501429 100644 --- a/internal/github/graphql.go +++ b/internal/github/graphql.go @@ -225,7 +225,12 @@ type gqlCommit struct { Message string Author struct { Name string - Date time.Time + Date *time.Time + User *struct{ Login string } + } + Committer struct { + Name string + Date *time.Time User *struct{ Login string } } } @@ -651,13 +656,20 @@ func adaptCommit(gql *gqlCommitNode) *gh.RepositoryCommit { Message: new(gql.Commit.Message), Author: &gh.CommitAuthor{ Name: new(gql.Commit.Author.Name), - Date: &gh.Timestamp{Time: gql.Commit.Author.Date}, + Date: ghTimestampPtr(gql.Commit.Author.Date), + }, + Committer: &gh.CommitAuthor{ + Name: new(gql.Commit.Committer.Name), + Date: ghTimestampPtr(gql.Commit.Committer.Date), }, }, } if gql.Commit.Author.User != nil { c.Author = &gh.User{Login: new(gql.Commit.Author.User.Login)} } + if gql.Commit.Committer.User != nil { + c.Committer = &gh.User{Login: new(gql.Commit.Committer.User.Login)} + } return c } diff --git a/internal/github/graphql_test.go b/internal/github/graphql_test.go index 131012e64..31ad5d3d9 100644 --- a/internal/github/graphql_test.go +++ b/internal/github/graphql_test.go @@ -345,15 +345,27 @@ func TestAdaptCommit(t *testing.T) { }, } gql.Commit.Author.Name = "Dave" - gql.Commit.Author.Date = now + gql.Commit.Author.Date = &now gql.Commit.Author.User = &struct{ Login string }{Login: "dave"} + gql.Commit.Committer.Name = "Eve" + committedAt := now.Add(time.Hour) + gql.Commit.Committer.Date = &committedAt + gql.Commit.Committer.User = &struct{ Login string }{Login: "eve"} c := adaptCommit(&gql) assert.Equal("sha123", c.GetSHA()) assert.Equal("fix: something", c.GetCommit().GetMessage()) assert.Equal("Dave", c.GetCommit().GetAuthor().GetName()) + assert.Equal(now, c.GetCommit().GetAuthor().GetDate().Time) assert.Equal("dave", c.GetAuthor().GetLogin()) + assert.Equal("Eve", c.GetCommit().GetCommitter().GetName()) + assert.Equal(committedAt, c.GetCommit().GetCommitter().GetDate().Time) + assert.Equal("eve", c.GetCommitter().GetLogin()) + + gql.Commit.Committer.Date = nil + withoutCommitterDate := adaptCommit(&gql) + assert.Nil(withoutCommitterDate.GetCommit().GetCommitter().Date) } func TestAdaptCheckContext(t *testing.T) { diff --git a/internal/github/normalize.go b/internal/github/normalize.go index a39fa2bf7..5e80dc9b4 100644 --- a/internal/github/normalize.go +++ b/internal/github/normalize.go @@ -146,8 +146,8 @@ func NormalizeReviewEvent(mrID int64, r *gh.PullRequestReview) db.MREvent { } // NormalizeCommitEvent converts a GitHub RepositoryCommit to a db.MREvent. -// Author is taken from the GitHub user login if available, falling back to -// the git commit author name. +// Author is the committer login or name when available, falling back to the +// commit author identity; a distinct original author is retained in metadata. func NormalizeCommitEvent(mrID int64, c *gh.RepositoryCommit) db.MREvent { event := platformgithub.NormalizeCommitEvent(platform.RepoRef{}, 0, c) return dbMREvent(mrID, event) diff --git a/internal/github/sync_test.go b/internal/github/sync_test.go index a8f4b638d..927666fb3 100644 --- a/internal/github/sync_test.go +++ b/internal/github/sync_test.go @@ -23393,6 +23393,15 @@ func TestWithObsoleteMetadata(t *testing.T) { } } +func TestWithCommitOrderMetadataPreservesCommitAuthor(t *testing.T) { + withOrder := withCommitOrderMetadata(`{"commit_author":"original-author"}`, 2, 4) + assert.JSONEq(t, `{"commit_author":"original-author","commit_order":2,"commit_order_key":4}`, withOrder) + + withObsolete, changed := withObsoleteMetadata(withOrder, true) + assert.True(t, changed) + assert.JSONEq(t, `{"commit_author":"original-author","commit_order":2,"commit_order_key":4,"obsolete":true}`, withObsolete) +} + // TestSyncRepoDropsStaleSettingsSnapshotBehindNewerObservation simulates a // notification sync committing fresher repository settings between this // sync's provider snapshot capture and its settings write. The delayed full diff --git a/internal/platform/github/normalize.go b/internal/platform/github/normalize.go index 8fe3c47cb..fbda01951 100644 --- a/internal/platform/github/normalize.go +++ b/internal/platform/github/normalize.go @@ -193,6 +193,20 @@ func NormalizeReviewCommentEvent( return event } +type commitEventMetadata struct { + CommitAuthor string `json:"commit_author,omitempty"` +} + +func commitIdentity(user *gh.User, signature *gh.CommitAuthor) string { + if login := loginOrEmpty(user); login != "" { + return login + } + if signature != nil { + return signature.GetName() + } + return "" +} + func NormalizeCommitEvent( repo platform.RepoRef, mrNumber int, @@ -204,9 +218,18 @@ func NormalizeCommitEvent( dedupeKey = sha[:12] } - author := loginOrEmpty(c.GetAuthor()) - if author == "" && c.GetCommit() != nil && c.GetCommit().GetAuthor() != nil { - author = c.GetCommit().GetAuthor().GetName() + commit := c.GetCommit() + authorSignature := (*gh.CommitAuthor)(nil) + committerSignature := (*gh.CommitAuthor)(nil) + if commit != nil { + authorSignature = commit.GetAuthor() + committerSignature = commit.GetCommitter() + } + author := commitIdentity(c.GetAuthor(), authorSignature) + committer := commitIdentity(c.GetCommitter(), committerSignature) + actor := committer + if actor == "" { + actor = author } event := platform.MergeRequestEvent{ @@ -214,13 +237,19 @@ func NormalizeCommitEvent( MergeRequestNumber: mrNumber, EventType: "commit", DedupeKey: fmt.Sprintf("commit-%s", dedupeKey), - Author: author, + Author: actor, Summary: sha, } - if c.GetCommit() != nil { - event.Body = c.GetCommit().GetMessage() - if c.GetCommit().Author != nil && c.GetCommit().Author.Date != nil { - event.CreatedAt = c.GetCommit().Author.Date.UTC() + if author != "" && author != actor { + metadata, _ := json.Marshal(commitEventMetadata{CommitAuthor: author}) + event.MetadataJSON = string(metadata) + } + if commit != nil { + event.Body = commit.GetMessage() + if committerSignature != nil && committerSignature.Date != nil { + event.CreatedAt = committerSignature.Date.UTC() + } else if authorSignature != nil && authorSignature.Date != nil { + event.CreatedAt = authorSignature.Date.UTC() } } return event diff --git a/internal/platform/github/normalize_test.go b/internal/platform/github/normalize_test.go index bd36074f9..028e7c738 100644 --- a/internal/platform/github/normalize_test.go +++ b/internal/platform/github/normalize_test.go @@ -1,6 +1,7 @@ package github import ( + "encoding/json" "testing" "time" @@ -51,6 +52,141 @@ func TestNormalizeReviewCommentEventPreservesHTMLURL(t *testing.T) { assert.Equal(t, commentURL, event.DirectURL) } +func TestNormalizeCommitEventUsesCommitterForRebasedCommit(t *testing.T) { + const payload = `{ + "sha": "abcdef1234567890", + "author": {"login": "original-author"}, + "committer": {"login": "rebase-committer"}, + "commit": { + "message": "feat: rewrite commit", + "author": {"name": "original-author", "date": "2026-09-03T02:21:43Z"}, + "committer": {"name": "rebase-committer", "date": "2026-09-03T04:52:04Z"} + } +}` + var commit gh.RepositoryCommit + require.NoError(t, json.Unmarshal([]byte(payload), &commit)) + + event := NormalizeCommitEvent(platform.RepoRef{Owner: "acme", Name: "widget"}, 7, &commit) + assert := assert.New(t) + assert.Equal("rebase-committer", event.Author) + assert.Equal(time.Date(2026, 9, 3, 4, 52, 4, 0, time.UTC), event.CreatedAt) + assert.Equal(`{"commit_author":"original-author"}`, event.MetadataJSON) + assert.Equal("feat: rewrite commit", event.Body) + assert.Equal("abcdef1234567890", event.Summary) + assert.Equal("commit-abcdef123456", event.DedupeKey) +} + +func TestNormalizeCommitEventFallbacks(t *testing.T) { + authoredAt := time.Date(2026, 9, 3, 2, 21, 43, 0, time.UTC) + committedAt := time.Date(2026, 9, 3, 4, 52, 4, 0, time.UTC) + user := func(login string) *gh.User { return &gh.User{Login: new(login)} } + signature := func(name string, date time.Time) *gh.CommitAuthor { + return &gh.CommitAuthor{Name: new(name), Date: &gh.Timestamp{Time: date}} + } + + cases := []struct { + name string + author *gh.User + committer *gh.User + commitAuthor *gh.CommitAuthor + commitCommitter *gh.CommitAuthor + wantAuthor string + wantCreatedAt time.Time + wantMetadata string + }{ + { + name: "associated committer login is preferred", + author: user("original-author"), + committer: user("rebase-committer"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: signature("rebase-name", committedAt), + wantAuthor: "rebase-committer", + wantCreatedAt: committedAt, + wantMetadata: `{"commit_author":"original-author"}`, + }, + { + name: "web-flow committer is displayed as reported", + author: user("original-author"), + committer: user("web-flow"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: signature("web-flow", committedAt), + wantAuthor: "web-flow", + wantCreatedAt: committedAt, + wantMetadata: `{"commit_author":"original-author"}`, + }, + { + name: "nested committer name is used without associated user", + author: user("original-author"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: signature("rebase-committer", committedAt), + wantAuthor: "rebase-committer", + wantCreatedAt: committedAt, + wantMetadata: `{"commit_author":"original-author"}`, + }, + { + name: "author login is used when committer identity is absent", + author: user("original-author"), + commitAuthor: signature("original-name", authoredAt), + wantAuthor: "original-author", + wantCreatedAt: authoredAt, + }, + { + name: "nested author name is used without associated users", + commitAuthor: signature("original-author", authoredAt), + wantAuthor: "original-author", + wantCreatedAt: authoredAt, + }, + { + name: "author date is used when committer date is absent", + author: user("original-author"), + committer: user("rebase-committer"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: &gh.CommitAuthor{Name: new("rebase-name")}, + wantAuthor: "rebase-committer", + wantCreatedAt: authoredAt, + wantMetadata: `{"commit_author":"original-author"}`, + }, + { + name: "committer date is used without committer identity", + author: user("original-author"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: &gh.CommitAuthor{Date: &gh.Timestamp{Time: committedAt}}, + wantAuthor: "original-author", + wantCreatedAt: committedAt, + }, + { + name: "same normalized identity omits redundant metadata", + author: user("same-user"), + committer: user("same-user"), + commitAuthor: signature("original-name", authoredAt), + commitCommitter: signature("committer-name", committedAt), + wantAuthor: "same-user", + wantCreatedAt: committedAt, + }, + { + name: "all identity and date fields absent remain empty", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + commit := &gh.RepositoryCommit{ + Author: tc.author, + Committer: tc.committer, + Commit: &gh.Commit{ + Author: tc.commitAuthor, + Committer: tc.commitCommitter, + }, + } + event := NormalizeCommitEvent(platform.RepoRef{}, 1, commit) + assert := assert.New(t) + assert.Equal(tc.wantAuthor, event.Author) + assert.Equal(tc.wantCreatedAt, event.CreatedAt) + assert.Equal(tc.wantMetadata, event.MetadataJSON) + }) + } +} + func TestNormalizePullRequestPreservesOptionalMergeMetrics(t *testing.T) { require := require.New(t) assert := assert.New(t)