From 7d98b2eac7a98310796909706ac5158c0a58795c Mon Sep 17 00:00:00 2001
From: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Date: Tue, 27 Oct 2026 05:51:55 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(holdguard):=20re-evaluate=20?=
 =?UTF-8?q?legacy=20stop=20records=20when=20base=20moves?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
---
 changelog.d/fixed-holdguard-legacy-base-move.md | 1 +
 src/cmd/hive/hold_guard_update_retry_test.go    | 7 +++++--
 src/pkg/holdguard/holdguard.go                  | 6 ++++--
 src/pkg/holdguard/update_state_test.go          | 7 +++++--
 4 files changed, 15 insertions(+), 6 deletions(-)
 create mode 100644 changelog.d/fixed-holdguard-legacy-base-move.md

diff --git a/changelog.d/fixed-holdguard-legacy-base-move.md b/changelog.d/fixed-holdguard-legacy-base-move.md
new file mode 100644
index 0000000000..dee64c2376
--- /dev/null
+++ b/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.
diff --git a/src/cmd/hive/hold_guard_update_retry_test.go b/src/cmd/hive/hold_guard_update_retry_test.go
index e4f582b006..d7f074d9bb 100644
--- a/src/cmd/hive/hold_guard_update_retry_test.go
+++ b/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")
 	}
diff --git a/src/pkg/holdguard/holdguard.go b/src/pkg/holdguard/holdguard.go
index ad1f68639e..00f933a86b 100644
--- a/src/pkg/holdguard/holdguard.go
+++ b/src/pkg/holdguard/holdguard.go
@@ -371,7 +371,9 @@ 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
@@ -379,7 +381,7 @@ func (e Entry) UpdateStoppedFor(headSHA, baseSHA string) (string, bool) {
 	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
diff --git a/src/pkg/holdguard/update_state_test.go b/src/pkg/holdguard/update_state_test.go
index 54e5d80be7..0fcd2c19f1 100644
--- a/src/pkg/holdguard/update_state_test.go
+++ b/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")
 	}
 }