Skip to content

ci: only check co-author trailers on commits the PR adds - #576

Open
lmo-selected wants to merge 1 commit into
yc-software:mainfrom
lmo-selected:coauthor-check-first-parent
Open

ci: only check co-author trailers on commits the PR adds#576
lmo-selected wants to merge 1 commit into
yc-software:mainfrom
lmo-selected:coauthor-check-first-parent

Conversation

@lmo-selected

@lmo-selected lmo-selected commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

The Co-author trailers check walks every commit reachable from the head:

base=$(git merge-base "$BASE_SHA" "$HEAD_SHA")
for sha in $(git rev-list "$base..$HEAD_SHA"); do

That range is not "the commits this PR adds" — it also contains everything a merge brought in. So a PR that merges another branch into itself is checked against that branch's commits as well as its own.

The failure mode is a check that cannot be satisfied. Merge main into a branch and the range picks up main's history, including 9ff90fc ("credits: record co-authors the squash merges dropped"), which records 13 @users.noreply.github.com trailers. The check flags it, and the author's options are rewriting published history or merging red. Neither is a review signal.

--first-parent restricts the range to the commits on the PR's own line of development.

It still catches what it is meant to catch

Both offending shapes sit on the first-parent path, so neither is affected:

  • a commit that introduces a banned trailer
  • a merge commit carrying a banned trailer in its own message

Only the contents of a merged-in branch are skipped, which is history that entered through whatever review that branch had. The added stderr line says so, so someone reading a failure knows the scope.

Repro

* fork: own clean commit
*   Merge branch 'upstream-main' into sync
|\
| * upstream: credit contributors      <- Co-authored-by: <...@users.noreply.github.com>
* | fork: prior work
|/
* base
scan result
git rev-list "$base..$HEAD_SHA" flags upstream: credit contributors
git rev-list --first-parent "$base..$HEAD_SHA" clean

Controls, both still flagged under --first-parent: a branch whose own commit adds a banned trailer, and a branch whose merge commit carries one in its own message.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

The trailer check walks `git rev-list "$base..$HEAD_SHA"`, which includes
every commit reachable from the head, not only the ones the PR contributes.
When a PR merges another branch in, the merged-in commits enter that range
and are checked as if the author had written them.

That makes the check fail on history nobody in the PR can change. A branch
that merges main in picks up main's commits; the check then reports an
offending trailer on a commit that is already on main, and the only ways out
are rewriting shared history or merging red.

Walking first-parent restricts the range to the commits the PR itself adds.
A commit that introduces a banned trailer is still caught, whether it is an
ordinary commit or a merge commit carrying the trailer in its own message,
because both sit on the first-parent path. Only the contents of a merged-in
branch are skipped, which is what the check was never meant to police.
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