forked from heavy-duty/rig
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>
124 lines
5.4 KiB
Bash
124 lines
5.4 KiB
Bash
#!/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 ]
|