From b5f4d1033898f8871266b59ba86d5b37c9412d62 Mon Sep 17 00:00:00 2001 From: omartelo Date: Thu, 10 Sep 2026 15:16:25 -0300 Subject: [PATCH] feat: say which GitHub account each commit on a pull request landed under A project's gh account governs what lich asks gh; it has never governed the push, which still signs with the global user.email. So a pull request could be read by one account and its commits land under another, with no error anywhere, and the Version Control pane refused to compare the two because comparing the strings is a false-positive farm. The answer was already in the payload. gh pr view --json commits carries the account GitHub itself resolved each author email to, checked against the verified addresses of every account, so noreply forms, vanity domains and org aliases all resolve correctly. toCommits threw it away: folding the login into the git name made a commit GitHub could not attribute read exactly like one it could. Keep the two apart, carry the author email for the unattributed case, and compare each commit against the account that opened the pull request. The Commits tab names the account a commit landed under, marks one linked to no account, and heads the list with a line when they are not all the pull request author's, while the branch can still be fixed. Nothing reads before a push, where GitHub has resolved nothing yet: gh api user/emails would answer there, but it needs the user scope that gh's own login does not request, and its refusal is a 404 no different from any other not-found. Both limits are in docs/ceilings.md. --- CHANGELOG.md | 14 ++ docs/ceilings.md | 22 ++- .../src/components/pulls/PullRequestView.tsx | 4 +- .../src/components/pulls/PullsCommits.tsx | 34 +++-- .../pulls/pulls-open-session.test.tsx | 1 + frontend/src/lib/api-types.ts | 13 +- frontend/src/lib/pulls/commit-authors.test.ts | 133 ++++++++++++++++++ frontend/src/lib/pulls/commit-authors.ts | 87 ++++++++++++ frontend/src/lib/pulls/merge-gate.test.ts | 1 + frontend/src/lib/pulls/pr-handoff.test.ts | 1 + internal/project/pr.go | 30 +++- internal/project/pr_test.go | 49 +++++-- 12 files changed, 357 insertions(+), 32 deletions(-) create mode 100644 frontend/src/lib/pulls/commit-authors.test.ts create mode 100644 frontend/src/lib/pulls/commit-authors.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a7f84c9b..b4d7363fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **The Commits tab says which GitHub account each commit actually landed + under.** A project's GitHub account governs what lich asks `gh`; it has never + governed the push, which still signs with your global `user.email`. So a pull + request could be read by one account and its commits land under another, or + under no account at all, and nothing said a word. Each commit row now shows + the account GitHub itself resolved the author's email to — GitHub checks it + against every account's verified addresses, so noreply forms, vanity domains + and organisation aliases all resolve correctly — and a commit belonging to no + account is marked as one. When the commits are not all the pull request + author's, a line above the list names the accounts they landed under and the + addresses no account owns, while the branch can still be fixed. + ### Changed - **The two sandbox grants say what they hand over.** In Settings › Sandbox, the diff --git a/docs/ceilings.md b/docs/ceilings.md index bd530e9dc..caa582aba 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -8,9 +8,25 @@ work when nobody knows it and that the call site never shows. The mechanism and - **A project's gh account governs gh, not git**: `vcs.account` (`internal/project/ghaccount.go`) puts one account's token in `GH_TOKEN` for every gh call lich makes for that project. A push still rides the remote's ssh key and signs with the global `user.email`, so a PR can be *read* by one account and its commits *land* - under another, with no error anywhere. The Version Control settings print both identities and never compare - them: noreply forms, vanity domains and org aliases make a mismatch warning a false-positive farm. lich never - writes `user.email`. + under another. lich never writes `user.email`, and the Version Control settings still print both identities + without comparing them — noreply forms, vanity domains and org aliases make a string comparison a + false-positive farm. + The Commits tab of the Pulls screen is where the two are finally reconciled, and only there + (`frontend/src/lib/pulls/commit-authors.ts`): GitHub has already resolved each commit's author email against + the verified addresses of every account, so its answer is set membership, not a guess. **The window is after + the push and before the merge.** Nothing reads before a push — the resolution does not exist until GitHub has + the commit — so a branch can still be built entirely under the wrong identity and only say so once its pull + request is open. Two things keep it there: `gh api user/emails` would answer for an unpushed commit, but it + needs the `user` scope that gh's own login does not request (measured: `repo, read:org, gist`) and its refusal + is an HTTP 404 indistinguishable from any other not-found, on a path where gh's stderr never reaches the UI; + and a commit's *signature* is a second identity this compares nothing about, so a commit signed by a key the + account does not list reads as clean. + The reading is also about the pull request, not about you: it compares each commit against whoever opened the + PR, the one identity the payload already carries. So a branch somebody else built collaboratively — a + teammate's commit, a Copilot-authored branch — reads as commits under another account on a PR you are only + reviewing. The statement is true and the tone is factual rather than an alarm, but the line does speak on pull + requests where nothing is wrong. Narrowing it to *your* pull requests means resolving which account lich runs + gh as, which `vcs.account` only answers when the project has named one. - **`LICH_WORKTREE_PORT` is reserved, never held** (`internal/terminal/worktreeport.go`): the number is a name the checkout owns, nothing binds it, and anything on the machine can take the port before the dev server starts. A Run card shortens that window rather than closing it — the process it starts is what binds the port, and diff --git a/frontend/src/components/pulls/PullRequestView.tsx b/frontend/src/components/pulls/PullRequestView.tsx index 5f44d9430..9a0f166f4 100644 --- a/frontend/src/components/pulls/PullRequestView.tsx +++ b/frontend/src/components/pulls/PullRequestView.tsx @@ -493,7 +493,9 @@ export function PullRequestView({ ) : (
{tab === "checks" && } - {tab === "commits" && } + {tab === "commits" && ( + + )} {tab === "conversation" && ( No commits. } + const notice = commitAuthorNotice(commits, authorLogin) return (
+ {notice && ( +

+ {notice} +

+ )} {commits.map((commit) => ( ))} @@ -40,7 +58,7 @@ function CommitRow({ commit }: { commit: PullRequestCommit }) { {commit.headline} - {commitMeta(commit)} + {commitMetaLine(commit)} {body !== "" && ( ) } - -// Who landed it and when, in git's own phrasing. Either half can be missing — a -// merge commit from the web flow carries no author — so the line is built from -// what is there instead of printing an empty "committed on". -function commitMeta(commit: PullRequestCommit): string { - const at = new Date(commit.date) - const date = Number.isNaN(at.getTime()) ? "" : at.toLocaleDateString() - if (commit.author && date) { - return `${commit.author} committed ${date}` - } - return commit.author || (date && `Committed ${date}`) || "" -} diff --git a/frontend/src/components/pulls/pulls-open-session.test.tsx b/frontend/src/components/pulls/pulls-open-session.test.tsx index 0cea15ed9..8886d96ce 100644 --- a/frontend/src/components/pulls/pulls-open-session.test.tsx +++ b/frontend/src/components/pulls/pulls-open-session.test.tsx @@ -26,6 +26,7 @@ const DETAIL: PullRequestDetail = { title: "A pull request", body: "", author: "someone", + authorLogin: "someone", state: "OPEN", isDraft: false, mergeable: "MERGEABLE", diff --git a/frontend/src/lib/api-types.ts b/frontend/src/lib/api-types.ts index 4fc43d4be..b309f9f10 100644 --- a/frontend/src/lib/api-types.ts +++ b/frontend/src/lib/api-types.ts @@ -107,8 +107,14 @@ export interface PullRequestCommit { headline: string /** The rest of the message; "" for a one-line commit. */ body: string - /** The first author's login, or their name; "" when gh reports none. */ - author: string + /** The GitHub account GitHub itself resolved the author's email to, and "" + * when it resolved none: the email belongs to no account, so the commit is + * attributed to nobody. Never falls back to the name — that is the fact. */ + login: string + /** git's author name: free text, and no evidence of who landed the commit. */ + name: string + /** git's author email, which is what GitHub did or did not resolve. */ + email: string /** gh's ISO committedDate. */ date: string } @@ -140,6 +146,9 @@ export interface PullRequestDetail { body: string /** Who opened it: a login, or a display name when gh reports no login. */ author: string + /** The same account without that fallback: "" rather than a display name, so + * it can be compared against a commit's login without inventing a mismatch. */ + authorLogin: string /** gh: OPEN | CLOSED | MERGED. Only a number-addressed lookup returns a * non-OPEN one; the branch lookup still hides them. */ state: string diff --git a/frontend/src/lib/pulls/commit-authors.test.ts b/frontend/src/lib/pulls/commit-authors.test.ts new file mode 100644 index 000000000..192221b7e --- /dev/null +++ b/frontend/src/lib/pulls/commit-authors.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it } from "vitest" +import type { PullRequestCommit } from "@/lib/api-types" +import { commitAuthorNotice, commitMetaLine } from "./commit-authors" + +const DATE = new Date("2026-07-25T22:32:47Z").toLocaleDateString() + +function commit(fields: Partial): PullRequestCommit { + return { + oid: "a4dbc1f", + headline: "feat: the panel", + body: "", + login: "", + name: "", + email: "", + date: "2026-07-25T22:32:47Z", + ...fields, + } +} + +describe("commitMetaLine", () => { + it("shows GitHub's own answer as an account", () => { + expect(commitMetaLine(commit({ login: "omartelo", name: "martelo" }))).toBe( + `@omartelo committed ${DATE}`, + ) + }) + + // The whole failure: without the mark, this row reads exactly like the one + // above it, and a commit belonging to nobody reaches a merge unremarked. + it("marks a commit GitHub could not attribute", () => { + expect(commitMetaLine(commit({ name: "Ada Lovelace", email: "ada@analytical.example" }))).toBe( + `Ada Lovelace committed ${DATE} · linked to no GitHub account`, + ) + }) + + // A merge commit from the web flow carries no author at all. Nobody's + // identity is wrong there, so the mark would be a false alarm. + it("leaves a commit with no author alone", () => { + expect(commitMetaLine(commit({}))).toBe(`Committed ${DATE}`) + }) + + it("keeps the line whole when gh reports no date", () => { + expect(commitMetaLine(commit({ login: "omartelo", date: "" }))).toBe("@omartelo") + expect(commitMetaLine(commit({ date: "" }))).toBe("") + }) +}) + +describe("commitAuthorNotice", () => { + it("stays quiet when every commit is the pull request author's", () => { + const commits = [commit({ login: "omartelo" }), commit({ oid: "f55ebe9", login: "omartelo" })] + expect(commitAuthorNotice(commits, "omartelo")).toBe("") + }) + + it("stays quiet with no commits at all", () => { + expect(commitAuthorNotice(null, "omartelo")).toBe("") + expect(commitAuthorNotice([], "omartelo")).toBe("") + }) + + // The ceiling this exists for: read by one account, landed under another. + it("names the account the commits actually landed under", () => { + const commits = [ + commit({ login: "marcelo-filho_snk" }), + commit({ oid: "f55ebe9", login: "marcelo-filho_snk" }), + ] + expect(commitAuthorNotice(commits, "omartelo")).toBe( + "2 commits landed under @marcelo-filho_snk, not @omartelo.", + ) + }) + + it("names each stray account once, however many commits it landed", () => { + const commits = [ + commit({ login: "omartelo" }), + commit({ oid: "b1", login: "marcelo-filho_snk" }), + commit({ oid: "b2", login: "marcelo-filho_snk" }), + commit({ oid: "b3", login: "octocat" }), + ] + expect(commitAuthorNotice(commits, "omartelo")).toBe( + "3 commits landed under @marcelo-filho_snk, @octocat, not @omartelo.", + ) + }) + + it("folds a long list of accounts into a count", () => { + const commits = ["a", "b", "c", "d", "e"].map((login, i) => commit({ oid: `oid-${i}`, login })) + expect(commitAuthorNotice(commits, "omartelo")).toBe( + "5 commits landed under @a, @b, @c and 2 more, not @omartelo.", + ) + }) + + it("names the address no account owns, which is what has to be fixed", () => { + const commits = [commit({ name: "Ada Lovelace", email: "ada@analytical.example" })] + expect(commitAuthorNotice(commits, "omartelo")).toBe( + "1 commit is linked to no GitHub account: ada@analytical.example.", + ) + }) + + it("reports both readings when a branch carries both", () => { + const commits = [ + commit({ login: "marcelo-filho_snk" }), + commit({ oid: "b1", name: "Ada", email: "ada@analytical.example" }), + ] + expect(commitAuthorNotice(commits, "omartelo")).toBe( + "1 commit landed under @marcelo-filho_snk, not @omartelo. " + + "1 commit is linked to no GitHub account: ada@analytical.example.", + ) + }) + + // A commit gh reports with no email at all is still unattributed; the line + // says so without an empty colon trailing it. + it("drops the address list when there is no address to name", () => { + expect(commitAuthorNotice([commit({ name: "Ada" })], "omartelo")).toBe( + "1 commit is linked to no GitHub account.", + ) + }) + + // Same reason the row leaves it alone: gh reported no author, so there is no + // identity to call wrong. + it("never counts an authorless merge commit as unattributed", () => { + const commits = [commit({ login: "omartelo" }), commit({ oid: "b1" })] + expect(commitAuthorNotice(commits, "omartelo")).toBe("") + }) + + // gh reports no login for a deleted account. Comparing against "" would call + // every commit a stray, so the comparison is withheld — the unattributed + // reading needs no second identity and still stands. + it("withholds the comparison when the pull request author has no login", () => { + const commits = [ + commit({ login: "omartelo" }), + commit({ oid: "b1", name: "Ada", email: "ada@analytical.example" }), + ] + expect(commitAuthorNotice(commits, "")).toBe( + "1 commit is linked to no GitHub account: ada@analytical.example.", + ) + }) +}) diff --git a/frontend/src/lib/pulls/commit-authors.ts b/frontend/src/lib/pulls/commit-authors.ts new file mode 100644 index 000000000..10858b2ea --- /dev/null +++ b/frontend/src/lib/pulls/commit-authors.ts @@ -0,0 +1,87 @@ +import type { PullRequestCommit } from "@/lib/api-types" + +// How many addresses or accounts the line names before it folds the rest into a +// count. Three is enough to recognise the mistake; a branch that made it under +// four different identities has a bigger problem than a list can render. +const NAMED = 3 + +const NO_ACCOUNT = "linked to no GitHub account" + +// Whether gh reported an author for this commit at all. A web-flow merge commit +// carries none, and calling that one unattributed would be the false alarm this +// whole reading exists to avoid — nobody's identity is wrong, there is nobody. +function hasAuthor(commit: PullRequestCommit): boolean { + return commit.email !== "" || commit.name !== "" +} + +function unattributed(commit: PullRequestCommit): boolean { + return commit.login === "" && hasAuthor(commit) +} + +/** The line under a commit's subject: who landed it, when, and — when GitHub + * resolved the author's email to no account — that nobody owns it. A login is + * GitHub's own answer and is shown as one; without the mark, a commit belonging + * to nobody prints the git name and reads exactly like one that does. */ +export function commitMetaLine(commit: PullRequestCommit): string { + const at = new Date(commit.date) + const date = Number.isNaN(at.getTime()) ? "" : at.toLocaleDateString() + const who = commit.login ? `@${commit.login}` : commit.name + const landed = + who && date ? `${who} committed ${date}` : who || (date && `Committed ${date}`) || "" + if (!unattributed(commit)) { + return landed + } + return landed ? `${landed} · ${NO_ACCOUNT}` : `Committed by an address ${NO_ACCOUNT}` +} + +/** The one line above the commit list, or "" when there is nothing to say. + * + * Nothing here guesses. GitHub resolved every author email against the verified + * addresses of every account before answering, so a login that differs is a + * different account and an empty one is an email no account owns — the two + * readings a noreply form, a vanity domain or an org alias used to make + * unsayable. What the line cannot see is a commit that has not been pushed: + * before that, nobody has resolved anything. */ +export function commitAuthorNotice( + commits: PullRequestCommit[] | null, + authorLogin: string, +): string { + if (!commits || commits.length === 0) { + return "" + } + const strays = commits.filter((c) => c.login !== "" && c.login !== authorLogin) + const orphans = commits.filter(unattributed) + return [ + // Only when the pull request's own author is a login: gh reports none for a + // deleted account, and comparing against "" would call every commit a stray. + authorLogin && strays.length > 0 + ? `${count(strays.length, "commit")} landed under ${list(distinct(strays.map((c) => `@${c.login}`)))}, not @${authorLogin}.` + : "", + orphans.length > 0 + ? `${count(orphans.length, "commit")} ${orphans.length === 1 ? "is" : "are"} ${NO_ACCOUNT}${emails(orphans)}.` + : "", + ] + .filter(Boolean) + .join(" ") +} + +function emails(commits: PullRequestCommit[]): string { + const addresses = distinct(commits.map((c) => c.email).filter(Boolean)) + return addresses.length > 0 ? `: ${list(addresses)}` : "" +} + +function distinct(values: string[]): string[] { + return Array.from(new Set(values)) +} + +// Up to NAMED of them, then how many were held back — so a long list stays one +// line and still says it is not the whole list. +function list(values: string[]): string { + const hidden = values.length - NAMED + const shown = values.slice(0, NAMED).join(", ") + return hidden > 0 ? `${shown} and ${hidden} more` : shown +} + +function count(n: number, noun: string): string { + return `${n} ${noun}${n === 1 ? "" : "s"}` +} diff --git a/frontend/src/lib/pulls/merge-gate.test.ts b/frontend/src/lib/pulls/merge-gate.test.ts index 105340062..a8bd93502 100644 --- a/frontend/src/lib/pulls/merge-gate.test.ts +++ b/frontend/src/lib/pulls/merge-gate.test.ts @@ -23,6 +23,7 @@ const detail = (over: Partial = {}): PullRequestDetail => ({ title: "feat: something", body: "", author: "omartelo", + authorLogin: "omartelo", state: "OPEN", isDraft: false, mergeable: "MERGEABLE", diff --git a/frontend/src/lib/pulls/pr-handoff.test.ts b/frontend/src/lib/pulls/pr-handoff.test.ts index 8ce501a0a..263a3f099 100644 --- a/frontend/src/lib/pulls/pr-handoff.test.ts +++ b/frontend/src/lib/pulls/pr-handoff.test.ts @@ -8,6 +8,7 @@ const detail = (over: Partial = {}): PullRequestDetail => ({ title: "feat: something", body: "", author: "omartelo", + authorLogin: "omartelo", state: "OPEN", isDraft: false, mergeable: "MERGEABLE", diff --git a/internal/project/pr.go b/internal/project/pr.go index 1c770d41a..380fd1ad2 100644 --- a/internal/project/pr.go +++ b/internal/project/pr.go @@ -98,6 +98,11 @@ type PRDetail struct { // reports no login. GitHub refuses a review request addressed to them, which // is what keeps them out of the picker. Author string `json:"author"` + // AuthorLogin is the same account as Author, minus the fallback to a display + // name — the value a commit's login is compared against. Author cannot serve: + // a name matches no login, so a PR whose author gh reports without one would + // read as every commit landing under the wrong account. + AuthorLogin string `json:"authorLogin"` // State is gh's OPEN | CLOSED | MERGED. Only a number-addressed lookup can // return a non-OPEN one; the branch lookup still gates them out. State string `json:"state"` @@ -133,11 +138,18 @@ type PRDetail struct { // PRCommit is one commit the pull request would land, split the way git itself // splits a message: the subject line, then the body (empty for a one-liner). +// +// Login and Name are kept apart because only one of them is evidence: Name is +// free text, Login is the account GitHub resolved the author's email to against +// every account's verified addresses — an answer lich cannot compute, and one +// whose "" means the email belongs to nobody rather than "unknown". type PRCommit struct { OID string `json:"oid"` Headline string `json:"headline"` Body string `json:"body"` - Author string `json:"author"` + Login string `json:"login"` + Name string `json:"name"` + Email string `json:"email"` Date string `json:"date"` // gh's ISO committedDate } @@ -178,6 +190,10 @@ type ghCommit struct { type ghCommitAuthor struct { Login string `json:"login"` Name string `json:"name"` + // Email is the address git recorded and GitHub resolved (or failed to). It + // is carried for the unresolved case alone: "this commit belongs to nobody" + // is only actionable next to the address that has to be added or corrected. + Email string `json:"email"` } // PullRequestDetail returns one open pull request in full: the given number, or @@ -218,6 +234,7 @@ func parsePRDetail(out []byte, openOnly bool) (*PRDetail, error) { Title: v.Title, Body: v.Body, Author: firstNonEmpty(v.Author.Login, v.Author.Name), + AuthorLogin: v.Author.Login, State: v.State, IsDraft: v.IsDraft, Mergeable: v.Mergeable, @@ -244,15 +261,20 @@ func toCommits(commits []ghCommit) []PRCommit { } out := make([]PRCommit, 0, len(commits)) for _, c := range commits { - author := "" + // Never collapsed to one display string: both kinds of commit carry a + // name, so falling back to it makes an unattributed commit read exactly + // like an attributed one. + var author ghCommitAuthor if len(c.Authors) > 0 { - author = firstNonEmpty(c.Authors[0].Login, c.Authors[0].Name) + author = c.Authors[0] } out = append(out, PRCommit{ OID: c.OID, Headline: c.MessageHeadline, Body: c.MessageBody, - Author: author, + Login: author.Login, + Name: author.Name, + Email: author.Email, Date: c.CommittedDate, }) } diff --git a/internal/project/pr_test.go b/internal/project/pr_test.go index 73011e622..c7f24d7cd 100644 --- a/internal/project/pr_test.go +++ b/internal/project/pr_test.go @@ -57,6 +57,24 @@ func TestParsePRDetail(t *testing.T) { } }) + // AuthorLogin is what a commit's login is compared against, so it must stay + // empty rather than borrow the display name: a name can never equal a login, + // and a borrowed one would report every commit as landing under another + // account. + t.Run("the author login never falls back to a display name", func(t *testing.T) { + out := []byte(`{"number":4,"state":"OPEN","author":{"name":"Ada Lovelace"}}`) + pr, err := parsePRDetail(out, true) + if err != nil || pr == nil { + t.Fatalf("unexpected parse result: %+v, %v", pr, err) + } + if pr.Author != "Ada Lovelace" { + t.Errorf("the printed author still falls back: %q", pr.Author) + } + if pr.AuthorLogin != "" { + t.Errorf("AuthorLogin borrowed the name: %q", pr.AuthorLogin) + } + }) + t.Run("draft flag survives", func(t *testing.T) { pr, err := parsePRDetail([]byte(`{"number":9,"state":"OPEN","isDraft":true}`), true) if err != nil { @@ -258,7 +276,9 @@ func TestToCommits(t *testing.T) { MessageHeadline: "fix: retire the badge", MessageBody: "The footer badge only looked up again on HEAD moves.", CommittedDate: "2026-07-25T22:32:47Z", - Authors: []ghCommitAuthor{{Login: "omartelo", Name: "omartelo"}}, + Authors: []ghCommitAuthor{ + {Login: "omartelo", Name: "omartelo", Email: "meopedevts@proton.me"}, + }, }, {OID: "f55ebe9", MessageHeadline: "docs: changelog"}, }) @@ -269,26 +289,39 @@ func TestToCommits(t *testing.T) { if first.OID != "a4dbc1f" || first.Headline != "fix: retire the badge" { t.Errorf("wrong header: %+v", first) } - if !strings.Contains(first.Body, "HEAD moves") || first.Author != "omartelo" { - t.Errorf("lost body or author: %+v", first) + if !strings.Contains(first.Body, "HEAD moves") || first.Login != "omartelo" { + t.Errorf("lost body or login: %+v", first) + } + if first.Email != "meopedevts@proton.me" { + t.Errorf("lost the author email: %q", first.Email) } if first.Date != "2026-07-25T22:32:47Z" { t.Errorf("wrong date: %q", first.Date) } // A one-line commit has no body and a web-flow merge commit no author; // both are rows, not errors. - if got[1].Body != "" || got[1].Author != "" { + if got[1].Body != "" || got[1].Login != "" || got[1].Name != "" { t.Errorf("second commit should be bare: %+v", got[1]) } }) - t.Run("a nameless login falls back to the name", func(t *testing.T) { + // Contract change: the login used to fall back to the name, which made a + // commit GitHub could not attribute read exactly like one it could. The two + // now stay apart, and an empty login is the answer "no account owns this + // email" rather than a missing value. + t.Run("an unattributed author keeps its name and reports no login", func(t *testing.T) { got := toCommits([]ghCommit{{ OID: "c0ffee0", - Authors: []ghCommitAuthor{{Name: "Ada Lovelace"}}, + Authors: []ghCommitAuthor{{Name: "Ada Lovelace", Email: "ada@analytical.example"}}, }}) - if len(got) != 1 || got[0].Author != "Ada Lovelace" { - t.Errorf("author fallback failed: %+v", got) + if len(got) != 1 { + t.Fatalf("want 1 commit, got %+v", got) + } + if got[0].Login != "" { + t.Errorf("a name must never be reported as a login: %+v", got[0]) + } + if got[0].Name != "Ada Lovelace" || got[0].Email != "ada@analytical.example" { + t.Errorf("lost the git author: %+v", got[0]) } }) }