forked from heavy-duty/rig
fix(labels): sweep on labeled so the handoff is immediate
A review landing was never a trigger for the labels workflow, so the exact moment `state:needs-human` became true — the third bot approving — fired nothing, and the label waited on the `*/15` cron. That cron does not run at its declared rate: measured across box, rig and cast over a two-hour window on 2026-07-20, one scheduled run each against the eight `*/15` implies. The obvious fix does not work. There is no `pull_request_review_target`, and on fork PRs — all of them here — `pull_request_review` runs with a read-only token and cannot label anything. So the handoff wakes the sweep itself: - `pull_request_target` also fires on `labeled`/`unlabeled` - the author sets `state:needs-human` at handoff, as the third act after the round summary and the review request The author's own label write fires the sweep that validates it — an optimistic write, not a transfer of ownership. The reconciler confirms or corrects it seconds later, and the cron falls back to a last resort. It cannot loop: the reconciler writes with GITHUB_TOKEN, which does not create workflow runs; agent writes use a PAT, which does. `labels-reconcile.sh` is unchanged — it already recomputes every open PR from scratch on every run, which is what makes the optimistic write safe. The `scope` job is skipped on label events, where no path can have changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
ac1bb3bf76
commit
23cca65855
4 changed files with 98 additions and 15 deletions
32
.github/workflows/labels.yml
vendored
32
.github/workflows/labels.yml
vendored
|
|
@ -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 }}
|
||||
|
|
|
|||
34
CHANGELOG.md
34
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#141, heavy-duty/cast#131) — `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:
|
||||
|
|
|
|||
|
|
@ -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. |
|
||||
|
|
|
|||
27
LABELS.md
27
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.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue