Commit Graph

1 Commits

Author SHA1 Message Date
Tonio 24913064ff
feat(commitperclip): surface the Co-Authored-By trailers a squash merge needs (#11498)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work, and it takes contributions from outside the core team
> - Those contributions arrive as PRs, and this repository squash-merges
every one of them
> - A squash collapses the whole branch into a single commit authored by
whoever pressed the button
> - So when a maintainer rebases and lands a contributor's stale PR, the
contributor's name survives only if the squash message carries a
`Co-Authored-By` trailer
> - Nothing prompts for that trailer, and the PR page keeps showing the
original author either way, so losing it is invisible at the moment it
happens
> - This pull request has commitperclip detect the situation and print
the exact trailers to paste
> - The benefit is that keeping an outside contributor's name is a
default rather than something a maintainer has to remember

## Linked Issues or Issue Description

No public issue exists. The problem follows, and it is not hypothetical.

**What happened?**

#11370, #11371 and #11379 landed @stubbi's work yesterday. Each of those
PRs carries a comment from me telling them their authorship would be
preserved. All three squash commits went in without a `Co-Authored-By`
trailer, so `git log` credits none of them:

| commit | landed from | credited |
| --- | --- | --- |
| `66515582e` | #9900 | Claude only |
| `bc0b5a164` | #9501 | Claude only |
| `35a9b9873` | #8982 | Claude only |
| `6542ad1f4` | #11259 |  Jannes Stubbemann + Claude |

The last one has the trailer because that message was written by hand
with the contributor in mind. The only difference between the two
outcomes was memory. Master history cannot be rewritten, so those three
are now credited by comment on the original PRs — which is a worse
record than a commit trailer, and the reason to make this automatic.

**Expected behavior**

When a branch carries commits by someone other than the PR author, the
merger is told what trailers the squash needs.

**Paperclip version or commit**

`master` at `92047cac4`.

## What Changed

- `.github/scripts/check-pr-coauthors.mjs` — new gate.
- `.github/scripts/run-quality-gates.mjs` — fetches the PR's commits and
runs it.
- `.github/scripts/tests/check-pr-coauthors.test.mjs` — 12 cases.
- `.github/workflows/pr.yml` — runs `.github/scripts/tests/`.

### Informational, not a failure

The squash message does not exist while the PR is open. This can neither
be verified there nor fixed there, so failing a PR on it would block
work on something its author cannot satisfy. The gate notices that the
situation applies and prints the lines to paste.

Run against #11370's actual commits it produces exactly what was
missing:

```
This branch carries commits by stubbi. Squash-merging drops that authorship
unless the squash message carries their trailers, and nothing else will notice
if it does not. Add to the squash body when merging:

      Co-Authored-By: Jannes Stubbemann <stubbi@users.noreply.github.com>
```

### Edge cases it handles

Bots skipped; the PR author's own commits skipped; logins compared
case-insensitively (`PR_AUTHOR` does not always arrive in the same case
as the commit author login); each contributor listed once however many
commits they wrote; and a commit GitHub could not match to an account
falls back to its raw git author — that identity being the one most
likely to be lost, not least likely.

Paging stops at the API's own 250-commit ceiling rather than spinning on
full pages of nothing new.

### The test directory was not running

`.github/scripts/tests/` held ten test files covering the existing
gates, and no workflow ran any of them. Adding an eleventh would have
meant adding a test that never executes, so `pr.yml` now runs the
directory. All **149** pass, including the 137 that were already there
and previously unverified in CI.

## Verification

- 149 tests pass via `node --test '.github/scripts/tests/*.test.mjs'` —
the exact command CI now runs.
- The gate was run against the real commit shape from #11370 and
produces the missing trailer verbatim.

This PR is its own negative control: the branch carries only my commits,
so the new gate should stay silent on it. If commitperclip prints a
co-author note below, the gate is wrong.

## Risks

Low. Informational output only — it cannot fail a PR, and `allPassed` is
unchanged.

It adds one API call per gate run (`/pulls/{n}/commits`), fetched in the
same `Promise.all` as the existing PR and files calls.

Enabling the previously-unrun test directory could in principle surface
a pre-existing failure; all 149 pass locally, so it does not.

Revert the commit to restore.

## Model Used

Claude Opus 5 (`claude-opus-5`), through Claude Code. Extended thinking
enabled. Tool use enabled: file read and edit, shell for test runs.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links
- [x] My branch name describes the change and contains no internal
ticket id
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-21 16:44:26 -07:00