Merge pull request #97 from dan-claude-bot/fix/labels-sweep-on-labeled
fix(labels): sweep on `labeled` so the handoff is immediate
This commit is contained in:
commit
27a5fa282e
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,
|
# 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.
|
# and reconcile checks out the BASE branch only. Keep it that way.
|
||||||
#
|
#
|
||||||
# Review-submitted transitions (bots finishing a round) ride the cron: there
|
# There is no pull_request_review_target, so a review landing cannot wake this
|
||||||
# is no pull_request_review_target, so the 15-minute tick is the wake signal —
|
# workflow directly — and the */15 cron is advisory: GitHub deprioritises short
|
||||||
# the same cadence the reviewer bots poll at.
|
# 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:
|
on:
|
||||||
schedule:
|
schedule:
|
||||||
- cron: "*/15 * * * *"
|
- cron: "*/15 * * * *"
|
||||||
workflow_dispatch: # also bootstraps missing labels — run once on a fresh repo
|
workflow_dispatch: # also bootstraps missing labels — run once on a fresh repo
|
||||||
pull_request_target:
|
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:
|
permissions:
|
||||||
contents: read
|
contents: read
|
||||||
|
|
@ -26,7 +43,12 @@ permissions:
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
scope:
|
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
|
runs-on: ubuntu-latest
|
||||||
concurrency:
|
concurrency:
|
||||||
group: labels-scope-${{ github.event.pull_request.number }}
|
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
|
### 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:*`
|
- **PR labels split into two axes: `state:*` (whose ball) and `blocker:*`
|
||||||
(what is in the way)** (heavy-duty/box#137) — `state:needs-rebase`, added
|
(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:
|
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
|
judgment belongs to the **author** — escalate by requesting the
|
||||||
maintainer's review (step 6), and the reconciler flips the label on that
|
maintainer's review (step 6), and the reconciler flips the label on that
|
||||||
request, because an explicit request is a fact it can trust.
|
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
|
6. **When the round passes, the author hands the PR to the maintainer** in
|
||||||
requesting their review — that request is what flips `state:needs-human`,
|
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
|
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
|
is still yours whatever the round said, so on a conflicted or red PR
|
||||||
PR will not flip it. With three formal head-current approvals the labels
|
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
|
workflow requests the maintainer automatically; when part of the panel is
|
||||||
comment-only, reading their agreement is the author's judgment, so the
|
comment-only, reading their agreement is the author's judgment, so the
|
||||||
author makes the request.
|
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
|
7. **Checks must be green**: `shellcheck`, `bash test/cli.sh` and
|
||||||
`bash test/release.sh` locally mirror what CI runs; the db dump/restore
|
`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
|
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 |
|
| 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. |
|
| `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. |
|
| `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. |
|
| `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
|
## Maintenance
|
||||||
|
|
||||||
State labels are written by automation, never by hand. Every state above is
|
State labels are machine-owned, with exactly one exception. Every state above
|
||||||
derivable from GitHub's own facts — the draft flag, requested reviewers,
|
is derivable from GitHub's own facts — the draft flag, requested reviewers,
|
||||||
review states, push timestamps — so the labels workflow
|
review states, push timestamps — so the labels workflow
|
||||||
([.github/workflows/labels.yml](.github/workflows/labels.yml)) recomputes the
|
([.github/workflows/labels.yml](.github/workflows/labels.yml)) recomputes the
|
||||||
state and reconciles labels statelessly, on a 15-minute cron plus PR events.
|
state and reconciles labels statelessly, on PR events (label changes included)
|
||||||
A hand-moved label is a lie waiting to happen; the workflow asserts the
|
plus a 15-minute cron. A hand-moved label is a lie waiting to happen; the
|
||||||
effective state instead. `scope:` labels on PRs are applied from the changed
|
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));
|
paths by actions/labeler ([.github/labeler.yml](.github/labeler.yml));
|
||||||
[CONTRIBUTING.md](CONTRIBUTING.md) says who sets what.
|
[CONTRIBUTING.md](CONTRIBUTING.md) says who sets what.
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue