[PR #177] [MERGED] ci(bump-callers): share one roster across the pr-risk and pr-derisk fleets (BE-7756) #185

New issue

Closed

opened 2026-10-26 22:37:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:37:08 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-7756-share-one-roster-pr-risk-derisk

📝 Commits (1)

e60695a ci(bump-callers): share one roster across the pr-risk and pr-derisk fleets (BE-7756)

📊 Changes

8 files changed (+300 additions, -20 deletions)

View changed files

📝 .github/bump-callers/README.md (+42 -1)
📝 .github/bump-callers/bump-callers.sh (+91 -6)
📝 .github/bump-callers/tests/test_bump_callers.sh (+119 -0)
📝 .github/workflows/bump-pr-derisk-callers.yml (+32 -9)
📝 .github/workflows/bump-pr-risk-callers.yml (+9 -0)
📝 AGENTS.md (+1 -1)
📝 docs/callers/README.md (+1 -1)
📝 docs/callers/pr-derisk.md (+5 -2)

📄 Description

ELI-5

The roster is a delivery list telling the bumper: go to this repo, open this file, update the pin inside.

Right now pr-risk and pr-derisk each want their own list. Two lists means two places to remember a repo — and forgetting the second one is completely silent. The caller merges, the pin never moves, every run stays green, and you find out weeks later.

That already happened. ComfyUI_frontend enrolled in pr-risk on 08-07, never made it into the roster, and sat 10 commits behind for ten days.

Every roster line already says which file it's about — nobody was using that. So: keep one list, and let each fleet pick its own lines out of it.

[
  {"repo": "Comfy-Org/example", "file": ".github/workflows/ci-pr-risk.yml"},
  {"repo": "Comfy-Org/example", "file": ".github/workflows/ci-pr-derisk.yml"}
]

The risk fleet takes the first line. The derisk fleet takes the second. One address book, two mail carriers, each reading only their own routes. Enrolling a repo is one edit in one place.

What changed

bump-callers.sh — new optional FILE_FILTER. When set, only roster entries whose file matches are bumped. Unset is byte-identical to today, which is every other fleet.
bump-pr-risk-callers.yml — filters to ci-pr-risk.yml.
bump-pr-derisk-callers.yml — filters to ci-pr-derisk.yml and reads PR_RISK_CALLERS. PR_DERISK_CALLERS is retired before it was ever seeded, so there is no migration.

Why not just merge the two fleets into one

Recorded so it isn't re-proposed:

WORKFLOW_FILE names the reusable whose @sha gets rewritten and is fleet-level. The two fleets pin different reusables, so a merged fleet needs it moved into the roster entry — a data-model change to the one script this repo is emphatic about not reshaping casually.
A fleet bumps every roster entry when it fires. Merged, a scripts/pr-derisk/-only change would open a no-op pin bump against every pr-risk caller — including repos running no derisk caller at all, every time the beta churns.

Failure modes, deliberately loud

Malformed FILE_FILTER → hard error. A typo would otherwise select zero entries, which on ALLOW_EMPTY=true is a green no-op indistinguishable from "not enrolled yet" — frozen callers, forever, with nothing red.
Selecting nothing defers to ALLOW_EMPTY, exactly as an empty roster does: derisk (growing) no-ops green, pr-risk (must always have callers) hard-fails naming the filter and the entry count.
Validation still covers filtered-out entries. A malformed entry belonging to the sibling fleet fails the run that sees it, rather than surfacing on whichever fleet happens to run next.
Both fleets log the same roster digest — it's taken over the whole secret, not the filtered subset. That identity is the evidence they read one roster; a per-fleet digest would make a shared roster look like two unrelated ones.

Residual, stated rather than implied

FILE_FILTER matches by string equality, so every repo in a filtered fleet must spell its caller the same way. Fleets don't have that property in general — CURSOR_REVIEW_CALLERS carries three spellings today — so a repo naming its caller differently is skipped silently. That's by design: skipping entries is the filter's entire job, and a warning on every run is noise nobody reads. The audit line's N caller(s), M after FILE_FILTER=… is the signal; glance at M after any enrolment.

Only put a fleet on FILE_FILTER where the caller filename convention is controlled — which pr-risk/pr-derisk are (their caller guides specify the names literally) and cursor-review is not. Documented in the script header and the bump-callers README.

Testing

bash .github/bump-callers/tests/test_bump_callers.sh    # 409 passed, 0 failed (36 new)
bash .github/bump-callers/tests/test_paths_contract.sh  #  56 passed, 0 failed
bash .github/bump-callers/tests/test_preflight.sh       # 278 passed, 0 failed
shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh

New coverage: each fleet takes only its half of a shared roster; the sibling takes the other half and logs an identical digest; filtered-to-zero both ways; four malformed-filter shapes rejected; validation reaching filtered-out entries; and unset FILE_FILTER bumping the whole roster with no filter wording in the audit line.

Noted, not fixed here

AGENTS.md is 245 lines — 45 over the 200-line hard ceiling this repo enforces on every other repo, and there's no ci-agents-md-integrity.yml caller running the checker on itself. Pre-existing; this PR is net-zero on that file rather than adding to it. Filed separately.

Provenance

Built from an interactive session reviewing pr-risk v1 status. The operator asked why /derisk is a separate workflow given the bump burden; a full merge was proposed, then withdrawn after reading bump-callers.sh (see "Why not just merge" above), and this smaller variant approved in its place.

_{🔄 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/177 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/17/2026 **Status:** ✅ Merged **Merged:** 8/17/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-7756-share-one-roster-pr-risk-derisk` --- ### 📝 Commits (1) - [`e60695a`](https://github.com/Comfy-Org/github-workflows/commit/e60695a5e2e8e93567f9293d7c92b729e1a24763) ci(bump-callers): share one roster across the pr-risk and pr-derisk fleets (BE-7756) ### 📊 Changes **8 files changed** (+300 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+42 -1) 📝 `.github/bump-callers/bump-callers.sh` (+91 -6) 📝 `.github/bump-callers/tests/test_bump_callers.sh` (+119 -0) 📝 `.github/workflows/bump-pr-derisk-callers.yml` (+32 -9) 📝 `.github/workflows/bump-pr-risk-callers.yml` (+9 -0) 📝 `AGENTS.md` (+1 -1) 📝 `docs/callers/README.md` (+1 -1) 📝 `docs/callers/pr-derisk.md` (+5 -2) </details> ### 📄 Description ## ELI-5 The **roster** is a delivery list telling the bumper: *go to this repo, open this file, update the pin inside.* Right now `pr-risk` and `pr-derisk` each want their own list. Two lists means two places to remember a repo — and forgetting the second one is completely silent. The caller merges, the pin never moves, every run stays green, and you find out weeks later. That already happened. `ComfyUI_frontend` enrolled in pr-risk on 08-07, never made it into the roster, and sat 10 commits behind for ten days. Every roster line **already says which file it's about** — nobody was using that. So: keep one list, and let each fleet pick its own lines out of it. ```json [ {"repo": "Comfy-Org/example", "file": ".github/workflows/ci-pr-risk.yml"}, {"repo": "Comfy-Org/example", "file": ".github/workflows/ci-pr-derisk.yml"} ] ``` The risk fleet takes the first line. The derisk fleet takes the second. One address book, two mail carriers, each reading only their own routes. Enrolling a repo is one edit in one place. ## What changed - **`bump-callers.sh`** — new optional `FILE_FILTER`. When set, only roster entries whose `file` matches are bumped. **Unset is byte-identical to today**, which is every other fleet. - **`bump-pr-risk-callers.yml`** — filters to `ci-pr-risk.yml`. - **`bump-pr-derisk-callers.yml`** — filters to `ci-pr-derisk.yml` and reads `PR_RISK_CALLERS`. `PR_DERISK_CALLERS` is retired before it was ever seeded, so there is no migration. ## Why not just merge the two fleets into one Recorded so it isn't re-proposed: - `WORKFLOW_FILE` names the reusable whose `@sha` gets rewritten and is **fleet-level**. The two fleets pin different reusables, so a merged fleet needs it moved into the roster entry — a data-model change to the one script this repo is emphatic about not reshaping casually. - A fleet bumps **every** roster entry when it fires. Merged, a `scripts/pr-derisk/`-only change would open a no-op pin bump against every pr-risk caller — including repos running no derisk caller at all, every time the beta churns. ## Failure modes, deliberately loud - **Malformed `FILE_FILTER` → hard error.** A typo would otherwise select zero entries, which on `ALLOW_EMPTY=true` is a green no-op indistinguishable from "not enrolled yet" — frozen callers, forever, with nothing red. - **Selecting nothing defers to `ALLOW_EMPTY`**, exactly as an empty roster does: derisk (growing) no-ops green, pr-risk (must always have callers) hard-fails naming the filter and the entry count. - **Validation still covers filtered-out entries.** A malformed entry belonging to the sibling fleet fails the run that sees it, rather than surfacing on whichever fleet happens to run next. - **Both fleets log the same roster digest** — it's taken over the whole secret, not the filtered subset. That identity is the evidence they read one roster; a per-fleet digest would make a shared roster look like two unrelated ones. ## Residual, stated rather than implied `FILE_FILTER` matches by string equality, so every repo in a filtered fleet must spell its caller the same way. Fleets don't have that property in general — `CURSOR_REVIEW_CALLERS` carries three spellings today — so a repo naming its caller differently is skipped **silently**. That's by design: skipping entries is the filter's entire job, and a warning on every run is noise nobody reads. The audit line's `N caller(s), M after FILE_FILTER=…` is the signal; glance at `M` after any enrolment. Only put a fleet on `FILE_FILTER` where the caller filename convention is controlled — which pr-risk/pr-derisk are (their caller guides specify the names literally) and cursor-review is not. Documented in the script header and the bump-callers README. ## Testing ``` bash .github/bump-callers/tests/test_bump_callers.sh # 409 passed, 0 failed (36 new) bash .github/bump-callers/tests/test_paths_contract.sh # 56 passed, 0 failed bash .github/bump-callers/tests/test_preflight.sh # 278 passed, 0 failed shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh ``` New coverage: each fleet takes only its half of a shared roster; the sibling takes the other half and logs an identical digest; filtered-to-zero both ways; four malformed-filter shapes rejected; validation reaching filtered-out entries; and unset `FILE_FILTER` bumping the whole roster with no filter wording in the audit line. ## Noted, not fixed here `AGENTS.md` is **245 lines — 45 over the 200-line hard ceiling this repo enforces on every other repo**, and there's no `ci-agents-md-integrity.yml` caller running the checker on itself. Pre-existing; this PR is net-zero on that file rather than adding to it. Filed separately. ## Provenance Built from an interactive session reviewing pr-risk v1 status. The operator asked why `/derisk` is a separate workflow given the bump burden; a full merge was proposed, then withdrawn after reading `bump-callers.sh` (see "Why not just merge" above), and this smaller variant approved in its place. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:37:08 +00:00