fix: source PR commit author identity and timestamps from the git author across providers (server#5479)#958
Conversation
Pure extraction of the GitLab commit mapping so it can be unit-tested directly. Behaviour unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…479) 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 <noreply@anthropic.com>
Pure extraction of the Azure commit mapping for direct unit testing. Behaviour unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Azure recorded the author display name only and populated author_username from the committer. Record the author as "Name <email>" (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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
Claude finished @AlexKantor87's task in 1m 57s —— View job PR Review: source PR commit author identity & timestamps from git author
Verdict: LGTM. Clean, well-tested completion of the server#5479 author/timestamp fix. Refactor-then-fix slicing with pure Static analysis
(I couldn't run Strengths
Observations (all non-blocking)
Nice, disciplined PR — small slices, tests-first, clear commit messages. |
|
Folded into #957 via fast-forward (signed commits preserved) so the full server#5479 change is one reviewable unit for the merge-to-main approval. No code changed in the fold. |
What and why
Completes the author/committer fix for kosli-dev/server#5479, clearing the three
TODO(server#5479)markers left by #957. PR-attestation commits now record the git author's identity and authored timestamp across all providers, not the committer's.Stacked on #957 (base branch
5479-pr-attestation-author); GitHub will retarget this tomainwhen #957 merges. Plan: https://github.com/kosli-dev/server/issues/5479#issuecomment-4690357750Per-provider before → after
authorauthor_usernametimestampAuthorName/AuthorEmail)Name <email>author.raw) — unchangeddatefield — unchangedAzure commits expose only git name/email/date (no login), so
author_usernameis dropped rather than populated from the wrong source;omitemptyremoves it from the wire. ItsAuthoris aligned to theName <email>format the other providers use.How it's built
Each provider's commit mapping was extracted into a pure
commitFrom<Provider>Commitfunction (GitHub already hadbuildPREvidence) so the logic is unit-testable without live API calls. Seven commits, refactor-then-fix per slice, each fix preceded by a failing test:authoredDateto the shared GraphQL commit node)Bitbucket was verified read-only: it already reads the API
authorfield and exposes only a single commitdate.Compatibility note
Wire schema unchanged; values change. Policies (Rego or jq) reading commit
authorortimestampmay legitimately flip outcomes — never-alone checks comparing commit author to approvers, and any commit-time-window logic. Azure payloads loseauthor_username(it previously held the committer's display name, which was wrong, and Azure has no commit login to replace it). This needs a changelog + docs note before release, alongside #957's.Verification
go build ./...,go vet,make lint(0 issues), and unit tests for all changed packages (github, gitlab, azure, bitbucket, types) pass locally. The fullmake test_integrationwas not run locally (noKOSLI_API_TOKEN_PRODin this environment); CI runs the authoritative full suite including the token-gated real-provider contract tests.🤖 Generated with Claude Code