[PR #46] [MERGED] fix(bump-callers): commit each repo's files atomically via the Git Data API (BE-3902) #58

New issue

Closed

opened 2026-10-26 22:35:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/46
Author: @mattmillerai
Created: 7/21/2026
Status: ✅ Merged
Merged: 7/21/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-3902-atomic-git-data-commit

📝 Commits (8)

eec9b38 fix(bump-callers): reuse one open PR per repo via a stable branch (BE-3882)
92330d8 fix(bump-callers): guard existing-PR lookup + fail loud on create error (BE-3882)
4455be2 fix(bump-callers): commit all of a repo's files onto one bump branch (BE-3896)
214a1a7 fix(bump-callers): harden per-file staging against partial/erroneous bumps (BE-3896)
cc45841 fix(bump-callers): commit each repo's files atomically via the Git Data API (BE-3902)
88f3294 fix(bump-callers): pass a tree SHA (not commit SHA) as base_tree (BE-3902)
5567aa2 Merge remote-tracking branch 'origin/main' into matt/be-3902-atomic-git-data-commit
17b8445 Merge remote-tracking branch 'origin/main' into matt/be-3902-atomic-git-data-commit

📊 Changes

2 files changed (+152 additions, -74 deletions)

View changed files

📝 .github/bump-callers/bump-callers.sh (+90 -46)
📝 .github/bump-callers/tests/test_bump_callers.sh (+62 -28)

📄 Description

STACKED — merging lands on matt/be-3896-multi-file-per-repo (owned by @mattmillerai, PR #45), NOT main. This PR is based on PR #45's branch. Merge #45 first; GitHub then auto-retargets this to main. Do not "merge to main" directly.

ELI-5

When we bump a caller repo, it might pin our reusable workflow from more than one file. The current code commits those files one at a time (a separate API call per file). If the 3rd call fails after the 1st and 2nd already landed, the branch is left with a partial bump — and if an open PR points at that branch, someone could merge a half-done bump. This PR makes all of a repo's files land in one commit or none: we build the whole commit off to the side (blobs → tree → commit) and only then move the branch to it in a single step. A failure anywhere leaves the branch untouched.

What changed

bump_repo() Pass 2 no longer loops PUT repos/<repo>/contents/<file> per file. Instead, for each staged file it:

POST git/blobs — create a blob for the new content.
POST git/trees — build one tree with base_tree=MAIN_SHA (the resolved default-branch tip) carrying all staged blobs, so every other path in the repo is preserved and only the bumped files appear.
POST git/commits — create one commit with that tree, parented on MAIN_SHA (clean "bump to @SHORT" diff vs the default branch).
POST/PATCH git/refs/heads/<branch> — point the stable bump branch at the finished commit (create if new, else force). The ref moves in one step from its old state to the complete commit; it never transiently sits at an empty/partial tree.

Because the commit is fully built before the ref moves, a failure at any step leaves only dangling (GC-able) blobs/tree/commit and the branch untouched — all-or-nothing. This closes the residual BE-3896 partial-bump window: a mid-sequence failure that previously left earlier files committed on the branch (and, if an open PR pointed at it, a partial-yet-mergeable bump).

Test changes

tests/test_bump_callers.sh — the gh stub now models the Git Data API calls instead of the per-file Contents PUT:

POST git/blobs → captures each blob's decoded content (put.$n.txt / put.last.txt; count = number of blobs) — same content assertions as before.
POST git/trees → records the tree's path list as branch_files (the atomic branch's final file set).
POST git/commits → returns a commit sha.
POST/PATCH git/refs → points the branch (no-op for file-set modeling).

The BE-3896 monorepo assertion is preserved: both files still land on the one branch (now via one tree carrying both blobs). All 69 assertions pass; shellcheck -x is clean.

Verification

shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh → clean.
bash .github/bump-callers/tests/test_bump_callers.sh → 69 passed, 0 failed.

Judgment calls

Tree entry mode 100644. Git tree entries need an explicit file mode; the bumped files are .github/workflows/*.yml callers, which are never executable, so 100644 is correct. (The old Contents PUT preserved mode implicitly; preserving it here would require an extra read of each existing tree entry's mode for no real-world benefit.)
One commit instead of N. The branch now carries a single "bump to @SHORT" commit rather than one commit per file. The PR diff vs the default branch is identical; reviewers just see one commit.
Commit author is the authenticated token identity (the Cloud Code Bot app token), exactly as the previous Contents PUT — no author/committer fields are set, so no attribution change.
Stacked on PR #45 because the code this modifies (bump_repo() Pass 2's per-file PUT loop) only exists on PR #45's branch, not on main.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/46 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/21/2026 **Status:** ✅ Merged **Merged:** 7/21/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-3902-atomic-git-data-commit` --- ### 📝 Commits (8) - [`eec9b38`](https://github.com/Comfy-Org/github-workflows/commit/eec9b380a3340ccd6564f45c34740dd45048350f) fix(bump-callers): reuse one open PR per repo via a stable branch (BE-3882) - [`92330d8`](https://github.com/Comfy-Org/github-workflows/commit/92330d88f95b137d2c1ca6430bf663b0d68eb1dd) fix(bump-callers): guard existing-PR lookup + fail loud on create error (BE-3882) - [`4455be2`](https://github.com/Comfy-Org/github-workflows/commit/4455be201228c4fb017bbfa4e55f8816b6dca5a5) fix(bump-callers): commit all of a repo's files onto one bump branch (BE-3896) - [`214a1a7`](https://github.com/Comfy-Org/github-workflows/commit/214a1a7bd843fd97848495cd31c4c8c1fe95d722) fix(bump-callers): harden per-file staging against partial/erroneous bumps (BE-3896) - [`cc45841`](https://github.com/Comfy-Org/github-workflows/commit/cc45841c910db0edaaebbc22af77d744deb83edf) fix(bump-callers): commit each repo's files atomically via the Git Data API (BE-3902) - [`88f3294`](https://github.com/Comfy-Org/github-workflows/commit/88f3294ddcc2503b22f530c28032d28de9f86a0c) fix(bump-callers): pass a tree SHA (not commit SHA) as base_tree (BE-3902) - [`5567aa2`](https://github.com/Comfy-Org/github-workflows/commit/5567aa25608b433c29770f44d80191251869ae00) Merge remote-tracking branch 'origin/main' into matt/be-3902-atomic-git-data-commit - [`17b8445`](https://github.com/Comfy-Org/github-workflows/commit/17b8445115bf1db5b1c3f9cb01bfcbbf6cc5980a) Merge remote-tracking branch 'origin/main' into matt/be-3902-atomic-git-data-commit ### 📊 Changes **2 files changed** (+152 additions, -74 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/bump-callers.sh` (+90 -46) 📝 `.github/bump-callers/tests/test_bump_callers.sh` (+62 -28) </details> ### 📄 Description > **STACKED — merging lands on `matt/be-3896-multi-file-per-repo` (owned by @mattmillerai, PR #45), NOT `main`.** This PR is based on PR #45's branch. Merge #45 first; GitHub then auto-retargets this to `main`. Do not "merge to main" directly. ## ELI-5 When we bump a caller repo, it might pin our reusable workflow from more than one file. The current code commits those files **one at a time** (a separate API call per file). If the 3rd call fails after the 1st and 2nd already landed, the branch is left with a *partial* bump — and if an open PR points at that branch, someone could merge a half-done bump. This PR makes all of a repo's files land in **one commit or none**: we build the whole commit off to the side (blobs → tree → commit) and only *then* move the branch to it in a single step. A failure anywhere leaves the branch untouched. ## What changed `bump_repo()` Pass 2 no longer loops `PUT repos/<repo>/contents/<file>` per file. Instead, for each staged file it: 1. `POST git/blobs` — create a blob for the new content. 2. `POST git/trees` — build **one** tree with `base_tree=MAIN_SHA` (the resolved default-branch tip) carrying all staged blobs, so every *other* path in the repo is preserved and only the bumped files appear. 3. `POST git/commits` — create **one** commit with that tree, parented on `MAIN_SHA` (clean "bump to @SHORT" diff vs the default branch). 4. `POST`/`PATCH git/refs/heads/<branch>` — point the stable bump branch at the finished commit (create if new, else force). The ref moves in one step from its old state to the complete commit; it never transiently sits at an empty/partial tree. Because the commit is fully built before the ref moves, a failure at any step leaves only dangling (GC-able) blobs/tree/commit and the branch untouched — **all-or-nothing**. This closes the residual BE-3896 partial-bump window: a mid-sequence failure that previously left earlier files committed on the branch (and, if an open PR pointed at it, a partial-yet-mergeable bump). ## Test changes `tests/test_bump_callers.sh` — the `gh` stub now models the Git Data API calls instead of the per-file Contents PUT: - `POST git/blobs` → captures each blob's decoded content (`put.$n.txt` / `put.last.txt`; `count` = number of blobs) — same content assertions as before. - `POST git/trees` → records the tree's path list as `branch_files` (the atomic branch's final file set). - `POST git/commits` → returns a commit sha. - `POST`/`PATCH git/refs` → points the branch (no-op for file-set modeling). The BE-3896 monorepo assertion is preserved: **both** files still land on the one branch (now via one tree carrying both blobs). All 69 assertions pass; `shellcheck -x` is clean. ## Verification - `shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh` → clean. - `bash .github/bump-callers/tests/test_bump_callers.sh` → **69 passed, 0 failed**. ## Judgment calls - **Tree entry mode `100644`.** Git tree entries need an explicit file mode; the bumped files are `.github/workflows/*.yml` callers, which are never executable, so `100644` is correct. (The old Contents PUT preserved mode implicitly; preserving it here would require an extra read of each existing tree entry's mode for no real-world benefit.) - **One commit instead of N.** The branch now carries a single "bump to @SHORT" commit rather than one commit per file. The PR *diff* vs the default branch is identical; reviewers just see one commit. - **Commit author** is the authenticated token identity (the Cloud Code Bot app token), exactly as the previous Contents PUT — no author/committer fields are set, so no attribution change. - **Stacked on PR #45** because the code this modifies (`bump_repo()` Pass 2's per-file PUT loop) only exists on PR #45's branch, not on `main`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:49 +00:00