Repository navigation
π fix(adoption): never adopt or re-home a repository maintainer's own PR - #14305
Conversation
Adoption stands in for a contributor relay that went quiet. It took over projectbluefin/utah-packages#619, whose author is a repository admin: adopted as contributor-offline 16 minutes after the 072 nudge, then closed in favour of a hive copy on a DCO re-home. authorMaintainsRepo reads the author's permission on the repository (admin, or role maintain), cached per repo and author for an hour; a failed read is not cached and does not adopt. It gates a new adoption, releases a live in-place adoption on the next pass, and backstops every supersede path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.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 |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
There was a problem hiding this comment.
Decision needed: @Danathar should decide which of #14304 and #14305 lands for #14301 β they implement the same maintainer-author guard and conflict in Engine and supersedeAdoptedPR, so only one can merge.
Conclusion: this is the same fix as #14304 (opened 33 s earlier, Fixes #14301), and the two conflict rather than stack. One real correctness gap here: the fail-closed error path also fires on the non-collaborator answer, which would disable adoption for exactly the authors it serves.
Read the PR head ce778a6 against v6 merge-base becce0a, plus authorRepoPermission in automerge_sweep.go, reporterHasWriteAccess/label_approval.go, and the test harness.
Overlap with #14304
Both PRs: read GET /repos/{o}/{r}/collaborators/{author}/permission, treat admin/maintain (via role_name) as maintainer, cache per repo/author, guard adoptContributorPR, supersedeAdoptedPR and the in-place settle path, and fail closed on a read error. Both add a new field pair to Engine and insert a guard at the top of supersedeAdoptedPR β a textual conflict, so they cannot both merge. Differences I verified:
- #14304 posts one maintainer note per head (
AdoptionMaintainerNotedHead) and uses a 10-minute cache; #14305 logs only, 1-hour cache, and updateslabels-and-control-signals.md. - #14304 carries a hive
changes_requestedfor a transient read error clearingAdoptedByafter the copy exists. #14305's supersede guard (automerge_contributor_adopt.go:615-620) runs before the copy is opened (callers at:611,:730), so it does not orphan a superseding PR β a point in its favour. - #14305 does not reference #14301 at all; it should
Closes #14301if it is the survivor.
I have not closed anything; flagging for a maintainer to pick one.
Finding (medium) β a 404 is treated as "maintainer"
src/pkg/github/automerge/automerge_contributor_adopt.go:798-802β any error fromGetPermissionLevelreturnstrue.src/pkg/github/label_approval.go:239-240β the same call elsewhere in this repo special-caseshttp.StatusNotFoundas "not a collaborator", i.e. the codebase already expects this endpoint to 404 for a non-collaborator (a fork contributor with no role on the repo β the population adoption exists for).- Consequence if that holds:
adoptContributorPR:565never adopts such a PR, andsettleInPlaceAdoption:876-882releases any in-place adoption already taken, every sweep (errors are not cached, so it never settles). - The harness only scripts a 500 and answers
writefor every login (automerge_contributor_adopt_test.go:68-80); there is no non-collaborator/404 case, so the tests cannot catch this.
Fix is small: branch on githubStatusError(err, http.StatusNotFound) β false the way label_approval.go does, and add a 404 fake to the harness. (Same applies to #14304's adoptionAuthorPermission.)
Minor
automerge_sweep.go:2406already hasauthorRepoPermissiondoing the same GET and normalisation;authorMaintainsRepocould wrap it instead of duplicating the parsing.settleInPlaceAdoption:876releases an in-place adoption on a transient error too (5xx / rate-limit). Low stakes β nothing is closed β but it will flap between adopted and released until the read recovers.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
* π fix: prevent contributor adoption of maintainer PRs Check cached repository permissions before adoption and supersession, release existing maintainer adoptions, and explain the refusal once per head. Fail closed when permissions cannot be read. Co-authored-by: Danathar <6772335+Danathar@users.noreply.github.com> Hive-Run: #14301 Hive-Plan: maintainer-adoption-guard Hive-Spec: issue-14301#acceptance-criteria Signed-off-by: arteeh <arteeh@users.noreply.github.com> * π fix(adoption): keep a live supersede adoption when the author permission read fails guardAdoption cleared the adoption record on every error, so one 5xx on GET collaborators/{author}/permission during the per-pass re-check wiped AdoptedBy for a PR superseded days earlier: settleAdoption then never closed the original when hive/adopt-N merged (or returned it to 072 when it closed), and after the 10-minute error cache the PR was eligible for a second adoption and a second superseding PR. A failed read now blocks the pass without touching the record; only a confirmed admin/maintain author releases the adoption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> * π fix(adoption): skip maintainer refusal on terminal PRs and close open superseders on release The adoption pre-guard in ApplyStageLabels now runs only for non-terminal observations: a maintainer who merged or closed their own adopted PR no longer gets the "will not take over" note on a PR that is forgotten in the same pass. Releasing a maintainer's live supersede adoption now closes a still-open hive superseder with a note before clearing the record, so the release never leaves an untracked duplicate of the maintainer's PR mergeable. A failed superseder lookup/close keeps the record so the next sweep retries instead of orphaning it; the refusal note names the closed superseder. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> * π fix(adoption): make the maintainer refusal marker distinct from the superseder note The refusal marker was a prefix of the superseder-close marker, so a prefix match counted the superseder note as a second refusal note (TestMaintainerAdoptionStaysReturnedAndReleasesLiveAdoption/*/supersede-open). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test: gate superseder-close expectation on first pass TestMaintainerAdoptionStaysReturnedAndReleasesLiveAdoption/supersede-open expected the hive superseder to be closed on every pass, but closeMaintainerSuperseder only runs while AdoptedBy is set; after the first pass clears adoption the second pass correctly does nothing. Expect the close and its note on pass 0 only, matching production. Hive-Fix-Attempt: 2/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> * π fix(adoption): converge on the v6 maintainer-adoption implementation (backport of #14305) Rework as the v5 backport of #14305: authorMaintainsRepo and repoMaintainerLevel with the same names and comments, no persisted Record.AdoptionMaintainerNotedHead and no refusal note. Kept beyond v6: a 10-minute permission cache whose failed reads defer (no takeover, no release), closing a still-open hive superseder with a note before a maintainer adoption is released (#14311), and a cache lock that is not held across GetPermissionLevel (#14313). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> * fix: thread sweep clock into settleInPlaceAdoption Use the now already computed by the stage-label sweep instead of a second c.now() reading, matching the sibling adoption helpers. Hive-Fix-Attempt: 3/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Signed-off-by: arteeh <arteeh@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: arteeh <arteeh@users.noreply.github.com> Co-authored-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Contributor adoption (stage
074, and its supersede and DCO re-home) stands in for a contributor relay that went quiet. It also took over a PR whose author is a repository admin, treated them as "offline", and then shut their PR in favour of a hive copy.Evidence: projectbluefin/utah-packages#619
The author is
hanthor.GET /repos/projectbluefin/utah-packages/collaborators/hanthor/permissionreturnspermission=admin role=admin. All times are 2026-10-25 UTC.author/clanker,role/trustedhive:adopted β¦ reason=contributor-offline mode=in-place: "your hive has been offline since the nudge" (16 minutes later)hive:adopted β¦ reason=dco-rehome mode=supersede superseded_by=621; the original was shutCause
Adoption is gated on author kind (labels and relay registration), presence, grace, hold labels and the diff fence (
adoptContributorPRinautomerge_contributor_adopt.go). Nothing asks whether the author maintains the repository.Change
authorMaintainsRepo. It readsGET /repos/{owner}/{repo}/collaborators/{author}/permissionand treatspermission=admin, orrole_nameadminormaintain, as a maintainer.adoptContributorPR: a maintainer's PR is never adopted, and it stays at 071/072 as today.settleInPlaceAdoption: an in-place adoption taken before this rule, or before the author's permission was raised, is released on the next pass.supersedeAdoptedPR: a backstop for every re-home path (grace, offline,dco-rehome,in-place-stalled).stage/074row inlabels-and-control-signals.md, and a changelog fragment.Testing
go test ./pkg/github/automerge/... ./pkg/stagelabels/... ./pkg/stageengine/... ./pkg/github/ ./cmd/hive/... -count=1(run fromsrc/): allok. That coverspkg/github/automerge2.8s,pkg/stagelabels,pkg/stageengine(withprstageandstageenginetest),pkg/github50.1s andcmd/hive10.5s.New tests in
automerge_adopt_maintainer_test.go:TestRepoMaintainerLeveladmin,maintain(served aspermission=writeplusrole_name=maintain, as GitHub does), case, andwrite/triage/read/noneTestAdoptionSkipsRepositoryMaintaineradminandmaintain: past the grace, across several sweeps, no adoption comment, branch or superseding PR; the PR stays at 072; the permission is read once (cached)TestAdoptionFailsClosedOnPermissionErrorwriteauthor is adopted as before (the error was not cached)TestInPlaceAdoptionOfMaintainerIsReleasedadminis released after the cache expires and is not re-homedThe adoption fake API serves the permission route, defaulting to
write, so every existing adoption test still runs its original path and passes unchanged.Real data: the live API answer for hanthor on utah-packages (
permission=admin role=admin) is whatrepoMaintainerLeveltreats as a maintainer.Guards and checks:
BASE_REF=v6 bash src/scripts/check-changelog-fragment.shis OK,go vetis clean, andgofmtis clean.Contributor checklist
changelog.d/fixed-adoption-skips-repo-maintainers.md.v6.π€ Generated with Claude Code
https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1