Skip to content

feat(runway): credit the change author on squash and merge commits - #482

Open
behinddwalls wants to merge 1 commit into
preetam/runway-uri-unit-docsfrom
preetam/runway-pr-author
Open

feat(runway): credit the change author on squash and merge commits#482
behinddwalls wants to merge 1 commit into
preetam/runway-uri-unit-docsfrom
preetam/runway-pr-author

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

Git records an author and a committer separately, and the merger was collapsing the two: command() pins user.name/user.email to the configured merger identity on every invocation, so any commit the merger creates is both authored and committed by SubmitQueue Runway <runway@submitqueue.invalid>. A landed change shows the service, not the person who wrote it — in git log, in blame, and in every tool built on them.

REBASE was never affected: cherry-pick carries each commit's author across on its own. The strategies that mint a fresh commit are the ones that lose it — SQUASH_REBASE builds its commit with reset --soft + git commit, and MERGE creates a --no-ff merge commit. PROMOTE creates no commit at all, so there is nothing to attribute.

What?

The committer stays the merger — it is what applied the change, and that is what a committer means. The author now comes from the change: the author recorded on the commit its URI pins. For a change spanning several commits by different people that is the head commit's author, which is the one identity the request actually names.

The author is read out of the local object store (git show --no-patch --format=%an%x00%ae), so this needs nothing on the wire and costs no network — every referenced commit is already fetched and verified before a step is applied. No change to the Change proto, and no resolver, keeping the property that the merger reads change URIs straight from the payload. A commit recording no usable author (either half missing) falls back to the committer identity rather than failing the merge.

The identity travels through GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL rather than git commit --author. Two reasons: git merge has no --author flag, so the merge commit could not use one; and the environment carries the name and address as separate values, where --author takes a single Name <address> string that git parses back apart — a display name containing an angle bracket splits in the wrong place, and one that parses to nothing makes git search history for a matching author and fail the commit outright.

New author.go holds authorIdent and commitAuthor. run/runCombined/command gained …As variants taking an author; the existing signatures delegate to them with a zero author, so every other call site is unchanged.

Test Plan

bazel test //runway/... — 6/6 targets pass.

New tests in git_merger_test.go: squash credits the change author, the --no-ff merge commit credits the change author, a multi-author change credits its head commit's author, and REBASE keeps each commit's own author. Every one also asserts the committer is still the merger, which is the invariant that separates attribution from impersonation. TestAuthorIdent covers the fallback when either half is missing, and a name containing angle brackets surviving verbatim.

Verified the tests actually catch the bug: reverting just the two attribution call sites fails exactly the three attribution tests and leaves the REBASE one passing, which is the expected profile since REBASE was already correct.

Ref: the equivalent change on the gitfarm merger, uber-code/go-code 454437cc — same intent, different mechanism, because that merger resolves the PR author over RPC where this one has the commits locally.

Stack

  1. feat(runway): wire the git merger into the server #463
  2. feat(runway): reject changes that disagree on provider #476
  3. docs: define the URI as the unit of change #477
  4. @ feat(runway): credit the change author on squash and merge commits #482

## Summary

### Why?

Git records an author and a committer separately, and the merger was collapsing the two: `command()` pins `user.name`/`user.email` to the configured merger identity on every invocation, so any commit the merger creates is both authored and committed by `SubmitQueue Runway <runway@submitqueue.invalid>`. A landed change shows the service, not the person who wrote it — in `git log`, in blame, and in every tool built on them.

`REBASE` was never affected: cherry-pick carries each commit's author across on its own. The strategies that mint a fresh commit are the ones that lose it — `SQUASH_REBASE` builds its commit with `reset --soft` + `git commit`, and `MERGE` creates a `--no-ff` merge commit. `PROMOTE` creates no commit at all, so there is nothing to attribute.

### What?

The committer stays the merger — it is what applied the change, and that is what a committer means. The author now comes from the change: the author recorded on the commit its URI pins. For a change spanning several commits by different people that is the head commit's author, which is the one identity the request actually names.

The author is read out of the local object store (`git show --no-patch --format=%an%x00%ae`), so this needs nothing on the wire and costs no network — every referenced commit is already fetched and verified before a step is applied. No change to the `Change` proto, and no resolver, keeping the property that the merger reads change URIs straight from the payload. A commit recording no usable author (either half missing) falls back to the committer identity rather than failing the merge.

The identity travels through `GIT_AUTHOR_NAME`/`GIT_AUTHOR_EMAIL` rather than `git commit --author`. Two reasons: `git merge` has no `--author` flag, so the merge commit could not use one; and the environment carries the name and address as separate values, where `--author` takes a single `Name <address>` string that git parses back apart — a display name containing an angle bracket splits in the wrong place, and one that parses to nothing makes git search history for a matching author and fail the commit outright.

New `author.go` holds `authorIdent` and `commitAuthor`. `run`/`runCombined`/`command` gained `…As` variants taking an author; the existing signatures delegate to them with a zero author, so every other call site is unchanged.

## Test Plan

✅ `bazel test //runway/...` — 6/6 targets pass.

New tests in `git_merger_test.go`: squash credits the change author, the `--no-ff` merge commit credits the change author, a multi-author change credits its head commit's author, and `REBASE` keeps each commit's own author. Every one also asserts the committer is still the merger, which is the invariant that separates attribution from impersonation. `TestAuthorIdent` covers the fallback when either half is missing, and a name containing angle brackets surviving verbatim.

Verified the tests actually catch the bug: reverting just the two attribution call sites fails exactly the three attribution tests and leaves the `REBASE` one passing, which is the expected profile since `REBASE` was already correct.

Ref: the equivalent change on the gitfarm merger, `uber-code/go-code` 454437cc — same intent, different mechanism, because that merger resolves the PR author over RPC where this one has the commits locally.
@behinddwalls
behinddwalls force-pushed the preetam/runway-pr-author branch from 6a019a1 to bec528c Compare July 31, 2026 23:13
@behinddwalls
behinddwalls marked this pull request as ready for review July 31, 2026 23:13
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners July 31, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant