[PR #119] [MERGED] ci(bump-callers): add the pr-risk caller-bump fleet (BE-6308) #130

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-6308-pr-risk-bump-fleet

📝 Commits (4)

eb8139e ci(bump-callers): add the pr-risk caller-bump fleet (BE-6308)
af20bc1 ci(bump-callers): address pr-risk fleet review findings (BE-6308)
9707274 ci(bump-callers): harden the pr-risk staleness guard (BE-6308)
26126a8 Merge remote-tracking branch 'origin/main' into matt/be-6308-pr-risk-bump-fleet

📊 Changes

4 files changed (+284 additions, -12 deletions)

View changed files

📝 .github/bump-callers/README.md (+5 -1)
➕ .github/workflows/bump-pr-risk-callers.yml (+264 -0)
📝 .github/workflows/test-bump-callers.yml (+3 -1)
📝 AGENTS.md (+12 -10)

📄 Description

ELI-5

Every repo that uses one of our shared workflows "pins" it to an exact commit, so the shared workflow can't change under it without review. That means somebody has to move the pin forward when the shared workflow changes — and we have a little robot per workflow that opens those bump PRs automatically. pr-risk was the one shared workflow that had real users but no robot, so its users' pins were frozen forever. This adds the robot.

What this does

Adds bump-pr-risk-callers.yml, a thin entrypoint over the shared .github/bump-callers/bump-callers.sh, and registers the new fleet in the three places a fleet has to be registered:

.github/workflows/bump-pr-risk-callers.yml — new (copied from bump-pr-size-callers.yml)
.github/workflows/test-bump-callers.yml — added to both the pull_request.paths and push.paths lists
.github/bump-callers/README.md — new row in "The fleets"
AGENTS.md — added to the thin-entrypoints list and the fleet-variables list

bump-callers.sh is unchanged. Its SHA rewrite address (/github-workflows|workflows_ref/) already moves both of a pr-risk caller's pins — the uses: line and the workflows_ref: input the reusable checks its grader/risk-map/label-script out at. That is the same double-pin shape a groom caller has, which the script documents explicitly.

Why it's urgent

Permission drift between a pinned caller and the reusable fails at caller startup. Open PR #112 widens the grade job's declared permissions, and pr-risk.yml has already moved twice since the pilot caller pinned it — so callers are drifting today with no mechanism to catch up.

Verification

shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh — clean
bash .github/bump-callers/tests/test_bump_callers.sh — 123 passed, 0 failed
actionlint on both changed workflow files — clean
python3 .github/agents-md-integrity/check_agents_md.py --root . — passed (144 lines; the one warning is the pre-existing missing-CODEOWNERS warning, unrelated to this change)
Diffed the new bumper against bump-pr-size-callers.yml: the only differences are the name, the concurrency group, the path filter, the decommission-guard paths, the four env values, and one added header paragraph (below).

Post-merge ops — needs repo-variable write access

1. Seed the roster. The spike's "seed it empty" step is now stale — there are TWO live callers on their default branches, not zero. Comfy-Org/comfy-cloud-mcp-server merged its pilot caller since the spike ran, and a second caller has landed in another registry repo. Seeding [] and then immediately adding both is pointless churn, so seed it with both entries directly. Shape matches the other fleets' variables:

gh variable set PR_RISK_CALLERS --repo Comfy-Org/github-workflows --body "$(jq -c . callers.json)"

where each entry is {"repo":"<owner/repo>","file":".github/workflows/ci-pr-risk.yml","label":""}. Keep the canonical callers.json in the private infra/ops repo per the README — the repo names are deliberately not written here, since this repo and its run logs are public.

2. workflow_dispatch the new fleet ONCE from main after seeding. Landing the fleet touches no path it watches, so its own merge fires no run and the already-stale callers stay stale until pr-risk.yml next changes. One dispatch converges them. (This ordering is the reverse of the spike's — seed first, then dispatch once, rather than dispatch-empty-then-seed-then-redispatch.)

Judgment calls

ALLOW_EMPTY: "true" kept as specified, even though the roster will be seeded non-empty. It matches the pr-size model the ticket named and means a not-yet-created variable is a clean no-op rather than a red run. Worth revisiting once the roster is stable: the cursor-review / auto-label fleets use hard-fail-if-empty precisely so an accidentally-cleared variable is loud instead of a silent fleet-wide no-op. Not changed here — that is a posture decision, not part of this ticket.
README "Seeded" cell reads empty [] allowed rather than the ticket's empty []. With two callers already live, writing "seeded empty" into a public doc would be false within a day of merge; the phrasing preserves the real semantic (this fleet tolerates an empty roster) without asserting a state that will not hold.
Three small doc-sync edits beyond the literal file list, all factual and non-behavioral: the double-pin note in AGENTS.md and the bump-callers README now say "groom or pr-risk" rather than groom alone, and test-bump-callers.yml's header comment lists pr-risk among the fleets its suite guards. Leaving these would have made three existing statements newly wrong.
The path filter scripts/pr-risk/** includes scripts/pr-risk/tests/, so a test-only change there fires a fan-out bump. That is exactly the pr-size precedent (scripts/check-pr-size/** contains main_test.go / size_test.go), and narrowing it risks missing a real behavior change — kept consistent with the model rather than special-cased.

Out of scope (unchanged, per the ticket)

PR #112 itself. If this fleet is live when #112 merges, #112's push to pr-risk.yml auto-fires the fan-out — that is the intended behavior.
detect-unreviewed-merge.yml's missing fleet. Noted while auditing: a DETECT_UNREVIEWED_MERGE_CALLERS variable now exists even though no bump-detect-unreviewed-merge-callers.yml entrypoint does, so the README's "no fleet — pins bumped by hand" row is still accurate but the half-built state is worth a follow-up ticket.
Any change to bump-callers.sh.

_{🔄 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/119 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/4/2026 **Status:** ✅ Merged **Merged:** 8/4/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-6308-pr-risk-bump-fleet` --- ### 📝 Commits (4) - [`eb8139e`](https://github.com/Comfy-Org/github-workflows/commit/eb8139e1b9e2fe7d4049159b8285a69033da2bfa) ci(bump-callers): add the pr-risk caller-bump fleet (BE-6308) - [`af20bc1`](https://github.com/Comfy-Org/github-workflows/commit/af20bc1793f6ff120f36a2925dac671323e0343a) ci(bump-callers): address pr-risk fleet review findings (BE-6308) - [`9707274`](https://github.com/Comfy-Org/github-workflows/commit/9707274305c2098508bab56e29bec13c2e307f9b) ci(bump-callers): harden the pr-risk staleness guard (BE-6308) - [`26126a8`](https://github.com/Comfy-Org/github-workflows/commit/26126a81c98ab763c723f2682e79ff0362258d30) Merge remote-tracking branch 'origin/main' into matt/be-6308-pr-risk-bump-fleet ### 📊 Changes **4 files changed** (+284 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+5 -1) ➕ `.github/workflows/bump-pr-risk-callers.yml` (+264 -0) 📝 `.github/workflows/test-bump-callers.yml` (+3 -1) 📝 `AGENTS.md` (+12 -10) </details> ### 📄 Description ## ELI-5 Every repo that uses one of our shared workflows "pins" it to an exact commit, so the shared workflow can't change under it without review. That means somebody has to move the pin forward when the shared workflow changes — and we have a little robot per workflow that opens those bump PRs automatically. `pr-risk` was the one shared workflow that had real users but **no robot**, so its users' pins were frozen forever. This adds the robot. ## What this does Adds `bump-pr-risk-callers.yml`, a thin entrypoint over the shared `.github/bump-callers/bump-callers.sh`, and registers the new fleet in the three places a fleet has to be registered: - `.github/workflows/bump-pr-risk-callers.yml` — new (copied from `bump-pr-size-callers.yml`) - `.github/workflows/test-bump-callers.yml` — added to both the `pull_request.paths` and `push.paths` lists - `.github/bump-callers/README.md` — new row in "The fleets" - `AGENTS.md` — added to the thin-entrypoints list and the fleet-variables list `bump-callers.sh` is **unchanged**. Its SHA rewrite address (`/github-workflows|workflows_ref/`) already moves both of a `pr-risk` caller's pins — the `uses:` line *and* the `workflows_ref:` input the reusable checks its grader/risk-map/label-script out at. That is the same double-pin shape a groom caller has, which the script documents explicitly. ## Why it's urgent Permission drift between a pinned caller and the reusable fails at *caller startup*. Open PR #112 widens the `grade` job's declared permissions, and `pr-risk.yml` has already moved twice since the pilot caller pinned it — so callers are drifting today with no mechanism to catch up. ## Verification - `shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh` — clean - `bash .github/bump-callers/tests/test_bump_callers.sh` — **123 passed, 0 failed** - `actionlint` on both changed workflow files — clean - `python3 .github/agents-md-integrity/check_agents_md.py --root .` — passed (144 lines; the one warning is the pre-existing missing-CODEOWNERS warning, unrelated to this change) - Diffed the new bumper against `bump-pr-size-callers.yml`: the only differences are the name, the concurrency group, the path filter, the decommission-guard paths, the four env values, and one added header paragraph (below). ## Post-merge ops — needs repo-variable write access **1. Seed the roster. The spike's "seed it empty" step is now stale — there are TWO live callers on their default branches, not zero.** `Comfy-Org/comfy-cloud-mcp-server` merged its pilot caller since the spike ran, and a second caller has landed in another registry repo. Seeding `[]` and then immediately adding both is pointless churn, so seed it with both entries directly. Shape matches the other fleets' variables: ```bash gh variable set PR_RISK_CALLERS --repo Comfy-Org/github-workflows --body "$(jq -c . callers.json)" ``` where each entry is `{"repo":"<owner/repo>","file":".github/workflows/ci-pr-risk.yml","label":""}`. Keep the canonical `callers.json` in the private infra/ops repo per the README — the repo names are deliberately not written here, since this repo and its run logs are public. **2. `workflow_dispatch` the new fleet ONCE from `main` after seeding.** Landing the fleet touches no path it watches, so its own merge fires no run and the already-stale callers stay stale until `pr-risk.yml` next changes. One dispatch converges them. (This ordering is the reverse of the spike's — seed first, then dispatch once, rather than dispatch-empty-then-seed-then-redispatch.) ## Judgment calls - **`ALLOW_EMPTY: "true"` kept as specified**, even though the roster will be seeded non-empty. It matches the pr-size model the ticket named and means a not-yet-created variable is a clean no-op rather than a red run. Worth revisiting once the roster is stable: the `cursor-review` / `auto-label` fleets use hard-fail-if-empty precisely so an accidentally-cleared variable is loud instead of a silent fleet-wide no-op. Not changed here — that is a posture decision, not part of this ticket. - **README "Seeded" cell reads `empty [] allowed` rather than the ticket's `empty []`.** With two callers already live, writing "seeded empty" into a public doc would be false within a day of merge; the phrasing preserves the real semantic (this fleet tolerates an empty roster) without asserting a state that will not hold. - **Three small doc-sync edits beyond the literal file list**, all factual and non-behavioral: the double-pin note in `AGENTS.md` and the `bump-callers` README now say "groom **or pr-risk**" rather than groom alone, and `test-bump-callers.yml`'s header comment lists `pr-risk` among the fleets its suite guards. Leaving these would have made three existing statements newly wrong. - **The path filter `scripts/pr-risk/**` includes `scripts/pr-risk/tests/`**, so a test-only change there fires a fan-out bump. That is exactly the `pr-size` precedent (`scripts/check-pr-size/**` contains `main_test.go` / `size_test.go`), and narrowing it risks missing a real behavior change — kept consistent with the model rather than special-cased. ## Out of scope (unchanged, per the ticket) - PR #112 itself. If this fleet is live when #112 merges, #112's push to `pr-risk.yml` auto-fires the fan-out — that is the intended behavior. - `detect-unreviewed-merge.yml`'s missing fleet. Noted while auditing: a `DETECT_UNREVIEWED_MERGE_CALLERS` variable now exists even though no `bump-detect-unreviewed-merge-callers.yml` entrypoint does, so the README's "no fleet — pins bumped by hand" row is still accurate but the half-built state is worth a follow-up ticket. - Any change to `bump-callers.sh`. --- <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:32 +00:00