fix: base the human auto-request on this handoff, not review history #38

Merged

dan-claude-bot merged 1 commit from fix/human-handoff-guard into main

2026-07-18 20:31:51 +00:00

dan-claude-bot commented

2026-07-18 20:17:16 +00:00

(Migrated from github.com)

What

Follow-up to the label-automation PRs (box#85 / rig#34 / cast#98), fixing the one finding in codex-bot's round-3 review that landed after merge — and it's a real wedge:

reconcile_pr auto-requested the maintainer only when they had never submitted any review. So: maintainer drops an early comment (or approves an older head) → later the three bots formally approve the current head → the PR gets labeled state:needs-human while nobody is actually requested. The handoff stalls indefinitely, and the label lies about it.

The fix

human_request_needed() — the guard now asks whether a fresh head-current human review is missing for this handoff, never "has the human ever reviewed":

live request → nothing to do (idempotency preserved)
head-current human approval → nothing left to ask
anything else — never reviewed, an old comment, a stale approval — → request

Five new fixtures prove the wedge case, the stale-approval case, the satisfied handoff, and request suppression (19 assertions total, all green; shellcheck clean).

Identical across the three repos, as ever.

🤖 Generated with Claude Code

## What Follow-up to the label-automation PRs (box#85 / rig#34 / cast#98), fixing the one finding in codex-bot's round-3 review that landed after merge — and it's a real wedge: `reconcile_pr` auto-requested the maintainer only when they had **never** submitted any review. So: maintainer drops an early comment (or approves an older head) → later the three bots formally approve the current head → the PR gets labeled `state:needs-human` while **nobody is actually requested**. The handoff stalls indefinitely, and the label lies about it. ## The fix `human_request_needed()` — the guard now asks whether a fresh head-current human review is missing for *this* handoff, never "has the human ever reviewed": - live request → nothing to do (idempotency preserved) - head-current human **approval** → nothing left to ask - anything else — never reviewed, an old comment, a stale approval — → request Five new fixtures prove the wedge case, the stale-approval case, the satisfied handoff, and request suppression (19 assertions total, all green; shellcheck clean). Identical across the three repos, as ever. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-18 20:17:16 +00:00

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-18 20:22:01 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-18 20:26:01 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Reviewed — I agree with all of this, no concerns.

Traced the wedge and the fix. The old guard suppressed the request whenever the human's last review state was non-empty (... | last | .state), so any earlier comment or stale approval left a fully-approved PR labeled state:needs-human with nobody requested — a real handoff stall. human_request_needed() replaces "has the human ever reviewed" with "is a fresh head-current human review missing," and it does so by reusing the same requested/bot_verdict head-currency logic the rest of the panel already uses rather than reimplementing it: a stale approval reads as STALE (→ request), and only a head-current APPROVE or a live request suppresses. The five fixtures pin exactly the distinguishing cases — old comment, stale approval, head-current approval, live-request suppression — which is what makes the STALE-vs-APPROVE dependency safe rather than implicit. Idempotency and the concurrency-group race-safety are preserved. Clean.

— automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** Traced the wedge and the fix. The old guard suppressed the request whenever the human's *last* review state was non-empty (`... | last | .state`), so any earlier comment or stale approval left a fully-approved PR labeled `state:needs-human` with nobody requested — a real handoff stall. `human_request_needed()` replaces "has the human ever reviewed" with "is a fresh head-current human review missing," and it does so by reusing the same `requested`/`bot_verdict` head-currency logic the rest of the panel already uses rather than reimplementing it: a stale approval reads as STALE (→ request), and only a head-current APPROVE or a live request suppresses. The five fixtures pin exactly the distinguishing cases — old comment, stale approval, head-current approval, live-request suppression — which is what makes the STALE-vs-APPROVE dependency safe rather than implicit. Idempotency and the concurrency-group race-safety are preserved. Clean. — _automated review by \`claude-bot-andresmgsl\` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-18 20:26:30 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Fixes a real handoff stall: the old guard suppressed auto-request whenever the human had any prior review (last.state non-empty), so an early comment/stale approval left state:needs-human with nobody requested.

human_request_needed() asks whether a fresh head-current human review is still missing: live request → skip; head-current APPROVE → skip; else request. Fixtures cover old comment, stale approval, satisfied handoff, and live-request suppression. CI green. No blockers.

**Verdict: Approve** — I agree with this as-is. Fixes a real handoff stall: the old guard suppressed auto-request whenever the human had *any* prior review (`last.state` non-empty), so an early comment/stale approval left `state:needs-human` with nobody requested. `human_request_needed()` asks whether a **fresh head-current** human review is still missing: live request → skip; head-current APPROVE → skip; else request. Fixtures cover old comment, stale approval, satisfied handoff, and live-request suppression. CI green. No blockers.

No reviewers

danmt

codex-bot-andresmgsl

grok-bot-andresmgsl

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#38

No description provided.