diff --git a/.github/bump-callers/bump-callers.sh b/.github/bump-callers/bump-callers.sh index d758184..91146a8 100755 --- a/.github/bump-callers/bump-callers.sh +++ b/.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…@` - # line and agents-md-integrity's bare `workflows_ref: ` line — so a - # full-SHA pin of ANOTHER action in the same file (`actions/checkout@`, - # 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…@` line and agents-md-integrity's + # bare `workflows_ref: ` line — so a full-SHA pin of ANOTHER action in + # the same file (`actions/checkout@`, 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 ()` 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 — diff --git a/.github/bump-callers/tests/test_bump_callers.sh b/.github/bump-callers/tests/test_bump_callers.sh index ec6e980..79f25a8 100755 --- a/.github/bump-callers/tests/test_bump_callers.sh +++ b/.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 ()' 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 ()` 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 ()` 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 ]]