From 0e64f46221bea046fd59698590de059a656e6aeb Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 9 Aug 2026 21:52:09 +0200 Subject: [PATCH] fix(#3129): treat a blank head sha as unknown too, not as a real value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit argus's review note: the emptiness guard was one-sided. A reviewed sha of "" was already treated as unknown, but a head of "" was not — so a forge returning an empty string rather than omitting the field would make every review compare unequal and mark the whole PR stale. That is the wrong-direction failure this function exists to prevent, and the asymmetry contradicted its own doc comment. Both sides now check emptiness, for the same reason: a blank string is a value the forge sent, not a sha it has. --- hive-forge/src/verbs/mod.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index ec1d1d42..38050701 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -274,7 +274,11 @@ fn reviewed_older_head(reviewed_sha: Option<&str>, head_sha: Option<&str>) -> bo let (Some(head), Some(reviewed)) = (head_sha, reviewed_sha) else { return false; }; - !reviewed.is_empty() && reviewed != head + // Both emptiness checks matter, and for the same reason: a blank string + // is a value the forge sent, not a sha it has. Treating one as real would + // make every review compare unequal and mark the whole PR stale — the + // direction this whole function exists to avoid. + !head.is_empty() && !reviewed.is_empty() && reviewed != head } /// Latest non-comment review per reviewer on a PR. Reviews come @@ -395,6 +399,10 @@ mod tests { assert!(!reviewed_older_head(None, Some("f4c47088"))); assert!(!reviewed_older_head(Some("81292f14"), None)); assert!(!reviewed_older_head(Some(""), Some("f4c47088"))); + // Absent and blank have to behave the same on BOTH sides — a blank + // head that counted as real would mark every review on the PR stale. + assert!(!reviewed_older_head(Some("81292f14"), Some(""))); + assert!(!reviewed_older_head(Some(""), Some(""))); } #[test]