fix(#3129): treat a blank head sha as unknown too, not as a real value

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

@@ -237,6 +237,14 @@ fn edit_distance(a: &str, b: &str) -> usize {
 /// string still reads `APPROVED` / `REQUEST_CHANGES` — so surfacing them
 /// stops the CLI from reporting a no-longer-valid review as still-good, and
 /// [`ReviewInfo::superseded`] rolls both into one "doesn't count" check.
+///
+/// ⚠️ **`stale` here is not forgejo's flag alone.** That flag is eventually
+/// consistent: seconds after a push it still reports the pre-push answer, so
+/// a verdict against the previous head reads as current in exactly the window
+/// where someone runs the CLI right after pushing. [`latest_reviews`] therefore
+/// ORs it with a direct comparison of the review's own `commit_id` against the
+/// PR head — the flag is right *eventually*, the comparison is right
+/// *immediately*, and either alone is worse than both.
 pub(crate) struct ReviewInfo {
     pub login: String,
     pub state: String,
@@ -253,6 +261,26 @@ impl ReviewInfo {
     }
 }
 
+/// Whether this review was submitted against a commit that is no longer the
+/// PR head — the race-free half of the staleness check.
+///
+/// Fails **closed on unknowns**, i.e. "not stale": either side missing means
+/// we cannot show the head moved, and the cost of guessing wrong in that
+/// direction is one redundant re-review, where the other direction would
+/// silently void every verdict on the PR (blocking nothing, but reporting a
+/// ready PR as unreviewed and inviting a re-request that dismisses a real
+/// approval).
+fn reviewed_older_head(reviewed_sha: Option<&str>, head_sha: Option<&str>) -> bool {
+    let (Some(head), Some(reviewed)) = (head_sha, reviewed_sha) else {
+        return false;
+    };
+    // 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
 /// oldest-first, so a later verdict from the same user supersedes an
 /// earlier one; `COMMENT` / `PENDING` reviews carry no verdict and are
@@ -266,6 +294,16 @@ impl ReviewInfo {
 pub(crate) fn latest_reviews(client: &Client, repo: &str, pr: u64) -> Result<Vec<ReviewInfo>> {
     let (owner, name) = crate::client::split_repo(repo)?;
     let pr = index(pr)?;
+    // The head this PR currently points at, used to age out verdicts forgejo
+    // has not marked stale yet (see `ReviewInfo`). Best-effort: on any failure
+    // we fall back to forgejo's flag alone, which is today's behaviour — a
+    // missing head must never make every review look superseded.
+    let head_sha = client
+        .api()
+        .repo_get_pull_request(owner, name, pr)
+        .send()
+        .ok()
+        .and_then(|pull| pull.head.as_ref().and_then(|h| h.sha.clone()));
     // Paginate (50/page, 10-page runaway cap — same ceiling the raw
     // client used) so a heavily re-reviewed PR doesn't truncate.
     let mut reviews = Vec::new();
@@ -291,7 +329,8 @@ pub(crate) fn latest_reviews(client: &Client, repo: &str, pr: u64) -> Result<Vec
         if st == "COMMENT" || st == "PENDING" || st.is_empty() {
             continue;
         }
-        let stale = r.stale.unwrap_or(false);
+        let stale = r.stale.unwrap_or(false)
+            || reviewed_older_head(r.commit_id.as_deref(), head_sha.as_deref());
         let dismissed = r.dismissed.unwrap_or(false);
         if let Some(slot) = latest.iter_mut().find(|info| info.login == login) {
             st.clone_into(&mut slot.state);
@@ -338,7 +377,33 @@ pub(crate) fn dependency_summaries(
 
 #[cfg(test)]
 mod tests {
-    use super::pct_encode;
+    use super::{pct_encode, reviewed_older_head};
+
+    #[test]
+    fn review_on_an_older_commit_is_stale() {
+        // The bug this exists for: forgejo still reports `stale: false` here
+        // in the seconds after a push, so the comparison has to catch it.
+        assert!(reviewed_older_head(Some("81292f14"), Some("f4c47088")));
+    }
+
+    #[test]
+    fn review_on_the_current_head_is_not_stale() {
+        assert!(!reviewed_older_head(Some("f4c47088"), Some("f4c47088")));
+    }
+
+    /// Every unknown fails *toward* keeping the verdict. Voiding every review
+    /// on a forge that stopped reporting one of these would be a far louder
+    /// wrong answer than one redundant re-review.
+    #[test]
+    fn unknown_commit_or_head_is_not_stale() {
+        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]
     fn pct_encode_passes_unreserved_through() {