Repository navigation
✨ Credit issue authors on clanker-opened PRs at merge time - #14585
Conversation
Every hive-performed merge (self-authored, trusted-author, contributor-lane
and queue sweeps, hive-merge relay) now appends Co-authored-by trailers for
the human authors of the issues the PR closes to the squash (or merge)
commit message, so reporters land on the contributor graph when an external
clanker fixed their issue — not only when the hive opened the PR itself.
Bots/Apps, the PR author and logins already credited on the branch are
skipped; branch commit bodies and Signed-off-by lines are reproduced ahead
of the new trailers; the logins are recorded as issue_authors= on the merge
audit entry and in review-evidence merge.issue_authors. One cached
GET /issues/{n} per newly referenced issue, attributed hive:merge_credit.
Switch: auto_merge.credit_issue_authors (default on).
Fixes #14581
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Changelog fragment in the wrong directoryFragments live in git mv src/changelog.d/added-merge-issue-author-credit.md changelog.d/added-merge-issue-author-credit.md
git commit -s -m "changelog: move fragment to repo-root changelog.d/"See |
|
@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this hive's repositories have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
There was a problem hiding this comment.
Read the core (merge_credit.go), all four sweep call sites, MergePRVia, and the audit/evidence hand-off at the PR head; go build of pkg/github, pkg/github/automerge, pkg/config, pkg/evidence is clean. Looks correct to me.
What I verified:
go-github'sPullRequests.Merge(..., commitMessage, opts)maps the fifth argument tocommit_messageonly;opts.CommitTitlestays unset inautomerge_merge_credit.go:26andpullrequest.go:434, so GitHub still writes its default<title> (#N)subject. Only the body is replaced.squashCreditMessage(merge_credit.go:318-348) reproduces GitHub'sCOMMIT_MESSAGESsquash body (single commit → body only; several →* subject+ body each) and keeps everySigned-off-byin place. On a commit-list read errorPreparereturns an empty message (merge_credit.go:196-203), so the DCO trail is never dropped for the trailer.uncreditedAuthors(merge_credit.go:277-307) matches by noreply address and by login (bothid+loginand bare local part), so a hive-opened PR that already carriesissue-coauthor.sh's trailer is not double-credited.MergePRVia(pullrequest.go:421-451): the credit reuses the PR already fetched for forward-merge resolution; only amerge-method call fetches it fresh, and only when the switch is on.fetchedTriedprevents a second GET after a failed one.RecordPRMergedAuditconsumes the note viatakeMergeCredit(client.go:4386), so the stash cannot leak across merges of the same number.
Two notes, neither a defect:
- Documented assumption. The rebuilt body matches GitHub's default only when the target repo's
squash_merge_commit_messageisCOMMIT_MESSAGES. I checked hive, rationguard and crux — all three are, so behaviour is unchanged for them. On a repo configuredPR_BODYorBLANK, enabling this (defaulttrue) silently switches the squash body to the commit messages. Worth one sentence in theoperator-reference.md:430row so an operator on such a repo knows to setcredit_issue_authors: false. MergeCreditor.Enabledreadsm.enabledunlocked whileSetCreditIssueAuthors(merge_credit.go:388-397) rewrites it on every review-config PUT (main.go:1644→installReviewRelaySettings). This mirrors the existingSetReviewEvidenceidiom (review_evidence.go:67), so it is not a regression — just noting the sweep can observe it concurrently.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Changelog: this PR changes code but carries no changelog entry If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
The switch is yaml-only, like auto_merge.squash_only. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
@clubanderson this PR is held at Failing checks:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
|
merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at fb5eeec. |
There was a problem hiding this comment.
Read merge_credit.go, the four sweep call sites, MergePRVia, and RecordPRMergedAudit against the tree at fb5eeec; go build ./pkg/github/... ./pkg/config/ ./pkg/evidence/ passes. The credit logic looks correct to me: Prepare never fails the merge, rebase is skipped, merge-from-base commits are excluded from the rebuilt squash body (merge_credit.go:323), every commit message — Signed-off-by included — is kept verbatim, and uncreditedAuthors dedupes by id-prefixed address and by login so hive-opened PRs don't gain a second trailer. The takeMergeCredit hand-off into the audit/evidence path is consumed exactly once.
One blocker, and it is the red check — the changelog fragment is in the wrong directory.
src/changelog.d/added-merge-issue-author-credit.md should be changelog.d/added-merge-issue-author-credit.md. The guard treats src/changelog.d/ as misplaced and refuses to autofix it: .github/workflows/changelog-fragment-guard.yml:181-190 — "A fragment under src/changelog.d/ is never compiled … fail with the move instruction instead" → ::error … Run: git mv $m changelog.d/$(basename "$m") and exit 1. That is the changelog-fragment-guard FAILURE on this head; the PR will stay UNSTABLE until the file moves.
Open question (not a defect, I could not verify it from the tree): the squash body is rebuilt in GitHub's commit-messages format (* subject + body per commit, merge_credit.go:316-351). Nothing in pkg/ reads a repo's squash_merge_commit_message setting, so on a repo configured for "PR title and description" a credited merge would replace the PR-body squash message with commit bullets. The existing "Sign-off survives the squash" check implies the hive's repos already use the commit-messages default, so this is probably moot — flagging only so the operator doc (hive-merge.md attribution section) can say which setting is assumed.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read merge_credit.go, automerge_merge_credit.go, the four sweep call sites and MergePRVia against v5 at merge-base 7d61076; checked the repo's merge settings (squash_merge_commit_message=COMMIT_MESSAGES, merge_commit_message=PR_TITLE), which is exactly the shape squashCreditMessage / mergeCommitCreditMessage rebuild. The mechanics look correct to me; one concrete fix is needed for the red check, and one open question.
Fix needed — the red changelog-fragment-guard. The fragment is at src/changelog.d/added-merge-issue-author-credit.md, but the convention is repo-root changelog.d/ (see changelog.d/README.md and the existing changelog.d/added-13290-relay-status.md style). .github/workflows/changelog-fragment-guard.yml:132 copies src/changelog.d/ fragments into the rejected set and :181 says "A fragment under src/changelog.d/ is never compiled". Move it to changelog.d/added-14585-merge-issue-author-credit.md and the check should go green.
Open question (not verified, so not a defect): when the hive squash-merges with GitHub's default message, GitHub generates Co-authored-by: trailers for branch commit authors who differ from the PR author. Once commit_message is supplied (automerge_merge_credit.go:37, pullrequest.go:449), the body is the one we send, and squashCreditMessage (merge_credit.go:300-335) rebuilds only from commit messages, not commit authors. If GitHub does not add those trailers to a supplied message, a mixed-author branch (e.g. a human pushing a fix onto a bot PR, or the reverse) would lose the auto co-author it gets today — every time the PR closes an issue with a human author. I could not confirm GitHub's behaviour from the docs; worth checking on one real mixed-author squash before relying on it, or carrying distinct commit authors into the trailer block as well.
What I verified as correct:
cachedPRRework-style caching inissueAuthor(merge_credit.go:220-247) is mutex-guarded;Enabled/Prepare/MergeCreditor()are nil-safe, so theMergePRViapath with no API client (pullrequest.go:442) and test transports lackingmergeCreditTransportmerge exactly as before.- Rebase returns an empty credit (
merge_credit.go:160); commits read failure keeps GitHub's default message (:190), so no path can drop aSigned-off-by. uncreditedAuthors(:270-298) matches by both noreply address and login, so hive-opened PRs withissue-coauthor.shtrailers are not double-credited.takeMergeCreditconsumes the stash on thepr_mergedaudit (client.gorecordMerge path), soissue_authors=only appears for merges that actually happened.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Second-round catch-up for #14340: the 7 v5 commits that landed after the main forward-merge (#14585 credit issue authors, #14583, #14579 merge gate follows live sentinel label / HumanApprover, #14580, #14571, #14427, #14428). Hand-resolved (16 files): - stagelabels/human.go: union of v6 Route.ManualMerge and v5 HumanApprover, ManualMergeRule, LaneReason. - config: v5 auto_merge.credit_issue_authors applied in review_config.go (v6 moved AutoMergeConfig there). - github/review_evidence*: v5 (blocked param, Notice = !blocked); sentinel_sweep.go keeps v6 adoption handling with the v5 one-line change. - automerge: linked-issue gate (v6 #14184 fallout) now seeds the v5 MergeCreditor's author cache so a merge-ready PR costs one issue GET; v5's lane-side decisionActions test dropped (v6 renders actions in stagelabels.DecisionActions). - dashboard/stage_tracker.go: v5 #14579 stageTrackerSentinel supersedes v6 #14053 (cleared label: alert off, findings still fill the tooltip). - dashboard/server.go: v5 FleetRun + v6 AdmissionLabels; index.html: v5 attention title + v6 "Hive end to end first" sort. - docs: v5 text with v6-only sentences re-applied; api-reference citations regenerated. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Fixes #14581
What
Every hive-performed merge now credits the human authors of the issues a PR closes with
Co-authored-by: <login> <id+login@users.noreply.github.com>trailers on the squash (or merge) commit, so a reporter whose issue was fixed by an external clanker lands on the contributor graph the way they already do when the hive opens the PR (RFC #6588 / #7208).Where the trailer is injected
pkg/github/merge_credit.go—MergeCreditor.Prepare: parsesFixes|Closes|Resolves #Nfrom the PR body (reusesParseClaimedIssues), one cachedGET /issues/{n}per newly referenced issue (WithRESTCaller(ctx, "hive:merge_credit")), skips bots/Apps (type=Bot,[bot]/-bot,github-actions*), the PR author, issues that are PRs; de-dupes againstCo-authored-bylines already on the branch (noreply address or login).pkg/github/automerge/automerge_merge_credit.go—Engine.mergeOptions, used by all four sweepMergecall sites (self-authored, trusted-author, queue, contributor lane).pkg/github/pullrequest.goMergePRVia—hive-mergerelay / forge path.Signed-off-bypreserved, merge-from-base commits skipped) with the trailers appended after a blank line;merge-method/merge→ trailer on the merge commit; rebase → untouched. If commits cannot be read, GitHub's default message is kept.issue_authors=kv on thepr_mergedaudit entry andMergeEvent.IssueAuthorsin review evidence.auto_merge.credit_issue_authors(default true), documented inoperator-reference.md; attribution section inhive-merge.md.Tests
pkg/github/merge_credit_test.go(trailer composition, dedupe, bot/self skip, multi-issue, cache, disabled, rebase, merge method, missing issue, commits error, MergePRVia on/off) andpkg/github/automerge/automerge_merge_credit_test.go(contributor-lane squash message with two preserved sign-offs, config off → nocommit_messageand zero issue/commit reads, self skip, hive-opened PR with existing trailer → no duplicate). Notime.Sleep. Coverage: pkg/github 88.9%, automerge 90.0%, evidence 96.8%, config 90.5%.Also fixed the
scale-envelope.mdline citation that moved withclient.go.