🐛 fix(automerge): an owner's approval carries over the hive's base merge when main only moved the PR's hunks

changelog.d/fixed-approval-carry-shifted-hunks.md

@@ -0,0 +1 @@
+- An approval or recorded decision now carries over the hive's own merge of the base branch when the base also edited a file the PR changes, as long as the edit is away from the PR's hunks: every hunk is still byte-for-byte what was approved and only its line numbers moved, so the workflow-path fence no longer re-arms and the PR no longer parks at `stage/085-needs-decision` waiting for a fresh approval of the same change (projectbluefin/utah-packages#924). Files under `.github/` still have to read exactly as approved, and a base merge that touched the PR's own hunks or their context lines still needs a new approval.

src/docs/hive-merge.md

@@ -409,7 +409,14 @@ a merge commit authored by the App, committed by GitHub (`web-flow`) or the
 App and carrying a signature GitHub verified, whose parents are the decided
 head and a commit on the base branch, and whose three-dot diff against the
 base is file-for-file (name, status, blob, line counts, patch) identical to
-the decided head's. An App login GitHub derived from an unsigned commit's
+the decided head's. One difference is allowed: when the base branch also
+edited a file the PR modifies, away from the PR's hunks, that file's blob
+differs but every hunk (context, added and removed lines, lengths, section
+heading) is the same and only its line numbers moved; that still counts as
+identical
+([projectbluefin/utah-packages#924](https://github.com/projectbluefin/utah-packages/pull/924)).
+A file under any `.github/` directory gets no such allowance: a workflow or
+action file must read exactly as decided. An App login GitHub derived from an unsigned commit's
 author email does not count, and a compare that lists GitHub's 300-file cap
 cannot be verified, so neither carries
 ([#16860](https://github.com/hivecommons/hive/issues/16860)). Up to five such refreshes may stack before any gate reads
@@ -1040,8 +1047,12 @@ A PR merges through this tier only when all of the following hold:
   head, or the hive's own audit marker for it; the approver must still be
   trusted. A contributor commit re-arms the fence even when it leaves the
   workflow files alone (the existing head-bound rule), as do a rebase, a
-  base merge pushed by anyone but the App, and a base merge that changed a
-  file the PR also changes. A typed `/hive decision approve` that
+  base merge pushed by anyone but the App, a base merge that changed the
+  PR's own hunks, and a base merge that changed anything in a `.github/`
+  file the PR also changes. A base merge that edited some other file the PR
+  changes only away from the PR's hunks (the hunks identical, their line
+  numbers moved) still carries
+  ([projectbluefin/utah-packages#924](https://github.com/projectbluefin/utah-packages/pull/924)). A typed `/hive decision approve` that
   releases the fence while the swarm's `requires_human` verdict asks for a
   person is also recorded as the answer to that question on the head, as
   the dashboard's approval is, so the owner of their own PR (whom GitHub

src/pkg/github/automerge/automerge_contributor_workflow_approval.go

@@ -268,8 +268,12 @@ func (c *Engine) workflowFenceApproval(ctx context.Context, owner, repo string,
 // waits for a fresh approval every time the base moves (projectbluefin/utah
 // #1111). The approver must still be a trusted merger. Any other head
 // change — a contributor commit, even one leaving the workflow files alone;
-// a rebase; a base merge by anyone but the App; a base merge that changed a
-// file the PR also changes — carries nothing and the fence stays armed.
+// a rebase; a base merge by anyone but the App; a base merge that changed
+// the PR's own hunks, or changed anything at all in a `.github/` file the PR
+// also changes — carries nothing and the fence stays armed. A base merge
+// that edited another file the PR changes only where the PR's hunks do not
+// reach (the hunks identical, only their line numbers moved) still carries
+// (projectbluefin/utah-packages#924).
 func (c *Engine) workflowFenceCarriedApproval(ctx context.Context, owner, repo string, pr *gh.PullRequest, number int, headSHA, appLogin string, reviews []*gh.PullRequestReview, comments []*gh.IssueComment) (workflowApproval, bool) {
 	baseRef := strings.TrimSpace(pr.GetBase().GetRef())
 	if c.gh == nil || appLogin == "" || baseRef == "" {

src/pkg/github/automerge/automerge_contributor_workflow_approval_test.go

@@ -333,15 +333,51 @@ func TestContributorLaneWorkflowFenceApprovalCarriesOverHiveBaseRefresh(t *testi
 	}
 	hiveBaseMerge := baseMerge(testHiveAppBotLogin, "web-flow", true)
 	approvedReview := []map[string]any{ownerApprovedReview("clubanderson", approvedHead)}
+	// docFile is a non-workflow file the PR also edits. Main editing the same
+	// doc above the PR's hunk changes the doc's blob and shifts the hunk's
+	// line numbers; editing next to the hunk changes its context lines too
+	// (projectbluefin/utah-packages#924 and #900).
+	docFile := func(blob, patch string) map[string]any {
+		return map[string]any{"filename": "docs/skills/upstream-version-bumps.md", "status": "modified", "sha": blob, "additions": 1, "deletions": 1, "changes": 2, "patch": patch}
+	}
+	const docPatch = "@@ -145,4 +145,4 @@ Verify a fix\n ctx one\n-old line\n+new line\n ctx two"
+	withDoc := func(diff []map[string]any, doc map[string]any) []map[string]any {
+		return append(append([]map[string]any{}, diff...), doc)
+	}
 	for _, tc := range []struct {
-		name        string
-		reviews     []map[string]any
-		comments    []map[string]any
-		headCommit  map[string]any
-		currentDiff []map[string]any
-		wantReason  string
-		wantMarker  string
+		name         string
+		reviews      []map[string]any
+		comments     []map[string]any
+		headCommit   map[string]any
+		approvedDiff []map[string]any
+		currentDiff  []map[string]any
+		wantReason   string
+		wantMarker   string
 	}{
+		{
+			name:         "a base merge that also edited another file the PR changes, away from its hunks, carries the approval",
+			reviews:      approvedReview,
+			headCommit:   hiveBaseMerge,
+			approvedDiff: withDoc(workflowDiff("blobA"), docFile("docOld", docPatch)),
+			currentDiff:  withDoc(workflowDiff("blobA"), docFile("docNew", "@@ -160,4 +160,4 @@ Verify a fix\n ctx one\n-old line\n+new line\n ctx two")),
+			wantMarker:   "<!-- hive:workflow-approval head=sha7 by=clubanderson from=sha6 -->",
+		},
+		{
+			name:         "a base merge that changed the context of the PR's hunk in another file re-arms the fence",
+			reviews:      approvedReview,
+			headCommit:   hiveBaseMerge,
+			approvedDiff: withDoc(workflowDiff("blobA"), docFile("docOld", docPatch)),
+			currentDiff:  withDoc(workflowDiff("blobA"), docFile("docNew", "@@ -160,4 +160,4 @@ Verify a fix\n ctx one\n-old line\n+new line\n main's new line")),
+			wantReason:   contributorLaneReasonWorkflowPath,
+		},
+		{
+			name:         "a base merge that edited the workflow file away from the PR's hunks still re-arms the fence",
+			reviews:      approvedReview,
+			headCommit:   hiveBaseMerge,
+			approvedDiff: workflowDiff("blobA"),
+			currentDiff:  []map[string]any{{"filename": ".github/workflows/ci.yml", "status": "modified", "sha": "blobA2", "additions": 3, "deletions": 1, "changes": 4, "patch": "@@ -9 +9,3 @@\n-old\n+new\n+steps\n+run"}},
+			wantReason:   contributorLaneReasonWorkflowPath,
+		},
 		{
 			name:        "review on the approved head carries over the hive's base merge",
 			reviews:     approvedReview,
@@ -417,7 +453,11 @@ func TestContributorLaneWorkflowFenceApprovalCarriesOverHiveBaseRefresh(t *testi
 			fx.reviews = tc.reviews
 			fx.prComments = tc.comments
 			fx.headCommit = tc.headCommit
-			fx.compareFiles = map[string][]map[string]any{approvedHead: workflowDiff("blobA"), "sha7": tc.currentDiff}
+			approvedDiff := tc.approvedDiff
+			if approvedDiff == nil {
+				approvedDiff = workflowDiff("blobA")
+			}
+			fx.compareFiles = map[string][]map[string]any{approvedHead: approvedDiff, "sha7": tc.currentDiff}
 			fx.compareMergeBase = map[string]string{approvedHead: "base1", "sha7": baseTip}
 			api := newContributorLaneAPI(t, &fx)
 			defer api.Close()

src/pkg/github/pr_decision_base_refresh.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -63,8 +64,10 @@ type BaseRefreshChecker struct {
 // GitHub-verified merge commit created by the App (appBotLogin, see
 // CommitIsVerifiedAppMerge) whose parents are oldHead
 // and a commit on baseRef, and whose three-dot diff against baseRef is
-// file-for-file identical to oldHead's (a compare at GitHub's 300-file cap
-// cannot show that and is false). That is exactly the head the
+// file-for-file identical to oldHead's (sameChangedFiles: hunks that only
+// moved to other line numbers in a file outside `.github/` still count as
+// identical; a compare at GitHub's 300-file cap cannot show any of that and
+// is false). That is exactly the head the
 // require_current_base gate produces when it merges the base into the
 // branch: the PR's content is unchanged, only its base moved. Any other
 // head change — a content commit (even one that also merged the base), a
@@ -241,6 +244,17 @@ func compareAllFiles(ctx context.Context, api BaseRefreshRepos, owner, repo, bas
 // sameChangedFiles reports whether two three-dot diffs touch the same files
 // the same way: name, status, rename source, resulting blob, line counts
 // and — when GitHub returned it for both — the patch text itself.
+//
+// One difference in the resulting blob is tolerated (sameHunksAtShiftedLines):
+// the base branch also edited a file the PR modifies, somewhere the PR's
+// hunks do not reach, so every hunk is byte-for-byte the same — context
+// lines, added and removed lines, hunk lengths — and only its line numbers
+// moved. That is still exactly the change the person approved
+// (projectbluefin/utah-packages#924: main edited a doc the PR also edits, 15
+// lines above the PR's hunks). A file under `.github/` never gets that
+// tolerance: a workflow or action file must read exactly as approved,
+// because a base edit elsewhere in it (its triggers, its permissions) can
+// change what the approved lines do.
 func sameChangedFiles(a, b []*gh.CommitFile) bool {
 	if len(a) != len(b) {
 		return false
@@ -264,16 +278,88 @@ func sameChangedFiles(a, b []*gh.CommitFile) bool {
 		if x.GetFilename() != y.GetFilename() || x.GetStatus() != y.GetStatus() || x.GetPreviousFilename() != y.GetPreviousFilename() {
 			return false
 		}
-		if x.GetSHA() != y.GetSHA() || x.GetAdditions() != y.GetAdditions() || x.GetDeletions() != y.GetDeletions() || x.GetChanges() != y.GetChanges() {
+		if x.GetAdditions() != y.GetAdditions() || x.GetDeletions() != y.GetDeletions() || x.GetChanges() != y.GetChanges() {
 			return false
 		}
+		if x.GetSHA() != y.GetSHA() {
+			if !sameHunksAtShiftedLines(x, y) {
+				return false
+			}
+			continue
+		}
 		if x.Patch != nil && y.Patch != nil && x.GetPatch() != y.GetPatch() {
 			return false
 		}
 	}
 	return true
 }
 
+// baseDriftHunkHeaderRE reads a unified-diff hunk header: the two start
+// lines, the two optional lengths, and the trailing section heading.
+var baseDriftHunkHeaderRE = regexp.MustCompile(`^@@ -\d+(,\d+)? \+\d+(,\d+)? @@(.*)$`)
+
+// sameHunksAtShiftedLines reports whether two entries for the same file,
+// whose resulting blobs differ, carry the same hunks moved to other line
+// numbers: both are plain modifications of a path outside any `.github/`
+// directory, GitHub returned a patch for both, and the patches are equal
+// once each hunk header's two start lines are dropped (hunk lengths and
+// section headings still have to match). Anything else — a missing or
+// empty patch, a header that does not parse, a rename, a `.github/` path —
+// is false.
+func sameHunksAtShiftedLines(x, y *gh.CommitFile) bool {
+	if !strings.EqualFold(x.GetStatus(), "modified") || !strings.EqualFold(y.GetStatus(), "modified") {
+		return false
+	}
+	for _, name := range []string{x.GetFilename(), x.GetPreviousFilename(), y.GetFilename(), y.GetPreviousFilename()} {
+		if underDotGitHub(name) {
+			return false
+		}
+	}
+	if x.Patch == nil || y.Patch == nil || x.GetPatch() == "" || y.GetPatch() == "" {
+		return false
+	}
+	px, okx := patchWithoutHunkStarts(x.GetPatch())
+	py, oky := patchWithoutHunkStarts(y.GetPatch())
+	return okx && oky && px == py
+}
+
+// underDotGitHub reports a path inside a `.github/` directory at any depth.
+func underDotGitHub(name string) bool {
+	name = strings.TrimSpace(name)
+	return strings.HasPrefix(name, ".github/") || strings.Contains(name, "/.github/")
+}
+
+// patchWithoutHunkStarts rewrites each hunk header of a unified-diff patch
+// to keep only its lengths and section heading ("@@ -,3 +,5 @@ heading"),
+// so two patches of the same hunks at different line numbers read the same.
+// false when a header does not parse or there is no hunk at all.
+func patchWithoutHunkStarts(patch string) (string, bool) {
+	lines := strings.Split(patch, "\n")
+	hunks := 0
+	for i, line := range lines {
+		if !strings.HasPrefix(line, "@@") {
+			continue
+		}
+		m := baseDriftHunkHeaderRE.FindStringSubmatch(line)
+		if m == nil {
+			return "", false
+		}
+		oldLen, newLen := strings.TrimPrefix(m[1], ","), strings.TrimPrefix(m[2], ",")
+		if oldLen == "" {
+			oldLen = "1"
+		}
+		if newLen == "" {
+			newLen = "1"
+		}
+		lines[i] = "@@ -," + oldLen + " +," + newLen + " @@" + m[3]
+		hunks++
+	}
+	if hunks == 0 {
+		return "", false
+	}
+	return strings.Join(lines, "\n"), true
+}
+
 // CommitByGitHubLogin reports whether the commit's GitHub-attributed author
 // or committer is login ([bot] suffix tolerated, case-insensitive). GitHub
 // resolves those logins from the git author and committer emails, so on its

src/pkg/github/pr_decision_base_refresh_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"log/slog"
 	"strconv"
+	"strings"
 	"sync"
 	"testing"
 
@@ -410,3 +411,39 @@ func TestHeadIsBaseRefreshOfFailsClosedAtCompareFileCap(t *testing.T) {
 		}
 	}
 }
+
+// A base merge that also edited a file the PR modifies, away from the PR's
+// hunks, leaves every hunk byte-for-byte the same at other line numbers: the
+// blob differs but the change is still the one approved
+// (projectbluefin/utah-packages#924). Anything more — a changed context
+// line or hunk length, a missing patch, a rename, any `.github/` file — is
+// still a different diff.
+func TestSameChangedFilesToleratesShiftedHunksOutsideDotGitHub(t *testing.T) {
+	const approved = "@@ -145,32 +145,82 @@ Verify a fix with `rebuild-rpms.yml`\n ctx\n-old\n+new\n ctx\n@@ -207,7 +257,7 @@ way as libcdio.\n ctx\n-a\n+b"
+	const shifted = "@@ -160,32 +160,82 @@ Verify a fix with `rebuild-rpms.yml`\n ctx\n-old\n+new\n ctx\n@@ -239,7 +289,7 @@ way as libcdio.\n ctx\n-a\n+b"
+	file := func(name, status, blob, patch string) []*gh.CommitFile {
+		f := refreshFile(name, blob, patch)
+		f.Status = gh.Ptr(status)
+		return []*gh.CommitFile{f}
+	}
+	const doc = "docs/skills/upstream-version-bumps.md"
+	if !sameChangedFiles(file(doc, "modified", "3604514", approved), file(doc, "modified", "4da6d8a", shifted)) {
+		t.Fatal("the same hunks at shifted line numbers must compare equal outside .github/")
+	}
+	noPatch := file(doc, "modified", "4da6d8a", "")
+	noPatch[0].Patch = nil
+	for name, tc := range map[string][2][]*gh.CommitFile{
+		"a context line changed":  {file(doc, "modified", "3604514", approved), file(doc, "modified", "4da6d8a", strings.Replace(shifted, " ctx\n@@", " main's line\n@@", 1))},
+		"a hunk length changed":   {file(doc, "modified", "3604514", approved), file(doc, "modified", "4da6d8a", strings.Replace(shifted, "+160,82", "+160,83", 1))},
+		"a section heading moved": {file(doc, "modified", "3604514", approved), file(doc, "modified", "4da6d8a", strings.Replace(shifted, "way as libcdio.", "way as libcue.", 1))},
+		"the patch was omitted":   {file(doc, "modified", "3604514", approved), noPatch},
+		"the file was renamed":    {file(doc, "renamed", "3604514", approved), file(doc, "renamed", "4da6d8a", shifted)},
+		"a workflow file":         {file(".github/workflows/bump.yml", "modified", "a0bbe66", approved), file(".github/workflows/bump.yml", "modified", "39b9f1e", shifted)},
+		"a nested action file":    {file("tools/.github/actions/x/action.yml", "modified", "a0bbe66", approved), file("tools/.github/actions/x/action.yml", "modified", "39b9f1e", shifted)},
+		"identical patch text":    {file(".github/workflows/bump.yml", "modified", "a0bbe66", approved), file(".github/workflows/bump.yml", "modified", "39b9f1e", approved)},
+	} {
+		if sameChangedFiles(tc[0], tc[1]) {
+			t.Errorf("%s: a different blob must not compare equal here", name)
+		}
+	}
+}