From 1b4594eafcf50874fd73e43e8efc7e387d64a2b8 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sat, 18 Jul 2026 18:31:51 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20label=20automation=20=E2=80=94=20th?= =?UTF-8?q?e=20state=20reconciler,=20path-scoped=20labeler,=20and=20CONTRI?= =?UTF-8?q?BUTING?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The machinery LABELS.md promised. labels.yml runs the reconciler on a 15-minute cron plus PR events (pull_request_target — every PR here is from a fork, where pull_request gets a read-only token; no PR code is ever checked out). The script derives each open PR's state:* from GitHub's own facts and converges labels statelessly; stale is judged from real activity (commits, comments, reviews), never label churn, so the sweep cannot un-stale its own mark. actions/labeler applies scope:* from changed paths. CONTRIBUTING.md is the guideline: the PR loop, and who sets which labels. Rehearsed with DRY_RUN=1 against the live repo; shellcheck-clean. Co-Authored-By: Claude Fable 5 --- .github/labeler.yml | 22 ++++ .github/scripts/labels-reconcile.sh | 149 ++++++++++++++++++++++++++++ .github/workflows/labels.yml | 49 +++++++++ CONTRIBUTING.md | 47 +++++++++ LABELS.md | 15 +-- 5 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 .github/labeler.yml create mode 100644 .github/scripts/labels-reconcile.sh create mode 100644 .github/workflows/labels.yml create mode 100644 CONTRIBUTING.md diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 0000000..e523415 --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,22 @@ +# path → scope:* map for actions/labeler — the PR half of LABELS.md's scope +# story (issues are hand-scoped at triage; paths only exist on PRs). Additive +# only: sync-labels stays off in labels.yml, so a hand-applied scope survives. +"scope:cli": + - changed-files: + - any-glob-to-any-file: ["bin/**", "test/cli.sh"] +"scope:installer": + - changed-files: + - any-glob-to-any-file: ["install.sh"] +"scope:host": + - changed-files: + - any-glob-to-any-file: ["host/**"] +"scope:tiers": + - changed-files: + - any-glob-to-any-file: + ["host/grant-user.sh", "host/revoke-user.sh", "drill/multiuser.sh"] +"scope:templates": + - changed-files: + - any-glob-to-any-file: ["templates/**", "profiles/**"] +"scope:drill": + - changed-files: + - any-glob-to-any-file: ["drill/**"] diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh new file mode 100644 index 0000000..876c11d --- /dev/null +++ b/.github/scripts/labels-reconcile.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +set -euo pipefail + +# labels-reconcile.sh — the automation LABELS.md promises: state labels are +# written by machinery, never by hand. Every run derives each open PR's +# state:* from GitHub's own facts (draft flag, requested reviewers, submitted +# reviews) and converges the labels to it, so a killed run or a hand-moved +# label heals on the next pass. Stale is judged from real activity — commits, +# comments, reviews — never from label churn, or the sweep would un-stale its +# own mark every tick. +# +# 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 +# the taxonomy (label create --force), which is how a fresh repo — or a label +# someone deleted — self-heals. + +REPO="${REPO:?set REPO to owner/name}" +HUMAN="${HUMAN_REVIEWER:-danmt}" +BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl) +STATES=(state:building state:bots-reviewing state:addressing state:needs-human) +STALE_AFTER=$((48 * 3600)) + +log() { printf 'labels: %s\n' "$*"; } + +run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing + if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi +} + +bootstrap_labels() { # dispatch-only: ~20 upserts is too chatty for every cron tick + while IFS='|' read -r name color desc; do + [ -n "$name" ] || continue + run gh label create "$name" -R "$REPO" --color "$color" --description "$desc" --force + done <<'EOF' +state:building|FBCA04|PR is a draft — the coding agent is still building +state:bots-reviewing|1D76DB|Waiting on the bot reviewers to finish the round +state:addressing|D93F0B|All bots reviewed — coding agent owes the single reply + fixes +state:needs-human|8250DF|All bots approve — waiting on the human reviewer +stale|B60205|No activity for 48h — needs a poke (sweep-managed) +blocked|6A737D|Waiting on another PR or issue to land first +release|0E8A16|Release flow and version/packaging work +scope:cli|C5DEF5|bin/box — the command surface +scope:installer|C5DEF5|install.sh, versioned installs, upgrade/uninstall +scope:host|C5DEF5|host/ — setup, teardown, firewall, isolation stack +scope:tiers|C5DEF5|restricted tier — grant/revoke, multi-user +scope:templates|C5DEF5|templates/ — the box seeds +scope:drill|C5DEF5|drill/ — rehearsals, doctor, RUNS.md +EOF +} + +if [ "${GITHUB_EVENT_NAME:-}" = workflow_dispatch ]; then + log "workflow_dispatch: bootstrapping the taxonomy" + bootstrap_labels +fi + +now="$(date +%s)" + +for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do + 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 + jq -r --arg u "$1" \ + '[.[] | select(.user.login == $u)] | sort_by(.submitted_at) | last | .state // empty' \ + <<<"$reviews" + } + 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 + log "#$n: requested $HUMAN (all bots approve)" + fi + + # ---- converge the state:* labels ---- + remove="" + for s in "${STATES[@]}"; do + if [ "$s" != "$desired" ] && has_label "$s"; then remove="$remove,$s"; fi + done + remove="${remove#,}" + if ! has_label "$desired" || [ -n "$remove" ]; then + args=(--add-label "$desired") + [ -n "$remove" ] && args+=(--remove-label "$remove") + run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null + log "#$n: state -> $desired${remove:+ (cleared $remove)}" + fi + + # ---- stale: real activity only, and blocked is legitimately quiet ---- + last_activity="$( + { + jq -r '.created_at' <<<"$pr" + jq -r '.[].submitted_at' <<<"$reviews" + 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/commits" --jq '.[].commit.committer.date' + } | sort | tail -n1 + )" + age=$((now - $(date -d "$last_activity" +%s))) + if has_label blocked || [ "$age" -le "$STALE_AFTER" ]; then + if has_label stale; then + run gh issue edit "$n" -R "$REPO" --remove-label stale >/dev/null + log "#$n: unstale" + fi + elif ! has_label stale; then + run gh issue edit "$n" -R "$REPO" --add-label stale >/dev/null + log "#$n: stale ($((age / 3600))h quiet)" + fi +done + +log "reconciled." diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml new file mode 100644 index 0000000..235441f --- /dev/null +++ b/.github/workflows/labels.yml @@ -0,0 +1,49 @@ +name: labels +# The automation LABELS.md promises. Two halves: +# scope — path-derived scope:* labels on PRs (actions/labeler) +# reconcile — the state:* machine + the stale sweep (.github/scripts/labels-reconcile.sh) +# +# pull_request_target, not pull_request: every PR here arrives from a fork, +# where pull_request (and pull_request_review) run with a READ-ONLY token and +# cannot label anything. _target is safe in this workflow because no PR code +# is ever checked out or executed — labeler reads changed paths via the API, +# and reconcile checks out the BASE branch only. Keep it that way. +# +# Review-submitted transitions (bots finishing a round) ride the cron: there +# is no pull_request_review_target, so the 15-minute tick is the wake signal — +# the same cadence the reviewer bots poll at. +on: + schedule: + - cron: "*/15 * * * *" + workflow_dispatch: # also bootstraps missing labels — run once on a fresh repo + pull_request_target: + types: [opened, reopened, ready_for_review, converted_to_draft, synchronize] + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: labels-${{ github.event.pull_request.number || 'cron' }} + cancel-in-progress: false + +jobs: + scope: + if: github.event_name == 'pull_request_target' + runs-on: ubuntu-latest + steps: + - uses: actions/labeler@v5 + with: + # additive only — a hand-applied scope must survive the machine + sync-labels: false + + reconcile: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 # base branch only — never the PR's code + - name: reconcile state + stale + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + run: bash .github/scripts/labels-reconcile.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ed5d42e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,47 @@ +# Contributing + +How change lands in this repo. The short version: PRs are born as drafts, +three reviewer bots take the first rounds, a human takes the last word — and +labels tell you where everything is without opening anything. + +## The PR loop + +1. **Fork and branch.** Contributors work from forks; upstream branches are + for maintainers. Title the PR conventionally (`feat:`, `fix:`, `docs:`), + and include a `CHANGELOG.md` entry under `## Unreleased` when the change + deserves one. +2. **Open as a draft** while you build. Drafts are invisible to the reviewer + bots on purpose. +3. **When it's ready**: mark ready-for-review and request all three bots — + `claude-bot-andresmgsl`, `codex-bot-andresmgsl`, `grok-bot-andresmgsl`. + They poll roughly every 15 minutes. +4. **Rounds are answered whole.** Wait until all three have reviewed, then + answer the entire round in a **single reply**, push the fixes, and + re-request the bots that didn't approve. Prefer verification over + argument: a test settles what a comment thread can't. +5. **When all three approve**, the final review goes to the maintainer — the + labels workflow requests it automatically. +6. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally + mirror what CI runs; the multi-user rehearsal runs in CI on a real Incus. + +## Labels — who sets what + +The full taxonomy lives in [LABELS.md](LABELS.md). What matters day to day is +who sets each kind — most of it is machinery, and hand-moving a +machine-owned label just gets corrected on the next pass: + +| Labels | Set by | +|---|---| +| `state:*` | the labels workflow ([.github/workflows/labels.yml](.github/workflows/labels.yml)) — recomputed from GitHub's own facts every 15 minutes and on PR events. Never by hand. | +| `stale` | the same workflow — 48h without commits, comments, or reviews. `blocked` PRs are exempt: they are quiet legitimately. | +| `scope:*` on PRs | actions/labeler, from the changed paths ([.github/labeler.yml](.github/labeler.yml)). Additive — you may add more, the machine won't remove them. | +| `scope:*` on issues | you, when opening or triaging — issues have no paths to derive from. | +| `blocked`, `release` | you — automation never guesses intent. | +| `bug` / `enhancement` / `documentation` | you, on issues only — a PR's type already lives in its title. | + +## Issues + +Give issues the same care as PR titles: say the surface in the title, apply a +`scope:` label and a type label (`bug` / `enhancement` / `documentation`) when +you open one, and `blocked` when it waits on something — that is what keeps +the board navigable as the issue count grows. diff --git a/LABELS.md b/LABELS.md index 45005dc..24e78b6 100644 --- a/LABELS.md +++ b/LABELS.md @@ -54,13 +54,16 @@ would just say the same thing twice, drifting apart eventually. State labels are written by automation, never by hand. Every state above is derivable from GitHub's own facts — the draft flag, requested reviewers, -review states, push timestamps — so a scheduled workflow recomputes the state -and reconciles labels statelessly. A hand-moved label is a lie waiting to -happen; the workflow asserts the effective state instead. Until that workflow -lands, treat `state:` labels as advisory. +review states, push timestamps — so the labels workflow +([.github/workflows/labels.yml](.github/workflows/labels.yml)) recomputes the +state and reconciles labels statelessly, on a 15-minute cron plus PR events. +A hand-moved label is a lie waiting to happen; the workflow asserts the +effective state instead. `scope:` labels on PRs are applied from the changed +paths by actions/labeler ([.github/labeler.yml](.github/labeler.yml)); +[CONTRIBUTING.md](CONTRIBUTING.md) says who sets what. -The same workflow bootstraps the taxonomy: it creates any missing label -idempotently. To create them by hand (needs push access): +The same workflow bootstraps the taxonomy: a manual dispatch creates any +missing label idempotently. To create them by hand (needs push access): ```sh gh label create "state:building" --color FBCA04 --description "PR is a draft — the coding agent is still building" --force -- 2.45.2 From 40fdfbb38985f0d807352b23eee70f9647aa1f85 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sat, 18 Jul 2026 18:50:50 +0000 Subject: [PATCH 2/4] =?UTF-8?q?docs:=20reviews=20end=20in=20a=20verdict=20?= =?UTF-8?q?=E2=80=94=20approve=20or=20request=20changes,=20never=20a=20bar?= =?UTF-8?q?e=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer rule: a comment-only review is a non-verdict the state machine (and the board) cannot read. Verdict carries blockingness only; nits ride an approval, blockers — including verdict-gating questions — are request-changes. Co-Authored-By: Claude Fable 5 --- CONTRIBUTING.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ed5d42e..680d45b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,9 +19,19 @@ labels tell you where everything is without opening anything. answer the entire round in a **single reply**, push the fixes, and re-request the bots that didn't approve. Prefer verification over argument: a test settles what a comment thread can't. -5. **When all three approve**, the final review goes to the maintainer — the +5. **Reviews end in a verdict.** A reviewer — bot or human — either + **approves** or **requests changes**, never a bare comment. A + comment-only review is a non-verdict: it doesn't say whether the round + passed, and the state machine (and anyone scanning the board) has to + guess. The verdict carries *blockingness only*, the body carries the + feedback: non-blocking nits ride an **approval** and the author addresses + them at their discretion; anything blocking — including a question that + gates the verdict — is **request changes**, saying what unblocks it. The + reconciler treats a comment-only review as not-approved, so commenting + without a verdict only stalls the PR. +6. **When all three approve**, the final review goes to the maintainer — the labels workflow requests it automatically. -6. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally +7. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally mirror what CI runs; the multi-user rehearsal runs in CI on a real Incus. ## Labels — who sets what -- 2.45.2 From 08d099cce287d8e6df869221b490f94079f3c979 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sat, 18 Jul 2026 19:31:00 +0000 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20verdict=20contract,=20head-bound=20a?= =?UTF-8?q?pprovals,=20serialized=20reconcile=20=E2=80=94=20and=20a=20test?= =?UTF-8?q?able=20state=20machine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/scripts/labels-reconcile.sh | 210 +++++++++++++++++++--------- .github/workflows/ci.yml | 2 + .github/workflows/labels.yml | 15 +- CONTRIBUTING.md | 6 +- test/labels-reconcile.sh | 124 ++++++++++++++++ 5 files changed, 284 insertions(+), 73 deletions(-) create mode 100644 test/labels-reconcile.sh diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index 876c11d..38de0b9 100644 --- a/.github/scripts/labels-reconcile.sh +++ b/.github/scripts/labels-reconcile.sh @@ -9,12 +9,24 @@ set -euo pipefail # comments, reviews — never from label churn, or the sweep would un-stale its # 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 # 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 -# someone deleted — self-heals. +# the taxonomy (label create --force) — that heal is dispatch-only; the cron +# 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}" BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl) 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 } +# --------------------------------------------------------------------------- +# 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 while IFS='|' read -r name color desc; do [ -n "$name" ] || continue @@ -47,68 +136,20 @@ scope:drill|C5DEF5|drill/ — rehearsals, doctor, RUNS.md EOF } -if [ "${GITHUB_EVENT_NAME:-}" = workflow_dispatch ]; then - log "workflow_dispatch: bootstrapping the taxonomy" - bootstrap_labels -fi +has_label() { grep -qxF "$1" <<<"$LABELS"; } -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 - 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")]')" + desired="$(decide_state)" - latest() { # $1 = login → their latest submitted review state, or empty - jq -r --arg u "$1" \ - '[.[] | select(.user.login == $u)] | sort_by(.submitted_at) | last | .state // empty' \ - <<<"$reviews" - } - 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 + # encode the runbook's last step: the round passed → 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. + if [ "$desired" = state:needs-human ] && ! requested "$HUMAN" \ + && [ -z "$(jq -r --arg u "$HUMAN" '[.[] | select(.user.login == $u)] | last | .state // empty' <<<"$REVIEWS_JSON")" ]; then 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 # ---- 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 args=(--add-label "$desired") [ -n "$remove" ] && args+=(--remove-label "$remove") - run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null - log "#$n: state -> $desired${remove:+ (cleared $remove)}" + if run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null; then + 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 # ---- stale: real activity only, and blocked is legitimately quiet ---- last_activity="$( { - jq -r '.created_at' <<<"$pr" - jq -r '.[].submitted_at' <<<"$reviews" + jq -r '.created_at' <<<"$PR_JSON" + jq -r '.[].submitted_at' <<<"$REVIEWS_JSON" 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/commits" --jq '.[].commit.committer.date' } | 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 stale; then 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 log "#$n: stale ($((age / 3600))h quiet)" 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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b36599d..fc23606 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,6 +21,8 @@ jobs: shellcheck -x "${files[@]}" - name: cli tests run: bash test/cli.sh + - name: labels state-machine tests + run: bash test/labels-reconcile.sh # The multi-user rehearsal, on a REAL incus — a GitHub runner is root on a # disposable VM, which is exactly the substrate the rehearsal needs. It runs diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 235441f..c451643 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -24,14 +24,13 @@ permissions: issues: write pull-requests: write -concurrency: - group: labels-${{ github.event.pull_request.number || 'cron' }} - cancel-in-progress: false - jobs: scope: if: github.event_name == 'pull_request_target' runs-on: ubuntu-latest + concurrency: + group: labels-scope-${{ github.event.pull_request.number }} + cancel-in-progress: true steps: - uses: actions/labeler@v5 with: @@ -40,6 +39,14 @@ jobs: reconcile: 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: - uses: actions/checkout@v4 # base branch only — never the PR's code - name: reconcile state + stale diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 680d45b..f080fdf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,7 +28,11 @@ labels tell you where everything is without opening anything. them at their discretion; anything blocking — including a question that gates the verdict — is **request changes**, saying what unblocks it. The 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 labels workflow requests it automatically. 7. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh new file mode 100644 index 0000000..13a5e5f --- /dev/null +++ b/test/labels-reconcile.sh @@ -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 ] -- 2.45.2 From 9d3fed890144f272e297da642451b1f6b94f2da5 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sat, 18 Jul 2026 19:51:52 +0000 Subject: [PATCH 4/4] =?UTF-8?q?refactor:=20the=20author=20escalates=20?= =?UTF-8?q?=E2=80=94=20drop=20the=20agreement-body=20heuristic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer direction: body-parsing agreement was a guess, and the machine must not guess. COMMENTED is now unconditionally a non-verdict; the judgment that a comment-only reviewer's round passed belongs to the PR AUTHOR, who escalates by requesting the human's review — an explicit request is a fact, and it is the machine's top-precedence input. Auto-request survives only for the no-judgment case: three formal head-current approvals. CONTRIBUTING and LABELS.md state the handoff; fixtures updated (14 transitions, including author-escalation and the three-formal-approvals path). Co-Authored-By: Claude Fable 5 --- .github/scripts/labels-reconcile.sh | 46 +++++++++++++---------------- CONTRIBUTING.md | 17 ++++++----- LABELS.md | 2 +- test/labels-reconcile.sh | 41 +++++++++++++------------ 4 files changed, 51 insertions(+), 55 deletions(-) diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index 38de0b9..4a225fd 100644 --- a/.github/scripts/labels-reconcile.sh +++ b/.github/scripts/labels-reconcile.sh @@ -10,13 +10,15 @@ set -euo pipefail # 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 +# request-changes. Some live bots are comment-only and post agreement as a +# COMMENTED review — a non-verdict this machine refuses to guess about (body +# parsing is a heuristic, and a wrong guess promotes an unapproved PR). The +# judgment call belongs to the PR AUTHOR, who reads the round and escalates +# by requesting the human's review — an explicit request is a fact, and it is +# the one this machine trusts (see decide_state's top precedence). The +# machine auto-requests the human only in the no-judgment-needed case: three +# formal head-current approvals. Any approval that counts 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 @@ -48,22 +50,14 @@ run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing 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 + local review state commit 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 @@ -71,13 +65,11 @@ bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK 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 ;; + *) + # COMMENTED and anything else: a non-verdict. The machine does not + # read bodies — if the comment is really an agreement, the AUTHOR + # says so by requesting the human's review. + echo FEEDBACK ;; esac } @@ -143,9 +135,11 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch desired="$(decide_state)" - # encode the runbook's last step: the round passed → 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. + # 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 run gh api "repos/$REPO/pulls/$n/requested_reviewers" -f "reviewers[]=$HUMAN" --silent diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f080fdf..2013c37 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,13 +28,16 @@ labels tell you where everything is without opening anything. them at their discretion; anything blocking — including a question that gates the verdict — is **request changes**, saying what unblocks it. The reconciler treats a comment-only review as not-approved, so commenting - 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 - labels workflow requests it automatically. + without a verdict only stalls the PR. The machine never reads review + bodies: when a comment-only reviewer's line is really an agreement, that + judgment belongs to the **author** — escalate by requesting the + maintainer's review (step 6), and the reconciler flips the label on that + request, because an explicit request is a fact it can trust. +6. **When the round passes, the author hands the PR to the maintainer** by + requesting their review — that request is what flips `state:needs-human`. + With three formal head-current approvals the labels workflow requests it + automatically; when part of the panel is comment-only, reading their + agreement is the author's judgment, so the author makes the request. 7. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally mirror what CI runs; the multi-user rehearsal runs in CI on a real Incus. diff --git a/LABELS.md b/LABELS.md index 24e78b6..612b38c 100644 --- a/LABELS.md +++ b/LABELS.md @@ -17,7 +17,7 @@ single reply, and a human takes the final review. | `state:building` | `#FBCA04` | the coding agent, still building | PR opened as draft | marked ready + bot reviews requested | | `state:bots-reviewing` | `#1D76DB` | the reviewer bots to finish the round | ready with reviews requested, or fixes pushed and reviews re-requested | all three bots have reviewed the round | | `state:addressing` | `#D93F0B` | the coding agent to reply and push fixes | all bots reviewed the round, not all approved | the single round-reply is posted and fixes pushed | -| `state:needs-human` | `#8250DF` | the human reviewer | all three bots approve | merged — or changes requested, which cycles back to `state:addressing` | +| `state:needs-human` | `#8250DF` | the human reviewer | the human review is requested — by the author when the round passes, or automatically on three formal head-current approvals | merged — or changes requested, which cycles back to `state:addressing` | `bots-reviewing` and `addressing` are deliberately distinct: staleness in the first means *poke the bots*, staleness in the second means *the agent dropped diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh index 13a5e5f..643546b 100644 --- a/test/labels-reconcile.sh +++ b/test/labels-reconcile.sh @@ -1,10 +1,10 @@ #!/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. +# Fixture tests for the labels-reconcile state machine: a comment is a +# non-verdict whatever its body says (the AUTHOR escalates by requesting the +# human), a stale approval does not promote unreviewed code, and an explicit +# human request outranks everything. # Dependency-free beyond jq; no network, no daemon — pure decide_state. cd "$(dirname "$0")/.." @@ -49,12 +49,23 @@ REQUESTED="" REVIEWS_JSON="$(reviews \ "$(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) ----------------- +# -- a comment is a non-verdict, agreement body or not: the author escalates -- 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)" + "$(rev "$BOT2" APPROVED head1 "" t2)" \ + "$(rev "$BOT3" APPROVED head1 "" t3)")" +expect "comment-only agreement still parks on the author" state:addressing "$(decide_state)" +# ...and the author's escalation — requesting the human — flips it +REQUESTED="$HUMAN" +expect "author escalation flips to needs-human" state:needs-human "$(decide_state)" +REQUESTED="" + +# -- three formal approvals need no author judgment --------------------------- +REVIEWS_JSON="$(reviews \ + "$(rev "$BOT1" APPROVED head1 "" t1)" \ + "$(rev "$BOT2" APPROVED head1 "" t2)" \ + "$(rev "$BOT3" APPROVED head1 "" t3)")" +expect "three formal approvals reach needs-human" state:needs-human "$(decide_state)" # -- a comment WITHOUT a verdict parks the PR on the agent -------------------- REVIEWS_JSON="$(reviews \ @@ -87,7 +98,7 @@ 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)")" + "$(rev "$BOT3" APPROVED head1 "" t4)")" expect "later approval supersedes earlier block" state:needs-human "$(decide_state)" # -- an explicit human request outranks the bot rounds ------------------------ @@ -108,17 +119,5 @@ 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 ] -- 2.45.2