From e877153e3cbf5dac4dd53cab5f245344b67f860e Mon Sep 17 00:00:00 2001
From: dan-claude-bot <dan-claude-bot@users.noreply.github.com>
Date: Sat, 18 Jul 2026 20:16:55 +0000
Subject: [PATCH] fix: base the human auto-request on THIS handoff, not review
 history
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

codex's late #85/#98 round-3 finding, valid post-merge: the needs-human
auto-request fired only when the human had NEVER reviewed, so any earlier
human comment or stale approval left a fully-approved PR labeled
needs-human with nobody actually requested — a wedged handoff.
human_request_needed() now asks whether a fresh head-current human review
is missing (live request or head-current approval → nothing to ask;
anything else → request). Five new fixtures cover the wedge, the stale
approval, the satisfied handoff, and request suppression (19 total).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .github/scripts/labels-reconcile.sh | 23 +++++++++++++++++------
 test/labels-reconcile.sh            | 27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh
index 4a225fd..c43e1cf 100644
--- a/.github/scripts/labels-reconcile.sh
+++ b/.github/scripts/labels-reconcile.sh
@@ -73,6 +73,16 @@ bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK
   esac
 }
 
+human_request_needed() { # 0 when needs-human requires a FRESH human request
+  # already requested → the handoff is live; head-current human approval →
+  # nothing left to ask. Anything else (never reviewed, an old comment, an
+  # approval of an older head) stalls the handoff unless we request —
+  # guarding on "has the human ever reviewed" wedged exactly that way.
+  if requested "$HUMAN"; then return 1; fi
+  if [ "$(bot_verdict "$HUMAN")" = APPROVE ]; then return 1; fi
+  return 0
+}
+
 decide_state() { # → the one state:* label this PR should carry
   if [ "$DRAFT" = true ]; then echo state:building; return; fi
   # an explicit human request outranks the bot rounds — it is the final
@@ -136,12 +146,13 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
   desired="$(decide_state)"
 
   # encode the runbook's last step for the no-judgment case: three formal
-  # head-current approvals → the human is asked, once. The guard (never
-  # requested, never reviewed) makes it idempotent — and the shared
-  # concurrency group in labels.yml makes it race-free. With a comment-only
-  # bot on the panel this path stays cold and the AUTHOR requests the human.
-  if [ "$desired" = state:needs-human ] && ! requested "$HUMAN" \
-    && [ -z "$(jq -r --arg u "$HUMAN" '[.[] | select(.user.login == $u)] | last | .state // empty' <<<"$REVIEWS_JSON")" ]; then
+  # head-current approvals → the human is asked, once. The guard asks whether
+  # a FRESH human review is needed for THIS head — never "has the human ever
+  # reviewed", which wedged the handoff after any earlier human comment.
+  # Idempotent (a live request suppresses it); race-free via the shared
+  # concurrency group in labels.yml. With a comment-only bot on the panel
+  # this path stays cold and the AUTHOR requests the human.
+  if [ "$desired" = state:needs-human ] && human_request_needed; then
     run gh api "repos/$REPO/pulls/$n/requested_reviewers" -f "reviewers[]=$HUMAN" --silent
     log "#$n: requested $HUMAN (round passed)"
   fi
diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh
index 643546b..10318a2 100644
--- a/test/labels-reconcile.sh
+++ b/test/labels-reconcile.sh
@@ -119,5 +119,32 @@ REQUESTED="$HUMAN"
 expect "re-requested human is needs-human again" state:needs-human "$(decide_state)"
 REQUESTED=""
 
+# -- an old human comment must not wedge the handoff (codex, #85 round 3) -----
+REVIEWS_JSON="$(reviews \
+  "$(rev "$HUMAN" COMMENTED old1 "early thoughts" t0)" \
+  "$(rev "$BOT1" APPROVED head1 "" t1)" \
+  "$(rev "$BOT2" APPROVED head1 "" t2)" \
+  "$(rev "$BOT3" APPROVED head1 "" t3)")"
+expect "old human comment + three approvals is needs-human" state:needs-human "$(decide_state)"
+expect "old human comment still needs a fresh request" needed "$(human_request_needed && echo needed || echo not-needed)"
+# ...a stale human APPROVAL likewise needs a re-request for the new head
+REVIEWS_JSON="$(reviews \
+  "$(rev "$HUMAN" APPROVED old1 "" t0)" \
+  "$(rev "$BOT1" APPROVED head1 "" t1)" \
+  "$(rev "$BOT2" APPROVED head1 "" t2)" \
+  "$(rev "$BOT3" APPROVED head1 "" t3)")"
+expect "stale human approval needs a fresh request" needed "$(human_request_needed && echo needed || echo not-needed)"
+# ...a HEAD-CURRENT human approval needs nothing more
+REVIEWS_JSON="$(reviews \
+  "$(rev "$HUMAN" APPROVED head1 "" t0)" \
+  "$(rev "$BOT1" APPROVED head1 "" t1)" \
+  "$(rev "$BOT2" APPROVED head1 "" t2)" \
+  "$(rev "$BOT3" APPROVED head1 "" t3)")"
+expect "head-current human approval needs no request" not-needed "$(human_request_needed && echo needed || echo not-needed)"
+# ...and a live request suppresses re-requesting
+REQUESTED="$HUMAN"
+expect "live human request suppresses re-request" not-needed "$(human_request_needed && echo needed || echo not-needed)"
+REQUESTED=""
+
 printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
 [ "$fail" -eq 0 ]