fix(bump-callers): commit each repo's files atomically via the Git Data API (BE-3902)

.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=<branch>`). 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=<branch>`), 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/<pid>/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

.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 \