[PR #137] [MERGED] ci(bump-callers): seed the pr-risk roster and hard-fail on an empty one (BE-6480) #146

New issue

Closed

opened 2026-10-26 22:36:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:36:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/137
Author: @mattmillerai
Created: 8/5/2026
Status: ✅ Merged
Merged: 8/12/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-6480-pr-risk-fleet-roster

📝 Commits (3)

0b99789 ci(bump-callers): seed the pr-risk roster and hard-fail on an empty one (BE-6480)
306c6d5 ci(bump-callers): make pr-risk's hard-fail explicit and close the roster doc gaps (BE-6480)
29aaf90 Merge origin/main into matt/be-6480-pr-risk-fleet-roster

📊 Changes

5 files changed (+63 additions, -25 deletions)

View changed files

📝 .github/bump-callers/README.md (+1 -1)
📝 .github/workflows/bump-pr-risk-callers.yml (+31 -14)
📝 README.md (+7 -4)
📝 docs/callers/README.md (+1 -1)
📝 docs/callers/pr-risk.md (+23 -5)

📄 Description

ELI-5

pr-risk is a workflow other repos borrow. When it changes, a "bumper" is supposed to open a PR in each borrowing repo moving its pin forward. That bumper was built last week but its address book was left empty, and it was configured to shrug and exit successfully when the address book is empty. So it did nothing, quietly, and one borrower has been running a two-revisions-old grader. This fills in the address book with the two real borrowers and removes the shrug, so an empty address book is now a loud failure instead of a silent one.

Scope note — most of this ticket already shipped

BE-6480 was written against a spike (BE-6474) that predates #119 (BE-6308), which already landed the entrypoint and the docs registration. Rather than re-do that, I verified each of the ticket's criteria against main and implemented only the genuine remainder. Criterion-by-criterion:

Ticket criterion	State
1. `bump-pr-risk-callers.yml` exists, correct `name`/concurrency/paths/`workflow_dispatch`/env/decommission guard/stale guard/no `WIRE_BOT_SCRIPT`	Already on `main` via #119 — verified line by line, all match (#119's decommission guard and stale-run guard are strictly stronger than the groom model the ticket specified)
1. no `ALLOW_EMPTY`	Was wrong — #119 set `ALLOW_EMPTY: "true"`. Fixed here
2. Seed `PR_RISK_CALLERS`	Done (out-of-band, see below)
3. README fleet row / AGENTS.md bullet / `test-bump-callers.yml` both path lists	Already on `main` via #119. The README row's Seeded column said `empty [] allowed`, now corrected
4. Post-merge `workflow_dispatch`	Cannot be done pre-merge — runbook below

Roster seeding (step 2) — done, not in this diff

Seeded as repo config, deliberately not as a commit: this repo is public, so caller names must never appear in a file, a commit message, or PR text. Two entries, {"repo","file","label":""}, matching the GROOM_CALLERS convention.

Verified in both directions, as AGENTS.md requires:

Roster → reality: both enrolled caller files exist and carry the double pin the rewrite moves (a full 40-hex uses: SHA and a bare workflows_ref:).
Reality → roster: swept all 200 Comfy-Org repos' .github/workflows/ via the contents API for any pr-risk caller. Exactly two exist, both enrolled. No third caller is missing from the roster, and no roster entry is dead.

I seeded both a variable and a same-named secret. #135 (BE-6472, open) flips every entrypoint from vars.X to secrets.X; its judgment call 1 assumes this roster is unseeded and therefore inert under the flip. With ALLOW_EMPTY gone that assumption would turn a merge-order accident into a red fleet, so the roster now resolves under either binding. Operational consequence worth knowing: there are now two copies, and a future edit to one silently staleness the other. Once #135 merges and the variables are deleted per its runbook, delete PR_RISK_CALLERS the variable too and keep only the secret.

Riskiest line

Removing ALLOW_EMPTY: "true". Safe because of ordering: the roster was seeded and read back before this change, and checked against bump-callers.sh's own jq validator, so there is no window where the fleet hard-fails against a roster that does not exist. If the roster ever does vanish, the new outcome is a loud red run — which is the point.

Verified empirically rather than by reading the script:

empty roster, ALLOW_EMPTY unset (new)  -> exit 1, ::error:: ... is not a non-empty JSON array
empty roster, ALLOW_EMPTY=true  (old)  -> exit 0, "has no callers yet — nothing to bump"

Negative-claim falsification

This diff adds a deny path (exit 0 → exit 1), so the falsification rule applies. The premise being asserted is "an empty pr-risk roster is never legitimate." That is falsifiable by finding zero callers — so I went and looked rather than assuming: the 200-repo org-wide sweep above returned exactly two live callers, both with pins the bumper can rewrite. Had it returned zero, the hard-fail would be wrong and ALLOW_EMPTY should stay. It returned two. The denial is also loud and one-line reversible, not a silent dead-end.

Verification

bash .github/bump-callers/tests/test_bump_callers.sh — 175 passed, 0 failed
shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh — clean
python3 .github/agents-md-integrity/check_agents_md.py --root . — passed (2 pre-existing warnings: AGENTS.md 186 lines, no CODEOWNERS — unchanged, AGENTS.md is not touched here)
All 32 .github/workflows/*.yml parse
test-bump-callers.yml already lists bump-pr-risk-callers.yml in both path filters, so CI exercises this change on the PR

Judgment calls

Not re-adding the entrypoint. The ticket says "add" it; it exists and is better than the groom copy the ticket asked for. Re-creating it would be a no-op diff or a regression.
Doc fixes beyond the ticket's three. docs/callers/pr-risk.md told enrollers "This workflow has no roster yet … so there is no vars.*_CALLERS entry to add" — actively wrong now, and it is precisely the "enrolling is two steps" half AGENTS.md calls the most repeated mistake here. Both enrollment roster tables (root README.md, docs/callers/README.md) also omitted PR_RISK_CALLERS. Fixed all three; they are the same registration the ticket's step 3 is about.
Left alone: those two tables also omit detect-unreviewed-merge. That fleet's roster is deliberately unseeded and its story is different, so adding it is someone else's call, not a drive-by here.
Conflict with #135 is likely and expected. It edits the same header region of this entrypoint and the same README table. Both diffs are small there; whoever merges second resolves. Nothing here changes the vars. binding — that is #135's job.

Merge-time runbook (ticket step 4 — cannot be done pre-merge)

gh workflow run bump-pr-risk-callers.yml -R Comfy-Org/github-workflows. Landing this touches no watched path, so nothing fires on its own and the stale caller stays stale until this one dispatch.
Expected: one bump PR on branch ci/bump-pr-risk. The other caller is already at main's tip (7f7c9bf) and should no-op.
Confirm the run masks caller names. Note the fleet-wide known gap still applies until #135 lands: the roster is printed unmasked in the step's env: dump before the script's ::add-mask:: can run.

Warning

Do not merge the generated bump PR as a bare pin bump. That caller predates the off-by-default enabled input from #115 and does not set enabled: true — I confirmed this against its current file. A pin-only bump would move it onto a revision where grading is off by default and silently disable its grading. A sibling ticket from the same spike fixes that caller atomically (pin bump plus enabled: true in one commit); it has not merged yet. Leave the bot's PR open and let that ticket supersede or close it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/137 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/5/2026 **Status:** ✅ Merged **Merged:** 8/12/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-6480-pr-risk-fleet-roster` --- ### 📝 Commits (3) - [`0b99789`](https://github.com/Comfy-Org/github-workflows/commit/0b997897d91beb1818740068195ef1d377276ab5) ci(bump-callers): seed the pr-risk roster and hard-fail on an empty one (BE-6480) - [`306c6d5`](https://github.com/Comfy-Org/github-workflows/commit/306c6d5dc0ab3dac9d7b99f54863ec4fc2241dda) ci(bump-callers): make pr-risk's hard-fail explicit and close the roster doc gaps (BE-6480) - [`29aaf90`](https://github.com/Comfy-Org/github-workflows/commit/29aaf9041f0f8d9fbb57d580a5d962e961d9bb37) Merge origin/main into matt/be-6480-pr-risk-fleet-roster ### 📊 Changes **5 files changed** (+63 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+1 -1) 📝 `.github/workflows/bump-pr-risk-callers.yml` (+31 -14) 📝 `README.md` (+7 -4) 📝 `docs/callers/README.md` (+1 -1) 📝 `docs/callers/pr-risk.md` (+23 -5) </details> ### 📄 Description ## ELI-5 `pr-risk` is a workflow other repos borrow. When it changes, a "bumper" is supposed to open a PR in each borrowing repo moving its pin forward. That bumper was built last week but its address book was left empty, and it was configured to shrug and exit successfully when the address book is empty. So it did nothing, quietly, and one borrower has been running a two-revisions-old grader. This fills in the address book with the two real borrowers and removes the shrug, so an empty address book is now a loud failure instead of a silent one. ## Scope note — most of this ticket already shipped BE-6480 was written against a spike (BE-6474) that predates **#119 (BE-6308)**, which already landed the entrypoint and the docs registration. Rather than re-do that, I verified each of the ticket's criteria against `main` and implemented only the genuine remainder. Criterion-by-criterion: | Ticket criterion | State | |---|---| | 1. `bump-pr-risk-callers.yml` exists, correct `name`/concurrency/paths/`workflow_dispatch`/env/decommission guard/stale guard/no `WIRE_BOT_SCRIPT` | Already on `main` via #119 — verified line by line, all match (#119's decommission guard and stale-run guard are strictly stronger than the groom model the ticket specified) | | 1. **no `ALLOW_EMPTY`** | **Was wrong** — #119 set `ALLOW_EMPTY: "true"`. Fixed here | | 2. Seed `PR_RISK_CALLERS` | **Done** (out-of-band, see below) | | 3. README fleet row / AGENTS.md bullet / `test-bump-callers.yml` both path lists | Already on `main` via #119. The README row's **Seeded** column said `empty [] allowed`, now corrected | | 4. Post-merge `workflow_dispatch` | Cannot be done pre-merge — runbook below | ## Roster seeding (step 2) — done, not in this diff Seeded as repo config, deliberately not as a commit: this repo is public, so caller names must never appear in a file, a commit message, or PR text. Two entries, `{"repo","file","label":""}`, matching the `GROOM_CALLERS` convention. **Verified in both directions, as AGENTS.md requires:** - *Roster → reality:* both enrolled caller files exist and carry the double pin the rewrite moves (a full 40-hex `uses:` SHA **and** a bare `workflows_ref:`). - *Reality → roster:* swept **all 200 `Comfy-Org` repos**' `.github/workflows/` via the contents API for any `pr-risk` caller. Exactly two exist, both enrolled. No third caller is missing from the roster, and no roster entry is dead. **I seeded both a variable and a same-named secret.** #135 (BE-6472, open) flips every entrypoint from `vars.X` to `secrets.X`; its judgment call 1 assumes this roster is unseeded and therefore inert under the flip. With `ALLOW_EMPTY` gone that assumption would turn a merge-order accident into a red fleet, so the roster now resolves under either binding. **Operational consequence worth knowing: there are now two copies, and a future edit to one silently staleness the other.** Once #135 merges and the variables are deleted per its runbook, delete `PR_RISK_CALLERS` the *variable* too and keep only the secret. ## Riskiest line Removing `ALLOW_EMPTY: "true"`. Safe because of ordering: the roster was seeded and read back **before** this change, and checked against `bump-callers.sh`'s own `jq` validator, so there is no window where the fleet hard-fails against a roster that does not exist. If the roster ever does vanish, the new outcome is a loud red run — which is the point. Verified empirically rather than by reading the script: ``` empty roster, ALLOW_EMPTY unset (new) -> exit 1, ::error:: ... is not a non-empty JSON array empty roster, ALLOW_EMPTY=true (old) -> exit 0, "has no callers yet — nothing to bump" ``` ## Negative-claim falsification This diff adds a deny path (exit 0 → exit 1), so the falsification rule applies. The premise being asserted is *"an empty pr-risk roster is never legitimate."* That is falsifiable by finding zero callers — so I went and looked rather than assuming: the 200-repo org-wide sweep above returned exactly two live callers, both with pins the bumper can rewrite. Had it returned zero, the hard-fail would be wrong and `ALLOW_EMPTY` should stay. It returned two. The denial is also loud and one-line reversible, not a silent dead-end. ## Verification - `bash .github/bump-callers/tests/test_bump_callers.sh` — **175 passed, 0 failed** - `shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh` — clean - `python3 .github/agents-md-integrity/check_agents_md.py --root .` — passed (2 pre-existing warnings: AGENTS.md 186 lines, no CODEOWNERS — unchanged, AGENTS.md is not touched here) - All 32 `.github/workflows/*.yml` parse - `test-bump-callers.yml` already lists `bump-pr-risk-callers.yml` in both path filters, so CI exercises this change on the PR ## Judgment calls 1. **Not re-adding the entrypoint.** The ticket says "add" it; it exists and is better than the groom copy the ticket asked for. Re-creating it would be a no-op diff or a regression. 2. **Doc fixes beyond the ticket's three.** `docs/callers/pr-risk.md` told enrollers *"This workflow has no roster yet … so there is no `vars.*_CALLERS` entry to add"* — actively wrong now, and it is precisely the "enrolling is two steps" half AGENTS.md calls the most repeated mistake here. Both enrollment roster tables (root `README.md`, `docs/callers/README.md`) also omitted `PR_RISK_CALLERS`. Fixed all three; they are the same registration the ticket's step 3 is about. 3. **Left alone:** those two tables also omit `detect-unreviewed-merge`. That fleet's roster is deliberately unseeded and its story is different, so adding it is someone else's call, not a drive-by here. 4. **Conflict with #135 is likely and expected.** It edits the same header region of this entrypoint and the same README table. Both diffs are small there; whoever merges second resolves. Nothing here changes the `vars.` binding — that is #135's job. ## Merge-time runbook (ticket step 4 — cannot be done pre-merge) 1. `gh workflow run bump-pr-risk-callers.yml -R Comfy-Org/github-workflows`. Landing this touches no watched path, so nothing fires on its own and the stale caller stays stale until this one dispatch. 2. Expected: **one** bump PR on branch `ci/bump-pr-risk`. The other caller is already at `main`'s tip (`7f7c9bf`) and should no-op. 3. Confirm the run masks caller names. Note the fleet-wide known gap still applies until #135 lands: the roster is printed unmasked in the step's `env:` dump before the script's `::add-mask::` can run. > [!WARNING] > **Do not merge the generated bump PR as a bare pin bump.** That caller predates the off-by-default `enabled` input from #115 and does not set `enabled: true` — I confirmed this against its current file. A pin-only bump would move it onto a revision where grading is off by default and **silently disable its grading**. A sibling ticket from the same spike fixes that caller atomically (pin bump **plus** `enabled: true` in one commit); it has **not** merged yet. Leave the bot's PR open and let that ticket supersede or close it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:36:46 +00:00