[PR #151] [MERGED] ci(bump-callers): switch every bump-* entrypoint to the shared preflight.sh (BE-6476) #160

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-6476-preflight-swap

📝 Commits (6)

b09346a ci(bump-callers): switch every bump-* entrypoint to the shared preflight.sh (BE-6476)
0674540 Merge remote-tracking branch 'origin/main' into matt/be-6476-preflight-swap
9bf9b02 ci(bump-callers): timeout, pre-token preflight, and a paths/WATCHED contract test (BE-6476)
602f584 ci(bump-callers): multi-asset WATCHED_ASSETS; watch scripts/check-pr-size in the cursor-review fleet (BE-7045) (#153)
0de05ac Merge origin/main into matt/be-6476-preflight-swap
369ebe3 Merge origin/main into matt/be-6476-preflight-swap (post-#137)

📊 Changes

13 files changed (+1610 additions, -454 deletions)

View changed files

📝 .github/bump-callers/README.md (+132 -32)
📝 .github/bump-callers/preflight.sh (+230 -76)
➕ .github/bump-callers/tests/test_paths_contract.sh (+640 -0)
📝 .github/bump-callers/tests/test_preflight.sh (+314 -1)
📝 .github/workflows/bump-agents-md-callers.yml (+32 -35)
📝 .github/workflows/bump-assign-reviewers-callers.yml (+31 -35)
📝 .github/workflows/bump-auto-label-callers.yml (+29 -59)
📝 .github/workflows/bump-cursor-review-callers.yml (+73 -35)
📝 .github/workflows/bump-detect-unreviewed-merge-callers.yml (+26 -106)
📝 .github/workflows/bump-groom-callers.yml (+33 -36)
📝 .github/workflows/bump-pr-size-callers.yml (+59 -35)
📝 .github/workflows/test-bump-callers.yml (+6 -1)
📝 AGENTS.md (+5 -3)

📄 Description

ELI-5

Every bump-*-callers.yml workflow had its own hand-written copy of the same "should I actually bump the consumer repos right now?" check. Eight near-copies, and they had drifted apart — most of them threw away a legitimate bump whenever an unrelated commit happened to land on main in the seconds between the trigger and the check. #136 extracted one tested implementation (.github/bump-callers/preflight.sh). This PR deletes the inline copies and wires seven entrypoints to that one script, as a Preflight step whose output gates the bump step.

What changed

Each of the seven entrypoints now carries two steps where it used to have one:

Preflight (staleness / decommission guard), id: preflight, with NEW_SHA: ${{ github.sha }} + literal WATCHED (+ WATCHED_ASSETS for the four multi-path fleets), running bash .github/bump-callers/preflight.sh.
The existing Bump SHA in caller repos step, gated if: steps.preflight.outputs.proceed == 'true', with NEW_SHA: ${{ steps.preflight.outputs.new_sha }} (never github.sha — that would discard the preflight's stale-re-run re-point) and its run: reduced to bash .github/bump-callers/bump-callers.sh.

Per-fleet inputs, taken from each file's own paths: filter and its old decommission check: agents-md (agents-md-integrity.yml + .github/agents-md-integrity), assign-reviewers (single-path), auto-label (single-path), cursor-review (cursor-review.yml + .github/cursor-review), groom (groom.yml + .github/groom), pr-size (pr-size.yml + scripts/check-pr-size, the one asset dir outside .github/), detect-unreviewed-merge (single-path). Header comments, triggers, concurrency groups, the if: github.ref == 'refs/heads/main' job guard, token minting and the rest of each bump step's env: are untouched; auto-label's log-exposure note and detect-unreviewed-merge's unseeded-roster commentary ride along unchanged.

Deliberate behavior changes — this is a convergence, not a behavior-preserving refactor

The five simple-guard fleets (agents-md, assign-reviewers, cursor-review, groom, pr-size) stop discarding a genuine bump when an unrelated commit lands on main between the trigger and the check. They previously skipped on a bare tip mismatch; they now skip only when the watched surface actually changed since.
Those five also gain the exact-refname tip parse (a branch literally named foo/refs/heads/main can no longer poison the ls-remote parse) and the stale-re-run NEW_SHA re-point.
bump-auto-label-callers.yml gains the re-point (its own guard compared content but then proceeded pinned to the stale github.sha) and the $WATCHED-variable deletion guard (it re-tested a second literal copy of the path).
All seven pick up the shared script's ancestry direction guard and its fail-loudly-on-an-unperformable-lookup posture: a failed fetch, an unresolvable FETCH_HEAD, a HEAD that is not GITHUB_SHA, or a main that moved backwards is now a red run instead of a silent green no-op. That is the intended trade — none of those is evidence of staleness.

Blast radius on merge: none

No fleet's paths: filter watches its own bump-*-callers.yml entrypoint — each watches its REUSABLE workflow plus its asset dir. Re-verified against this branch's base (eb4b26c): the only workflow whose filter matches bump-*-callers.yml (or .github/bump-callers/**, for the README edit) is test-bump-callers.yml. So merging this fires zero bump fleets and opens no consumer-repo SHA-bump PRs; the only workflow it triggers is the test one, which must be green here.

Judgment call: `bump-pr-risk-callers.yml` is deliberately NOT swapped

The ticket's per-fleet table lists seven fleets and pr-risk is not among them, and the merged preflight.sh header already settled why: pr-risk's paths: filter carries :(exclude) entries (!scripts/pr-risk/tests/**, !scripts/pr-risk/README.md) that a single WATCHED_ASSETS string cannot express. Pointing WATCHED_ASSETS at the whole scripts/pr-risk directory would make it wider than the filter, so a test-only or docs-only commit — which starts no run of its own — would change the tree OID, be read as "the watched surface changed since", and freeze the fleet waiting on a run that never comes. The script's own guidance for an excluding fleet is "narrow the inputs, or leave that fleet on its own guard"; there is no narrowing that expresses an exclusion, so it keeps its guard. Consequence for the acceptance criteria: grep -l 'ls_remote' .github/workflows/bump-*-callers.yml returns bump-pr-risk-callers.yml rather than nothing, and it retains NEW_SHA: ${{ github.sha }}. Every other criterion is met for the seven in scope. I also documented the exception in .github/bump-callers/README.md so the next reader does not "finish the job" by accident.

Recommended follow-up (not done here — out of scope)

Five of the seven jobs (agents-md, assign-reviewers, cursor-review, groom, pr-size) have no timeout-minutes; auto-label and detect-unreviewed-merge have 20. preflight.sh's header names the caller job's timeout-minutes as the backstop for an origin that hangs during its --unshallow fetch, so those five now lean on a backstop they do not declare — with cancel-in-progress: false a hung fetch could hold the fleet for the 6h default. The pre-existing inline guards fetched unbounded too, so this is not a regression, but it is worth a one-line follow-up. I left it alone because the ticket says to keep each file's surrounding structure untouched.

Verification

bash .github/bump-callers/tests/test_bump_callers.sh — 175 passed, 0 failed
bash .github/bump-callers/tests/test_preflight.sh — 107 passed, 0 failed
shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/preflight.sh .github/bump-callers/tests/test_bump_callers.sh .github/bump-callers/tests/test_preflight.sh — clean
actionlint — clean on all eight edited/adjacent bump-* workflows (the only findings repo-wide are pre-existing github.job_workflow_sha / job.workflow_sha expression warnings in groom.yml, untouched here)
YAML parse of every file in .github/workflows/ — OK
Structural check of all seven: no ref: override on any checkout (the preflight hard-errors if HEAD != GITHUB_SHA, so this was verified rather than assumed), id: preflight present, bump step gated on proceed, NEW_SHA from new_sha.

Self-review note on the negative-claim rule: this diff adds a skip path (proceed == 'false') and new hard-error paths, but it denies no capability — for the five simple fleets the skip conditions are strictly narrower than what they replace, and the hard-error paths live in the already-merged, already-tested preflight.sh rather than being authored here.

_{🔄 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/151 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/11/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-6476-preflight-swap` --- ### 📝 Commits (6) - [`b09346a`](https://github.com/Comfy-Org/github-workflows/commit/b09346a22334e0cd31a85ef30ccc7f7d21c6ff39) ci(bump-callers): switch every bump-* entrypoint to the shared preflight.sh (BE-6476) - [`0674540`](https://github.com/Comfy-Org/github-workflows/commit/067454030f7c6ae8a4d2dabd83b9e00854bc153f) Merge remote-tracking branch 'origin/main' into matt/be-6476-preflight-swap - [`9bf9b02`](https://github.com/Comfy-Org/github-workflows/commit/9bf9b026495bee8cd14798547bf73720f5d8b3e4) ci(bump-callers): timeout, pre-token preflight, and a paths/WATCHED contract test (BE-6476) - [`602f584`](https://github.com/Comfy-Org/github-workflows/commit/602f584d50113fae3a2e2c38be15f46d80df5b2c) ci(bump-callers): multi-asset WATCHED_ASSETS; watch scripts/check-pr-size in the cursor-review fleet (BE-7045) (#153) - [`0de05ac`](https://github.com/Comfy-Org/github-workflows/commit/0de05aca6962242a46d845626bd80c596d7ffb4b) Merge origin/main into matt/be-6476-preflight-swap - [`369ebe3`](https://github.com/Comfy-Org/github-workflows/commit/369ebe355a97d0d07b98db326d795091a6a64b24) Merge origin/main into matt/be-6476-preflight-swap (post-#137) ### 📊 Changes **13 files changed** (+1610 additions, -454 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+132 -32) 📝 `.github/bump-callers/preflight.sh` (+230 -76) ➕ `.github/bump-callers/tests/test_paths_contract.sh` (+640 -0) 📝 `.github/bump-callers/tests/test_preflight.sh` (+314 -1) 📝 `.github/workflows/bump-agents-md-callers.yml` (+32 -35) 📝 `.github/workflows/bump-assign-reviewers-callers.yml` (+31 -35) 📝 `.github/workflows/bump-auto-label-callers.yml` (+29 -59) 📝 `.github/workflows/bump-cursor-review-callers.yml` (+73 -35) 📝 `.github/workflows/bump-detect-unreviewed-merge-callers.yml` (+26 -106) 📝 `.github/workflows/bump-groom-callers.yml` (+33 -36) 📝 `.github/workflows/bump-pr-size-callers.yml` (+59 -35) 📝 `.github/workflows/test-bump-callers.yml` (+6 -1) 📝 `AGENTS.md` (+5 -3) </details> ### 📄 Description ## ELI-5 Every `bump-*-callers.yml` workflow had its own hand-written copy of the same "should I actually bump the consumer repos right now?" check. Eight near-copies, and they had drifted apart — most of them threw away a legitimate bump whenever an unrelated commit happened to land on `main` in the seconds between the trigger and the check. #136 extracted one tested implementation (`.github/bump-callers/preflight.sh`). This PR deletes the inline copies and wires seven entrypoints to that one script, as a `Preflight` step whose output gates the bump step. ## What changed Each of the seven entrypoints now carries two steps where it used to have one: 1. `Preflight (staleness / decommission guard)`, `id: preflight`, with `NEW_SHA: ${{ github.sha }}` + literal `WATCHED` (+ `WATCHED_ASSETS` for the four multi-path fleets), running `bash .github/bump-callers/preflight.sh`. 2. The existing `Bump SHA in caller repos` step, gated `if: steps.preflight.outputs.proceed == 'true'`, with `NEW_SHA: ${{ steps.preflight.outputs.new_sha }}` (never `github.sha` — that would discard the preflight's stale-re-run re-point) and its `run:` reduced to `bash .github/bump-callers/bump-callers.sh`. Per-fleet inputs, taken from each file's own `paths:` filter and its old decommission check: agents-md (`agents-md-integrity.yml` + `.github/agents-md-integrity`), assign-reviewers (single-path), auto-label (single-path), cursor-review (`cursor-review.yml` + `.github/cursor-review`), groom (`groom.yml` + `.github/groom`), pr-size (`pr-size.yml` + `scripts/check-pr-size`, the one asset dir outside `.github/`), detect-unreviewed-merge (single-path). Header comments, triggers, concurrency groups, the `if: github.ref == 'refs/heads/main'` job guard, token minting and the rest of each bump step's `env:` are untouched; auto-label's log-exposure note and detect-unreviewed-merge's unseeded-roster commentary ride along unchanged. ## Deliberate behavior changes — this is a convergence, not a behavior-preserving refactor - The five simple-guard fleets (agents-md, assign-reviewers, cursor-review, groom, pr-size) **stop discarding a genuine bump** when an unrelated commit lands on `main` between the trigger and the check. They previously skipped on a bare tip mismatch; they now skip only when the watched surface actually changed since. - Those five also gain the **exact-refname tip parse** (a branch literally named `foo/refs/heads/main` can no longer poison the `ls-remote` parse) and the **stale-re-run `NEW_SHA` re-point**. - `bump-auto-label-callers.yml` gains the **re-point** (its own guard compared content but then proceeded pinned to the stale `github.sha`) and the **`$WATCHED`-variable deletion guard** (it re-tested a second literal copy of the path). - All seven pick up the shared script's **ancestry direction guard** and its **fail-loudly-on-an-unperformable-lookup** posture: a failed fetch, an unresolvable `FETCH_HEAD`, a `HEAD` that is not `GITHUB_SHA`, or a `main` that moved backwards is now a red run instead of a silent green no-op. That is the intended trade — none of those is evidence of staleness. ## Blast radius on merge: none No fleet's `paths:` filter watches its own `bump-*-callers.yml` entrypoint — each watches its REUSABLE workflow plus its asset dir. Re-verified against this branch's base (`eb4b26c`): the only workflow whose filter matches `bump-*-callers.yml` (or `.github/bump-callers/**`, for the README edit) is `test-bump-callers.yml`. So merging this fires **zero** bump fleets and opens no consumer-repo SHA-bump PRs; the only workflow it triggers is the test one, which must be green here. ## Judgment call: `bump-pr-risk-callers.yml` is deliberately NOT swapped The ticket's per-fleet table lists seven fleets and `pr-risk` is not among them, and the merged `preflight.sh` header already settled why: pr-risk's `paths:` filter carries `:(exclude)` entries (`!scripts/pr-risk/tests/**`, `!scripts/pr-risk/README.md`) that a single `WATCHED_ASSETS` string cannot express. Pointing `WATCHED_ASSETS` at the whole `scripts/pr-risk` directory would make it **wider** than the filter, so a test-only or docs-only commit — which starts no run of its own — would change the tree OID, be read as "the watched surface changed since", and freeze the fleet waiting on a run that never comes. The script's own guidance for an excluding fleet is "narrow the inputs, or leave that fleet on its own guard"; there is no narrowing that expresses an exclusion, so it keeps its guard. Consequence for the acceptance criteria: `grep -l 'ls_remote' .github/workflows/bump-*-callers.yml` returns `bump-pr-risk-callers.yml` rather than nothing, and it retains `NEW_SHA: ${{ github.sha }}`. Every other criterion is met for the seven in scope. I also documented the exception in `.github/bump-callers/README.md` so the next reader does not "finish the job" by accident. ## Recommended follow-up (not done here — out of scope) Five of the seven jobs (agents-md, assign-reviewers, cursor-review, groom, pr-size) have no `timeout-minutes`; auto-label and detect-unreviewed-merge have `20`. `preflight.sh`'s header names the caller job's `timeout-minutes` as the backstop for an origin that hangs during its `--unshallow` fetch, so those five now lean on a backstop they do not declare — with `cancel-in-progress: false` a hung fetch could hold the fleet for the 6h default. The pre-existing inline guards fetched unbounded too, so this is not a regression, but it is worth a one-line follow-up. I left it alone because the ticket says to keep each file's surrounding structure untouched. ## Verification - `bash .github/bump-callers/tests/test_bump_callers.sh` — **175 passed, 0 failed** - `bash .github/bump-callers/tests/test_preflight.sh` — **107 passed, 0 failed** - `shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/preflight.sh .github/bump-callers/tests/test_bump_callers.sh .github/bump-callers/tests/test_preflight.sh` — clean - `actionlint` — clean on all eight edited/adjacent `bump-*` workflows (the only findings repo-wide are pre-existing `github.job_workflow_sha` / `job.workflow_sha` expression warnings in `groom.yml`, untouched here) - YAML parse of every file in `.github/workflows/` — OK - Structural check of all seven: no `ref:` override on any checkout (the preflight hard-errors if `HEAD != GITHUB_SHA`, so this was verified rather than assumed), `id: preflight` present, bump step gated on `proceed`, `NEW_SHA` from `new_sha`. Self-review note on the negative-claim rule: this diff adds a skip path (`proceed == 'false'`) and new hard-error paths, but it denies no capability — for the five simple fleets the skip conditions are strictly **narrower** than what they replace, and the hard-error paths live in the already-merged, already-tested `preflight.sh` rather than being authored here. --- <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:53 +00:00