fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine

Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
  durable markers (Verdict: Approve / I agree with everything / leading ) —
  the gate to needs-human can actually close. Formal verdicts remain the
  contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
  PR in addressing (agent owes re-request) instead of promoting unreviewed
  code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
  per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
  edits warn instead of wedging; the self-heal claim now matches reality
  (dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
  test/labels-reconcile.sh proves 14 fixture transitions — comment-only
  agreement, stale approval, comment-without-verdict, human precedence and
  human-block — wired into CI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-18 19:31:04 +00:00
parent d3065cbc9a
commit 00c748c879
5 changed files with 284 additions and 73 deletions

View file

@ -9,12 +9,24 @@ set -euo pipefail
# comments, reviews — never from label churn, or the sweep would un-stale its # comments, reviews — never from label churn, or the sweep would un-stale its
# own mark every tick. # own mark every tick.
# #
# The verdict contract (CONTRIBUTING.md): reviews end in approve or
# request-changes. Reality check (#85 round 1): at least one live bot posts
# its agreement as a COMMENTED review and can never formally approve, which
# would park every fully-agreed PR in state:addressing forever. So COMMENTED
# reviews whose body carries a durable agreement signal count as approval —
# the workaround the state machine owes the fleet until every bot speaks the
# formal contract. Any verdict that counts toward needs-human must be bound
# to the CURRENT head SHA: GitHub keeps approvals alive across pushes, and a
# stale approval must never promote unreviewed code to the human.
#
# DRY_RUN=1 narrates every mutation instead of performing it (how this script # DRY_RUN=1 narrates every mutation instead of performing it (how this script
# is rehearsed against the live repo). A workflow_dispatch run also bootstraps # is rehearsed against the live repo). A workflow_dispatch run also bootstraps
# the taxonomy (label create --force), which is how a fresh repo — or a label # the taxonomy (label create --force) — that heal is dispatch-only; the cron
# someone deleted — self-heals. # sweep tolerates a missing label rather than recreating it.
#
# The state machine below is pure (globals in, state out) and covered by
# fixture tests in test/labels-reconcile.sh.
REPO="${REPO:?set REPO to owner/name}"
HUMAN="${HUMAN_REVIEWER:-danmt}" HUMAN="${HUMAN_REVIEWER:-danmt}"
BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl) BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl)
STATES=(state:building state:bots-reviewing state:addressing state:needs-human) STATES=(state:building state:bots-reviewing state:addressing state:needs-human)
@ -26,6 +38,83 @@ run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing
if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi
} }
# ---------------------------------------------------------------------------
# The state machine. Pure functions over four globals, set per PR:
# DRAFT true|false
# HEAD_SHA the PR's current head commit
# REQUESTED newline-separated logins with a review currently requested
# REVIEWS_JSON JSON array of submitted (non-PENDING) reviews
# ---------------------------------------------------------------------------
requested() { grep -qxF "$1" <<<"$REQUESTED"; }
agreement_signal() { # $1 = review body → 0 when it carries a durable agreement
# the signals the live bots actually emit: grok "**Verdict: Approve**",
# codex "Verdict: I agree with everything", claude "✅ … I agree with
# everything". Conservative on purpose: "I agree with most" is NOT a match.
grep -qiE 'verdict:? ?\**approve|i agree with everything|^✅' <<<"$1"
}
bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK
local review state commit body
review="$(jq -c --arg u "$1" \
'[.[] | select(.user.login == $u)] | sort_by(.submitted_at) | last // empty' \
<<<"$REVIEWS_JSON")"
if [ -z "$review" ]; then echo MISSING; return; fi
state="$(jq -r '.state' <<<"$review")"
commit="$(jq -r '.commit_id' <<<"$review")"
body="$(jq -r '.body // ""' <<<"$review")"
case "$state" in
CHANGES_REQUESTED)
# blocks at ANY head — GitHub's own semantic: only a newer review
# from the same reviewer clears it
echo BLOCK ;;
APPROVED)
if [ "$commit" = "$HEAD_SHA" ]; then echo APPROVE; else echo STALE; fi ;;
COMMENTED)
if agreement_signal "$body"; then
if [ "$commit" = "$HEAD_SHA" ]; then echo APPROVE; else echo STALE; fi
else
echo FEEDBACK
fi ;;
*) echo FEEDBACK ;;
esac
}
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
# gate, and a maintainer pulling a PR to themselves early counts too
if requested "$HUMAN"; then echo state:needs-human; return; fi
local b v verdicts=""
for b in "${BOTS[@]}"; do
if requested "$b"; then echo state:bots-reviewing; return; fi
done
for b in "${BOTS[@]}"; do
v="$(bot_verdict "$b")"
if [ "$v" = MISSING ]; then echo state:bots-reviewing; return; fi
verdicts="$verdicts $v"
done
case "$verdicts" in
# FEEDBACK = a comment with no verdict → the agent owes the round-reply.
# STALE = a verdict for an older head → the agent owes a re-request.
*BLOCK* | *FEEDBACK* | *STALE*) echo state:addressing; return ;;
esac
# the bots all approve — but if the human's standing word is
# changes-requested (and nobody re-requested them yet), the agent owes
# fixes, not the human a nag
if [ "$(bot_verdict "$HUMAN")" = BLOCK ]; then
echo state:addressing
else
echo state:needs-human
fi
}
# ---------------------------------------------------------------------------
# The sweep: fetch facts, decide, converge. One PR's failure never aborts the
# others — each PR reconciles in a subshell and a failure just logs.
# ---------------------------------------------------------------------------
bootstrap_labels() { # dispatch-only: ~20 upserts is too chatty for every cron tick bootstrap_labels() { # dispatch-only: ~20 upserts is too chatty for every cron tick
while IFS='|' read -r name color desc; do while IFS='|' read -r name color desc; do
[ -n "$name" ] || continue [ -n "$name" ] || continue
@ -47,68 +136,20 @@ scope:coolify-api|C5DEF5|coolify.ts + OpenAPI reference — the client
EOF EOF
} }
if [ "${GITHUB_EVENT_NAME:-}" = workflow_dispatch ]; then has_label() { grep -qxF "$1" <<<"$LABELS"; }
log "workflow_dispatch: bootstrapping the taxonomy"
bootstrap_labels
fi
now="$(date +%s)" reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
local n="$1" desired remove s args last_activity age
for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do desired="$(decide_state)"
pr="$(gh api "repos/$REPO/pulls/$n")"
draft="$(jq -r '.draft' <<<"$pr")"
labels="$(jq -r '.labels[].name' <<<"$pr")"
requested_logins="$(jq -r '.requested_reviewers[].login' <<<"$pr")"
# PENDING reviews are unsubmitted drafts sitting in someone's browser — not a verdict
reviews="$(gh api --paginate "repos/$REPO/pulls/$n/reviews" --jq '.[]' \
| jq -s '[.[] | select(.state != "PENDING")]')"
latest() { # $1 = login → their latest submitted review state, or empty # encode the runbook's last step: the round passed → the human is asked,
jq -r --arg u "$1" \ # once. The guard (never requested, never reviewed) makes it idempotent —
'[.[] | select(.user.login == $u)] | sort_by(.submitted_at) | last | .state // empty' \ # and the shared concurrency group in labels.yml makes it race-free.
<<<"$reviews" if [ "$desired" = state:needs-human ] && ! requested "$HUMAN" \
} && [ -z "$(jq -r --arg u "$HUMAN" '[.[] | select(.user.login == $u)] | last | .state // empty' <<<"$REVIEWS_JSON")" ]; then
requested() { grep -qxF "$1" <<<"$requested_logins"; }
has_label() { grep -qxF "$1" <<<"$labels"; }
# ---- who is the ball with? (the LABELS.md state machine) ----
desired=""
if [ "$draft" = true ]; then
desired=state:building
elif requested "$HUMAN"; then
# an explicit human request outranks the bot rounds — it is the final
# gate, and a maintainer pulling a PR to themselves early counts too
desired=state:needs-human
else
for b in "${BOTS[@]}"; do
# in requested_reviewers = round (re-)requested and unanswered; never
# reviewed at all = the round hasn't even started for this bot
if requested "$b" || [ -z "$(latest "$b")" ]; then desired=state:bots-reviewing; fi
done
if [ -z "$desired" ]; then
all_approved=1
for b in "${BOTS[@]}"; do
[ "$(latest "$b")" = APPROVED ] || all_approved=0
done
if [ "$all_approved" = 1 ]; then
# the ball is the human's — unless their last word was CHANGES_REQUESTED
# and nobody has re-requested them since (then the agent owes fixes)
if ! requested "$HUMAN" && [ "$(latest "$HUMAN")" = CHANGES_REQUESTED ]; then
desired=state:addressing
else
desired=state:needs-human
fi
else
desired=state:addressing
fi
fi
fi
# encode the runbook's last step: all bots approve → the human is asked, once.
# The guard (never requested, never reviewed) is what makes this idempotent.
if [ "$desired" = state:needs-human ] && ! requested "$HUMAN" && [ -z "$(latest "$HUMAN")" ]; then
run gh api "repos/$REPO/pulls/$n/requested_reviewers" -f "reviewers[]=$HUMAN" --silent run gh api "repos/$REPO/pulls/$n/requested_reviewers" -f "reviewers[]=$HUMAN" --silent
log "#$n: requested $HUMAN (all bots approve)" log "#$n: requested $HUMAN (round passed)"
fi fi
# ---- converge the state:* labels ---- # ---- converge the state:* labels ----
@ -120,21 +161,25 @@ for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[
if ! has_label "$desired" || [ -n "$remove" ]; then if ! has_label "$desired" || [ -n "$remove" ]; then
args=(--add-label "$desired") args=(--add-label "$desired")
[ -n "$remove" ] && args+=(--remove-label "$remove") [ -n "$remove" ] && args+=(--remove-label "$remove")
run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null if run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null; then
log "#$n: state -> $desired${remove:+ (cleared $remove)}" log "#$n: state -> $desired${remove:+ (cleared $remove)}"
else
# a deleted label must not wedge the sweep — dispatch heals the taxonomy
log "#$n: WARNING: label edit failed (missing label? run the workflow manually to bootstrap)"
fi
fi fi
# ---- stale: real activity only, and blocked is legitimately quiet ---- # ---- stale: real activity only, and blocked is legitimately quiet ----
last_activity="$( last_activity="$(
{ {
jq -r '.created_at' <<<"$pr" jq -r '.created_at' <<<"$PR_JSON"
jq -r '.[].submitted_at' <<<"$reviews" jq -r '.[].submitted_at' <<<"$REVIEWS_JSON"
gh api --paginate "repos/$REPO/issues/$n/comments" --jq '.[].created_at' gh api --paginate "repos/$REPO/issues/$n/comments" --jq '.[].created_at'
gh api --paginate "repos/$REPO/pulls/$n/comments" --jq '.[].created_at' gh api --paginate "repos/$REPO/pulls/$n/comments" --jq '.[].created_at'
gh api --paginate "repos/$REPO/pulls/$n/commits" --jq '.[].commit.committer.date' gh api --paginate "repos/$REPO/pulls/$n/commits" --jq '.[].commit.committer.date'
} | sort | tail -n1 } | sort | tail -n1
)" )"
age=$((now - $(date -d "$last_activity" +%s))) age=$((NOW - $(date -d "$last_activity" +%s)))
if has_label blocked || [ "$age" -le "$STALE_AFTER" ]; then if has_label blocked || [ "$age" -le "$STALE_AFTER" ]; then
if has_label stale; then if has_label stale; then
run gh issue edit "$n" -R "$REPO" --remove-label stale >/dev/null run gh issue edit "$n" -R "$REPO" --remove-label stale >/dev/null
@ -144,6 +189,35 @@ for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[
run gh issue edit "$n" -R "$REPO" --add-label stale >/dev/null run gh issue edit "$n" -R "$REPO" --add-label stale >/dev/null
log "#$n: stale ($((age / 3600))h quiet)" log "#$n: stale ($((age / 3600))h quiet)"
fi fi
done }
log "reconciled." main() {
REPO="${REPO:?set REPO to owner/name}"
NOW="$(date +%s)"
if [ "${GITHUB_EVENT_NAME:-}" = workflow_dispatch ]; then
log "workflow_dispatch: bootstrapping the taxonomy"
bootstrap_labels
fi
local n
for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do
(
PR_JSON="$(gh api "repos/$REPO/pulls/$n")"
DRAFT="$(jq -r '.draft' <<<"$PR_JSON")"
HEAD_SHA="$(jq -r '.head.sha' <<<"$PR_JSON")"
LABELS="$(jq -r '.labels[].name' <<<"$PR_JSON")"
REQUESTED="$(jq -r '.requested_reviewers[].login' <<<"$PR_JSON")"
# PENDING reviews are unsubmitted drafts in someone's browser — not a verdict
REVIEWS_JSON="$(gh api --paginate "repos/$REPO/pulls/$n/reviews" --jq '.[]' \
| jq -s '[.[] | select(.state != "PENDING")]')"
reconcile_pr "$n"
) || log "#$n: reconcile failed — continuing with the remaining PRs"
done
log "reconciled."
}
# sourced by test/labels-reconcile.sh for the fixture tests; executed in CI
if [ "${BASH_SOURCE[0]}" = "$0" ]; then
main "$@"
fi

View file

@ -22,3 +22,5 @@ jobs:
- run: npm test - run: npm test
- name: installer is valid bash - name: installer is valid bash
run: bash -n install.sh bin/cast scripts/*.sh run: bash -n install.sh bin/cast scripts/*.sh
- name: labels state-machine tests
run: bash test/labels-reconcile.sh

View file

@ -24,14 +24,13 @@ permissions:
issues: write issues: write
pull-requests: write pull-requests: write
concurrency:
group: labels-${{ github.event.pull_request.number || 'cron' }}
cancel-in-progress: false
jobs: jobs:
scope: scope:
if: github.event_name == 'pull_request_target' if: github.event_name == 'pull_request_target'
runs-on: ubuntu-latest runs-on: ubuntu-latest
concurrency:
group: labels-scope-${{ github.event.pull_request.number }}
cancel-in-progress: true
steps: steps:
- uses: actions/labeler@v5 - uses: actions/labeler@v5
with: with:
@ -40,6 +39,14 @@ jobs:
reconcile: reconcile:
runs-on: ubuntu-latest runs-on: ubuntu-latest
# ONE shared group: every reconcile sweeps every open PR, so cron and
# PR-event runs must serialize or two sweeps race the same PR's labels
# and both pass the request-the-human-once guard. GitHub keeps at most
# one queued run per group (older queued runs are superseded), which
# coalesces bursts instead of piling them up.
concurrency:
group: labels-reconcile
cancel-in-progress: false
steps: steps:
- uses: actions/checkout@v4 # base branch only — never the PR's code - uses: actions/checkout@v4 # base branch only — never the PR's code
- name: reconcile state + stale - name: reconcile state + stale

View file

@ -26,7 +26,11 @@ labels tell you where everything is without opening anything.
them at their discretion; anything blocking — including a question that them at their discretion; anything blocking — including a question that
gates the verdict — is **request changes**, saying what unblocks it. The gates the verdict — is **request changes**, saying what unblocks it. The
reconciler treats a comment-only review as not-approved, so commenting reconciler treats a comment-only review as not-approved, so commenting
without a verdict only stalls the PR. without a verdict only stalls the PR. (Transitional workaround: until
every bot speaks the formal contract, the reconciler counts a COMMENTED
review whose body carries a durable agreement signal — "Verdict: Approve",
"I agree with everything", a leading ✅ — as an approval, bound to the
current head SHA.)
6. **When all three approve**, the final review goes to the maintainer — the 6. **When all three approve**, the final review goes to the maintainer — the
labels workflow requests it automatically. labels workflow requests it automatically.
7. **Checks must be green**: `npm run check`, `npm run build`, and 7. **Checks must be green**: `npm run check`, `npm run build`, and

124
test/labels-reconcile.sh Normal file
View file

@ -0,0 +1,124 @@
#!/usr/bin/env bash
set -euo pipefail
# Fixture tests for the labels-reconcile state machine — the transitions #85's
# review demanded proof of: comment-only agreement closes the gate, a stale
# approval does not promote unreviewed code, a comment without a verdict parks
# the PR on the agent, and an explicit human request outranks everything.
# Dependency-free beyond jq; no network, no daemon — pure decide_state.
cd "$(dirname "$0")/.."
# shellcheck source=.github/scripts/labels-reconcile.sh
. .github/scripts/labels-reconcile.sh
# The DRAFT/HEAD_SHA/REQUESTED/REVIEWS_JSON assignments below are the state
# machine's inputs, consumed inside the sourced decide_state — not unused.
# shellcheck disable=SC2034
BOT1="${BOTS[0]}" BOT2="${BOTS[1]}" BOT3="${BOTS[2]}"
pass=0 fail=0
expect() { # $1 = description, $2 = want, $3 = got
if [ "$2" = "$3" ]; then
pass=$((pass + 1))
else
fail=$((fail + 1))
printf 'FAIL: %s — want %s, got %s\n' "$1" "$2" "$3"
fi
}
rev() { # $1=login $2=state $3=commit $4=body $5=submitted_at → one review object
jq -n --arg u "$1" --arg s "$2" --arg c "$3" --arg b "$4" --arg t "$5" \
'{user: {login: $u}, state: $s, commit_id: $c, body: $b, submitted_at: $t}'
}
reviews() { jq -s '.' <<<"$*"; } # collect review objects into an array
# -- drafts are building, whoever is requested --------------------------------
DRAFT=true HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON='[]'
expect "draft PR is building" state:building "$(decide_state)"
# -- fresh ready PR with bots requested ---------------------------------------
DRAFT=false REQUESTED="$BOT1
$BOT2
$BOT3" REVIEWS_JSON='[]'
expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state)"
# -- a bot that never reviewed keeps the round open ---------------------------
REQUESTED="" REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)")"
expect "missing bot review means bots-reviewing" state:bots-reviewing "$(decide_state)"
# -- comment-only agreement closes the gate (the #85 blocker) -----------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "✅ **Reviewed — I agree with everything.**" t1)" \
"$(rev "$BOT2" APPROVED head1 "Verdict: I agree with everything and have no additional feedback." t2)" \
"$(rev "$BOT3" COMMENTED head1 "**Verdict: Approve** — I agree with this as-is." t3)")"
expect "comment-only agreement counts as approval" state:needs-human "$(decide_state)"
# -- a comment WITHOUT a verdict parks the PR on the agent --------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "🔧 Reviewed — I agree with most; feedback below." t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "comment without verdict is addressing" state:addressing "$(decide_state)"
# -- changes requested blocks, at any head ------------------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" CHANGES_REQUESTED old1 "blockers below" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "changes-requested blocks even from an old head" state:addressing "$(decide_state)"
# -- a stale approval must not promote unreviewed code ------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED old1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "stale approval is addressing (agent owes re-request)" state:addressing "$(decide_state)"
# -- a re-requested bot reopens the round even with an old approval on file ---
REQUESTED="$BOT1"
expect "re-requested bot means bots-reviewing" state:bots-reviewing "$(decide_state)"
REQUESTED=""
# -- only the LATEST review per bot counts ------------------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" CHANGES_REQUESTED head1 "blockers" t1)" \
"$(rev "$BOT1" APPROVED head1 "" t2)" \
"$(rev "$BOT2" APPROVED head1 "" t3)" \
"$(rev "$BOT3" COMMENTED head1 "Verdict: Approve" t4)")"
expect "later approval supersedes earlier block" state:needs-human "$(decide_state)"
# -- an explicit human request outranks the bot rounds ------------------------
REQUESTED="$HUMAN" REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "feedback, no verdict" t1)")"
expect "human requested outranks bots" state:needs-human "$(decide_state)"
REQUESTED=""
# -- human CHANGES_REQUESTED puts the ball back on the agent ------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)" \
"$(rev "$HUMAN" CHANGES_REQUESTED head1 "not yet" t4)")"
expect "human block with bots approving is addressing" state:addressing "$(decide_state)"
# ...and re-requesting the human hands it back to them
REQUESTED="$HUMAN"
expect "re-requested human is needs-human again" state:needs-human "$(decide_state)"
REQUESTED=""
# -- agreement_signal is conservative -----------------------------------------
if agreement_signal "I agree with most; feedback below"; then
fail=$((fail + 1)); echo "FAIL: 'agree with most' must NOT be agreement"
else
pass=$((pass + 1))
fi
if agreement_signal "**Verdict: Request changes** — blockers listed below."; then
fail=$((fail + 1)); echo "FAIL: 'Verdict: Request changes' must NOT be agreement"
else
pass=$((pass + 1))
fi
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]