diff --git a/.github/bump-callers/bump-callers.sh b/.github/bump-callers/bump-callers.sh index c656afd..d758184 100755 --- a/.github/bump-callers/bump-callers.sh +++ b/.github/bump-callers/bump-callers.sh @@ -110,11 +110,14 @@ fi # the loop) instead of killing the run. A repo whose files are all missing or # already pinned commits nothing and is a successful skip (`return 0`). # -# Grouping by repo is load-bearing: the branch is reset to the default-branch -# tip ONCE per repo, then each file is committed onto it. The previous per-entry -# loop reset the branch before EACH file, so a second same-repo file's reset -# discarded the first file's commit and the PR shipped only the last file — a -# silent partial bump (BE-3896). +# Grouping by repo is load-bearing: all of a repo's files are built into ONE +# atomic commit (Git Data API) off the default-branch tip and the bump branch is +# moved to it in a single step, so the PR either bumps every file or none — never +# a partial-yet-mergeable bump. The earlier per-file Contents PUT loop committed +# files one at a time with no rollback, so a mid-sequence failure left the +# earlier files already committed on the branch (BE-3902); and before that a +# per-ENTRY branch reset discarded earlier commits so only the last file shipped +# (BE-3896). One commit for the whole repo closes both. bump_repo() { local REPO="$1"; shift # remaining args: this repo's "repo|file|label" entries @@ -124,12 +127,12 @@ bump_repo() { return 1 } - # Resolve the default-branch tip ONCE, up front, and pin every Pass-1 blob - # fetch to this immutable SHA (not the mutable `?ref=`). This closes a - # TOCTOU race: if the caller's default branch moved between the fetches and the - # reset below, a staged blob `sha` would be stale against the new tip and its - # Pass-2 PUT would 409 after earlier files already committed. Reading the tip - # is a GET (no branch churn), so it is safe before the all-skip early return. + # Resolve the default-branch tip ONCE, up front. Every Pass-1 fetch is pinned + # to this immutable SHA (not the mutable `?ref=`), and Pass 2 uses the + # same commit as the new commit's parent AND (via its resolved tree) as the + # tree's `base_tree` — so the whole bump is built against one consistent + # snapshot of the caller repo, never a moving tip. Reading the tip is a GET (no + # branch churn), so it is safe before the all-skip early return. local MAIN_SHA MAIN_SHA=$(gh api "repos/${REPO}/git/refs/heads/${DEFAULT_BRANCH}" --jq '.object.sha') || { echo "::warning::${REPO}: cannot read ${DEFAULT_BRANCH} ref — skipping" @@ -140,9 +143,9 @@ bump_repo() { # anything yet. This keeps the reset/commit path off entirely for a repo whose # files are all missing or already pinned (the old per-entry skips), so it # never resets a branch or opens a PR it doesn't need. - local -a PEND_FILE_ENC=() PEND_CONTENT=() PEND_BLOB=() LABELS=() + local -a PEND_FILE=() PEND_CONTENT=() LABELS=() local WIRING_ADDED_ANY="" - local ENTRY FILE LABEL WIRE_BOT FILE_ENC CURRENT BLOB_SHA OLD_CONTENT NEW_CONTENT + local ENTRY FILE LABEL WIRE_BOT FILE_ENC CURRENT OLD_CONTENT NEW_CONTENT for ENTRY in "$@"; do FILE=$(cut -d'|' -f2 <<<"$ENTRY") LABEL=$(cut -d'|' -f3 <<<"$ENTRY") @@ -156,8 +159,8 @@ bump_repo() { # duplicate entry's label into the set (it still applies to the one PR) but # do not re-stage the file. local SEEN_FILE=0 P - for P in ${PEND_FILE_ENC[@]+"${PEND_FILE_ENC[@]}"}; do - [[ "$P" == "$FILE_ENC" ]] && { SEEN_FILE=1; break; } + for P in ${PEND_FILE[@]+"${PEND_FILE[@]}"}; do + [[ "$P" == "$FILE" ]] && { SEEN_FILE=1; break; } done if (( SEEN_FILE )); then [[ -n "$LABEL" ]] && LABELS+=("$LABEL") @@ -185,7 +188,6 @@ bump_repo() { fi rm -f "$ERRFILE" - BLOB_SHA=$(jq -r '.sha' <<<"$CURRENT") OLD_CONTENT=$(jq -r '.content' <<<"$CURRENT" | base64 -d) # Rewrite the github-workflows pin(s) to NEW_SHA and normalize the stale @@ -239,50 +241,92 @@ bump_repo() { # newline, so printf '%s\n' restores exactly one — otherwise the committed # caller loses its EOF newline and trips the receiver repo's formatter # (oxfmt/prettier both require a trailing newline). - PEND_FILE_ENC+=("$FILE_ENC") + PEND_FILE+=("$FILE") PEND_CONTENT+=("$(printf '%s\n' "$NEW_CONTENT" | base64 | tr -d '\n')") - PEND_BLOB+=("$BLOB_SHA") done # Nothing to bump for this repo → clean skip: no branch churn, no PR. - if (( ${#PEND_FILE_ENC[@]} == 0 )); then + if (( ${#PEND_FILE[@]} == 0 )); then return 0 fi - # Rebuild the stable bump branch from the resolved default-branch tip (MAIN_SHA, - # captured up front) ONCE for the repo, so the PR is always a clean "bump to - # @SHORT" diff — it never accumulates stale commits across bumps and never - # drifts if the caller's default branch moved since the last bump (BE-3882). - # Create the branch at the tip; if it already exists (an open bump PR, or - # residue from a prior run) force it back to the tip so the diff is rebuilt - # from scratch rather than committed on top of the old bump. + # Pass 2 — land EVERY staged file in ONE atomic commit via the Git Data API, + # then point the stable bump branch at it. The previous per-file Contents PUT + # loop committed files one at a time with no rollback, so a mid-sequence + # failure left the earlier files already committed on the branch — and if an + # open bump PR pointed at it, a partial-yet-mergeable bump (BE-3902). Building + # one commit off the tip and moving the ref in a single step makes the bump + # all-or-nothing: either every file lands or the branch is untouched. + + # 1. Create a blob for each staged file's new content. + local -a TREE_ENTRIES=() + local i BLOB_NEW + for (( i = 0; i < ${#PEND_FILE[@]}; i++ )); do + # Pass the base64 body on stdin (like the tree/commit calls below), not via + # --field on argv — this keeps the content off /proc//cmdline and out of + # the ARG_MAX bound, and matches the surrounding Git Data API calls. + BLOB_NEW=$(jq -n --arg content "${PEND_CONTENT[$i]}" \ + '{content: $content, encoding: "base64"}' \ + | gh api --method POST "repos/${REPO}/git/blobs" --input - --jq '.sha') || { + echo "::warning::${REPO}: blob create for ${PEND_FILE[$i]} failed — skipping" + return 1 + } + # Callers are regular files (mode 100644 — the `.github/workflows/*.yml` + # they pin from is never executable). `base_tree` below carries every OTHER + # path in the repo unchanged, so the tree lists only the bumped files. + TREE_ENTRIES+=("$(jq -n --arg path "${PEND_FILE[$i]}" --arg sha "$BLOB_NEW" \ + '{path: $path, mode: "100644", type: "blob", sha: $sha}')") + done + + # 2. Build ONE tree off the default-branch tip carrying all staged blobs. + # `base_tree` must be a TREE SHA, not a commit SHA — the Create-a-tree API + # rejects a commit SHA (422), and a tree built with no valid `base_tree` + # would carry ONLY the bumped files, so merging its PR would delete every + # other path in the caller repo. `MAIN_SHA` is the tip *commit* SHA (still + # correct as the commit's parent below), so resolve its tree first. + local MAIN_TREE_SHA + MAIN_TREE_SHA=$(gh api "repos/${REPO}/git/commits/${MAIN_SHA}" --jq '.tree.sha') || { + echo "::warning::${REPO}: cannot resolve base tree — skipping" + return 1 + } + local TREE_SHA + TREE_SHA=$(printf '%s\n' "${TREE_ENTRIES[@]}" \ + | jq -s --arg base "$MAIN_TREE_SHA" '{base_tree: $base, tree: .}' \ + | gh api --method POST "repos/${REPO}/git/trees" --input - --jq '.sha') || { + echo "::warning::${REPO}: tree create failed — skipping" + return 1 + } + + # 3. Create ONE commit with that tree, parented on the tip so the PR is always + # a clean "bump to @SHORT" diff against the default branch (never stale + # commits accumulated across bumps, never drift if the caller's default + # branch moved since the last bump — BE-3882). + local COMMIT_SHA + COMMIT_SHA=$(jq -n \ + --arg msg "ci: bump ${TAG} to github-workflows@${SHORT}" \ + --arg tree "$TREE_SHA" --arg parent "$MAIN_SHA" \ + '{message: $msg, tree: $tree, parents: [$parent]}' \ + | gh api --method POST "repos/${REPO}/git/commits" --input - --jq '.sha') || { + echo "::warning::${REPO}: commit create failed — skipping" + return 1 + } + + # 4. Point the stable bump branch at the finished commit. Create it there if it + # is new; otherwise force it there so the diff is rebuilt from scratch (an + # open bump PR's branch, or residue from a prior run) rather than committed + # on top of the old bump (BE-3882). The ref moves in ONE step from its old + # state to the complete new commit — it never transiently sits at an empty + # or partial tree. if ! gh api --method POST "repos/${REPO}/git/refs" \ --field ref="refs/heads/${BRANCH}" \ - --field sha="${MAIN_SHA}" >/dev/null 2>&1; then + --field sha="${COMMIT_SHA}" >/dev/null 2>&1; then gh api --method PATCH "repos/${REPO}/git/refs/heads/${BRANCH}" \ - --field sha="${MAIN_SHA}" --field force=true >/dev/null 2>&1 || { - echo "::warning::${REPO}: cannot reset ${BRANCH} to ${DEFAULT_BRANCH} tip — skipping" + --field sha="${COMMIT_SHA}" --field force=true >/dev/null 2>&1 || { + echo "::warning::${REPO}: cannot point ${BRANCH} at the bump commit — skipping" return 1 } fi - # Pass 2 — commit each staged file onto the (single) branch in turn. Each PUT - # carries that file's own blob SHA from the tip; because the files are - # distinct, committing one never changes another's blob, so every PUT applies - # cleanly on top of the previous one and the branch accumulates ALL of them. - local i - for (( i = 0; i < ${#PEND_FILE_ENC[@]}; i++ )); do - gh api --method PUT "repos/${REPO}/contents/${PEND_FILE_ENC[$i]}" \ - --field message="ci: bump ${TAG} to github-workflows@${SHORT}" \ - --field content="${PEND_CONTENT[$i]}" \ - --field sha="${PEND_BLOB[$i]}" \ - --field branch="${BRANCH}" \ - > /dev/null || { - echo "::warning::${REPO}: commit to ${BRANCH} failed — skipping" - return 1 - } - done - # The title/body embed ${SHORT}. Build them once so the create and the # update-in-place paths post the identical, current text. The body is a single # line so the workflow parses it — a multi-line --body collapsed the step's diff --git a/.github/bump-callers/tests/test_bump_callers.sh b/.github/bump-callers/tests/test_bump_callers.sh index eabdf8a..ec6e980 100755 --- a/.github/bump-callers/tests/test_bump_callers.sh +++ b/.github/bump-callers/tests/test_bump_callers.sh @@ -14,7 +14,8 @@ # fleet still hard-fails, and a malformed variable hard-fails. # # No network: `gh` is a PATH stub that serves a fixture file and captures the -# contents PUT so we can inspect exactly what would be committed. +# Git Data API calls (the blob content + the tree's file list) so we can inspect +# exactly what would be committed. set -uo pipefail @@ -42,8 +43,9 @@ mkdir -p "$STUB_BIN" cat > "${STUB_BIN}/gh" <<'STUB' #!/usr/bin/env bash # Minimal `gh` stub. Serves $STUB_CONTENT_FILE for a contents GET and captures -# the decoded contents PUT to $STUB_PUT_DIR; everything else returns a canned -# value so bump-callers.sh runs end to end offline. +# the atomic Git Data API commit (blobs/tree/commit/ref) to $STUB_PUT_DIR; +# everything else returns a canned value so bump-callers.sh runs end to end +# offline. sub="$1"; shift || true if [[ "$sub" == "pr" ]]; then action="$1"; shift || true @@ -87,26 +89,44 @@ while (( i < ${#args[@]} )); do esac done -# Model the ONE bump branch's committed file set in $STUB_PUT_DIR/branch_files: -# a ref create/reset (POST/PATCH on git/refs) rebuilds the branch at the tip and -# so DROPS every prior bump commit (truncate), while a contents PUT commits a -# file onto it (append its path). This is what lets a test assert the branch's -# final contents — and catch the BE-3896 regression where resetting the branch -# per file left only the last file on it. (One branch is modeled; a same-repo -# test drives a single repo, so branch_files reflects exactly that repo's PR.) -case "$method" in - POST) # branch create at the tip — starts a fresh (empty) bump branch - [[ "$path" == *"/git/refs"* ]] && : > "$STUB_PUT_DIR/branch_files" - exit 0;; - PATCH) # force-reset of the bump branch ref — discards prior bump commits - [[ "$path" == *"/git/refs"* ]] && : > "$STUB_PUT_DIR/branch_files" - exit 0;; - PUT) +# Model the ONE atomic bump commit in $STUB_PUT_DIR. The script builds a blob +# per staged file (POST git/blobs), one tree carrying all of them off the tip +# (POST git/trees, body on stdin), one commit (POST git/commits), then points +# the bump branch at that commit (POST/PATCH git/refs). We record each blob's +# decoded content (put.$n.txt / put.last.txt; count = number of blobs = files +# committed) and the tree's path list as $STUB_PUT_DIR/branch_files — the +# atomic branch's final file set. Because the whole commit is built BEFORE the +# ref moves, an earlier failure (e.g. a Pass-1 fetch error) leaves NO blobs, NO +# tree, and the ref untouched — the all-or-nothing property this asserts +# (BE-3902) — while the tree still lists BOTH files of a monorepo caller on the +# one branch (BE-3896). (One branch/commit is modeled; a same-repo test drives a +# single repo, so branch_files reflects exactly that repo's PR.) +case "$method:$path" in + POST:*/git/blobs*) # blob create — capture the new file content, count it + # The script now sends the base64 body on stdin (--input -), so read the + # content out of the JSON body rather than the old --field content= on argv. + content=$(jq -r '.content' <(cat)) n=$(( $(cat "$STUB_PUT_DIR/count" 2>/dev/null || echo 0) + 1 )) echo "$n" > "$STUB_PUT_DIR/count" printf '%s' "$content" | { base64 -d 2>/dev/null || base64 -D; } > "$STUB_PUT_DIR/put.$n.txt" cp "$STUB_PUT_DIR/put.$n.txt" "$STUB_PUT_DIR/put.last.txt" - echo "${path##*/contents/}" >> "$STUB_PUT_DIR/branch_files" # file now on the branch + echo "blobsha${n}" + exit 0;; + POST:*/git/trees*) # tree create — the stdin body lists every bumped path + body=$(cat) + jq -r '.tree[].path' <<<"$body" > "$STUB_PUT_DIR/branch_files" + # Record base_tree so the suite can assert it is the TIP's TREE sha (resolved + # via GET git/commits below), NOT the tip COMMIT sha — the real Create-a-tree + # API rejects a commit sha, and a missing/invalid base_tree drops every other + # file in the caller repo. + jq -r '.base_tree // ""' <<<"$body" > "$STUB_PUT_DIR/branch_base_tree" + echo "treesha1" + exit 0;; + POST:*/git/commits*) # commit create — drain the body, return a commit sha + cat >/dev/null + echo "commitsha1" + exit 0;; + POST:*/git/refs*|PATCH:*/git/refs*) # point the bump branch at the commit exit 0;; esac @@ -126,6 +146,11 @@ if [[ "$path" == *"/contents/"* ]]; then fi b64=$(base64 < "$STUB_CONTENT_FILE" | tr -d '\n') printf '{"sha":"blobsha123","content":"%s"}' "$b64" +elif [[ "$path" == *"/git/commits/"* ]]; then + # Resolve the tip commit's TREE sha (distinct from the commit sha) — the script + # must pass THIS as base_tree, not the commit sha it was parented on. The stub + # discards --jq (see arg loop), so emit the post-`.tree.sha` value directly. + echo "maintreesha1" elif [[ "$path" == *"/git/refs/heads/"* ]]; then echo "1234567890abcdef1234567890abcdef12345678" else @@ -178,6 +203,12 @@ check "stale pin comment removed" "! grep -qF '# github-workflows#27 # exactly one trailing newline (#23): last byte is \n (tail -c1 strips to empty), # and the last two bytes are not both \n (tail -c2 keeps a non-newline byte). check "single trailing newline" "[[ -z \"\$(tail -c1 \"$PUT\")\" && -n \"\$(tail -c2 \"$PUT\")\" ]]" +# base_tree must be the tip's TREE sha (resolved via GET git/commits), NOT the +# tip COMMIT sha — a commit sha 422s the real Create-a-tree API, and a bad +# base_tree drops every other file in the caller repo (BE-3902). +BBT="${STUB_PUT_DIR}/branch_base_tree" +check "base_tree is the resolved tree sha" "[[ \"\$(cat \"$BBT\")\" == 'maintreesha1' ]]" +check "base_tree is NOT the commit sha" "! grep -qF '1234567890abcdef1234567890abcdef12345678' \"$BBT\"" # No open PR for the stable branch → the create path runs, not the edit path. check "opened a new PR (pr create called)" "grep -q '^pr-create' \"\$STUB_PUT_DIR/pr.log\"" check "did not edit (no open PR existed)" "! grep -q '^pr-edit' \"\$STUB_PUT_DIR/pr.log\"" @@ -310,18 +341,20 @@ check "'# v1' comment left intact" "grep -qF '# v1' \"$PUT\"" echo "== monorepo: TWO files in the SAME repo BOTH land on the one branch (BE-3896) ==" # A repo listed more than once (a monorepo pinning the reusable workflow from -# two workflow files) must land BOTH files on its single stable branch. The old -# per-entry loop reset the branch before each file, so the second file's reset -# discarded the first file's commit and the PR shipped only the last file — a -# silent partial bump. The stub now models the branch's file set (a reset -# truncates it, a PUT appends), so this asserts the branch keeps BOTH files. +# two workflow files) must land BOTH files on its single stable branch. Both are +# now built into ONE atomic commit (one tree carrying both blobs), so the branch +# holds them together or not at all. The stub records the tree's path list as +# the branch's file set, so this asserts the branch keeps BOTH files — the old +# per-entry loop reset the branch before each file and shipped only the last one +# (BE-3896), and the per-file PUT loop that replaced it could still leave a +# partial commit on failure (BE-3902). new_case mono STUB_CONTENT_FILE="$CR_FIXTURE" run_bump \ VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \ CALLERS_JSON='[{"repo":"Comfy-Org/secret-mono","file":".github/workflows/ci-a.yml","label":""},{"repo":"Comfy-Org/secret-mono","file":".github/workflows/ci-b.yml","label":""}]' BF="${STUB_PUT_DIR}/branch_files" check "exit 0" "[[ $RC -eq 0 ]]" -check "committed both files (2 PUTs)" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 2 ]]" +check "committed both files (2 blobs)" "[[ \$(cat \"\$STUB_PUT_DIR/count\") -eq 2 ]]" check "branch holds exactly two files" "[[ \$(wc -l < \"$BF\") -eq 2 ]]" check "first file present on the branch" "grep -q 'ci-a.yml' \"$BF\"" # the file the old code dropped check "second file present on the branch" "grep -q 'ci-b.yml' \"$BF\"" @@ -381,9 +414,10 @@ 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 "== same repo+file listed twice is de-duped to ONE commit (no stale-sha 409) ==" -# A repo listed twice for the same path must stage that file once; a second PUT -# with the now-stale pre-bump blob sha would 409 and fail the repo. +echo "== same repo+file listed twice is de-duped to ONE blob/tree entry ==" +# A repo listed twice for the same path must stage that file once; a duplicate +# tree entry for the same path is ambiguous (the atomic commit must carry each +# path exactly once), so the dedup keeps the commit well-formed. new_case dedup STUB_CONTENT_FILE="$CR_FIXTURE" run_bump \ VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \