🐛 fix(holdguard): re-evaluate legacy stop records when base moves

changelog.d/fixed-holdguard-legacy-base-move.md

@@ -0,0 +1 @@
+- Held PRs stopped by an older build's workflows-refusal record are retried once when the base branch moves, instead of being skipped until the next head push.

src/cmd/hive/hold_guard_update_retry_test.go

@@ -250,12 +250,15 @@ func TestMaintainHeldPRMergeabilityRetriesRefusedHeadOnlyAfterHeadOrBaseMoves(t
 }
 
 // A record written by a build that only knew the workflows refusal keeps
-// stopping on its head.
+// stopping on its head only while no base is known; a known base retries.
 func TestMaintainHeldPRMergeabilityHonoursLegacyWorkflowRefusal(t *testing.T) {
 	rec := holdguard.Entry{WorkflowRefusedHeadSHA: "c1"}
-	if reason, ok := rec.UpdateStoppedFor("c1", "b9"); !ok || reason != holdguard.UpdateStopWorkflowPermission {
+	if reason, ok := rec.UpdateStoppedFor("c1", ""); !ok || reason != holdguard.UpdateStopWorkflowPermission {
 		t.Fatalf("UpdateStoppedFor(c1) = %q, %v; want the legacy workflows refusal", reason, ok)
 	}
+	if _, ok := rec.UpdateStoppedFor("c1", "b9"); ok {
+		t.Fatal("a known base must re-evaluate a legacy record")
+	}
 	if _, ok := rec.UpdateStoppedFor("c2", "b9"); ok {
 		t.Fatal("a moved head must not be stopped by the legacy record")
 	}

src/pkg/holdguard/holdguard.go

@@ -371,15 +371,17 @@ const UpdateConfirmWindow = 30 * time.Minute
 
 // UpdateStoppedFor reports whether the guard stopped updating this head on
 // this base, and why. A record from a build that only knew the workflows
-// refusal (WorkflowRefusedHeadSHA) stops on its head alone.
+// refusal (WorkflowRefusedHeadSHA) records no base, so it stops only when the
+// caller has no base to compare; with a known base it falls through so a base
+// move earns a fresh attempt, which records both SHAs.
 func (e Entry) UpdateStoppedFor(headSHA, baseSHA string) (string, bool) {
 	if headSHA == "" {
 		return "", false
 	}
 	if e.UpdateStoppedHeadSHA == headSHA && e.UpdateStoppedBaseSHA == baseSHA && e.UpdateStoppedReason != "" {
 		return e.UpdateStoppedReason, true
 	}
-	if e.WorkflowRefusedHeadSHA == headSHA {
+	if e.WorkflowRefusedHeadSHA == headSHA && baseSHA == "" {
 		return UpdateStopWorkflowPermission, true
 	}
 	return "", false

src/pkg/holdguard/update_state_test.go

@@ -95,8 +95,11 @@ func TestUpdateStoppedForOneHeadAndBase(t *testing.T) {
 	}
 
 	legacy := Entry{WorkflowRefusedHeadSHA: "h3"}
-	if reason, ok := legacy.UpdateStoppedFor("h3", "any-base"); !ok || reason != UpdateStopWorkflowPermission {
-		t.Fatalf("legacy workflows refusal = (%q, %v), want (%q, true)", reason, ok, UpdateStopWorkflowPermission)
+	if reason, ok := legacy.UpdateStoppedFor("h3", ""); !ok || reason != UpdateStopWorkflowPermission {
+		t.Fatalf("legacy workflows refusal without a base = (%q, %v), want (%q, true)", reason, ok, UpdateStopWorkflowPermission)
+	}
+	if _, ok := legacy.UpdateStoppedFor("h3", "any-base"); ok {
+		t.Fatal("a legacy record has no base, so a known base must re-evaluate rather than stop")
 	}
 }