diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index c451643..26587ab 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -9,15 +9,32 @@ name: labels # 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. +# There is no pull_request_review_target, so a review landing cannot wake this +# workflow directly — and the */15 cron is advisory: GitHub deprioritises short +# intervals hard enough that a quiet repo goes hours between ticks. So the +# handoff wakes the sweep itself: the author sets state:needs-human when handing +# the PR to the maintainer (CONTRIBUTING step 6), and `labeled` fires this +# workflow, which confirms or corrects that optimistic write within seconds. The +# cron stays as the last resort, for the round an agent forgets to hand off. +# +# This cannot loop: the reconciler's own label writes use GITHUB_TOKEN, and +# GitHub does not create workflow runs from GITHUB_TOKEN-triggered events. Agent +# writes use a PAT and therefore do trigger — exactly the asymmetry wanted. 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] + types: + [ + opened, + reopened, + ready_for_review, + converted_to_draft, + synchronize, + labeled, + unlabeled, + ] permissions: contents: read @@ -26,7 +43,12 @@ permissions: jobs: scope: - if: github.event_name == 'pull_request_target' + # Not on labeled/unlabeled: those events change no paths, so labeler has + # nothing new to derive — and label churn is precisely what they are. + if: >- + github.event_name == 'pull_request_target' && + github.event.action != 'labeled' && + github.event.action != 'unlabeled' runs-on: ubuntu-latest concurrency: group: labels-scope-${{ github.event.pull_request.number }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0a755..b3b47ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -188,6 +188,40 @@ on the way to cutting its first release, and this file starts there. ### Changed +- **`state:needs-human` no longer waits on the cron to become true** (#96) + — the labels workflow now also wakes on `pull_request_target: labeled` and + `unlabeled`, and the author sets `state:needs-human` themselves when handing + a PR to the maintainer. + + A review landing was never a trigger. There is no + `pull_request_review_target`, and on fork PRs — which is all of them here — + `pull_request_review` runs with a read-only token and cannot label anything. + So the exact moment the label became true, the third bot approving, fired + nothing at all: the label waited for the `*/15` cron, or for somebody to + touch an unrelated PR. And that cron does not run at its declared rate — + GitHub deprioritises short intervals hard enough that, measured across the + three repos over a two-hour window on 2026-07-20, each got **one** scheduled + run against the eight `*/15` implies. heavy-duty/rig#94 took its third + approval and sat on `state:bots-reviewing` for hours; box and cast happened + to catch a tick and flipped correctly, on byte-identical workflow files. The + lag was worst on the quietest repo, which depends on the cron most and + receives it least. + + The two halves fix each other: the author's own label write is what fires + the sweep that validates it. That makes it an optimistic write rather than a + transfer of ownership — seconds later the reconciler either confirms the + label or corrects it, and the cron falls back to being a genuine last + resort, for the round an agent forgets to hand off. It cannot loop, because + the reconciler's own writes use `GITHUB_TOKEN` and GitHub does not create + workflow runs from `GITHUB_TOKEN`-triggered events, while agent writes use a + PAT and do. `labels-reconcile.sh` is unchanged: it already recomputes every + open PR from scratch on every run, which is exactly what makes the + optimistic write safe. The `scope` job is skipped on the two new actions, + where no path can have changed and labeler has nothing to derive. + + Landed in all three repos together (heavy-duty/box#142, + heavy-duty/cast#132) — `labels.yml` and the label taxonomy are shared. + - **PR labels split into two axes: `state:*` (whose ball) and `blocker:*` (what is in the way)** (heavy-duty/box#137) — `state:needs-rebase`, added here only days ago by #87, is retired in the same breath. In its place: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 43a567c..7e1c4a5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,14 +31,24 @@ labels tell you where everything is without opening anything. 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`, +6. **When the round passes, the author hands the PR to the maintainer** in + three acts, in this order: post the tagged round summary, request the + maintainer's review, then set `state:needs-human` yourself — removing the + state label it replaces. The review request is what *earns* the label, provided the PR carries **no `blocker:*` label**. A blocker means the work - is still yours whatever the round said, so a request on a conflicted or red - PR will not flip it. With three formal head-current approvals the labels + is still yours whatever the round said, so on a conflicted or red PR + neither the request nor your own label write will stick — the sweep takes + it straight back off. With three formal head-current approvals the labels workflow requests the maintainer automatically; when part of the panel is comment-only, reading their agreement is the author's judgment, so the author makes the request. + + Writing the label by hand is an **optimistic write, not a transfer of + ownership**. The machine stays the authority — but because the workflow + wakes on `labeled`, the author's own write fires the sweep that validates + it, and a handoff that had not earned the label is corrected seconds later. + Forgetting the write is not a failure either; it only means the label waits + for the cron, which is the lag this replaced. 7. **Checks must be green**: `shellcheck`, `bash test/cli.sh` and `bash test/release.sh` locally mirror what CI runs; the db dump/restore round-trip (`test/db-integration.sh`) executes in CI where Docker is @@ -97,7 +107,7 @@ 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. Exactly one per PR: *whose ball is it.* | +| `state:*` | the labels workflow ([.github/workflows/labels.yml](.github/workflows/labels.yml)) — recomputed from GitHub's own facts on PR events (label changes included) and every 15 minutes. Machine-owned, with one exception: the author sets `state:needs-human` at handoff (step 6) and the workflow reconciles it. Otherwise never by hand. Exactly one per PR: *whose ball is it.* | | `blocker:*` | the same workflow, from the same facts — *what is in the way.* Any number per PR, or none. Never by hand: applying one does not stop a merge, and removing one does not unblock anything. Fix the thing and the next sweep drops the label. | | `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. | diff --git a/LABELS.md b/LABELS.md index f352850..a9d2408 100644 --- a/LABELS.md +++ b/LABELS.md @@ -107,13 +107,30 @@ would just say the same thing twice, drifting apart eventually. ## Maintenance -State labels are written by automation, never by hand. Every state above is -derivable from GitHub's own facts — the draft flag, requested reviewers, +State labels are machine-owned, with exactly one exception. Every state above +is derivable from GitHub's own facts — the draft flag, requested reviewers, 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 +state and reconciles labels statelessly, on PR events (label changes included) +plus a 15-minute cron. A hand-moved label is a lie waiting to happen; the +workflow asserts the effective state instead. + +The exception is `state:needs-human`, which the author sets at handoff +([CONTRIBUTING.md](CONTRIBUTING.md), step 6). That is an optimistic write, not +a transfer of ownership: because `pull_request_target: labeled` wakes the +workflow, the author's own label write fires the sweep that validates it, and +a handoff that had not earned the label is corrected within seconds. + +It exists because the wake signal was missing. There is no +`pull_request_review_target` — on fork PRs, which is all of them here, +`pull_request_review` runs read-only and cannot label anything — so the moment +the label becomes true, the third approval landing, fired nothing at all. What +was left was the `*/15` cron, and GitHub deprioritises short intervals hard +enough that the delivered rate is closer to hourly. The label could therefore +lag the round it described by hours, worst on the quietest repo: every sweep +reconciles the whole board, so a busy repo stays fresh by piggybacking on +unrelated PR events, while a quiet one depends on the cron most and receives +it least. `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.