[PR #82] [MERGED] ci(bump-callers): harden the main-tip lookup in the 5 sibling entrypoints (BE-4683) #91

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/82
Author: @mattmillerai
Created: 7/27/2026
Status: ✅ Merged
Merged: 7/28/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-4683-harden-main-tip-lookup

📝 Commits (1)

feda217 ci(bump-callers): harden the main-tip lookup in the 5 sibling entrypoints (BE-4683)

📊 Changes

5 files changed (+70 additions, -5 deletions)

View changed files

📝 .github/workflows/bump-agents-md-callers.yml (+14 -1)
📝 .github/workflows/bump-assign-reviewers-callers.yml (+14 -1)
📝 .github/workflows/bump-cursor-review-callers.yml (+14 -1)
📝 .github/workflows/bump-groom-callers.yml (+14 -1)
📝 .github/workflows/bump-pr-size-callers.yml (+14 -1)

📄 Description

ELI-5

Five of our "bump the pinned SHA in every caller repo" workflows looked up the current tip of main like this: main_tip=$(git ls-remote origin refs/heads/main | cut -f1). A shell pipeline reports the exit status of its last command, so bash -e only ever saw cut succeed — a failed ls-remote (network blip, remote hiccup, rate limit) quietly left main_tip empty. Empty then compared unequal to github.sha, so the guard printed "stale run/re-run; nothing to bump" and exit 0. The workflow reported success while bumping nothing, and a green run that bumped nothing looks exactly like a green run that bumped everything unless a human reads the log. That is precisely the silent-drift failure this machinery exists to prevent (AGENTS.md records a pin that never moved causing a real ci-groom.yml startup failure).

This runs ls-remote on its own so its exit status is actually checked, rejects empty output, and only then compares against github.sha. A lookup we couldn't perform is not evidence of staleness — so it now fails loudly instead of pretending the run was stale.

What changed

The fix already landed in bump-auto-label-callers.yml (commit 7b360ed, branch ci/auto-label-bump-fleet / #81) is ported verbatim to the five sibling entrypoints that #81 left untouched:

.github/workflows/bump-cursor-review-callers.yml
.github/workflows/bump-groom-callers.yml
.github/workflows/bump-assign-reviewers-callers.yml
.github/workflows/bump-pr-size-callers.yml
.github/workflows/bump-agents-md-callers.yml

Each file's surrounding explanatory comment is preserved and the stale-run branch itself is byte-for-byte unchanged — the only behavior change is that a failed or empty lookup now exits 1 with a ::error:: annotation instead of falling through to the stale-run exit 0.

Verification

bash .github/bump-callers/tests/test_bump_callers.sh → 123 passed, 0 failed.
shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh → clean.
These tests cover the shared script, not the entrypoints' inline run: blocks, so additionally: all five files parse as YAML, and each edited run: block passes bash -n and shellcheck -s bash after extraction.
All three branches of the new guard exercised locally: success parses a 40-hex SHA (daf635a…); a bogus remote exits 1 with ::error::Could not look up the current main tip; a non-existent ref (empty output) exits 1 with ::error::git ls-remote returned no SHA. Confirmed under both set -euo pipefail and plain set -e (GitHub's default run shell), since if ! var=$(cmd) suspends -e in the condition and must not swallow the failure.

Risk / falsification

The change converts a silent skip into a hard failure, so the one real risk is git ls-remote origin not working in the runner at all — it runs after actions/checkout with persist-credentials: false. Falsified empirically against the real runner, not assumed: the log of run 30222872404 (bump-groom-callers, 2026-07-26, success) shows the ls-remote line executing, no stale-tip message, and the job reaching groom bump complete for all callers. The repo is public, so anonymous HTTPS ls-remote succeeds; reproduced locally against https://github.com/Comfy-Org/github-workflows.git.

Judgment calls / notes

The riskiest line is main_tip=${ls_remote%%$'\t'*}: $'\t' is a bash-ism, and under a non-bash shell it would strip nothing and reintroduce the exact silent no-op. That is safe here because the surrounding guard already uses [[ ]], so these blocks are bash-only by construction — and it is verified by the extraction checks above rather than by inspection.
Completeness: grep -rn ls-remote over the repo returns only these five files, and there are no other unguarded x=$(… | cut) assignments under .github/workflows/. bump-auto-label-callers.yml is not on main yet — it lands with #81 already carrying this fix.
Based on main, not stacked on #81 — these five files are pre-existing and #81 does not touch them, so there is no dependency in either direction.
No new tests: the defect lives in inline workflow run: blocks, which the repo has no harness for; the extraction + branch-exercise checks above are the substitute. Adding such a harness would be a bigger, separate change than this mechanical port.

Closes BE-4683.

_{🔄 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/82 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/27/2026 **Status:** ✅ Merged **Merged:** 7/28/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-4683-harden-main-tip-lookup` --- ### 📝 Commits (1) - [`feda217`](https://github.com/Comfy-Org/github-workflows/commit/feda217b82e77fa8c22ea6eba312da7262254c36) ci(bump-callers): harden the main-tip lookup in the 5 sibling entrypoints (BE-4683) ### 📊 Changes **5 files changed** (+70 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/bump-agents-md-callers.yml` (+14 -1) 📝 `.github/workflows/bump-assign-reviewers-callers.yml` (+14 -1) 📝 `.github/workflows/bump-cursor-review-callers.yml` (+14 -1) 📝 `.github/workflows/bump-groom-callers.yml` (+14 -1) 📝 `.github/workflows/bump-pr-size-callers.yml` (+14 -1) </details> ### 📄 Description ## ELI-5 Five of our "bump the pinned SHA in every caller repo" workflows looked up the current tip of `main` like this: `main_tip=$(git ls-remote origin refs/heads/main | cut -f1)`. A shell pipeline reports the exit status of its *last* command, so `bash -e` only ever saw `cut` succeed — a failed `ls-remote` (network blip, remote hiccup, rate limit) quietly left `main_tip` empty. Empty then compared unequal to `github.sha`, so the guard printed "stale run/re-run; nothing to bump" and `exit 0`. The workflow reported **success while bumping nothing**, and a green run that bumped nothing looks exactly like a green run that bumped everything unless a human reads the log. That is precisely the silent-drift failure this machinery exists to prevent (AGENTS.md records a pin that never moved causing a real `ci-groom.yml` startup failure). This runs `ls-remote` on its own so its exit status is actually checked, rejects empty output, and only then compares against `github.sha`. A lookup we couldn't perform is not evidence of staleness — so it now fails loudly instead of pretending the run was stale. ## What changed The fix already landed in `bump-auto-label-callers.yml` (commit `7b360ed`, branch `ci/auto-label-bump-fleet` / #81) is ported verbatim to the five sibling entrypoints that #81 left untouched: - `.github/workflows/bump-cursor-review-callers.yml` - `.github/workflows/bump-groom-callers.yml` - `.github/workflows/bump-assign-reviewers-callers.yml` - `.github/workflows/bump-pr-size-callers.yml` - `.github/workflows/bump-agents-md-callers.yml` Each file's surrounding explanatory comment is preserved and the stale-run branch itself is byte-for-byte unchanged — the only behavior change is that a *failed or empty* lookup now exits 1 with a `::error::` annotation instead of falling through to the stale-run `exit 0`. ## Verification - `bash .github/bump-callers/tests/test_bump_callers.sh` → **123 passed, 0 failed**. - `shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh` → clean. - These tests cover the shared script, not the entrypoints' inline `run:` blocks, so additionally: all five files parse as YAML, and each edited `run:` block passes `bash -n` and `shellcheck -s bash` after extraction. - All three branches of the new guard exercised locally: success parses a 40-hex SHA (`daf635a…`); a bogus remote exits **1** with `::error::Could not look up the current main tip`; a non-existent ref (empty output) exits **1** with `::error::git ls-remote returned no SHA`. Confirmed under both `set -euo pipefail` and plain `set -e` (GitHub's default `run` shell), since `if ! var=$(cmd)` suspends `-e` in the condition and must not swallow the failure. ## Risk / falsification The change converts a silent skip into a hard failure, so the one real risk is `git ls-remote origin` not working in the runner at all — it runs after `actions/checkout` with `persist-credentials: false`. Falsified empirically against the real runner, not assumed: the log of run [30222872404](https://github.com/Comfy-Org/github-workflows/actions/runs/30222872404) (bump-groom-callers, 2026-07-26, `success`) shows the `ls-remote` line executing, **no** stale-tip message, and the job reaching `groom bump complete for all callers`. The repo is public, so anonymous HTTPS `ls-remote` succeeds; reproduced locally against `https://github.com/Comfy-Org/github-workflows.git`. ## Judgment calls / notes - The riskiest line is `main_tip=${ls_remote%%$'\t'*}`: `$'\t'` is a bash-ism, and under a non-bash shell it would strip nothing and reintroduce the exact silent no-op. That is safe here because the surrounding guard already uses `[[ ]]`, so these blocks are bash-only by construction — and it is verified by the extraction checks above rather than by inspection. - Completeness: `grep -rn ls-remote` over the repo returns only these five files, and there are no other unguarded `x=$(… | cut)` assignments under `.github/workflows/`. `bump-auto-label-callers.yml` is not on `main` yet — it lands with #81 already carrying this fix. - Based on `main`, **not** stacked on #81 — these five files are pre-existing and #81 does not touch them, so there is no dependency in either direction. - No new tests: the defect lives in inline workflow `run:` blocks, which the repo has no harness for; the extraction + branch-exercise checks above are the substitute. Adding such a harness would be a bigger, separate change than this mechanical port. Closes BE-4683. --- <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:08 +00:00