[PR #107] [MERGED] fix(groom): pin ci-groom.yml by SHA and stop the bumper reporting unpinnable callers as converged (BE-6015) #118

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-6015-groom-caller-registration

📝 Commits (6)

80219aa fix(groom): pin ci-groom.yml by real SHA so the caller starts and the fleet can bump it (BE-6015)
5ad406b ci(bump-callers): warn instead of reporting "already at" for an unpinnable caller (BE-6015)
963557f ci(bump-callers): check pin integrity before the no-op test, and fail the run (BE-6015)
5d55309 Merge branch 'main' into matt/be-6015-groom-caller-registration
026e3a9 docs(bump-callers): correct un-bumpable claim now that BE-4662 self-heals pin shape (BE-6015)
e367d08 fix(bump-callers): stop staging a sibling-only roster entry's workflows_ref (BE-6015)

📊 Changes

4 files changed (+168 additions, -3 deletions)

View changed files

📝 .github/bump-callers/README.md (+25 -0)
📝 .github/bump-callers/bump-callers.sh (+74 -1)
📝 .github/bump-callers/tests/test_bump_callers.sh (+56 -0)
📝 AGENTS.md (+13 -2)

📄 Description

ELI-5

The groom workflow lives here, and other repos "call" it by pointing at a specific commit of this repo. A robot keeps those pointers up to date, but it only knows about repos listed in a config list. Four repos were calling groom without being on that list, so their pointers had gone stale — and this repo's own caller had a leftover REPLACE_ME placeholder instead of a real pointer, so it had been crashing on startup every single day while the robot cheerfully logged it as "already up to date." This fixes our own pointer, and makes the robot say "I can't move this one" instead of pretending it's fine.

What this does

Two changes, both from the same root cause — a caller that is registered with the fleet but that the fleet cannot actually move.

1. ci-groom.yml is pinned by a real SHA. BE-4004 (#56) landed the interval gate behind a deliberate fail-loud REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA placeholder in both uses: and workflows_ref:, to be hand-bumped to that PR's squash SHA after merge. The hand-bump never happened. Every scheduled ci-groom run since has failed at startup on an unresolvable ref — six consecutive schedule failures, each with zero jobs. Both refs now pin bcde90f4, the same SHA the other groom callers carry, which puts this caller back under bump-groom-callers.yml so future bumps move it automatically.

2. bump-callers.sh no longer reports an unpinnable caller as converged. The skip branch treats a no-op rewrite as "already at NEW_SHA", but a no-op rewrite only means that when the file carries a 40-hex pin the pass could have moved. A caller that names the reusable and pins it with a placeholder, tag or branch produces a byte-identical no-op — so the log said already at <short> — skipping for a file that had been crashing for weeks. It now emits a ::warning:: naming the file instead. Control flow is unchanged: the entry is still skipped, still a warning rather than a failure, so one unpinnable caller cannot abort the fan-out. It stays silent when the reusable-name probe (GW_USES, the same probe the rewrite address uses) cannot identify a uses: spelling, so a converged caller never warns spuriously.

Out-of-repo work this PR completes (no commit — it is an Actions variable)

The bulk of BE-6015 is a GROOM_CALLERS edit, already applied and verified: four live callers that were never registered are now on the roster, and the stale pre-rename repo name was corrected in the same edit. Dispatching bump-groom-callers.yml then opened bump PRs against exactly those four, each moving uses: and workflows_ref: in lock-step and touching nothing else. An independent sweep of all 196 Comfy-Org repos confirms nine live groom callers exist and all nine are now registered — no tenth was missed. Consumer names are deliberately not listed here; this repo is public and most callers are private.

Merging this PR is what gets the ninth caller onto the same SHA as the other eight.

Verification

shellcheck -x clean; bash .github/bump-callers/tests/test_bump_callers.sh → 128 passed, 0 failed; the cursor-review (40), agents-md-integrity (18) and groom (170) unittest suites all pass; check_agents_md.py --root . passes (its one warning, no CODEOWNERS, is pre-existing on main). The new bump-callers test is a genuine red→green regression test: reverting only bump-callers.sh drops it to 126 passed / 2 failed.

Judgment calls and things a reviewer should weigh

Scope. The ticket's plan is entirely operational (edit the variable, dispatch the bumper). I added this in-repo change because acceptance criterion 3 — "after merge all nine pin the same SHA" — is unreachable without it, and because the ticket's own measured state records this repo's pin as a "local path," which is not what is on main. Flagging rather than assuming: if you would rather split the bump-callers.sh warning into its own PR, it is the second commit and drops cleanly.
This makes a dormant scheduled job actually run, for the first time since it was written. Checked before pinning: all six inputs it passes are declared in groom.yml at bcde90f4; both secrets it forwards resolve (ANTHROPIC_API_KEY from the org, CLOUD_CODE_BOT_PRIVATE_KEY from the repo); builder is unset so it defaults to false — issues only, no auto-PRs; and interval_days falls back to 7, so weekly. Reasonable to workflow_dispatch it once with dry_run: true after merge before letting the schedule take it.
A tag-pinned caller in any fleet will now emit the new warning. That is intended — this repo mandates full-SHA pins and a bare @v1 fails the consumer-side pin validation — but it is a behavior change visible in every fleet's run summary, not just groom's.
Not done, and it needs a human: the ticket asks for the before/after roster JSON in a Linear comment. It cannot go here — the roster names four private repos and this repo is public. The JSON is captured on the runner at .supervisor-state/BE-6015-groom-callers/{before,after}.json.
Adjacent, deliberately not fixed: CURSOR_REVIEW_CALLERS and AGENTS_MD_CALLERS still carry the same stale pre-rename repo name. BE-5921 owns that class across every *_CALLERS variable, and the ticket says to fix it in one place, not two.
The public-log gap the bump-callers README already documents (a step's env: block prints before masking) now exposes four more names. Pre-existing and fleet-wide, but this edit widened 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/107 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/2/2026 **Status:** ✅ Merged **Merged:** 8/11/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-6015-groom-caller-registration` --- ### 📝 Commits (6) - [`80219aa`](https://github.com/Comfy-Org/github-workflows/commit/80219aa9fb4aa37af1a08ef211330a643e76a1db) fix(groom): pin ci-groom.yml by real SHA so the caller starts and the fleet can bump it (BE-6015) - [`5ad406b`](https://github.com/Comfy-Org/github-workflows/commit/5ad406b1ae25651b614a8e771e564caf8e651994) ci(bump-callers): warn instead of reporting "already at" for an unpinnable caller (BE-6015) - [`963557f`](https://github.com/Comfy-Org/github-workflows/commit/963557f750a9c15940aa5d3ce83b21794a156be5) ci(bump-callers): check pin integrity before the no-op test, and fail the run (BE-6015) - [`5d55309`](https://github.com/Comfy-Org/github-workflows/commit/5d55309b6ee22068b8c21c3885c3a822ffad09c6) Merge branch 'main' into matt/be-6015-groom-caller-registration - [`026e3a9`](https://github.com/Comfy-Org/github-workflows/commit/026e3a984b92722aba6aa545d6f065a6b7cd75f1) docs(bump-callers): correct un-bumpable claim now that BE-4662 self-heals pin shape (BE-6015) - [`e367d08`](https://github.com/Comfy-Org/github-workflows/commit/e367d08ae5bd0695f711aeb92a4dc90f915fd304) fix(bump-callers): stop staging a sibling-only roster entry's workflows_ref (BE-6015) ### 📊 Changes **4 files changed** (+168 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+25 -0) 📝 `.github/bump-callers/bump-callers.sh` (+74 -1) 📝 `.github/bump-callers/tests/test_bump_callers.sh` (+56 -0) 📝 `AGENTS.md` (+13 -2) </details> ### 📄 Description ## ELI-5 The groom workflow lives here, and other repos "call" it by pointing at a specific commit of this repo. A robot keeps those pointers up to date, but it only knows about repos listed in a config list. Four repos were calling groom without being on that list, so their pointers had gone stale — and this repo's own caller had a leftover `REPLACE_ME` placeholder instead of a real pointer, so it had been crashing on startup every single day while the robot cheerfully logged it as "already up to date." This fixes our own pointer, and makes the robot say "I can't move this one" instead of pretending it's fine. ## What this does Two changes, both from the same root cause — a caller that is registered with the fleet but that the fleet cannot actually move. **1. `ci-groom.yml` is pinned by a real SHA.** BE-4004 (#56) landed the interval gate behind a deliberate fail-loud `REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA` placeholder in both `uses:` and `workflows_ref:`, to be hand-bumped to that PR's squash SHA after merge. The hand-bump never happened. Every scheduled `ci-groom` run since has failed at startup on an unresolvable ref — six consecutive schedule failures, each with zero jobs. Both refs now pin `bcde90f4`, the same SHA the other groom callers carry, which puts this caller back under `bump-groom-callers.yml` so future bumps move it automatically. **2. `bump-callers.sh` no longer reports an unpinnable caller as converged.** The skip branch treats a no-op rewrite as "already at `NEW_SHA`", but a no-op rewrite only *means* that when the file carries a 40-hex pin the pass could have moved. A caller that names the reusable and pins it with a placeholder, tag or branch produces a byte-identical no-op — so the log said `already at <short> — skipping` for a file that had been crashing for weeks. It now emits a `::warning::` naming the file instead. Control flow is unchanged: the entry is still skipped, still a warning rather than a failure, so one unpinnable caller cannot abort the fan-out. It stays silent when the reusable-name probe (`GW_USES`, the same probe the rewrite address uses) cannot identify a `uses:` spelling, so a converged caller never warns spuriously. ## Out-of-repo work this PR completes (no commit — it is an Actions variable) The bulk of BE-6015 is a `GROOM_CALLERS` edit, already applied and verified: four live callers that were never registered are now on the roster, and the stale pre-rename repo name was corrected in the same edit. Dispatching `bump-groom-callers.yml` then opened bump PRs against exactly those four, each moving `uses:` and `workflows_ref:` in lock-step and touching nothing else. An independent sweep of all 196 `Comfy-Org` repos confirms nine live groom callers exist and all nine are now registered — no tenth was missed. Consumer names are deliberately not listed here; this repo is public and most callers are private. Merging this PR is what gets the ninth caller onto the same SHA as the other eight. ## Verification `shellcheck -x` clean; `bash .github/bump-callers/tests/test_bump_callers.sh` → **128 passed, 0 failed**; the cursor-review (40), agents-md-integrity (18) and groom (170) unittest suites all pass; `check_agents_md.py --root .` passes (its one warning, no CODEOWNERS, is pre-existing on `main`). The new bump-callers test is a genuine red→green regression test: reverting only `bump-callers.sh` drops it to 126 passed / 2 failed. ## Judgment calls and things a reviewer should weigh - **Scope.** The ticket's plan is entirely operational (edit the variable, dispatch the bumper). I added this in-repo change because acceptance criterion 3 — "after merge all nine pin the same SHA" — is unreachable without it, and because the ticket's own measured state records this repo's pin as a "local path," which is not what is on `main`. Flagging rather than assuming: if you would rather split the `bump-callers.sh` warning into its own PR, it is the second commit and drops cleanly. - **This makes a dormant scheduled job actually run**, for the first time since it was written. Checked before pinning: all six inputs it passes are declared in `groom.yml` at `bcde90f4`; both secrets it forwards resolve (`ANTHROPIC_API_KEY` from the org, `CLOUD_CODE_BOT_PRIVATE_KEY` from the repo); `builder` is unset so it defaults to `false` — issues only, no auto-PRs; and `interval_days` falls back to 7, so weekly. Reasonable to `workflow_dispatch` it once with `dry_run: true` after merge before letting the schedule take it. - **A tag-pinned caller in any fleet will now emit the new warning.** That is intended — this repo mandates full-SHA pins and a bare `@v1` fails the consumer-side pin validation — but it is a behavior change visible in every fleet's run summary, not just groom's. - **Not done, and it needs a human:** the ticket asks for the before/after roster JSON in a Linear comment. It cannot go here — the roster names four private repos and this repo is public. The JSON is captured on the runner at `.supervisor-state/BE-6015-groom-callers/{before,after}.json`. - **Adjacent, deliberately not fixed:** `CURSOR_REVIEW_CALLERS` and `AGENTS_MD_CALLERS` still carry the same stale pre-rename repo name. BE-5921 owns that class across every `*_CALLERS` variable, and the ticket says to fix it in one place, not two. - The public-log gap the bump-callers README already documents (a step's `env:` block prints before masking) now exposes four more names. Pre-existing and fleet-wide, but this edit widened 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:26 +00:00