diff --git a/.github/bump-callers/README.md b/.github/bump-callers/README.md index d3544c3..757392b 100644 --- a/.github/bump-callers/README.md +++ b/.github/bump-callers/README.md @@ -42,11 +42,62 @@ The **groom** fleet is the one that most needs this: a groom caller pins the reusable **twice** — the `uses:` SHA *and* the `workflows_ref:` input that loads the finder/verifier/builder briefs plus the dedup ledger. Those must stay in lock-step or a run executes one version's workflow against another version's -briefs. `bump-callers.sh`'s pin rewrite moves both (it matches the `uses:` line -and any bare `workflows_ref:` line), so the fleet cannot drift into that split -state through a hand-bump of only one. It also re-points the `# main @ ` -pin comment those callers carry — a comment still naming the old commit after the -pin moved is worse than no comment. +briefs. `bump-callers.sh`'s pin rewrite moves both, so the fleet cannot drift +into that split state through a hand-bump of only one. It also re-points the +`# main @ ` pin comment those callers carry — a comment still naming the +old commit after the pin moved is worse than no comment. + +A caller pinning **two** github-workflows reusables in the same file is a +special case: the `uses:` pin rewrite and the `# main @ ` comment rewrite +are both address-restricted to the line calling THIS fleet's `WORKFLOW_FILE`, so +a sibling fleet's pin and annotation are left untouched rather than stamped with +this fleet's SHA (BE-4523). The legacy `# github-workflows#NN` / already-converted +`github-workflows main ()` markers name a SHA but not which reusable they +annotate, so they are refreshed only when the file is provably ours alone; +otherwise they are left as found and the run logs a warning. Inert for every +caller today (all call exactly one reusable); it exists so a caller that starts +calling two cannot be corrupted. + +## How the pin rewrite is scoped (and why it asserts afterwards) + +The rewrite targets the **pin token**, not "any 40-hex on a line that mentions +`github-workflows`" (BE-4662). Two patterns, matched by position rather than by +what the ref *looks like*: + +- `Comfy-Org/github-workflows…@` — the `uses:` pin. The owner/repo is matched + **case-insensitively**, because GitHub resolves `uses:` that way and a caller + written `comfy-org/…` is calling this repo; what follows the repo name must be + the `/` of a path or the `@` of a ref, so a **sibling** repo whose name merely + starts the same (`github-workflows-tools/action@v1`) is out of reach. +- `workflows_ref: ` as a block-mapping key, optionally quoted — the input pin. + +Whatever sits right after the token is the ref, so **any literal ref shape moves** +— full sha, short sha, or a tag like `v1`. That matters because a caller whose +`workflows_ref` is a tag used to be skipped by the old 40-hex rule while its +`uses:` pin moved: a green-looking bump PR on a caller that is now running one +version's workflow against another version's briefs. In the other direction, an +unrelated full SHA that merely *shares* a line with the words `github-workflows` +or `workflows_ref` is now unreachable, and a prose comment mentioning +`workflows_ref:` is left as prose. + +Precision cuts both ways, though — a pin form the patterns don't know how to move +would be silently left behind. So before a rewritten file can be staged, the +script re-reads it with a deliberately **broader** reader (any non-whitespace +value sitting where a ref belongs, comments excluded) and **asserts every +github-workflows pin now equals the new SHA**. If one does not — today the one +known case is a `workflows_ref` fed by a `${{ … }}` expression, which is +intentionally never rewritten — it emits a `::warning::` naming the file and the +stale value and **fails that repo**, exactly as it does for a transient fetch +error. A partial bump is worse than no bump (BE-3896). An empty pin +(`workflows_ref: ""`) and a value the rewrite could only half-move (a `#` inside +the ref) fail the same way rather than reading back as clean. + +Because that reader is the one place a false positive would block an otherwise +clean caller's bump on every run, it is bounded on both sides: comments are +dropped by YAML's own rule (a `#` preceded by whitespace, so a `#` *inside* a +value survives to be compared), and the `workflows_ref` key needs a real left +boundary, so a longer key that merely ends in it (`upstream_workflows_ref: v1`) +is not read as this repo's pin. ## The caller variables diff --git a/.github/bump-callers/bump-callers.sh b/.github/bump-callers/bump-callers.sh index a9e51c9..64a9bb1 100755 --- a/.github/bump-callers/bump-callers.sh +++ b/.github/bump-callers/bump-callers.sh @@ -64,6 +64,59 @@ SHORT="${NEW_SHA:0:7}" # the commit/title/body, not the branch name. BRANCH="ci/bump-${TAG}" +# The two forms a caller uses to pin THIS repo, written as regex prefixes that +# end exactly where the ref begins. They live here, together, because the +# rewrite below and the post-rewrite assertion that guards it must never drift +# apart: whatever the rewrite is expected to move, the assertion re-reads. +# 1. the `uses:` pin — `Comfy-Org/github-workflows/@` +# 2. the input pin — `workflows_ref: ` (optionally quoted), which loads +# this repo's prompts/briefs/scripts at run time +# INPUT_PIN_RE is applied `^`-anchored (see the rewrite), i.e. only where +# `workflows_ref:` is a block-mapping KEY. That is what keeps the rewrite off a +# prose comment that merely mentions the input — `# workflows_ref: keep in sync +# with uses:` must not come back with a SHA spliced into the middle of the +# sentence. The assertion below is deliberately NOT anchored that way, so the +# one shape this misses (flow style, `with: {workflows_ref: v1}`) fails the repo +# loudly instead of silently half-bumping it. +# +# The repo name is spelled case-INSENSITIVELY because GitHub resolves a `uses:` +# owner/repo that way — a caller written `comfy-org/github-workflows/…@` is +# calling THIS repo and must be bumped like any other. Missing it would be the +# worst kind of miss: rule 2 below is repo-agnostic, so the caller's +# `workflows_ref:` half would move while `uses:` stayed stale, and the assertion +# (which reuses this same regex to read `uses:` back) would not see the stale +# half either — shipping exactly the split bump this change exists to prevent. +# Character classes rather than a case-insensitive flag: sed's `I` and grep's +# `-i` are not portable/scopable the same way, and this regex is shared by both. +REPO_RE='[Cc][Oo][Mm][Ff][Yy]-[Oo][Rr][Gg]/[Gg][Ii][Tt][Hh][Uu][Bb]-[Ww][Oo][Rr][Kk][Ff][Ll][Oo][Ww][Ss]' +# The path segment is OPTIONAL but the delimiter after the repo name is NOT: +# what follows `github-workflows` must be the `/` that starts the path or the `@` +# that starts the ref. Without that, `[^@[:space:]]*` also swallows a SIBLING +# repo's name — `Comfy-Org/github-workflows-tools/action@v1` would be repinned to +# THIS repo's SHA, and because the assertion reuses this regex it would read the +# corrupted value back as NEW_SHA and stage it silently. +USES_PIN_RE="${REPO_RE}(/[^@[:space:]]*)?@" +INPUT_PIN_RE='[[:space:]]*workflows_ref:[[:space:]]*['\''"]?' +# The assertion's reader for the input pin. Deliberately looser than +# INPUT_PIN_RE's `^`-anchored use — it also catches flow style +# (`with: {workflows_ref: v1}`), which the rewrite cannot move — but it still +# needs a LEFT boundary. Unanchored, `workflows_ref:` matches inside a longer key +# like `upstream_workflows_ref: v1`, whose value rule 2 correctly leaves alone; +# the assertion would then read that value as a stale github-workflows pin and +# hard-fail an otherwise-clean repo on every run. The boundary is a plain +# character class, not `(^|…)`: `^` inside a group is a GNU extension, and the +# assertion pads every line with a leading space first (see below) so a key at +# column 0 is covered by the same class. +INPUT_KEY_RE='[[:space:],{]workflows_ref:' +# A LITERAL ref: a full sha, a short sha, or a tag (`v1`). Deliberately excludes +# quotes, `#` and `$ { } ( )` so a closing quote or a trailing comment is never +# swallowed, and a GitHub expression (`workflows_ref: ${{ inputs.workflows_ref }}`) +# is never half-rewritten into a broken value — an expression matches nothing +# here, is left untouched, and is then caught by the assertion, which fails the +# repo rather than shipping a caller whose two pins disagree. +# shellcheck disable=SC2016 # `$ { }` here are regex literals, not an expansion +REF_RE='[^[:space:]'\''"#$(){}]+' + STRIPPED="${CALLERS_JSON//[[:space:]]/}" # Empty handling is fleet-specific. A fleet seeded empty (ALLOW_EMPTY=true) @@ -208,64 +261,82 @@ bump_repo() { 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. + # Rewrite the github-workflows pin(s) to NEW_SHA and normalize the stale pin + # comments. # - # 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 + # Rules 1-2 are anchored to the PIN TOKEN itself (BE-4662) — + # `Comfy-Org/github-workflows…@` and `workflows_ref: ` — NOT to + # "any 40-hex on a line that mentions github-workflows or workflows_ref", + # which is what this used to key on. Keying on the line was wrong in both + # directions, and silently so: + # * UNDER-rewrite. A caller pins this repo TWICE — the `uses:` sha and the + # `workflows_ref:` input that loads the briefs/prompts/scripts at run + # time — and the halves must move in lock-step or a run executes one + # version's workflow against another version's assets. A `workflows_ref` + # pinned to a TAG (`v1`) or a short sha is not 40 hex, so it was left + # behind while `uses:` moved, and the content-equality check below still + # saw a difference — a green-looking bump PR on a split caller. + # * OVER-rewrite. An unrelated 40-hex value that merely shared such a line + # was clobbered to NEW_SHA. + # Matching the ref by POSITION (right after the pin token) instead of by + # 40-hex-ness fixes both at once: any literal ref shape is moved, and a + # full-SHA pin of ANOTHER action (`actions/checkout@`, the org's + # mandated practice) or a co-located digest is unreachable by both patterns. + # Rule 2 is additionally `^`-anchored so it only fires where `workflows_ref:` + # is a block-mapping key, never inside a prose comment that mentions it. + # + # In a file that also calls a SIBLING github-workflows reusable, rule 1 (the + # `uses:` pin — token-anchored on repo name alone, not on WHICH reusable) is + # additionally address-restricted to OUR reusable's path via SHA_ADDR: without + # it, a caller pinning TWO github-workflows reusables would have BOTH `uses:` + # lines bumped to this fleet's SHA — a cross-fleet stamp that silently points + # the sibling caller at a commit its own fleet never shipped. Rules 4-6 (the + # `# main @` comment, below) ride on the SAME address, since the annotation + # belongs to whichever pin line it sits on. The tightening is inert for every + # caller today (all 23 call exactly one github-workflows reusable, so the + # address matches everywhere rule 1 would anyway); 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:` + # Rule 2 (`workflows_ref:`) stays unaddressed 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/' + # needs YAML block structure 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 rule. + local SHA_ADDR='github-workflows|workflows_ref' if (( GW_HAS_SIBLING )); then - SHA_ADDR="/github-workflows[^[:space:]]*${WORKFLOW_FILE//./\\.}|workflows_ref/" + SHA_ADDR="github-workflows[^[:space:]]*${WORKFLOW_FILE//./\\.}|workflows_ref" fi # - # The groom callers' `# main @ ` annotation rides on the pin line - # itself, so it moves HERE, under the SAME `${SHA_ADDR}` — the address is the - # exact set of lines whose pin this rule rewrites, which makes it honest - # attribution for that form (the `main ()` form below cannot use it: it - # also appears in prose ABOVE the `uses:` line, naming no pin context of its - # own, so it needs the caller-level guard instead). The two MUST stay - # identical: a groom caller pins TWICE and its `workflows_ref: # main @ - # ` line is rewritten by the 40-hex rule, so a narrower anchor here - # would bump that pin while leaving its comment naming the old commit — - # reintroducing, on the second of the two groom pins, exactly the confident lie - # these rewrites exist to kill. Sharing the address also means the sibling - # tightening covers this form for free: a sibling fleet's `uses: …/other.yml@… - # # main @ ` line is outside the tightened address, so neither its pin - # nor its annotation is stamped with our SHA. + # The `# github-workflows#NN` legacy marker (and its already-converted + # `main ()` form) is deliberately NOT rewritten here — it names a SHA + # but not WHICH reusable's pin it annotates (it can sit in prose ABOVE the + # `uses:` line, outside any single pin's address), so it cannot be + # address-scoped like the rules below. It is normalized after this rewrite, + # gated on GW_ONLY_OURS (see below), instead of unconditionally here. # - # The short form is bounded to {7,12} hex so a deliberate FULL-sha annotation - # keeps its full form (the 40-hex rule has already corrected it in the same - # pass) instead of being shortened — and that bound only holds if the hex run - # ENDS there, hence two rules rather than one: `[0-9a-f]{7,12}` alone is happy - # to match the first 12 characters of a longer run, so on a `# main @ <40hex>` - # comment it would swap 12 hex for the 7-char SHORT and strand the remaining 28 - # — mangling the very full-form comment the bound exists to protect. Requiring - # a non-hex character (or end of line) after the run makes the match a whole - # token: rule 2 catches the short form mid-line, rule 3 the same at EOL, and a - # 13+ hex run matches neither and is left intact. A hex-boundary assertion - # (`\b`, `[[:>:]]`) would be simpler but spells differently in GNU and BSD sed; - # this is portable ERE. + # Rules 4-6 normalize the `# main @ ` comment (the groom callers' form), + # because a comment that still names the OLD commit after the pin moved is + # worse than no comment — it is a confident lie in the one file where the pin + # is the whole point: `# main @ 29a81ca …` -> `# main @ …`. Rule 4 (the + # FULL-sha form) is load-bearing now that rules 1-2 no longer sweep every + # 40-hex on the line: a deliberate `# main @ <40hex>` note used to be + # corrected as collateral of the old broad substitution, and would otherwise + # be left naming the old commit. Rules 5-6 handle the SHORT form and are + # bounded to {7,12} hex, which only holds if the hex run ENDS there — + # `[0-9a-f]{7,12}` alone is happy to match the first 12 characters of a longer + # run, so on a 40-hex comment it would swap 12 hex for the 7-char SHORT and + # leave the remaining 28 dangling, mangling the very full-form comment rule 4 + # just corrected. Requiring a non-hex character (rule 5) or end of line + # (rule 6) after the run makes the match a whole token, so a 13+ hex run + # matches neither. A hex-boundary assertion (`\b`, `[[:>:]]`) would be simpler + # but spells differently in GNU and BSD sed; this is portable ERE. NEW_CONTENT=$(sed -E " - ${SHA_ADDR} s/[0-9a-f]{40}/${NEW_SHA}/g - ${SHA_ADDR} s|# main @ [0-9a-f]{7,12}([^0-9a-f])|# main @ ${SHORT}\1|g - ${SHA_ADDR} s|# main @ [0-9a-f]{7,12}\$|# main @ ${SHORT}|g + /${SHA_ADDR}/ s|(${USES_PIN_RE})${REF_RE}|\1${NEW_SHA}|g + s|^(${INPUT_PIN_RE})${REF_RE}|\1${NEW_SHA}| + /${SHA_ADDR}/ s|# main @ [0-9a-f]{40}|# main @ ${NEW_SHA}|g + /${SHA_ADDR}/ s|# main @ [0-9a-f]{7,12}([^0-9a-f])|# main @ ${SHORT}\1|g + /${SHA_ADDR}/ s|# main @ [0-9a-f]{7,12}\$|# main @ ${SHORT}|g " <<<"$OLD_CONTENT") # Normalize the human-readable pin annotation so it never disagrees with the @@ -323,6 +394,68 @@ bump_repo() { fi fi + # ASSERT that the rewrite actually moved EVERY github-workflows pin in this + # file, before it can be staged (BE-4662). The patterns above are precise by + # design, and precision cuts both ways: a pin form they do not know how to + # move is silently left behind. So re-read the result with a DELIBERATELY + # BROADER reader — any non-whitespace value sitting where a ref belongs — and + # fail the repo if anything but NEW_SHA is still there. The gap between the + # two (today: a `workflows_ref` fed by a `${{ … }}` expression, which is + # deliberately never rewritten) then surfaces as a loud, named failure + # instead of a green-looking PR that bumps `uses:` and leaves the assets ref + # behind. A caller running one version's workflow against another version's + # briefs is exactly the split this fleet exists to prevent, and — as with the + # transient fetch error above — a partial bump is worse than no bump (BE-3896). + # + # Comments are not pins, so drop them before scanning: a prose note that + # happens to say `workflows_ref:` must not fail an otherwise-clean repo. A + # comment is stripped by YAML's OWN rule — a `#` preceded by whitespace (every + # line is padded with a leading space first, so that also covers a `#` at + # column 0, and gives INPUT_KEY_RE's boundary class a character to match at + # line start) — NOT at the first `#` anywhere on the line. That distinction is + # what keeps the assertion honest about a `#` INSIDE a value: REF_RE stops at + # `#`, so `workflows_ref: 'feature#1'` is rewritten to `'#1'`, and a + # first-`#` strip would read back a bare NEW_SHA and accept that corrupted + # value. Under YAML's rule the value survives intact, compares unequal, and + # fails the repo — the loud outcome, as designed. + local LIVE_CONTENT USES_SCAN STALE_PINS SAFE_PINS + LIVE_CONTENT=$(sed -E 's|^| |; s|[[:space:]]#.*$||' <<<"$NEW_CONTENT") + # The `uses:` pin scan is restricted to the same SHA_ADDR-eligible lines as + # rule 1's rewrite above (a no-op restriction outside the sibling case): a + # sibling reusable's `uses:` pin is deliberately left untouched by rule 1, so + # scanning it here too would read its (unrelated, un-moved) ref as a stale + # github-workflows pin and fail an otherwise-clean repo. The `workflows_ref:` + # scan below stays unrestricted — rule 2 is unaddressed for the same reason. + USES_SCAN=$(grep -E "$SHA_ADDR" <<<"$LIVE_CONTENT") || true + STALE_PINS=$( + { + # A `uses:` ref cannot contain whitespace, so the token after `@` is the + # whole ref. `workflows_ref` is a YAML scalar, so take the rest of the + # line — that keeps an expression value readable in the warning instead + # of truncating it to `${{`. + grep -oE "${USES_PIN_RE}[^[:space:]]+" <<<"$USES_SCAN" | sed -E 's|^.*@||' + grep -oE "${INPUT_KEY_RE}[[:space:]]*[^[:space:]].*$" <<<"$LIVE_CONTENT" \ + | sed -E 's|^.*workflows_ref:[[:space:]]*||; s|[[:space:]]+$||' + # An extracted value that is EMPTY after unquoting is named rather than + # dropped: `workflows_ref: ""` is a pin the rewrite cannot move either + # (REF_RE needs ≥1 character), so filtering blanks out here would let it + # slip past the assertion while `uses:` moved — the exact silent half-bump + # this guard exists to catch. It is not a legal ref, so it fails loudly. + } | sed -E "s|^['\"]||; s|['\"]\$||" \ + | sed -E 's|^$|(empty)|' | grep -vFx "$NEW_SHA" | sort -u + ) + if [[ -n "$STALE_PINS" ]]; then + # Sanitize before echoing: these values come from a caller file, and the + # run logs of this public repo are a workflow-command sink. `tr` collapses + # the newline join but not a carriage return, so a value carrying `\r::` + # could inject a command of its own; `::` is neutralized for the same + # reason. (The value itself is a ref of THIS public repo, so there is + # nothing private to redact — only a control character to defang.) + SAFE_PINS=$(tr -d '\r' <<<"$STALE_PINS" | sed -E 's|::|:|g' | tr '\n' ' ' | sed -E 's|[[:space:]]+$||') + echo "::warning::${REPO}: ${FILE} still pins github-workflows at ${SAFE_PINS} after the rewrite (expected ${NEW_SHA}) — failing repo to avoid a half-bumped caller" + return 1 + fi + # Already fully pinned → the rewrite is a no-op → nothing to do for this file. # Comparing the rewritten content to the original (rather than grepping for # NEW_SHA appearing *anywhere*) also repairs a half-bumped file: if a prior diff --git a/.github/bump-callers/tests/test_bump_callers.sh b/.github/bump-callers/tests/test_bump_callers.sh index a63e6e8..98d7903 100755 --- a/.github/bump-callers/tests/test_bump_callers.sh +++ b/.github/bump-callers/tests/test_bump_callers.sh @@ -10,6 +10,15 @@ # * every private repo name is masked out of the public run logs, # * the caller's pinned SHA (and only it) is rewritten, the pin comment is # normalized, and the committed file keeps its single trailing newline, +# * BOTH pin halves move in lock-step — the `uses:` ref and the +# `workflows_ref` input, whatever shape the latter carries (full sha, short +# sha, tag) — while an unrelated 40-hex merely sharing such a line is left +# alone, and a pin the rewrite cannot move fails the repo rather than +# shipping a half-bumped caller, +# * the pin token's edges hold in both directions: a differently-cased +# owner/repo is still this repo, while a sibling repo whose name starts the +# same and a longer key ending in `workflows_ref` are neither rewritten nor +# misread as a stale pin, # * an empty seeded-empty fleet is a clean no-op while a must-have-callers # fleet still hard-fails, and a malformed variable hard-fails. # @@ -550,6 +559,270 @@ 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 "== a TAG-pinned workflows_ref moves in lock-step with uses: (BE-4662) ==" +# The under-rewrite half of BE-4662. A caller pins this repo TWICE — the `uses:` +# sha and the `workflows_ref` input that loads the briefs/prompts/scripts. The +# old substitution rewrote "any 40-hex on a line mentioning github-workflows", so +# a `workflows_ref` pinned to a TAG was left behind while `uses:` moved: a +# green-looking bump PR running one version's workflow against another version's +# assets. The rewrite is anchored to the pin token now, so ref SHAPE is irrelevant. +new_case reftag +TAG_FIXTURE="${WORK}/tag_ref_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' workflows_ref: v1' \ + > "$TAG_FIXTURE" +STUB_CONTENT_FILE="$TAG_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-tagref","file":".github/workflows/agents-md-integrity.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0" "[[ $RC -eq 0 ]]" +check "staged the caller" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]" +check "uses: pin bumped" "grep -qF 'agents-md-integrity.yml@$NEW_SHA' \"$PUT\"" +check "TAG workflows_ref bumped too" "grep -qF 'workflows_ref: $NEW_SHA' \"$PUT\"" +check "no tag left in workflows_ref" "! grep -qE '^[[:space:]]*workflows_ref:[[:space:]]*v1[[:space:]]*\$' \"$PUT\"" +check "both pins at the new SHA" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]" +check "'# v1' comment left intact" "grep -qF '# v1' \"$PUT\"" + +echo "== a SHORT-SHA (and quoted) workflows_ref also moves in lock-step (BE-4662) ==" +# The other non-40-hex shape a caller can carry. The quotes must survive — only +# the ref token inside them is replaced, so the YAML stays valid. +new_case refshort +SHORT_FIXTURE="${WORK}/short_ref_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + " workflows_ref: '2222222'" \ + > "$SHORT_FIXTURE" +STUB_CONTENT_FILE="$SHORT_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-shortref","file":".github/workflows/agents-md-integrity.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0" "[[ $RC -eq 0 ]]" +check "staged the caller" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]" +check "short-sha workflows_ref bumped" "grep -qF \"workflows_ref: '$NEW_SHA'\" \"$PUT\"" +check "closing quote preserved" "! grep -qF \"workflows_ref: $NEW_SHA'\" \"$PUT\"" +check "short sha gone" "! grep -qF \"workflows_ref: '2222222'\" \"$PUT\"" +check "both pins at the new SHA" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]" + +echo "== an unrelated 40-hex SHARING a github-workflows line is NOT clobbered (BE-4662) ==" +# The over-rewrite half of BE-4662. The old substitution keyed on the LINE ("any +# 40-hex on a line that mentions github-workflows or workflows_ref"), so an +# unrelated full-SHA value that merely shared such a line — another action pinned +# next to a mention of this repo, a digest documented as tracking workflows_ref — +# was rewritten to github-workflows' SHA. Only the pin TOKEN may move. (The +# sibling `anchor` case above covers the easy version, where the third-party pin +# sits on its own line.) +new_case coloc +COLOC_FIXTURE="${WORK}/coloc_caller.yml" +printf '%s\n' \ + 'name: CI cursor-review' \ + 'jobs:' \ + ' review:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows#27' \ + ' audit:' \ + ' runs-on: ubuntu-latest' \ + ' steps:' \ + ' - uses: some-org/mirror-check@cccccccccccccccccccccccccccccccccccccccc # verifies the github-workflows mirror' \ + ' - name: digest' \ + ' run: echo dddddddddddddddddddddddddddddddddddddddd # kept in sync with workflows_ref' \ + > "$COLOC_FIXTURE" +STUB_CONTENT_FILE="$COLOC_FIXTURE" run_bump \ + VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-coloc","file":".github/workflows/ci.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0" "[[ $RC -eq 0 ]]" +check "github-workflows pin bumped" "grep -qF 'cursor-review.yml@$NEW_SHA' \"$PUT\"" +check "co-located action SHA left intact" "grep -qF 'some-org/mirror-check@cccccccccccccccccccccccccccccccccccccccc' \"$PUT\"" +check "co-located digest left intact" "grep -qF 'echo dddddddddddddddddddddddddddddddddddddddd' \"$PUT\"" +check "exactly ONE pin was rewritten" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 1 ]]" + +echo "== a pin the rewrite cannot move FAILS the repo — no commit, no PR (BE-4662) ==" +# The assertion is the backstop for anything the (deliberately precise) rewrite +# does not know how to move — here a `workflows_ref` fed by a GitHub expression, +# which must never be half-rewritten into a broken value. Staging it would open a +# green-looking PR whose `uses:` moved and whose assets ref did not, so the repo +# fails instead: a partial bump is worse than no bump (BE-3896). +new_case assertfire +EXPR_FIXTURE="${WORK}/expr_ref_caller.yml" +# shellcheck disable=SC2016 # the `${{ }}` must reach the fixture verbatim +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' workflows_ref: ${{ inputs.workflows_ref }}' \ + > "$EXPR_FIXTURE" +STUB_CONTENT_FILE="$EXPR_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-expr","file":".github/workflows/agents-md-integrity.yml","label":""}]' +check "exit 1 — the repo failed" "[[ $RC -eq 1 ]]" +check "warning names the file" "grep -q 'agents-md-integrity.yml still pins github-workflows' <<<\"\$OUT\"" +check "warning names the un-bumped pin" "grep -qF 'inputs.workflows_ref' <<<\"\$OUT\"" +check "warning reads as a sentence" "grep -qF 'inputs.workflows_ref }} after the rewrite' <<<\"\$OUT\"" +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\"" +check "job failed for the repo" "grep -q 'bump failed for 1 repo' <<<\"\$OUT\"" + +echo "== the assertion reads PINS, not prose — a commented workflows_ref is not a stale pin ==" +# The assertion is deliberately broader than the rewrite, which makes it the one +# place a false positive would hard-fail an otherwise-clean repo. Comments are +# stripped before it scans, so a human note that happens to say `workflows_ref:` +# cannot masquerade as an un-bumped pin — while the real pin on the next line is +# still asserted (it is checked before the `#`). +new_case prosecomment +PROSE_FIXTURE="${WORK}/prose_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' # workflows_ref: keep this in lock-step with the uses: pin above' \ + ' workflows_ref: 2222222222222222222222222222222222222222 # bumped by CI' \ + > "$PROSE_FIXTURE" +STUB_CONTENT_FILE="$PROSE_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-prose","file":".github/workflows/agents-md-integrity.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0 — prose did not trip the assert" "[[ $RC -eq 0 ]]" +check "staged the caller" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]" +check "both real pins bumped" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]" +check "old pin gone" "! grep -qF '2222222222222222222222222222222222222222' \"$PUT\"" +check "prose comment still readable" "grep -qF 'keep this in lock-step with the uses: pin above' \"$PUT\"" + +echo "== a SIBLING repo that merely starts with our name is NOT repinned (BE-4662) ==" +# The pin token ends at a DELIMITER. Without one, the path glob after the repo +# name also swallows a sibling repo's name — `github-workflows-tools/action@v1` +# would be repinned to THIS repo's SHA, and because the assertion reads pins with +# the same pattern it would read the corrupted value back as NEW_SHA and stage it. +new_case sibling +SIB_FIXTURE="${WORK}/sibling_caller.yml" +printf '%s\n' \ + 'name: CI cursor-review' \ + 'jobs:' \ + ' review:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows#27' \ + ' tools:' \ + ' uses: Comfy-Org/github-workflows-tools/.github/workflows/lint.yml@cccccccccccccccccccccccccccccccccccccccc # v3' \ + ' steps:' \ + ' - uses: Comfy-Org/github-workflows-actions/setup@v2' \ + > "$SIB_FIXTURE" +STUB_CONTENT_FILE="$SIB_FIXTURE" run_bump \ + VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-sibling","file":".github/workflows/ci.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0" "[[ $RC -eq 0 ]]" +check "our pin bumped" "grep -qF 'cursor-review.yml@$NEW_SHA' \"$PUT\"" +check "sibling repo's SHA pin left intact" "grep -qF 'github-workflows-tools/.github/workflows/lint.yml@cccccccccccccccccccccccccccccccccccccccc' \"$PUT\"" +check "sibling repo's tag pin left intact" "grep -qF 'github-workflows-actions/setup@v2' \"$PUT\"" +check "exactly ONE pin was rewritten" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 1 ]]" + +echo "== a lowercase owner/repo is still THIS repo — both halves move (BE-4662) ==" +# GitHub resolves `uses:` owner/repo case-insensitively, so a caller written +# `comfy-org/…` is calling this repo. A case-SENSITIVE match would skip its +# `uses:` half while rule 2 (repo-agnostic) bumped `workflows_ref` anyway, and the +# assertion — reading `uses:` with the same pattern — would not see the stale half +# either: a silently half-bumped caller, the split this change exists to prevent. +new_case lowercase +LC_FIXTURE="${WORK}/lowercase_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: comfy-org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' workflows_ref: v1' \ + > "$LC_FIXTURE" +STUB_CONTENT_FILE="$LC_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-lowercase","file":".github/workflows/agents-md-integrity.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0" "[[ $RC -eq 0 ]]" +check "staged the caller" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]" +check "lowercase uses: pin bumped" "grep -qF 'comfy-org/github-workflows/.github/workflows/agents-md-integrity.yml@$NEW_SHA' \"$PUT\"" +check "workflows_ref bumped in lock-step" "grep -qF 'workflows_ref: $NEW_SHA' \"$PUT\"" +check "owner case preserved as written" "! grep -qF 'Comfy-Org/github-workflows/.github' \"$PUT\"" +check "both pins at the new SHA" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]" + +echo "== a longer key ENDING in workflows_ref is not a stale pin ==" +# The assertion is looser than the rewrite by design, which makes it the one place +# a false positive hard-fails a clean repo. `upstream_workflows_ref: v1` is not +# this repo's input — rule 2 correctly leaves it alone — so reading it as an +# un-bumped github-workflows pin would block this caller's bump on every run. +new_case longkey +LONGKEY_FIXTURE="${WORK}/longkey_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' workflows_ref: 2222222222222222222222222222222222222222' \ + ' upstream_workflows_ref: v1' \ + > "$LONGKEY_FIXTURE" +STUB_CONTENT_FILE="$LONGKEY_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-longkey","file":".github/workflows/agents-md-integrity.yml","label":""}]' +PUT="${STUB_PUT_DIR}/put.last.txt" +check "exit 0 — the foreign key did not trip it" "[[ $RC -eq 0 ]]" +check "staged the caller" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 1 ]]" +check "both real pins bumped" "[[ \$(grep -cF '$NEW_SHA' \"$PUT\") -eq 2 ]]" +check "foreign key's value left alone" "grep -qE '^[[:space:]]*upstream_workflows_ref:[[:space:]]*v1\$' \"$PUT\"" + +echo "== a '#' inside the ref cannot fake its way past the assertion ==" +# `#` is legal in a git ref name, and REF_RE stops at it, so rule 2 rewrites only +# the part before: `'feature#1'` -> `'#1'`. Stripping comments at the +# FIRST `#` would read that back as a bare NEW_SHA and accept the corrupted value; +# stripping by YAML's rule (whitespace-preceded `#`) keeps the value whole, so it +# compares unequal and fails the repo instead of shipping broken YAML. +new_case hashref +HASH_FIXTURE="${WORK}/hash_ref_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + " workflows_ref: 'feature#1'" \ + > "$HASH_FIXTURE" +STUB_CONTENT_FILE="$HASH_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-hashref","file":".github/workflows/agents-md-integrity.yml","label":""}]' +check "exit 1 — the repo failed" "[[ $RC -eq 1 ]]" +check "warning names the half-rewritten value" "grep -qF '#1' <<<\"\$OUT\"" +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 EMPTY workflows_ref is a stale pin, not a blank to skip ==" +# `workflows_ref: \"\"` is a pin rule 2 cannot move either (REF_RE needs >=1 +# character), so dropping empty extracted values would let it slip past while +# `uses:` moved — the silent half-bump the assertion exists to catch. +new_case emptyref +EMPTY_FIXTURE="${WORK}/empty_ref_caller.yml" +printf '%s\n' \ + 'name: AGENTS.md Integrity' \ + 'jobs:' \ + ' agents-md:' \ + ' uses: Comfy-Org/github-workflows/.github/workflows/agents-md-integrity.yml@2222222222222222222222222222222222222222 # v1' \ + ' with:' \ + ' workflows_ref: ""' \ + > "$EMPTY_FIXTURE" +STUB_CONTENT_FILE="$EMPTY_FIXTURE" run_bump \ + VAR_NAME=AGENTS_MD_CALLERS TAG=agents-md-integrity WORKFLOW_FILE=agents-md-integrity.yml ALLOW_EMPTY=true \ + CALLERS_JSON='[{"repo":"Comfy-Org/secret-emptyref","file":".github/workflows/agents-md-integrity.yml","label":""}]' +check "exit 1 — the repo failed" "[[ $RC -eq 1 ]]" +check "warning names the empty pin" "grep -qF '(empty)' <<<\"\$OUT\"" +check "committed NOTHING" "[[ ! -f \"\$STUB_PUT_DIR/count\" ]]" + 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 diff --git a/.github/workflows/bump-groom-callers.yml b/.github/workflows/bump-groom-callers.yml index c23d8c8..da95685 100644 --- a/.github/workflows/bump-groom-callers.yml +++ b/.github/workflows/bump-groom-callers.yml @@ -15,9 +15,12 @@ name: Bump groom callers # reusable TWICE — the `uses:` SHA and the `workflows_ref:` input that loads the # finder/verifier/builder briefs + the dedup ledger. Those two must stay in # lock-step or a run executes one version's workflow against another version's -# briefs. bump-callers.sh rewrites both (its rewrite matches the `uses:` line and -# any bare `workflows_ref:` line), so the fleet cannot drift into that split -# state by hand-bumping only one of them. +# briefs. bump-callers.sh rewrites both — it targets the pin TOKEN +# (`Comfy-Org/github-workflows…@` and the `workflows_ref:` key), so the ref's +# shape does not matter and both halves move whatever they are pinned to — and it +# then asserts every pin in the file reached the new SHA before staging it. So the +# fleet cannot drift into that split state by hand-bumping only one of them, nor +# ship a half-bumped caller (BE-4662). # # The caller list is NOT hardcoded here. This repo is PUBLIC (workflow file and # Actions run logs are both publicly viewable) and callers may be private, so