Merge pull request #54 from claude-bot-andresmgsl/build/51-needs-ruling
feat(labels): `needs-ruling` — the label, the doctrine, and the reconciler's exclusion rule (#51)
This commit is contained in:
commit
2f0d3c65af
7 changed files with 146 additions and 5 deletions
|
|
@ -58,7 +58,13 @@ CONTRIBUTING; everything below is the shared flow.)
|
|||
3. Never dismiss a review, never merge, never mark your own work as passed.
|
||||
A blocking point you disagree with is answered with evidence or escalated
|
||||
in the PR — a maintainer can be asked for a ruling; silence and
|
||||
force-forward are not options.
|
||||
force-forward are not options. The ruling ask has mechanics: set
|
||||
`needs-ruling` on the PR, with one comment carrying the question, the
|
||||
options, and your recommendation — a panel deadlock is consolidated into
|
||||
that one question, never forwarded as three phrasings of it. The label
|
||||
stays until agreement is *reached*, not until the maintainer replies;
|
||||
you record the ruling, remove the label, and return the PR to its flow
|
||||
([LABELS.md](LABELS.md)).
|
||||
|
||||
## Handoff
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ so entries say what changed, cite the issue, and stop.
|
|||
|
||||
## Unreleased
|
||||
|
||||
- `needs-ruling` — the cross-cutting flag for a pending human decision, excluded from `state:needs-human` and from the staleness sweep (#51).
|
||||
|
||||
## 0.1.0 — 2026-07-22
|
||||
|
||||
- `lib/version.sh` — one version abstraction, `file` and `package-json` backends (#3).
|
||||
|
|
|
|||
24
LABELS.md
24
LABELS.md
|
|
@ -18,7 +18,7 @@ and the reconciler recomputes it from GitHub's own facts.
|
|||
|---|---|---|
|
||||
| `state:building` | `#FBCA04` | the builder — PR is a draft |
|
||||
| `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 |
|
||||
| `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 |
|
||||
|
||||
`bots-reviewing` vs `addressing` is deliberate: staleness in the first means
|
||||
|
|
@ -66,9 +66,31 @@ staleness sweep will reclaim (issue #18); until that machinery exists,
|
|||
|---|---|---|
|
||||
| `stale` | `#B60205` | no activity for 48h — sweep-managed, never hand-applied |
|
||||
| `blocked` | `#6A737D` | (see above — same label serves PRs waiting on another PR/issue; legitimately quiet, the staleness sweep skips it) |
|
||||
| `needs-ruling` | `#D4C5F9` | a human decision is required; the question, options and a recommendation are in the flagging comment. Set by triage or the builder; a state, not a signal — it clears on agreement, not on a reply |
|
||||
| `release` | `#0E8A16` | release flow, versioning, packaging work — and the ceremony PR itself |
|
||||
| `merge-next` | `#0E8A16` | head of the merge queue — merge this one next. Queue order is *intent*: never set by the reconciler, only cleared by it |
|
||||
|
||||
`needs-ruling` marks where the human's turn is when the pending thing is a
|
||||
*decision*, not a merge (#50 settled it, D1–D10). It is not
|
||||
`state:needs-human`: that label means exactly "this PR could be merged right
|
||||
now", and the retired `state:needs-rebase` is the family's proof that a
|
||||
label meaning two things lies about both. It is not a `blocker:*` either:
|
||||
every blocker names work the *builder* owes, a ruling is owed by the human —
|
||||
and the flag must live on issues too, where blockers do not exist. On issues
|
||||
it coexists with the queue labels (the one-of-three invariant above ignores
|
||||
it); its color is the light shade of `state:needs-human`'s, so the human
|
||||
axis reads as one family. It is a state, not a signal: set only with the
|
||||
escalation contract (the question, the options, a recommendation — a bare
|
||||
flag is noise), it stays up until agreement is *reached* — a human reply
|
||||
alone does not clear it — and its setter closes it out: records the ruling
|
||||
as a decision in one comment, removes the label, and returns the item to
|
||||
its flow in that same comment, never as a side effect. If the human
|
||||
disagrees that agreement was reached, the label goes back on. The machine
|
||||
reads it and never writes it: the reconciler refuses `state:needs-human`
|
||||
while it stands (the PR falls to `state:addressing` — the ball on the PR is
|
||||
the builder's, who carries the ruling in), and the staleness sweep skips
|
||||
it, because waiting on a human is legitimately quiet.
|
||||
|
||||
## Scope — which surface? (PRs and issues, any number)
|
||||
|
||||
All scopes share one calm color, `#C5DEF5` — scopes locate, states alert. The
|
||||
|
|
|
|||
|
|
@ -68,3 +68,9 @@ saw Y" outranks one that says "this looks like it might".
|
|||
with evidence until one side concedes or the builder escalates to the
|
||||
maintainer for a ruling. Two reviewers pulling a builder in opposite
|
||||
directions without resolution is a panel failure, not a builder failure.
|
||||
The escalation is flagged `needs-ruling` — by the **builder**, never by
|
||||
you: one accountable flag-setter per PR is what keeps the escalation
|
||||
contract enforceable and hands the human one consolidated question
|
||||
instead of three phrasings of it. Your job is to state the disagreement
|
||||
precisely enough that the builder can write that escalation
|
||||
([LABELS.md](LABELS.md)).
|
||||
|
|
|
|||
|
|
@ -35,7 +35,14 @@ is the failure this whole flow exists to prevent.
|
|||
3. **Escalate.** The blocker is a *decision* only a human owns — scope,
|
||||
money, product direction, breaking a public contract. Say precisely what
|
||||
the decision is, list the options with your recommendation, and name the
|
||||
decider. The discussion is where humans decide; wait there.
|
||||
decider. The discussion is where humans decide; wait there. When the
|
||||
decision blocks something already on the board — an existing issue, or
|
||||
minted work a discussion's ruling gates — set `needs-ruling` on it too,
|
||||
so the board shows where the human's turn is; the issue keeps its queue
|
||||
label. You set the flag, so you close it out ([LABELS.md](LABELS.md)):
|
||||
judge when agreement is reached, record the ruling as a decision in one
|
||||
comment, remove the label, and return the issue to its flow in that same
|
||||
comment.
|
||||
4. **Decline.** Real idea, wrong repo or wrong time. Say why plainly, link
|
||||
where it belongs if anywhere, close. A refusal with reasons is a good
|
||||
outcome; a zombie discussion is not.
|
||||
|
|
|
|||
|
|
@ -40,6 +40,11 @@ BOTS=()
|
|||
REQUIRED_BOTS=()
|
||||
STATES=(state:building state:bots-reviewing state:addressing state:needs-human)
|
||||
BLOCKERS=(blocker:conflict blocker:ci-red blocker:unrequested)
|
||||
# The PR's current labels, set per PR by the sweep. Initialized here because
|
||||
# the script runs under `set -u` even when sourced, and the pure-function
|
||||
# fixtures call decide_state — which now reads has_label — without ever
|
||||
# setting it (#51). An empty default keeps has_label honest for every caller.
|
||||
LABELS=""
|
||||
# Labels this machine used to own and no longer does. Cleared on sight so a
|
||||
# retirement heals the board instead of stranding a label nothing recomputes.
|
||||
RETIRED=(state:needs-rebase)
|
||||
|
|
@ -111,13 +116,14 @@ set_required_bots() { # the PR author is recused by construction
|
|||
}
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The state machine. Pure functions over four globals, set per PR:
|
||||
# The state machine. Pure functions over these 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
|
||||
# MERGEABLE MERGEABLE | CONFLICTING | UNKNOWN (GitHub's own verdict)
|
||||
# CHECKS SUCCESS | FAILURE | PENDING | NONE (the check rollup)
|
||||
# LABELS newline-separated labels currently on the PR
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
requested() { grep -qxF "$1" <<<"$REQUESTED"; }
|
||||
|
|
@ -290,6 +296,20 @@ decide_state() { # → the one state:* label this PR should carry
|
|||
if [ "$s" = state:needs-human ] && [ -n "$(blockers)" ]; then
|
||||
echo state:addressing; return
|
||||
fi
|
||||
|
||||
# A pending ruling disqualifies needs-human the same way (#51): while
|
||||
# `needs-ruling` is up, the human's turn lives in the THREAD — the flag
|
||||
# marks it — and "mergeable right now" must not read true beside an open
|
||||
# decision. state:addressing is the honest landing because the ball ON THE
|
||||
# PR is the builder's: the flag-setter judges when agreement is reached and
|
||||
# carries the ruling in (#50 D6). The label is deliberately NOT in BLOCKERS:
|
||||
# that array is machine-owned, and the converge loop strips every entry the
|
||||
# current facts do not re-derive — `needs-ruling` is hand-set intent the
|
||||
# machine reads and never writes (#50 D9), so parking it there would strip
|
||||
# a live escalation on the next 15-minute tick.
|
||||
if [ "$s" = state:needs-human ] && has_label needs-ruling; then
|
||||
echo state:addressing; return
|
||||
fi
|
||||
echo "$s"
|
||||
}
|
||||
|
||||
|
|
@ -366,6 +386,7 @@ blocker:unrequested|E99695|Somebody still owes a verdict and nobody was asked fo
|
|||
merge-next|0E8A16|Head of the merge queue — merge this one next (set by hand/agent, cleared here)
|
||||
stale|B60205|No activity for 48h — needs a poke (sweep-managed)
|
||||
blocked|6A737D|Waiting on another PR or issue to land first
|
||||
needs-ruling|D4C5F9|A human decision is pending — question, options and a recommendation are in the comment
|
||||
release|0E8A16|Release flow and version/packaging work
|
||||
needs-triage|FBCA04|Did not come through triage — owes normalization or conversion to a discussion
|
||||
ready|0E8A16|Triaged, spec complete, unblocked — a builder can start now and succeed
|
||||
|
|
@ -496,7 +517,9 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
|
|||
} | sort | tail -n1
|
||||
)"
|
||||
age=$((NOW - $(date -d "$last_activity" +%s)))
|
||||
if has_label blocked || [ "$age" -le "$STALE_AFTER" ]; then
|
||||
# needs-ruling joins blocked here: waiting on a human is legitimately quiet
|
||||
# (#50 D10). The 7-day nudge is #52's, once for both surfaces.
|
||||
if has_label blocked || has_label needs-ruling || [ "$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"
|
||||
|
|
|
|||
|
|
@ -414,5 +414,80 @@ warm="$(reconcile_probe "$(printf 'state:addressing\nmerge-next\nstale\nblocker:
|
|||
expect "a bootstrapped repo converges the state as well" \
|
||||
yes "$(grep -q 'state -> state:addressing' <<<"$warm" && echo yes || echo no)"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# needs-ruling (#51): a pending human decision. Hand-set intent the machine
|
||||
# reads and never writes — an EXCLUSION on needs-human, never a blocker and
|
||||
# never a latch. #50's D8 by construction: needs-ruling and state:needs-human
|
||||
# can never share a PR.
|
||||
# ---------------------------------------------------------------------------
|
||||
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE CHECKS=SUCCESS
|
||||
LABELS=""
|
||||
expect "the ruling-free fixture hands off (control)" state:needs-human "$(decide_state)"
|
||||
LABELS="needs-ruling"
|
||||
expect "a pending ruling excludes needs-human" state:addressing "$(decide_state)"
|
||||
LABELS=""
|
||||
expect "...and clearing it hands off again — an exclusion, not a latch" state:needs-human "$(decide_state)"
|
||||
LABELS="needs-ruling" DRAFT=true
|
||||
expect "a draft with a ruling pending is still building" state:building "$(decide_state)"
|
||||
DRAFT=false
|
||||
|
||||
# blockers() must not know the label exists: it is not a branch fact, and the
|
||||
# converge loop strips every BLOCKERS entry the facts do not re-derive —
|
||||
# emitting it there is exactly the trap #51 names.
|
||||
MERGEABLE=CONFLICTING
|
||||
LABELS=""
|
||||
expect "conflict fixture emits its blocker (control)" blocker:conflict "$(blockers)"
|
||||
LABELS="needs-ruling"
|
||||
expect "needs-ruling adds nothing to blockers()" blocker:conflict "$(blockers)"
|
||||
MERGEABLE=MERGEABLE LABELS=""
|
||||
|
||||
# The guard the other fixtures cannot see: an UNGUARDED has_label read under
|
||||
# set -u does not go red — bash treats the unset expansion inside the
|
||||
# herestring redirection as a redirection error (bash 5.2: rc 127, the shell
|
||||
# survives), so has_label fails OPEN, answering "label absent" with only a
|
||||
# stderr complaint. For needs-ruling that would wave a live escalation
|
||||
# through to needs-human in any caller that never set LABELS. Pinned by
|
||||
# re-sourcing in a clean shell: the LABELS="" init keeps the read silent,
|
||||
# and deleting the init turns this red.
|
||||
guard_noise="$(bash -uc '. actions/labels-reconcile/labels-reconcile.sh
|
||||
DRAFT=false HEAD_SHA=h REQUESTED="" REVIEWS_JSON="[]" MERGEABLE=MERGEABLE CHECKS=SUCCESS
|
||||
decide_state' 2>&1 >/dev/null)"
|
||||
expect "a fresh source reads LABELS cleanly (no unbound complaint)" "" "$guard_noise"
|
||||
|
||||
# The full-sweep probes ride the needs-human-otherwise fixture (three
|
||||
# head-current approvals, mergeable, green). reconcile_probe cannot serve
|
||||
# here — its empty round lands on addressing for its own reasons, and the
|
||||
# exclusion must be the ONLY thing moving the state.
|
||||
ruling_probe() { # $1 = the PR's labels → the log lines reconcile_pr emits
|
||||
(
|
||||
REPO_LABELS="$(printf 'state:addressing\nstate:needs-human\nmerge-next\nstale\nneeds-ruling')"
|
||||
REPO=owner/repo NOW="$(date +%s)"
|
||||
LABELS="$1"
|
||||
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON="$ALL_APPROVE"
|
||||
MERGEABLE=MERGEABLE CHECKS=SUCCESS
|
||||
PR_JSON='{"created_at":"2020-01-01T00:00:00Z"}'
|
||||
run() { :; } # swallow mutations
|
||||
gh() { :; } # no network
|
||||
reconcile_pr 888 2>&1
|
||||
)
|
||||
}
|
||||
|
||||
ruled="$(ruling_probe "$(printf 'needs-ruling\nmerge-next')")"
|
||||
expect "the exclusion drives the full sweep to addressing" \
|
||||
yes "$(grep -q 'state -> state:addressing' <<<"$ruled" && echo yes || echo no)"
|
||||
expect "...retracting merge-next: a PR awaiting a ruling is not merge-me-next" \
|
||||
yes "$(grep -q 'cleared merge-next' <<<"$ruled" && echo yes || echo no)"
|
||||
expect "...and the sweep never touches needs-ruling itself" \
|
||||
no "$(grep -q 'needs-ruling' <<<"$ruled" && echo yes || echo no)"
|
||||
|
||||
# Staleness: waiting on a human is legitimately quiet (#50 D10) — same
|
||||
# treatment as blocked, including taking an already-applied stale back off.
|
||||
quiet="$(ruling_probe "needs-ruling")"
|
||||
expect "quiet under a pending ruling is never stale" \
|
||||
no "$(grep -q 'stale (' <<<"$quiet" && echo yes || echo no)"
|
||||
unstale="$(ruling_probe "$(printf 'needs-ruling\nstale')")"
|
||||
expect "...and an already-applied stale comes off" \
|
||||
yes "$(grep -q 'unstale' <<<"$unstale" && echo yes || echo no)"
|
||||
|
||||
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
|
||||
[ "$fail" -eq 0 ]
|
||||
|
|
|
|||
Loading…
Reference in a new issue