forked from heavy-duty/ceremony
feat: per-author review panels — labels.conf gains panel[<login>]= rows
One resolution point (panel_for_author) feeds set_required_bots; the author's row when the conf defines one, the base panel= otherwise, minus the author in either case. Bracket prefixes are matched quoted so the case patterns cannot glob (D7, panela= tripwire). configured_label_rows skips the rows so a dispatch bootstrap cannot mint a label named after one. BUILDER.md/REVIEWER.md carry the one D9 wording; CONSUMERS.md publishes the row as unreleased with the parse-failure warning. Refs #224 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot
parent
80da0a8a1f
commit
8db6c3ae29
8 changed files with 229 additions and 9 deletions
|
|
@ -212,11 +212,12 @@ triage bug, and the move is to say so on the issue, not to guess.
|
|||
repo-specific facts such as the panel roster live in that repo's own
|
||||
CONTRIBUTING; the shared flow lives here and is not restated there.)
|
||||
|
||||
1. Mark ready-for-review; request **the whole panel**. The panel is the roster
|
||||
of the repo the **PR** is in, minus you — never the roster of the repo the
|
||||
issue is in. The PR repo's `.github/labels.conf` `panel=` line is the
|
||||
1. Mark ready-for-review; request **the whole panel**. The panel is the PR
|
||||
repo's `panel[<your-login>]=` line if it defines one, else its `panel=`
|
||||
line; minus the author in either case (#224) — and never the roster of
|
||||
the repo the issue is in. The PR repo's `.github/labels.conf` is the
|
||||
machine's answer; its CONTRIBUTING roster is the human-readable answer,
|
||||
and `panel=` governs if they disagree because that is what the state
|
||||
and the conf governs if they disagree because that is what the state
|
||||
machine reads. If the PR repo names no roster, ask triage on the
|
||||
authorizing issue before marking ready-for-review; do not guess. You may
|
||||
request an off-panel reviewer, but say that their verdict is advisory and
|
||||
|
|
|
|||
|
|
@ -65,7 +65,9 @@ saw Y" outranks one that says "this looks like it might".
|
|||
wait for the repo to appear on a list: review is reversible
|
||||
read-plus-comment work, and the requester already decided it should happen.
|
||||
- **A request is authorization, not panel membership.** Convergence is
|
||||
measured against the target repo's `panel=` roster minus the author. If you
|
||||
measured against the target repo's `panel[<author>]=` line if its
|
||||
`labels.conf` defines one for the PR author, else its `panel=` line; minus
|
||||
the author in either case (#224). If you
|
||||
are requested off-panel, post the verdict anyway and say in its body that
|
||||
it is advisory; neither your silence nor your request-changes is a gate the
|
||||
reconciler enforces. The nine-hour wait for kimi's off-panel verdict on
|
||||
|
|
|
|||
|
|
@ -37,6 +37,12 @@ fi
|
|||
|
||||
HUMAN="${HUMAN_REVIEWER:-danmt}"
|
||||
BOTS=()
|
||||
# Per-author panels (#224): parallel arrays because the conf is tiny and an
|
||||
# associative array buys nothing but a bash-4 dependency statement. One entry
|
||||
# per panel[<login>]= row — PANEL_AUTHORS holds the login, PANEL_ROWS the
|
||||
# space-joined reviewer set at the same index.
|
||||
PANEL_AUTHORS=()
|
||||
PANEL_ROWS=()
|
||||
REQUIRED_BOTS=()
|
||||
STATES=(state:building state:bots-reviewing state:addressing state:needs-human)
|
||||
BLOCKERS=(blocker:conflict blocker:ci-red blocker:unrequested)
|
||||
|
|
@ -127,8 +133,16 @@ load_config() { # $1 = consumer labels.conf; panel is mandatory, scopes optional
|
|||
return 1
|
||||
}
|
||||
BOTS=()
|
||||
PANEL_AUTHORS=()
|
||||
PANEL_ROWS=()
|
||||
# shellcheck disable=SC2094 # parse_panel_author_row takes $conf for its
|
||||
# error messages only — nothing in this loop writes the file it reads
|
||||
while IFS= read -r line || [ -n "$line" ]; do
|
||||
[ -n "$line" ] || continue
|
||||
# The panel[ prefix is matched QUOTED (#224 D7): in a case pattern an
|
||||
# unquoted panel[abc]=* is a bracket expression that matches panela=…,
|
||||
# panelb=…, panelc=… — silently rerouting ordinary settings. The
|
||||
# panela= tripwire in test/labels.test.sh goes red if this regresses.
|
||||
case "$line" in
|
||||
panel=*)
|
||||
[ "$panel_seen" = false ] || {
|
||||
|
|
@ -142,6 +156,7 @@ load_config() { # $1 = consumer labels.conf; panel is mandatory, scopes optional
|
|||
return 1
|
||||
}
|
||||
;;
|
||||
"panel["*) parse_panel_author_row "$line" "$conf" || return ;;
|
||||
triage-actors=*) ;;
|
||||
*) parse_label_row "$line" >/dev/null || return ;;
|
||||
esac
|
||||
|
|
@ -152,6 +167,43 @@ load_config() { # $1 = consumer labels.conf; panel is mandatory, scopes optional
|
|||
}
|
||||
}
|
||||
|
||||
parse_panel_author_row() { # panel[<login>]=<space-separated logins> (#224)
|
||||
# Every failure here is a hard one that names the offending line (D3): a
|
||||
# conf error takes the whole board down, and the run log is the only place
|
||||
# the operator can read why. A malformed bracket is refused AS a bracket
|
||||
# (D4) — falling through to parse_label_row would report it as a
|
||||
# "malformed label row", the misleading diagnostic #224 was filed over.
|
||||
local line="$1" conf="$2" login rest existing
|
||||
case "$line" in
|
||||
"panel["*"]="*) ;;
|
||||
*)
|
||||
echo "labels: malformed panel[<login>]= row (expected panel[<login>]=<reviewers>): $line in $conf" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
login="${line#panel[}"
|
||||
login="${login%%]=*}"
|
||||
[ -n "$login" ] || {
|
||||
echo "labels: empty login in panel row: $line in $conf" >&2
|
||||
return 1
|
||||
}
|
||||
for existing in ${PANEL_AUTHORS[@]+"${PANEL_AUTHORS[@]}"}; do
|
||||
[ "$existing" != "$login" ] || {
|
||||
echo "labels: duplicate panel[$login]= row in $conf: $line" >&2
|
||||
return 1
|
||||
}
|
||||
done
|
||||
local -a row=()
|
||||
rest="${line#*]=}"
|
||||
read -r -a row <<<"$rest"
|
||||
[ "${#row[@]}" -gt 0 ] || {
|
||||
echo "labels: panel[$login]= must name at least one reviewer in $conf: $line" >&2
|
||||
return 1
|
||||
}
|
||||
PANEL_AUTHORS+=("$login")
|
||||
PANEL_ROWS+=("${row[*]}")
|
||||
}
|
||||
|
||||
parse_label_row() { # exact name|color|description; pipes in descriptions are refused
|
||||
local line="$1" name color desc extra
|
||||
IFS='|' read -r name color desc extra <<<"$line"
|
||||
|
|
@ -167,15 +219,38 @@ configured_label_rows() { # validated scope rows, excluding the panel setting
|
|||
[ -f "$conf" ] || return 0
|
||||
while IFS= read -r line || [ -n "$line" ]; do
|
||||
[ -n "$line" ] || continue
|
||||
case "$line" in panel=* | triage-actors=*) continue ;; esac
|
||||
# "panel["* quoted for the same D7 reason as load_config's case; skipping
|
||||
# the bracketed rows (D5) keeps a dispatch bootstrap from trying to
|
||||
# create a label named panel[<login>].
|
||||
case "$line" in panel=* | "panel["* | triage-actors=*) continue ;; esac
|
||||
parse_label_row "$line" || return
|
||||
done <"$conf"
|
||||
}
|
||||
|
||||
panel_for_author() { # $1 = author → the effective panel, space-joined (#224 D2)
|
||||
# THE resolution point: the author's panel[<login>]= row when the conf
|
||||
# defines one, the base panel= otherwise. Everything that computes a
|
||||
# required set goes through here, because two places computing the panel
|
||||
# is how the engine and the reconciler came to disagree in the first place.
|
||||
local author="$1" i
|
||||
for i in ${PANEL_AUTHORS[@]+"${!PANEL_AUTHORS[@]}"}; do
|
||||
if [ "${PANEL_AUTHORS[i]}" = "$author" ]; then
|
||||
printf '%s\n' "${PANEL_ROWS[i]}"
|
||||
return
|
||||
fi
|
||||
done
|
||||
printf '%s\n' "${BOTS[*]}"
|
||||
}
|
||||
|
||||
set_required_bots() { # the PR author is recused by construction
|
||||
# Minus-the-author applies to WHICHEVER set panel_for_author returns (#224
|
||||
# D2's safety net): an author who mistakenly appears inside its own
|
||||
# bracketed row is still recused.
|
||||
local author="$1" bot
|
||||
local -a effective=()
|
||||
read -r -a effective <<<"$(panel_for_author "$author")"
|
||||
REQUIRED_BOTS=()
|
||||
for bot in "${BOTS[@]}"; do
|
||||
for bot in ${effective[@]+"${effective[@]}"}; do
|
||||
[ "$bot" = "$author" ] || REQUIRED_BOTS+=("$bot")
|
||||
done
|
||||
}
|
||||
|
|
|
|||
6
changelog.d/224.md
Normal file
6
changelog.d/224.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
### Added
|
||||
|
||||
- `labels.conf` accepts optional `panel[<login>]=` rows: the required set for
|
||||
a PR authored by that login is the row minus the author; other authors keep
|
||||
`panel=`. Consumers gain the row at their next pin bump — adding it before
|
||||
that bump is a parse failure that takes the label board down (#224).
|
||||
|
|
@ -482,10 +482,12 @@ quiet repo wears that flag until the backstop cron; a consumer picks them up
|
|||
by pinning `0.3.0` or later, never through mixed refs.
|
||||
|
||||
`.github/labels.conf` has one mandatory panel setting, one mandatory
|
||||
`triage-actors` setting, and then zero or more scope rows:
|
||||
`triage-actors` setting, zero or more optional per-author panel rows, and
|
||||
then zero or more scope rows:
|
||||
|
||||
```text
|
||||
panel=claude-bot example-codex-bot example-grok-bot
|
||||
panel[example-builder]=example-codex-bot example-grok-bot
|
||||
triage-actors=example-triage-bot
|
||||
scope:cli|C5DEF5|The command-line surface
|
||||
scope:docs|C5DEF5|Documentation
|
||||
|
|
@ -497,11 +499,24 @@ rows only; adding `triage-actors=` is a parse failure, not an ignored setting.
|
|||
Add it at the same pin bump as the `issues:` trigger — `0.2.0` or later —
|
||||
never before it and never through mixed refs.
|
||||
|
||||
The optional `panel[<login>]=` rows are **unreleased** (#224). A row names
|
||||
the effective panel for PRs authored by exactly that login — the reconciler
|
||||
computes that PR's required set from the row, minus the author as always —
|
||||
and every other author keeps the base `panel=`, which stays mandatory. The
|
||||
panel is configured or it is the base one: ceremony never infers a reviewer
|
||||
set from the model behind a login. On any earlier pin a bracketed row is a
|
||||
**parse failure, not an ignored setting** — the same shape `triage-actors=`
|
||||
bought at `0.2.0`, but harsher in practice: the reconcile job dies on every
|
||||
PR event and every sweep until the row is removed, so the whole label board
|
||||
goes down. Add the row only at or after the pin bump that carries it, never
|
||||
before it and never through mixed refs.
|
||||
|
||||
Both actor lists are whitespace-separated. `triage-actors` names the identities
|
||||
allowed to mint issues without the sweep applying `needs-triage`. Label rows use exactly
|
||||
`name|color|description`; blank lines are ignored and extra pipes are refused.
|
||||
There are no comment lines: every non-blank line must be the `panel=`
|
||||
setting, the `triage-actors=` setting, or a label row, so `#`-prefixed prose
|
||||
setting, a `panel[<login>]=` row, the `triage-actors=` setting, or a label
|
||||
row, so `#`-prefixed prose
|
||||
is a parse failure, not a comment (rig #13's conversion found this the hard
|
||||
way — keep the file data only).
|
||||
Core state, blocker, work-queue, and release labels come from ceremony. Scope
|
||||
|
|
|
|||
|
|
@ -22,6 +22,16 @@ printf '%s\n' 'panel=one' >"$TMP/missing.conf"
|
|||
check "missing triage actors fails loudly" 1 "missing triage-actors=" load_issueflow_config "$TMP/missing.conf"
|
||||
printf '%s\n' 'triage-actors=one' 'triage-actors=two' >"$TMP/duplicate.conf"
|
||||
check "duplicate triage actors fails loudly" 1 "duplicate triage-actors" load_issueflow_config "$TMP/duplicate.conf"
|
||||
# A panel[<login>]= row (#224 D8) must not take the issue board down: this
|
||||
# loader ignores every line that is not triage-actors=. That tolerance was
|
||||
# incidental; this row makes it deliberate, so a future tightening cannot
|
||||
# break the sweep as a side effect.
|
||||
printf '%s\n' \
|
||||
'panel=one two' \
|
||||
'panel[builder-z]=two' \
|
||||
'triage-actors=triage-one' >"$TMP/bracketed.conf"
|
||||
check "a per-author panel row is tolerated by the issue-flow loader" 0 "" \
|
||||
load_issueflow_config "$TMP/bracketed.conf"
|
||||
|
||||
# The dogfood caller and reusable workflow must expose the same runtime facts
|
||||
# as the documented consumer stub. Static pins catch YAML blocks drifting to
|
||||
|
|
|
|||
|
|
@ -986,5 +986,48 @@ for ev in schedule pull_request_target; do
|
|||
expect "...and deletes nothing" \
|
||||
no "$(grep -q '^delete ' "$EXEC/record" && echo yes || echo no)"
|
||||
done
|
||||
# -- per-author panels (#224): the required set flows from the one ----------
|
||||
# resolution point, and convergence counts the effective set — never the
|
||||
# base panel beside a reduced request set (the must-fail the issue names)
|
||||
PANEL_DIR="$RTMP/panel-author"
|
||||
mkdir -p "$PANEL_DIR"
|
||||
printf '%s\n' 'panel=bot-a bot-b bot-c bot-d' \
|
||||
'panel[builder-z]=bot-b bot-c bot-d' >"$PANEL_DIR/labels.conf"
|
||||
load_config "$PANEL_DIR/labels.conf"
|
||||
set_required_bots builder-z
|
||||
expect "a bracketed author requires exactly its configured row" \
|
||||
"bot-b bot-c bot-d" "${REQUIRED_BOTS[*]}"
|
||||
set_required_bots bot-a
|
||||
expect "an unbracketed author beside a bracketed row requires panel minus self" \
|
||||
"bot-b bot-c bot-d" "${REQUIRED_BOTS[*]}"
|
||||
set_required_bots outsider
|
||||
expect "an unbracketed non-panelist author requires the whole base panel" \
|
||||
"bot-a bot-b bot-c bot-d" "${REQUIRED_BOTS[*]}"
|
||||
|
||||
# The engine shape: the three configured reviewers approving the head IS the
|
||||
# whole round for a bracketed author — bot-a's absent verdict must not hold
|
||||
# convergence, or the request side and the convergence side disagree forever
|
||||
# (the deadlock crew#285 was filed over).
|
||||
set_required_bots builder-z
|
||||
THREE_APPROVE="$(reviews \
|
||||
"$(rev bot-b APPROVED head1 ok 2026-08-02T10:00:00Z)" \
|
||||
"$(rev bot-c APPROVED head1 ok 2026-08-02T10:01:00Z)" \
|
||||
"$(rev bot-d APPROVED head1 ok 2026-08-02T10:02:00Z)")"
|
||||
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON="$THREE_APPROVE" \
|
||||
MERGEABLE=MERGEABLE CHECKS=SUCCESS LABELS=""
|
||||
expect "the bracketed author's round converges on its three approvals" \
|
||||
state:needs-human "$(decide_state)"
|
||||
expect "...with no blocker standing" "" "$(blockers)"
|
||||
# The control: the same three approvals under the base panel are NOT a full
|
||||
# round — the fourth verdict is owed and unrequested. If this pair ever
|
||||
# reads the same, one side stopped consulting the resolution point.
|
||||
printf '%s\n' 'panel=bot-a bot-b bot-c bot-d' >"$PANEL_DIR/labels.conf"
|
||||
load_config "$PANEL_DIR/labels.conf"
|
||||
set_required_bots builder-z
|
||||
expect "without the row the same approvals leave the round incomplete" \
|
||||
state:addressing "$(decide_state)"
|
||||
expect "...and the owed, unasked verdict is named" \
|
||||
blocker:unrequested "$(blockers)"
|
||||
|
||||
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
|
||||
[ "$fail" -eq 0 ]
|
||||
|
|
|
|||
|
|
@ -53,6 +53,74 @@ load_config "$TMP/good.conf"
|
|||
set_required_bots two
|
||||
check "PR author is recused from the required panel" 0 "one three" printf '%s\n' "${REQUIRED_BOTS[*]}"
|
||||
|
||||
# -- per-author panel rows (#224): the config-parse matrix -------------------
|
||||
# required_for loads a conf fresh in a subshell and prints the required set
|
||||
# behind a RESULT: anchor, so substring matching cannot confuse "b c" with
|
||||
# "a b c".
|
||||
# shellcheck disable=SC2016 # expansion belongs to the nested bash
|
||||
required_for() { # $1 = conf, $2 = author → RESULT:<required set>
|
||||
bash -c 'source "$1"; load_config "$2" || exit 1
|
||||
set_required_bots "$3"; printf "RESULT:%s\n" "${REQUIRED_BOTS[*]}"' _ \
|
||||
"$ROOT/actions/labels-reconcile/labels-reconcile.sh" "$1" "$2"
|
||||
}
|
||||
printf '%s\n' 'panel=a b c' >"$TMP/plain.conf"
|
||||
check "no bracketed row: panelist author gets panel minus self" 0 "RESULT:b c" \
|
||||
required_for "$TMP/plain.conf" a
|
||||
check "no bracketed row: outside author gets the whole panel" 0 "RESULT:a b c" \
|
||||
required_for "$TMP/plain.conf" z
|
||||
printf '%s\n' 'panel=a b c' 'panel[z]=b c' >"$TMP/author.conf"
|
||||
check "bracketed author gets exactly its row" 0 "RESULT:b c" \
|
||||
required_for "$TMP/author.conf" z
|
||||
check "unbracketed author beside a bracketed row is unchanged" 0 "RESULT:b c" \
|
||||
required_for "$TMP/author.conf" a
|
||||
printf '%s\n' 'panel[z]=b c' 'panel=a b c' >"$TMP/reversed.conf"
|
||||
check "row order is irrelevant: bracketed row before panel=" 0 "RESULT:b c" \
|
||||
required_for "$TMP/reversed.conf" z
|
||||
check "row order is irrelevant for the base panel too" 0 "RESULT:b c" \
|
||||
required_for "$TMP/reversed.conf" a
|
||||
printf '%s\n' 'panel=a b c' 'panel[a]=a b' >"$TMP/self.conf"
|
||||
check "author inside its own bracketed row is still recused" 0 "RESULT:b" \
|
||||
required_for "$TMP/self.conf" a
|
||||
# shellcheck disable=SC2016 # expansion belongs to the nested bash
|
||||
check "base panel is byte-identical with the bracketed rows deleted" 0 "SAME" \
|
||||
bash -c 'source "$1"; load_config "$2"; with="${BOTS[*]}"
|
||||
load_config "$3"; [ "$with" = "${BOTS[*]}" ] && echo SAME' _ \
|
||||
"$ROOT/actions/labels-reconcile/labels-reconcile.sh" \
|
||||
"$TMP/author.conf" "$TMP/plain.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[z]=b' 'panel[z]=c' >"$TMP/dup-author.conf"
|
||||
check "duplicate rows for one login fail naming the line" 1 \
|
||||
"duplicate panel[z]= row" load_config "$TMP/dup-author.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[z]=' >"$TMP/empty-set.conf"
|
||||
check "a bracketed row naming zero reviewers fails loudly" 1 \
|
||||
"panel[z]= must name at least one reviewer" load_config "$TMP/empty-set.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[]=b c' >"$TMP/empty-login.conf"
|
||||
check "an empty login fails loudly" 1 "empty login in panel row" \
|
||||
load_config "$TMP/empty-login.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[z=b c' >"$TMP/broken-bracket.conf"
|
||||
check "a malformed bracket is refused as a bracket (D4)" 1 \
|
||||
"malformed panel[<login>]= row" load_config "$TMP/broken-bracket.conf"
|
||||
# shellcheck disable=SC2016 # expansion belongs to the nested bash
|
||||
check "...and never as a label row" 1 "" bash -c \
|
||||
'source "$1"; load_config "$2" 2>&1 | grep -F "malformed label row"' _ \
|
||||
"$ROOT/actions/labels-reconcile/labels-reconcile.sh" "$TMP/broken-bracket.conf"
|
||||
# The D7 tripwire: in a case pattern an unquoted panel[abc]=* is a bracket
|
||||
# expression matching panela=… — this row going green as a panel setting is
|
||||
# exactly the silent mis-route the quoted prefix exists to prevent.
|
||||
printf '%s\n' 'panel=a b c' 'panela=b c' >"$TMP/glob-guard.conf"
|
||||
check "panela= is still a malformed label row, never a panel setting (D7)" 1 \
|
||||
"malformed label row" load_config "$TMP/glob-guard.conf"
|
||||
printf '%s\n' 'panel[z]=b c' >"$TMP/bracket-only.conf"
|
||||
check "a bracketed row does not satisfy the mandatory panel=" 1 \
|
||||
"missing panel= line" load_config "$TMP/bracket-only.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[z]=b c' \
|
||||
'scope:one|C5DEF5|First scope' >"$TMP/mixed.conf"
|
||||
check "configured_label_rows returns the scope rows alone" 0 \
|
||||
"scope:one|C5DEF5|First scope" configured_label_rows "$TMP/mixed.conf"
|
||||
# shellcheck disable=SC2016 # expansion belongs to the nested bash
|
||||
check "no panel[...] row reaches the bootstrap" 1 "" bash -c \
|
||||
'source "$1"; configured_label_rows "$2" | grep -F "panel["' _ \
|
||||
"$ROOT/actions/labels-reconcile/labels-reconcile.sh" "$TMP/mixed.conf"
|
||||
|
||||
# LABELS.md is mirrored byte-identically into every governed repo, so any
|
||||
# scope enumeration it carries is true at home and false everywhere else —
|
||||
# 14 of 16 vendored rows were false across the family when this fired (#104).
|
||||
|
|
|
|||
Loading…
Reference in a new issue