fix(bump-callers): refresh the already-converted main (<short>) pin marker (BE-4523)

.github/bump-callers/bump-callers.sh

@@ -190,15 +190,82 @@ bump_repo() {
 
     OLD_CONTENT=$(jq -r '.content' <<<"$CURRENT" | base64 -d)
 
-    # Rewrite the github-workflows pin(s) to NEW_SHA and normalize the stale
-    # `# github-workflows#NN` pin comment. Anchor the 40-hex substitution to the
-    # two known pin contexts — the `uses: …Comfy-Org/github-workflows…@<sha>`
-    # line and agents-md-integrity's bare `workflows_ref: <sha>` line — so a
-    # full-SHA pin of ANOTHER action in the same file (`actions/checkout@<sha>`,
-    # the org's mandated practice) is never clobbered to github-workflows' SHA.
-    # The comment rewrite is a no-op for callers that use a different comment
-    # form (e.g. agents-md-integrity's `# v1`), so it is safe to share.
-    NEW_CONTENT=$(sed -E "/github-workflows|workflows_ref/ s/[0-9a-f]{40}/${NEW_SHA}/g; s|# github-workflows#[0-9]+|# github-workflows main (${SHORT})|g" <<<"$OLD_CONTENT")
+    # Which github-workflows reusables does this file actually CALL? Taken from
+    # `uses:` lines only (`^[^#]*uses:` keeps a commented-out `# uses:` out), so a
+    # prose comment or a docs URL that merely NAMES a sibling workflow cannot make
+    # a single-reusable caller look multi-reusable — that would wrongly trip the
+    # attribution guard below and leave the caller's marker stale, i.e. re-create
+    # the very BE-4523 bug. Empty means we could not identify one (an unusual
+    # spelling): treated as "not provably ours", which only ever costs the
+    # marker refresh, never a pin bump.
+    local GW_USES
+    GW_USES=$(grep -oE '^[^#]*uses:[[:space:]]*[^[:space:]]*github-workflows/\.github/workflows/[A-Za-z0-9._-]+\.ya?ml' <<<"$OLD_CONTENT" \
+      | sed -E 's|.*/||' | sort -u) || true
+    # Provably ours alone → safe to rewrite unattributed marker comments.
+    # Provably NOT ours alone → this file also calls a SIBLING fleet's reusable.
+    local GW_ONLY_OURS=0 GW_HAS_SIBLING=0
+    if [[ -n "$GW_USES" ]]; then
+      if [[ "$GW_USES" == "$WORKFLOW_FILE" ]]; then GW_ONLY_OURS=1; else GW_HAS_SIBLING=1; fi
+    fi
+
+    # Rewrite the github-workflows pin(s) to NEW_SHA. Anchor the 40-hex
+    # substitution to the two known pin contexts — the
+    # `uses: …Comfy-Org/github-workflows…@<sha>` line and agents-md-integrity's
+    # bare `workflows_ref: <sha>` line — so a full-SHA pin of ANOTHER action in
+    # the same file (`actions/checkout@<sha>`, the org's mandated practice) is
+    # never clobbered to github-workflows' SHA.
+    #
+    # In a file that also calls a SIBLING github-workflows reusable, tighten the
+    # first context to OUR reusable's path: the broad `/github-workflows/` address
+    # matched the sibling's `uses: …/other.yml@<40-hex>` line too and repinned it
+    # to this fleet's SHA — a cross-fleet stamp that silently pointed the sibling
+    # caller at a commit its own fleet never shipped. The tightening is inert for
+    # every caller today (all 23 call exactly one github-workflows reusable, so
+    # the address is byte-for-byte the broad one); it exists so a caller that
+    # starts calling two cannot be corrupted.
+    #
+    # The bare `workflows_ref:` context stays broad in both cases: it is a `with:`
+    # input carrying no workflow name, so telling OUR job's from a sibling job's
+    # needs YAML block structure that a line-wise sed does not have — and leaving
+    # ours un-bumped (a `workflows_ref` disagreeing with its own `uses:` pin) is
+    # the worse failure. Unreachable while no caller calls two reusables; if one
+    # ever does, parse the YAML instead of extending this address.
+    local SHA_ADDR='/github-workflows|workflows_ref/'
+    if (( GW_HAS_SIBLING )); then
+      SHA_ADDR="/github-workflows[^[:space:]]*${WORKFLOW_FILE//./\\.}|workflows_ref/"
+    fi
+    NEW_CONTENT=$(sed -E "${SHA_ADDR} s/[0-9a-f]{40}/${NEW_SHA}/g" <<<"$OLD_CONTENT")
+
+    # Normalize the human-readable pin annotation so it never disagrees with the
+    # pin above. Two spellings, both handled here:
+    #   * the LEGACY `# github-workflows#NN` form (converted on first sight), and
+    #   * the ALREADY-CONVERTED `github-workflows main (<short>)` form (BE-4523).
+    # Only the legacy rule existed before, and it fires once — so after a caller
+    # was migrated, every later bump advanced the real 40-hex pin and left the
+    # annotation frozen at whatever it was on conversion day. Comfy-iOS shipped a
+    # bump whose `uses:`/`workflows_ref` were current while both of its marker
+    # comments still named a SHA the file no longer used, and it had to be
+    # hand-fixed. The marker pattern is deliberately NOT anchored to `# ` so it
+    # also catches the prose form callers put ABOVE the `uses:` line ("# Pinned
+    # to github-workflows main (dc65b8b). …"), which is the same annotation and
+    # goes stale the same way; `{7,40}` covers both the short and full-SHA
+    # spellings. Re-running against an already-current marker rewrites it to
+    # itself, so this stays a no-op for a converged caller (the already-pinned
+    # clean-skip path still skips). A caller using some other comment form
+    # (agents-md-integrity's `# v1`) matches neither rule and is untouched.
+    #
+    # Attribution guard: an annotation names a SHA but not WHICH reusable it
+    # annotates, so in a file that calls several github-workflows reusables there
+    # is no honest way to tell whose is whose — rewriting them all would stamp
+    # this fleet's SHA onto a sibling fleet's annotation. So rewrite only when the
+    # file is provably ours alone; otherwise leave every annotation as found and
+    # say so. (No caller does this today; the guard is why the new rule cannot
+    # regress one that starts to.)
+    if (( GW_ONLY_OURS )); then
+      NEW_CONTENT=$(sed -E "s|# github-workflows#[0-9]+|# github-workflows main (${SHORT})|g; s|github-workflows main \([0-9a-f]{7,40}\)|github-workflows main (${SHORT})|g" <<<"$NEW_CONTENT")
+    elif grep -qE '# github-workflows#[0-9]+|github-workflows main \([0-9a-f]{7,40}\)' <<<"$OLD_CONTENT"; then
+      echo "::warning::${REPO}: ${FILE} does not call ${WORKFLOW_FILE} alone — leaving its github-workflows pin comments untouched"
+    fi
 
     # Wire an extra identity/config into this file when its entry is flagged
     # (BE-1814's cloud-code-bot review identity is the first user). Idempotent —

.github/bump-callers/tests/test_bump_callers.sh

@@ -492,6 +492,101 @@ check "reported already at SHORT"              "grep -q 'already at $SHORT' <<<\
 check "committed nothing"                       "[[ ! -f \"\$STUB_PUT_DIR/count\" ]]"
 check "opened no PR"                            "[[ ! -f \"\$STUB_PUT_DIR/pr.log\" ]] || ! grep -q '^pr-create' \"\$STUB_PUT_DIR/pr.log\""
 
+echo "== an ALREADY-CONVERTED 'main (<short>)' marker is refreshed, not frozen (BE-4523) =="
+# The legacy `# github-workflows#NN` rule only fires once. After a caller has
+# been migrated to the `main (<short>)` marker, every later bump used to advance
+# the real 40-hex pin and leave the human-readable annotation at the SHA the file
+# no longer uses (Comfy-iOS shipped exactly that and hand-fixed it). Both marker
+# spellings must track the pin: the prose comment ABOVE the `uses:` line and the
+# trailing comment ON it.
+new_case converted
+CONVERTED_FIXTURE="${WORK}/converted_caller.yml"
+printf '%s\n' \
+  'name: CI cursor-review' \
+  'jobs:' \
+  '  review:' \
+  '    # Pinned to github-workflows main (1111111). The bump-cursor-review-callers' \
+  '    # workflow auto-opens a SHA-bump PR here when cursor-review.yml changes' \
+  '    # upstream; keep workflows_ref matching.' \
+  '    uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows main (1111111)' \
+  '    with:' \
+  '      workflows_ref: 1111111111111111111111111111111111111111' \
+  > "$CONVERTED_FIXTURE"
+STUB_CONTENT_FILE="$CONVERTED_FIXTURE" run_bump \
+  VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
+  CALLERS_JSON='[{"repo":"Comfy-Org/secret-converted","file":".github/workflows/ci-cursor-review.yml","label":""}]'
+PUT="${STUB_PUT_DIR}/put.last.txt"
+check "exit 0" "[[ $RC -eq 0 ]]"
+check "staged the file (not a skip)"            "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]"
+check "both SHA refs bumped"                    "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]"
+check "old 40-hex pin removed"                  "! grep -qF '1111111111111111111111111111111111111111' \"$PUT\""
+check "no stale short SHA left in any marker"   "! grep -qF 'main (1111111)' \"$PUT\""
+check "BOTH markers refreshed to the new short" "[[ \$(grep -cF 'github-workflows main ($SHORT)' \"$PUT\") -eq 2 ]]"
+check "prose marker above uses: refreshed"      "grep -qF '# Pinned to github-workflows main ($SHORT).' \"$PUT\""
+check "no attribution warning for a single-reusable caller" \
+  "! grep -q 'pin comments untouched' <<<\"\$OUT\""
+
+echo "== a SECOND reusable in the same file keeps BOTH its pin and its marker (BE-4523) =="
+# A `main (<short>)` marker names a SHA but not WHICH reusable it annotates, so a
+# file calling several github-workflows reusables gives no honest way to tell
+# whose marker is whose. The bumper must refuse to guess: leave every marker as
+# found and warn, rather than stamping this fleet's SHA onto a sibling fleet's
+# annotation (Comfy-iOS pins cursor-review-auto-label.yml independently, at its
+# own older SHA — in a separate file today, but the guard is what keeps a
+# single-file variant from regressing).
+#
+# The sibling's REAL PIN matters at least as much as its comment: the 40-hex
+# substitution used to be addressed at any `/github-workflows/` line, so it
+# repinned the sibling's `uses: …@<40-hex>` to THIS fleet's SHA — pointing that
+# caller at a commit its own fleet never shipped. Asserting only the comment here
+# would have let that clobber stay green, so both are asserted.
+new_case multireusable
+MULTI_FIXTURE="${WORK}/multi_caller.yml"
+printf '%s\n' \
+  'name: CI cursor-review' \
+  'jobs:' \
+  '  review:' \
+  '    uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows main (1111111)' \
+  '  auto-label:' \
+  '    uses: Comfy-Org/github-workflows/.github/workflows/cursor-review-auto-label.yml@2222222222222222222222222222222222222222 # github-workflows#27' \
+  > "$MULTI_FIXTURE"
+STUB_CONTENT_FILE="$MULTI_FIXTURE" run_bump \
+  VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
+  CALLERS_JSON='[{"repo":"Comfy-Org/secret-multi","file":".github/workflows/ci-cursor-review.yml","label":""}]'
+PUT="${STUB_PUT_DIR}/put.last.txt"
+check "exit 0" "[[ $RC -eq 0 ]]"
+check "warned that pin comments were left alone" "grep -q 'pin comments untouched' <<<\"\$OUT\""
+check "OUR reusable's pin still bumped"          "grep -q \"cursor-review.yml@$NEW_SHA\" \"$PUT\""
+check "the SIBLING's 40-hex pin is UNCHANGED"    "grep -qF 'cursor-review-auto-label.yml@2222222222222222222222222222222222222222' \"$PUT\""
+check "the sibling's legacy marker is untouched" "grep -qF '# github-workflows#27' \"$PUT\""
+check "our own marker left alone (ambiguous)"    "grep -qF 'github-workflows main (1111111)' \"$PUT\""
+check "this fleet's short SHA was NOT stamped in" "! grep -qF 'github-workflows main ($SHORT)' \"$PUT\""
+
+echo "== merely NAMING a sibling workflow in a comment is not a second caller (BE-4523) =="
+# The multi-reusable check reads `uses:` lines only. A single-reusable caller that
+# mentions a sibling workflow in prose (or a docs URL, or a commented-out block)
+# must NOT be misread as multi-reusable — that would suppress the marker refresh
+# and leave the annotation stale, which is exactly the bug BE-4523 fixes.
+new_case mentiononly
+MENTION_FIXTURE="${WORK}/mention_caller.yml"
+printf '%s\n' \
+  'name: CI cursor-review' \
+  '# Labels come from github-workflows/.github/workflows/cursor-review-auto-label.yml' \
+  '# (see also https://github.com/Comfy-Org/github-workflows/.github/workflows/groom.yml)' \
+  'jobs:' \
+  '  review:' \
+  '    # uses: Comfy-Org/github-workflows/.github/workflows/pr-size.yml@3333333333333333333333333333333333333333' \
+  '    uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows main (1111111)' \
+  > "$MENTION_FIXTURE"
+STUB_CONTENT_FILE="$MENTION_FIXTURE" run_bump \
+  VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
+  CALLERS_JSON='[{"repo":"Comfy-Org/secret-mention","file":".github/workflows/ci-cursor-review.yml","label":""}]'
+PUT="${STUB_PUT_DIR}/put.last.txt"
+check "exit 0" "[[ $RC -eq 0 ]]"
+check "no spurious attribution warning"          "! grep -q 'pin comments untouched' <<<\"\$OUT\""
+check "the marker WAS refreshed"                 "grep -qF 'github-workflows main ($SHORT)' \"$PUT\""
+check "no stale short SHA left behind"           "! grep -qF 'main (1111111)' \"$PUT\""
+
 echo
 echo "== $PASS passed, $FAIL failed =="
 [[ $FAIL -eq 0 ]]