Merge pull request #227 from dan-claude-bot/build/224-cut-0-5-0-bundle
feat: per-author panels, draft vs the round, the marker sweep, the write-token rule — the 0.5.0 bundle
This commit is contained in:
commit
b18c0bc0b2
12 changed files with 435 additions and 33 deletions
16
BUILDER.md
16
BUILDER.md
|
|
@ -200,6 +200,13 @@ triage bug, and the move is to say so on the issue, not to guess.
|
|||
guard still refuses anything that deletes a shipped heading.
|
||||
- Follow the repo's conventions file and match the code you touch. Tests are
|
||||
not optional: the issue's test plan is the floor, not the ceiling.
|
||||
- **A write-capable job gets a repo-owned script, not a third-party action.**
|
||||
If the job's token can write (`packages: write`, `contents: write`,
|
||||
`id-token: write`, deploy secrets), default to a script in the repo that a
|
||||
test can drive; a third-party action there needs an established publisher
|
||||
and a full-commit-SHA pin. Read-only jobs still SHA-pin. The full rule and
|
||||
the red-flag profile a reviewer will apply are in REVIEWER.md §What you
|
||||
review against, item 2 (incubator#53/#54; #216).
|
||||
- **Scope discipline: the PR does the issue — whole, and nothing else.**
|
||||
Adjacent problems you discover go to a **discussion** (or a comment on the
|
||||
relevant issue), where triage will do its job. You do not mint issues —
|
||||
|
|
@ -212,11 +219,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
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ and the reconciler recomputes it from GitHub's own facts.
|
|||
|
||||
| Label | Color | Waiting on |
|
||||
|---|---|---|
|
||||
| `state:building` | `#FBCA04` | the builder — PR is a draft |
|
||||
| `state:building` | `#FBCA04` | the builder — pre-round: no verdict stands against the head. Draft is evidence for it, not the definition of it: a draft carrying a standing non-approving verdict is a fix round and reads `state:addressing` (#205) |
|
||||
| `state:bots-reviewing` | `#1D76DB` | the reviewer panel to finish the round (a request is live) |
|
||||
| `state:addressing` | `#D93F0B` | the builder — round complete without full approval, or nobody was asked, or a blocker is up, or a ruling is pending |
|
||||
| `state:needs-human` | `#8250DF` | the human — **this PR could be merged right now**: zero blockers, whole panel approved the current head |
|
||||
|
|
|
|||
17
REVIEWER.md
17
REVIEWER.md
|
|
@ -49,6 +49,19 @@ In order of authority:
|
|||
`0.1.0`'s `load_config` rejected `triage-actors=...` with
|
||||
`malformed label row` and `exit=1`. CI green on a conversion PR proves
|
||||
nothing about the new config: the base branch's workflow is what ran.
|
||||
- **Third-party actions never hold a write-capable token by default.** In
|
||||
any job whose token is write-capable (`packages: write`,
|
||||
`contents: write`, `id-token: write`, or one carrying deploy secrets),
|
||||
the default is a repo-owned script a test can drive. A third-party
|
||||
action may hold that token only if it comes from an **established
|
||||
publisher** — a real organization with maintenance history and more
|
||||
than one maintainer, not a memberless shell or a lone account shipping
|
||||
an unauditable `dist/` blob — and is **pinned by full commit SHA**. An
|
||||
action matching the incubator red-flag profile never holds a write
|
||||
token, however well it works. Read-only jobs: ordinary dependency
|
||||
judgement, SHA-pinning still required. This is bot-run infrastructure —
|
||||
no human watches runtime logs, so a compromised action's window is
|
||||
unbounded (incubator#53/#54; #216).
|
||||
3. **The code itself** — correctness first, then tests (does the test plan's
|
||||
floor exist? do the failure cases actually fail?), then conventions.
|
||||
Changelog line present for behavior changes; comments carry why, not
|
||||
|
|
@ -65,7 +78,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,55 @@ 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
|
||||
}
|
||||
# The login must be exactly one well-formed bracket pair of login
|
||||
# characters. Without this, panel[z]]=b parses: the case above only
|
||||
# establishes that SOME ]= occurs, ${login%%]=*} keeps the stray ] inside
|
||||
# the login (z]), and set_required_bots for the real z then silently falls
|
||||
# back to the base panel — the misroute D4 exists to refuse. GitHub logins
|
||||
# are [A-Za-z0-9-], per the #285 spec.
|
||||
case "$login" in
|
||||
*[!A-Za-z0-9-]*)
|
||||
echo "labels: malformed panel[<login>]= row (a login is [A-Za-z0-9-] only): $line in $conf" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
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 +231,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
|
||||
}
|
||||
|
|
@ -394,12 +481,55 @@ blockers() { # → the blocker:* labels this PR should carry, one per line
|
|||
fi
|
||||
}
|
||||
|
||||
round_outranks_draft() { # 0 when the round's standing word survives a re-draft (#205)
|
||||
# A standing non-approving verdict outranks draft: a PR that took a round,
|
||||
# carries CHANGES_REQUESTED (or a comment owed a reply, or approvals a push
|
||||
# staled), and is then converted back to draft is a fix round in progress,
|
||||
# not a build — and hiding it behind state:building is a dropped ball the
|
||||
# staleness sweep reads as work in progress. Approvals do NOT outrank
|
||||
# draft: a re-draft after a passed round is deliberately building again,
|
||||
# and a draft must never read state:needs-human.
|
||||
#
|
||||
# A LIVE panel request on a draft also falls through — deliberately
|
||||
# surfaced, not absorbed (#205's must-not-paper-over): the bots ignore
|
||||
# drafts by design, so a draft wearing state:bots-reviewing on the board
|
||||
# is the visible symptom of a real defect (a request nobody cleared at
|
||||
# round close, or a hand-requested draft), and reading it as building
|
||||
# would hide exactly that.
|
||||
local b
|
||||
for b in "${REQUIRED_BOTS[@]}"; do
|
||||
requested "$b" && return 0
|
||||
case "$(bot_verdict "$b")" in BLOCK | FEEDBACK | STALE) return 0 ;; esac
|
||||
done
|
||||
[ "$(bot_verdict "$HUMAN")" = BLOCK ]
|
||||
}
|
||||
|
||||
decide_state() { # → the one state:* label this PR should carry
|
||||
if [ "$DRAFT" = true ]; then echo state:building; return; fi
|
||||
# Draft decides the state only when the round implies nothing else (#205):
|
||||
# a draft with no round history reads state:building exactly as it always
|
||||
# has, and round_outranks_draft is what "nothing else" means.
|
||||
if [ "$DRAFT" = true ] && ! round_outranks_draft; then
|
||||
echo state:building
|
||||
return
|
||||
fi
|
||||
|
||||
local s
|
||||
s="$(round_state)"
|
||||
|
||||
# A draft disqualifies needs-human unconditionally (#205, round 1): with
|
||||
# the short-circuit above now conditional, a draft carrying a live human
|
||||
# request plus a standing bot block or comment fell through to
|
||||
# round_state, whose explicit-human-request precedence sits above the
|
||||
# BLOCK/FEEDBACK cases — and GitHub cannot merge a draft at all, so
|
||||
# "a human could merge this right now" would lie no matter what the
|
||||
# round says. state:addressing is the same honest landing the blocker/
|
||||
# needs-ruling/blocked clauses below use: the round's word stands, only
|
||||
# the mergeable-now claim is off the table while the PR is a draft.
|
||||
if [ "$s" = state:needs-human ] && [ "$DRAFT" = true ]; then
|
||||
echo state:addressing
|
||||
return
|
||||
fi
|
||||
|
||||
# The one rule joining the two axes: state:needs-human means a human could
|
||||
# merge this RIGHT NOW, so it requires a clear branch. Any blocker at all
|
||||
# means the work is the agent's — whatever the review round says — and the
|
||||
|
|
@ -502,7 +632,7 @@ round_state() { # → the state the REVIEW ROUND alone implies; knows no branch
|
|||
|
||||
core_label_rows() {
|
||||
cat <<'EOF'
|
||||
state:building|FBCA04|PR is a draft — the coding agent is still building
|
||||
state:building|FBCA04|Pre-round: the builder is still building — draft is evidence for it, not the definition
|
||||
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|No blockers, all bots approve — waiting on the human reviewer
|
||||
|
|
|
|||
6
changelog.d/205.md
Normal file
6
changelog.d/205.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
### Fixed
|
||||
|
||||
- A standing non-approving verdict now outranks draft in `decide_state`: a
|
||||
re-drafted PR mid-round reads `state:addressing`, a live panel request on a
|
||||
draft surfaces as `state:bots-reviewing`, and a draft with no round history
|
||||
still reads `state:building` (#205).
|
||||
7
changelog.d/216.md
Normal file
7
changelog.d/216.md
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
### Changed
|
||||
|
||||
- Doctrine: third-party actions never hold a write-capable token by default —
|
||||
repo-owned scripts in write-capable jobs, established publisher plus
|
||||
full-SHA pin for the exception, SHA pins everywhere. Canonical in
|
||||
REVIEWER.md, short form in BUILDER.md; consumers adopt at the pin bump
|
||||
(#216).
|
||||
9
changelog.d/221.md
Normal file
9
changelog.d/221.md
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
### Fixed
|
||||
|
||||
- Five stale **unreleased** markers in `docs/CONSUMERS.md` now name their
|
||||
tags: fragment mode, `changelog-assembled` and `runner-isolated` at
|
||||
`0.2.0`; the additive labeler at `0.3.0`; the two-caller split at `0.4.1`
|
||||
(#221).
|
||||
- The marker convention now names its clearing owner: the release PR that
|
||||
ships machinery clears, in that same PR, every marker its assembled
|
||||
section makes false (#221).
|
||||
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).
|
||||
|
|
@ -60,10 +60,10 @@ the machinery at all:
|
|||
the release PR assembles the section
|
||||
([Assembling a release section](#assembling-a-release-section)).
|
||||
|
||||
Fragment mode is **unreleased** and not in `0.1.0`. A consumer pinned
|
||||
to `0.1.0` bootstraps the legacy shape instead — the preamble plus an
|
||||
empty `## Unreleased` section for entries to land under — and converts
|
||||
on the pin bump to the first tag carrying fragment mode; never mix
|
||||
Fragment mode is available at `0.2.0` and later, and not in `0.1.0`.
|
||||
A consumer pinned to `0.1.0` bootstraps the legacy shape instead — the
|
||||
preamble plus an empty `## Unreleased` section for entries to land
|
||||
under — and converts on the pin bump to `0.2.0` or later; never mix
|
||||
refs to adopt it early.
|
||||
3. **`drills/README.md`** defining what a drill *means* in this repo —
|
||||
each repo names its own
|
||||
|
|
@ -85,14 +85,16 @@ the machinery at all:
|
|||
fetch-depth: 0
|
||||
- uses: heavy-duty/ceremony/actions/changelog-armed@<pinned-tag>
|
||||
- uses: heavy-duty/ceremony/actions/changelog-monotonic@<pinned-tag>
|
||||
# Unreleased: changelog-assembled is not in 0.1.0. Adopt this step
|
||||
# with the pin bump to the first tag that carries it; never mix
|
||||
# refs. Green NOTICE on every non-release PR; on a release PR it
|
||||
# asserts the stamped section is exactly the fragments it consumed.
|
||||
# changelog-assembled is available at 0.2.0 and later, not in
|
||||
# 0.1.0. Adopt this step with the pin bump to 0.2.0 or later;
|
||||
# never mix refs. Green NOTICE on every non-release PR; on a
|
||||
# release PR it asserts the stamped section is exactly the
|
||||
# fragments it consumed.
|
||||
- uses: heavy-duty/ceremony/actions/changelog-assembled@<pinned-tag>
|
||||
- uses: heavy-duty/ceremony/actions/drill-recorded@<pinned-tag>
|
||||
# Unreleased: runner-isolated is not in 0.1.0. Adopt this step with
|
||||
# the pin bump to the first tag that carries it; never mix refs.
|
||||
# runner-isolated is available at 0.2.0 and later, not in 0.1.0.
|
||||
# Adopt this step with the pin bump to 0.2.0 or later; never mix
|
||||
# refs.
|
||||
- uses: heavy-duty/ceremony/actions/runner-isolated@<pinned-tag>
|
||||
```
|
||||
|
||||
|
|
@ -112,7 +114,11 @@ the machinery at all:
|
|||
somebody adds one.
|
||||
|
||||
This guide documents `main`. New machinery is marked **unreleased**
|
||||
here until a release tag ships it. If an action does not exist at the
|
||||
here until a release tag ships it — and the release PR that ships the
|
||||
machinery clears, in that same PR, every marker its own assembled
|
||||
section makes false: the section cites its issues, each marker cites
|
||||
the same issue, and the release PR's diff is the one place both
|
||||
halves are visible at once (#221). If an action does not exist at the
|
||||
consumer's pinned tag, adopt it with the pin bump to the first tag that
|
||||
carries it; never mix a moving or newer ref into an otherwise exact-pin
|
||||
consumer. In particular, `0.1.0` carries `changelog-armed`,
|
||||
|
|
@ -298,7 +304,8 @@ together at the same pin:
|
|||
The consumer keeps its path mapping in `.github/labeler.yml` and its
|
||||
review panel plus scope taxonomy in `.github/labels.conf`.
|
||||
|
||||
**Additive means additive** (unreleased — #130): the scope job's only label
|
||||
**Additive means additive** (available at `0.3.0` and later — #130): the
|
||||
scope job's only label
|
||||
write is `POST /issues/{n}/labels`, which adds the derived scopes and removes
|
||||
nothing, so a label applied while the job runs survives it. Earlier tags used
|
||||
`actions/labeler@v5`, which — even under `sync-labels: false` — replaces the
|
||||
|
|
@ -436,16 +443,16 @@ mint→`needs-triage` check and `closed` the blocker-closes→`ready` self-heal;
|
|||
the stub and ceremony's own caller stay byte-for-byte identical, the parity
|
||||
#144 established.
|
||||
|
||||
The two-caller split (ceremony#209) is **unreleased**. A consumer pinned to
|
||||
`0.4.0` or earlier keeps the previous single-caller shape — the labels
|
||||
caller carrying the cron, `workflow_dispatch`, and `actions: read` — and
|
||||
adopts the split at the pin bump to the first tag carrying ceremony#209.
|
||||
Never mix refs to adopt it early.
|
||||
The two-caller split (ceremony#209) is available at `0.4.1` and later. A
|
||||
consumer pinned to `0.4.0` or earlier keeps the previous single-caller
|
||||
shape — the labels caller carrying the cron, `workflow_dispatch`, and
|
||||
`actions: read` — and adopts the split at the pin bump to `0.4.1` or
|
||||
later. Never mix refs to adopt it early.
|
||||
|
||||
The migration is **one atomic PR** with exactly four edits — crew, the
|
||||
consumer whose displaced-check evidence drove #209 (crew#227, crew#250),
|
||||
is the worked example; written here against `0.4.1` as the illustrative
|
||||
first tag carrying the split:
|
||||
is the worked example; written here against `0.4.1`, the first tag
|
||||
carrying the split:
|
||||
|
||||
1. **Pin bump, every reference together** ([Version pinning](#version-pinning)):
|
||||
`0.4.0` → `0.4.1` in the labels caller's `uses:` line **and in every
|
||||
|
|
@ -482,10 +489,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,13 +506,25 @@ 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
|
||||
is a parse failure, not a comment (rig #13's conversion found this the hard
|
||||
way — keep the file data only).
|
||||
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
|
||||
rows remain consumer-owned because paths and surfaces differ by repository.
|
||||
|
||||
|
|
|
|||
|
|
@ -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,119 @@ for ev in schedule pull_request_target; do
|
|||
expect "...and deletes nothing" \
|
||||
no "$(grep -q '^delete ' "$EXEC/record" && echo yes || echo no)"
|
||||
done
|
||||
# -- a re-drafted fix round is not a build (#205) ----------------------------
|
||||
# Draft used to short-circuit decide_state before the round was consulted, so
|
||||
# a PR carrying a standing CHANGES_REQUESTED that its builder converted back
|
||||
# to draft read state:building — and the staleness sweep read a dropped fix
|
||||
# round as a build in progress.
|
||||
load_config .github/labels.conf
|
||||
set_required_bots codex-bot-andresmgsl
|
||||
MERGEABLE=MERGEABLE CHECKS=SUCCESS LABELS="" HEAD_SHA=head1
|
||||
DRAFT=true REQUESTED="" REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" CHANGES_REQUESTED head1 no t1)" \
|
||||
"$(rev "$BOT2" APPROVED head1 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head1 ok t3)")"
|
||||
expect "a re-drafted PR with a standing block is addressing, not building" \
|
||||
state:addressing "$(decide_state)"
|
||||
REVIEWS_JSON="$(reviews "$(rev "$BOT1" COMMENTED head1 thoughts t1)")"
|
||||
expect "a re-drafted PR owing a round-reply is addressing" \
|
||||
state:addressing "$(decide_state)"
|
||||
REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" APPROVED head0 ok t1)" \
|
||||
"$(rev "$BOT2" APPROVED head0 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head0 ok t3)")"
|
||||
expect "a re-drafted PR whose approvals a push staled is addressing" \
|
||||
state:addressing "$(decide_state)"
|
||||
REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" APPROVED head1 ok t1)" \
|
||||
"$(rev "$BOT2" APPROVED head1 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head1 ok t3)" \
|
||||
"$(rev "$HUMAN" CHANGES_REQUESTED head1 no t4)")"
|
||||
expect "the human's standing changes-requested outranks draft too" \
|
||||
state:addressing "$(decide_state)"
|
||||
# Approvals do NOT outrank draft: a re-draft after a passed round is
|
||||
# deliberately building again — and a draft must never read needs-human.
|
||||
REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" APPROVED head1 ok t1)" \
|
||||
"$(rev "$BOT2" APPROVED head1 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head1 ok t3)")"
|
||||
expect "a re-draft after a passed round is building again" \
|
||||
state:building "$(decide_state)"
|
||||
REQUESTED="$HUMAN"
|
||||
expect "...even with the human requested — a draft never reads needs-human" \
|
||||
state:building "$(decide_state)"
|
||||
# Round 1's 224-case hole (claude's differential): a draft with a LIVE HUMAN
|
||||
# REQUEST plus a standing block or comment fell through to round_state,
|
||||
# whose human-request precedence sits above BLOCK/FEEDBACK — and read
|
||||
# needs-human on a PR GitHub cannot merge. These are the same inputs as the
|
||||
# addressing rows above with REQUESTED="$HUMAN", which is where the
|
||||
# criterion can actually fail.
|
||||
REQUESTED="$HUMAN" REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" CHANGES_REQUESTED head1 no t1)" \
|
||||
"$(rev "$BOT2" APPROVED head1 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head1 ok t3)")"
|
||||
expect "a draft with a human request and a standing block is addressing" \
|
||||
state:addressing "$(decide_state)"
|
||||
REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" COMMENTED head1 thoughts t1)" \
|
||||
"$(rev "$BOT2" APPROVED head1 ok t2)" \
|
||||
"$(rev "$BOT3" APPROVED head1 ok t3)")"
|
||||
expect "a draft with a human request and an owed reply is addressing" \
|
||||
state:addressing "$(decide_state)"
|
||||
|
||||
# The must-not-paper-over combination: a live panel request on a draft is a
|
||||
# board defect (the bots ignore drafts by design) and stays VISIBLE as
|
||||
# bots-reviewing rather than being absorbed into building.
|
||||
REQUESTED="$BOT2" REVIEWS_JSON='[]'
|
||||
expect "a live panel request on a draft surfaces as bots-reviewing" \
|
||||
state:bots-reviewing "$(decide_state)"
|
||||
# The byte-identical baseline: a virgin draft still reads building.
|
||||
REQUESTED="" REVIEWS_JSON='[]'
|
||||
expect "a draft with no round history still reads building" \
|
||||
state:building "$(decide_state)"
|
||||
|
||||
# -- 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,82 @@ 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"
|
||||
# codex's round-1 probe: the stray ] used to parse, record login z], and
|
||||
# silently misroute z to the base panel — exactly the D4 refusal owed.
|
||||
printf '%s\n' 'panel=a b c' 'panel[z]]=b' >"$TMP/stray-bracket.conf"
|
||||
check "a stray ] inside the bracket is refused as a bracket" 1 \
|
||||
"malformed panel[<login>]= row" load_config "$TMP/stray-bracket.conf"
|
||||
printf '%s\n' 'panel=a b c' 'panel[a_b]=c' >"$TMP/bad-login.conf"
|
||||
check "a non-login character in the bracket is refused" 1 \
|
||||
"malformed panel[<login>]= row" load_config "$TMP/bad-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