diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 04a4adb..0f68fd1 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -10,11 +10,14 @@ name: labels # and reconcile checks out the BASE branch only. Keep it that way. # # 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. The -# handoff wakes the sweep itself: the author sets state:needs-human, and the -# caller's `labeled` event confirms or corrects that optimistic write within -# seconds. The cron stays as the last resort for a forgotten handoff. +# workflow directly — which is why the caller's cron is load-bearing, not a +# safety net (#199 relaxed it from */15 to hourly, but did NOT drop it). The +# cron is the sweep's only discovery path for every transition no subscribed +# event carries: a verdict landing, blocker:ci-red set/cleared, a +# blocker:conflict when another PR merges under this one, and the time-based +# stale / 48h claim-reclaim. Where an event IS subscribed the wake is direct — +# the handoff sets state:needs-human and the caller's `labeled` event confirms +# or corrects that optimistic write within seconds. # # This cannot loop: reconciler writes use GITHUB_TOKEN, and GitHub does not # create workflow runs from GITHUB_TOKEN-triggered events. Agent writes use a diff --git a/.github/workflows/self-labels.yml b/.github/workflows/self-labels.yml index e9b8dac..e955e52 100644 --- a/.github/workflows/self-labels.yml +++ b/.github/workflows/self-labels.yml @@ -5,11 +5,36 @@ name: labels # unpinned — correct only for the repo that IS the source). Consumers write: # uses: heavy-duty/ceremony/.github/workflows/labels.yml@ on: - schedule: [{cron: "*/15 * * * *"}] # advisory; the handoff label is the real wake + # Hourly, not */15 (#199): the cron is the sweep's ONLY wake for the four + # transition classes no subscribed event carries — a review verdict landing + # (there is no pull_request_review trigger here), blocker:ci-red set or cleared + # (no check_suite/check_run/workflow_run), a blocker:conflict when ANOTHER PR + # merges under this one, and the time-based stale / 48h claim-reclaim. The + # events below carry the rest in seconds. So hourly trades ≤1h of latency on + # those four for dropping */15's four full-board sweeps an hour at GitHub's + # 1-minute billing floor — 52% of one private consumer's runs. Do NOT delete + # this cron — it is the discovery path for those four, not a forgotten-handoff net. + schedule: [{cron: "0 * * * *"}] workflow_dispatch: # bootstraps missing labels on a fresh repo + # Narrowed (#199) to the actions that carry a queue-state change the hourly + # cron cannot wait one cadence for — dropping only labeled/unlabeled/assigned/ + # unassigned, which feed validation and the 48h claim clock (caught within one + # cadence) and were the dominant issues-churn source. Kept: `opened` (the + # mint→needs-triage check, issueflow's opened-only path), `closed` (the + # blocker-closes→ready self-heal, crew#96/#98), `edited` (a body rewrite of the + # `Blocked by #N` declaration the sweep parses — issueflow-reconcile.sh:179), + # `reopened` (a closed issue re-entering the queue wearing labels derived when + # it closed). The must-fail in #199 is exactly "a queue-state transition waits + # on the schedule when an event could have carried it", so edited/reopened stay + # on events. The PR handoff wake is pull_request_target:labeled, NOT issues, so + # this does not touch the handoff. issues: - types: [opened, edited, assigned, unassigned, labeled, unlabeled, closed, reopened] + types: [opened, closed, edited, reopened] pull_request_target: + # Every PR arrives from a fork, so these carry the head/draft/review facts + # the sweep derives state:* from. labeled/unlabeled are the handoff wake — + # the author's optimistic state:needs-human write, confirmed or corrected + # here in seconds (#11); synchronize re-derives on every push; # review_requested/review_request_removed wake the sweep that clears (or # restores) blocker:unrequested — without them the one event that makes # the label false could not clear it, and a quiet repo wore the red flag diff --git a/changelog.d/199.md b/changelog.d/199.md new file mode 100644 index 0000000..69dd207 --- /dev/null +++ b/changelog.d/199.md @@ -0,0 +1,17 @@ +### Changed + +- `labels` — the reconcile cron relaxes from `*/15` to hourly (#199), cutting a + private consumer's schedule-triggered full-board sweeps ~4× at GitHub's + 1-minute billing floor. +- `labels` — the hourly cron is the sweep's only wake for transitions no + subscribed event carries — a verdict landing, blocker:ci-red, a + blocker:conflict when another PR merges, the time-based stale/reclaim — so it + bounds their latency to ≤1h, delaying no event-carried transition (#199). +- `labels` — the caller's `issues:` trigger narrows to + `[opened, closed, edited, reopened]` (#199), the actions that carry a + queue-state change the cron cannot wait a cadence for. The churn/validation + actions — labeled/unlabeled/assigned/unassigned — come off; the PR handoff + wake is unaffected. +- `labels` — each caller trigger now carries a comment saying why it is + subscribed, and reconcile keeps `cancel-in-progress: false` (#199) — + cancelling a sweep mid-board is the race that guard exists to prevent. diff --git a/docs/CONSUMERS.md b/docs/CONSUMERS.md index 3d02e52..5a4a4c9 100644 --- a/docs/CONSUMERS.md +++ b/docs/CONSUMERS.md @@ -296,20 +296,34 @@ The complete caller is: ```yaml name: labels on: - schedule: [{cron: "*/15 * * * *"}] # advisory; the handoff label is the real wake + # Hourly, not */15 (#199): the cron is the sweep's only wake for the + # transitions no subscribed event carries — a review verdict landing (no + # pull_request_review trigger), blocker:ci-red set/cleared, a blocker:conflict + # when another PR merges under this one, and the time-based stale / 48h + # claim-reclaim. Events below carry the rest in seconds. Hourly trades ≤1h of + # latency on those four for dropping */15's four sweeps an hour at GitHub's + # 1-minute floor. Keep the cron — it is the discovery path, not a safety net. + schedule: [{cron: "0 * * * *"}] workflow_dispatch: # bootstraps missing labels on a fresh repo pull_request_target: - # review_requested/review_request_removed are unreleased — not in 0.2.0; - # add them with the pin bump to the first tag carrying ceremony#137. They - # wake the sweep that clears blocker:unrequested when the panel is asked. + # Fork PRs; these carry the head/draft/review facts state:* derives from. + # labeled/unlabeled are the handoff wake (state:needs-human confirmed here); + # synchronize re-derives on every push. review_requested/review_request_removed + # (shipped in 0.3.0, ceremony#137) wake the sweep that clears + # blocker:unrequested when the panel is asked. types: [opened, reopened, ready_for_review, converted_to_draft, synchronize, labeled, unlabeled, review_requested, review_request_removed] # Available at 0.2.0 and later (the first tag carrying ceremony#32); a # consumer pinned to 0.1.0 omits this block. issues: - # edited/reopened are unreleased — not in 0.2.0; add them with the pin - # bump to the first tag carrying ceremony#144, the same bump as the - # pull_request_target additions above. - types: [opened, edited, assigned, unassigned, labeled, unlabeled, closed, reopened] + # Narrowed (#199) to the actions carrying a queue-state change the hourly + # cron cannot wait one cadence for: opened → the mint→needs-triage check, + # closed → the blocker-closes→ready self-heal, edited → a body rewrite of the + # `Blocked by #N` declaration the sweep parses, reopened → a closed issue + # re-entering the queue. Dropped: labeled/unlabeled/assigned/unassigned — + # validation + the 48h claim clock, caught within one cadence, and + # labeled/unlabeled were the issues-churn source. The handoff wake is + # pull_request_target:labeled, not issues, so this leaves it intact. + types: [opened, closed, edited, reopened] permissions: contents: read checks: read # mergeability/check-rollup read for PR state @@ -330,23 +344,32 @@ needs all three explicit reads above; without them the failure appears as an emp The `issues:` trigger is available at `0.2.0` and later — `0.2.0` is the first tag carrying ceremony#32. A consumer pinned to `0.1.0` omits it. Adopt it only by bumping every ceremony reference to `0.2.0` or later; never mix -refs to adopt it early. At `0.2.0` the type list stops at `closed`: `edited` -and `reopened` are newer, part of the pending stub edit below. +refs to adopt it early. The type list has grown then narrowed across tags: +`0.2.0` (ceremony#32) shipped `[opened, labeled, unlabeled, assigned, +unassigned, closed]`; `0.3.0` (ceremony#144) added `edited` and `reopened`; +ceremony#199 narrows it to `[opened, closed, edited, reopened]` and relaxes the +cron to hourly, so a consumer picks up the smaller trigger surface at the pin +bump to the first tag carrying ceremony#199. The narrowing drops +`labeled`/`unlabeled`/`assigned`/`unassigned` — validation and the 48h claim +clock, which the hourly cron catches within one cadence, and `labeled`/ +`unlabeled` were the issues-churn source — while **keeping** #144's `edited`/ +`reopened`: those carry a queue-state change an event uniquely carries (a body +rewrite of `Blocked by #N`, and a closed issue re-entering the queue), so the +must-fail in ceremony#199 keeps them on events. `opened` drives the +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. `pull_request_target` is intentional: fork PRs need the base repository's token to write labels. The reusable workflow executes no PR code. It checks out only the consumer's base branch and the pinned ceremony implementation. The #52 ruling invariants ride exactly these triggers — but the caller above is no longer the #18 shape, so adopting current triggers is a stub edit, not -a bare pin bump. The pending edits are `review_requested` and -`review_request_removed` on `pull_request_target:` (#137) — the wake that +a bare pin bump. `review_requested` and `review_request_removed` on +`pull_request_target:` shipped in `0.3.0` (ceremony#137) — the wake that clears `blocker:unrequested` the moment the panel is asked, without which a -quiet repo wears that flag until the advisory cron — and `edited` and -`reopened` on `issues:` (#144): the wakes for a body edit rewriting the -`Blocked by #N` declaration the reconcile sweep parses, and for a closed -issue re-entering the queue wearing labels derived when it closed. Make both -edits together, with the pin bump to the first tag carrying ceremony#137 and -ceremony#144 — never before it and never through mixed refs. +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: diff --git a/test/labels-triggers.test.sh b/test/labels-triggers.test.sh new file mode 100644 index 0000000..b408663 --- /dev/null +++ b/test/labels-triggers.test.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +set -u + +# The labels TRIGGER SURFACE is a cost lever (#199): a full-board sweep is +# billed a 1-minute minimum every time a trigger fires, so how OFTEN it fires +# is what exhausted the fleet's shared Actions allotment. These assertions +# pin the reductions #199 made and the guard it must not trade away — none of +# them touch the reconciler's LOGIC, which its own fixtures cover. + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=test/harness.sh +source "$ROOT/test/harness.sh" + +REUSABLE="$ROOT/.github/workflows/labels.yml" +SELF="$ROOT/.github/workflows/self-labels.yml" +STUB="$ROOT/docs/CONSUMERS.md" # the published caller stub, a fenced yaml block + +# The `cancel-in-progress:` value of a named top-level job, read from the first +# such line inside that job's block. Job keys sit at two-space indent. +job_cancel_in_progress() { # $1 = file, $2 = job name + awk -v job="^ $2:\$" ' + $0 ~ job { f = 1; next } + f && /^ [a-z]/ { exit } # next job — stop before leaking into it + f && /cancel-in-progress:/ { sub(/.*cancel-in-progress:[[:space:]]*/, ""); print; exit } + ' "$1" +} + +# The `types:` list of a trigger key (issues:, pull_request_target:), read from +# the first `types:` line after the bare key. The key is bare (nothing after +# the colon) so it never collides with `issues: write` in the permissions block. +trigger_types() { # $1 = file, $2 = trigger key + awk -v key="^ $2:\$" ' + $0 ~ key { f = 1; next } + f && /^ types:/ { sub(/^ types:[[:space:]]*/, ""); print; exit } + f && /^ [a-z]/ { exit } + ' "$1" +} + +# ---- the guard the cost fix must never trade away (#199 test plan must-fail) -- +# cancel-in-progress: true on reconcile kills a sweep mid-board, the exact race +# the shared concurrency group exists to prevent. It WOULD cut run count — by +# trading correctness for minutes — so it stays false, forever. +check "reconcile serializes, never cancels mid-board" 0 "false" \ + job_cancel_in_progress "$REUSABLE" reconcile +# shellcheck disable=SC2016 # the awk program runs in the nested bash, not here +check "reconcile is never cancel-in-progress: true" 1 "" \ + bash -c 'job_cancel_in_progress() { + awk -v job="^ reconcile:\$" "\$0 ~ job{f=1;next} f&&/^ [a-z]/{exit} f&&/cancel-in-progress:/{sub(/.*cancel-in-progress:[[:space:]]*/,\"\");print;exit}" "$1" + }; [ "$(job_cancel_in_progress "$1")" = true ]' _ "$REUSABLE" +# scope MAY cancel — it is per-PR and additive, so a superseded run is waste, +# not a lost sweep. This asserts the must-fail above is scoped to reconcile. +check "scope stays cancel-in-progress: true (per-PR, additive)" 0 "true" \ + job_cancel_in_progress "$REUSABLE" scope + +# ---- the cron is a backstop, relaxed to hourly (#199 candidate 1) ----------- +# Scope the */15 assertion to the cron LINE — the prose comments cite */15 by +# name to explain the change, and must not re-red their own documentation. +check "self caller cron is hourly" 0 '0 * * * *' grep -F 'cron:' "$SELF" +# shellcheck disable=SC2016 # $1 expands in the nested bash, not here +check "self caller cron line no longer fires */15" 1 "" \ + bash -c 'grep -F "cron:" "$1" | grep -qF "*/15"' _ "$SELF" +check "stub cron is hourly" 0 '0 * * * *' grep -F 'cron:' "$STUB" +# shellcheck disable=SC2016 # $1 expands in the nested bash, not here +check "stub cron line no longer fires */15" 1 "" \ + bash -c 'grep -F "cron:" "$1" | grep -qF "*/15"' _ "$STUB" + +# ---- issues: is narrowed to the queue-state-changing actions (#199) ---------- +# Kept because each carries a queue-state change an event uniquely carries, so +# dropping it would trip #199's must-fail (a transition waiting on the schedule +# when an event could have carried it): opened → mint→needs-triage; closed → +# blocker-closes→ready self-heal; edited → a body rewrite of the `Blocked by #N` +# declaration the sweep parses; reopened → a closed issue re-entering the queue. +# (labels.test.sh owns the exact-list and caller<->stub parity assertions.) +for keep in opened closed edited reopened; do + # shellcheck disable=SC2016 # the awk program runs in the nested bash, not here + check "self caller issues surface keeps '$keep'" 0 "" \ + bash -c 'trigger_types() { + awk -v key="^ issues:\$" "\$0 ~ key{f=1;next} f&&/^ types:/{sub(/^ types:[[:space:]]*/,\"\");print;exit} f&&/^ [a-z]/{exit}" "$1" + }; trigger_types "$1" | grep -qw "$2"' _ "$SELF" "$keep" +done +# The churn actions must not reappear on the issues surface without a fresh why. +# labeled/unlabeled were the dominant issues-churn source; assigned/unassigned +# only feed validation and the 48h claim clock, caught within one cadence. +for churn in labeled unlabeled assigned unassigned; do + # shellcheck disable=SC2016 # the awk program runs in the nested bash, not here + check "self caller issues surface drops '$churn'" 1 "" \ + bash -c 'trigger_types() { + awk -v key="^ issues:\$" "\$0 ~ key{f=1;next} f&&/^ types:/{sub(/^ types:[[:space:]]*/,\"\");print;exit} f&&/^ [a-z]/{exit}" "$1" + }; trigger_types "$1" | grep -qw "$2"' _ "$SELF" "$churn" +done + +# ---- the PR handoff wake is NOT collateral of the issues narrowing ---------- +# The handoff (state:needs-human, confirmed by the caller's labeled event) rides +# pull_request_target, not issues. A future edit that strips it there re-reds. +check "pull_request_target keeps the labeled handoff wake" 0 "labeled" \ + trigger_types "$SELF" pull_request_target + +summary diff --git a/test/labels.test.sh b/test/labels.test.sh index 79f53fa..e08d7c0 100755 --- a/test/labels.test.sh +++ b/test/labels.test.sh @@ -69,15 +69,16 @@ check "LABELS.md enumerates no repo's scope labels" 1 "0" \ # same lists. review_requested/review_request_removed are the wake that # clears blocker:unrequested — the label sat false for as long as a quiet # repo stayed quiet because the one event that falsifies it was never -# listed (#137). edited/reopened are the wakes for the two events that -# falsify issue labels silently — an edited body rewrites the `Blocked by -# #N` declaration the reconcile sweep parses, and a reopened issue -# re-enters the queue wearing labels derived at close; PR #32 widened the -# caller by both and the stub never followed (#144). The stub is prose, so -# nothing but these rows keeps the lists from drifting: a type in one file -# only is a wake that fires at home and nowhere in the fleet, or the -# reverse. The NF guard keeps `issues: write` under permissions: from -# matching the issues: trigger key. +# listed (#137). The issues list narrowed to [opened, closed, edited, reopened] +# (#199): each carries a queue-state change the hourly cron cannot wait one +# cadence for — opened drives mint→needs-triage, closed the blocker-closes→ready +# self-heal, edited a body rewrite of the `Blocked by #N` line the sweep parses, +# reopened a closed issue re-entering the queue — while the churn/validation +# actions (labeled/unlabeled/assigned/unassigned) came off. The stub is prose, so nothing but these rows +# keeps the lists from drifting: a type in one file only is a wake that fires +# at home and nowhere in the fleet, or the reverse — the drift #144 caught. +# The NF guard keeps `issues: write` under permissions: from matching the +# issues: trigger key. event_types() { # $1 = file, $2 = trigger key → that trigger's types line, unindented awk -v key="$2:" '$1 == key && NF == 1 {f=1; next} f && /types: /{sub(/^ */,""); print; exit}' "$1" } @@ -88,17 +89,20 @@ types_in_sync() { # $1 = trigger key, $2 = caller, $3 = stub → 0 when both lis } CALLER="$ROOT/.github/workflows/self-labels.yml" STUB="$ROOT/docs/CONSUMERS.md" +# event_types anchors on the bare trigger key (NF == 1), so it reads the real +# types line even though the #199 comments name pull_request_target: and +# issues: in prose above the keys — an inline /pull_request_target:/ scan would +# latch onto the first mention and read the wrong list. +pr_has_both_review_wakes() { + event_types "$CALLER" pull_request_target | grep -F review_requested | grep -qF review_request_removed +} check "caller and stub pull_request_target lists are identical" 0 "" \ types_in_sync pull_request_target "$CALLER" "$STUB" -# shellcheck disable=SC2016 # expansion belongs to the nested bash -check "the caller lists both review-request wakes" 0 "" bash -c \ - 'awk "/pull_request_target:/{f=1; next} f && /types: /{print; exit}" "$1" | - grep -F review_requested | grep -qF review_request_removed' _ "$CALLER" +check "the caller lists both review-request wakes" 0 "" pr_has_both_review_wakes check "caller and stub issues lists are identical" 0 "" \ types_in_sync issues "$CALLER" "$STUB" -check "the caller still lists all eight issue types" 0 \ - "types: [opened, edited, assigned, unassigned, labeled, unlabeled, closed, reopened]" \ - event_types "$CALLER" issues +check "the caller lists exactly the queue-state-changing issue types" 0 \ + "types: [opened, closed, edited, reopened]" event_types "$CALLER" issues # the failing cases: drop a type from either file, or reorder one list only, # and the identity rows above go red — exercised here on mutated copies mut_caller="$TMP/mut-caller.yml" mut_stub="$TMP/mut-stub.md" @@ -112,13 +116,13 @@ sed 's/review_requested, review_request_removed/review_request_removed, review_r "$STUB" >"$mut_stub" check "a reorder in one list only goes red" 1 "" \ types_in_sync pull_request_target "$CALLER" "$mut_stub" -sed 's/, edited//' "$CALLER" >"$mut_caller" +sed 's/, closed//' "$CALLER" >"$mut_caller" check "an issue type dropped from the caller goes red" 1 "" \ types_in_sync issues "$mut_caller" "$STUB" -sed 's/, edited//' "$STUB" >"$mut_stub" +sed 's/, closed//' "$STUB" >"$mut_stub" check "an issue type dropped from the stub goes red" 1 "" \ types_in_sync issues "$CALLER" "$mut_stub" -sed 's/closed, reopened/reopened, closed/' "$STUB" >"$mut_stub" +sed 's/opened, closed/closed, opened/' "$STUB" >"$mut_stub" check "an issue-list reorder in one file only goes red" 1 "" \ types_in_sync issues "$CALLER" "$mut_stub"