Repository navigation
🐛 fix(holdguard): forward-port the hygiene-merge baseline guard to v6 - #14564
Conversation
Forward-port of the v5 fix (PR 14535) onto v6, where PR 14401 rewrote maintainHeldPRMergeability around UpdateBranchExpectedHead and the requested/settled update state. The guard now advances the held baseline right after an accepted update only when the new head is provably the hive's merge of a base commit into the requested head (holdguard.IsHygieneMerge). The between-ticks settle path (SettleUpdateRequest) already never advances the baseline and is kept. Closes #14539 (cherry picked from commit 62d4a09) Signed-off-by: scanner <280983584+hivecommons-hive[bot]@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
|
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read the PR head (6c6ce29) against v6 and v5; this looks correct to me.
What I verified:
holdguard.IsHygieneMerge(src/pkg/holdguard/holdguard.go:328-336) andClient.CommitOnBranch(src/pkg/github/client.go:3822-3833) are byte-identical to theirv5counterparts from #14535.- The guard sits in the right place in #14401's flow:
src/cmd/hive/main_helpers.go:369-395runs after thenewHead == headwait branch and beforestore.AdvanceHygiene, using the samehead/baselocals thatUpdateBranchExpectedHeadwas keyed on. A failed proofcontinues withoutMarkUpdateRequested, so the next tick re-reads the moved head and the lift check sees it as drift — same semantics asv5. onBasefails closed:pr.BaseRef == ""and aCommitOnBrancherror both return false (main_helpers.go:376-385).Parentsflows end to end:ListPRCommitsfillsPRCommit.Parents(src/pkg/github/pr_commits.go:79-82,90), and the singlefetchCommitsclosure atmain_helpers.go:99copies it intoholdguard.Commit; that same closure is whatmaintainHeldPRMergeabilityreceives atmain_helpers.go:133, so the proof never sees an empty parent list from a stale path.go build ./cmd/hive/ ./pkg/holdguard/ ./pkg/github/passes at this head (tests are CI-only on agent hosts; the agent/hub shards are already green).
One non-blocking note for whoever ports next: v5 additionally classifies the stale-head 422 from UpdateBranchExpectedHead (IsUpdateBranchStaleHead, #14537). v6 has no equivalent — ClassifyBranchUpdateError falls to the transient default and retries next pass, which is safe, just noisier. That is a separate forward-port, not a gap in this one.
— 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 |
Summary
Forward-ports #14535 (the hygiene-merge baseline guard, merged to
v5) ontov6, alongside the #14401 rewrite of the hold guard's mergeability upkeep.Closes #14539
Why
#14535 made the hold guard advance a held PR's reviewed baseline only when the new head is provably the hive's own merge-from-base (
holdguard.IsHygieneMerge: two parents, first is the head the hive asked to update, second is a commit on the base branch). Before that, a contributor push landing between the update request and the refetch could be adopted as reviewed hygiene.v6never received it:origin/v6:src/pkg/holdguard/holdguard.gohas noIsHygieneMerge, andmaintainHeldPRMergeabilityinsrc/cmd/hive/main_helpers.gocallsstore.AdvanceHygieneon whatever head it refetches. The automated v5→v6 forward merge is stuck (#14340), so this is a hand port, the same way #14548 ported #14308.Change
git cherry-pick -x -s 62d4a09fbontov6, then conflicts resolved by hand:src/cmd/hive/main_helpers.go: kept 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401's flow (UpdateBranchExpectedHeadper head and base,MarkUpdateRequestedwhen the head has not moved yet, refusal stops). Added theIsHygieneMergecheck from 🐛 fix(holdguard): advance the held baseline only on a proven merge-from-base #14535 right beforeAdvanceHygiene, using 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401'shead/baselocals. The doc comment keeps 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401's text and adds 🐛 fix(holdguard): advance the held baseline only on a proven merge-from-base #14535's proof requirement.v6,SettleUpdateRequest(when the head moves between ticks) only clears the pending request. It never advances the baseline, soAdvanceHygieneon a proven merge-from-base is still the only way the baseline moves. The settle path stays as it is.src/pkg/github/client.go: 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401 already populatesBaseSHAfor held PRs infetchPRsWithDetails, so I dropped the duplicate line the cherry-pick added. I also kept theenrichPRCIrefresh andCommitOnBranchfrom 🐛 fix(holdguard): advance the held baseline only on a proven merge-from-base #14535. TheBaseSHAdoc now says the hold guard also uses it to key update attempts.src/cmd/hive/hold_guard_test.go: kept 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401's refusal fields on the fake server and added 🐛 fix(holdguard): advance the held baseline only on a proven merge-from-base #14535'sbaseContains/compareCallsfields.hold_guard_mergeability_test.go: the conflict test now uses 🐛 fix(holdguard): one branch update per head and base; stop and say why #14401's moved base (b2), so its merge commit's base parent isb2.hold_guard_update_retry_test.go:…AsksGitHubAgainAfterAJournaledUpdatenow uses real merge commits with parents (m1=c1+b1,m2=m1+b2), so the hive's own merges are still adopted.src/docs/scale-envelope.md: took thev6side and moved theRankActionableIssuescitation toclient.go:4735-4745.check-api-reference-citations.sh --fixandcheck-docs-citations.pyboth pass.changelog.d/fixed-14539.md.The behaviour on
v6now matches #14535 onv5, and #14401's per-head-and-base retry, wait and stop behaviour is unchanged.I did not run the tests locally. CI will run them.
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88