Skip to content

PREQ-7781 Claim build numbers atomically via Git refs instead of verify-and-retry #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
julien-carsique-sonarsource wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

PREQ-7781 Claim build numbers atomically via Git refs instead of verify-and-retry #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ and set it as an environment variable named `BUILD_NUMBER`, and as a GitHub Acti

The build number is unique per workflow run ID. It is not incremented on workflow reruns.

Concurrent runs (e.g. several GitHub Stacked PRs opened at once) claim a build number atomically: the
action reads the `build_number` property as a starting hint, then claims that number exclusively by
creating a Git reference under `refs/build-locks/<number>` — ref creation fails if the ref already
exists, so it acts as a compare-and-swap. On a collision the action retries with the next number; no
two concurrent runs can ever claim the same number. The `build_number` property is still updated
afterwards, but only as a best-effort hint for where the next run should start searching — correctness
never depends on it. `refs/build-locks/*` accumulate over time and are not currently pruned; cleanup is
tracked separately and does not affect correctness.

During execution the action temporarily writes `.build_number.txt` at the repository root (for
`actions/cache`); the file is removed before the action completes. Do not track a file named
`.build_number.txt` in your repository.
Expand All @@ -94,7 +103,9 @@ The action authenticates `gh` with a Vault-issued GitHub token. It sets both `GI

#### Required Vault Permissions

- `build-number`: GitHub preset to read and write the build number property. This is built-in to the Vault `auth.github` permission.
- `build-number`: GitHub preset to read and write the build number property, and to create the
`refs/build-locks/*` references used to claim it atomically. This is built-in to the Vault
`auth.github` permission.

### Usage

Expand Down
52 changes: 47 additions & 5 deletions get-build-number/get_build_number.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,63 @@
set -euo pipefail

: "${GITHUB_REPOSITORY:?}"
: "${GITHUB_SHA:?}"
GH_API_VERSION_HEADER="X-GitHub-Api-Version: 2022-11-28"
BUILD_NUMBER_FILE="${BUILD_NUMBER_FILE:-.build_number.txt}"
PROPERTIES_API_URL="repos/${GITHUB_REPOSITORY}/properties/values"
REFS_API_URL="repos/${GITHUB_REPOSITORY}/git/refs"
# The custom-properties API has no conditional/atomic update (no If-Match support), so a plain
# GET-increment-PATCH on build_number races under concurrent runs (e.g. GitHub Stacked PRs) and can
# hand out the same number twice. Git ref creation IS atomic (it fails if the ref already exists), so
# it's used here as a compare-and-swap: exclusively claim refs/build-locks/<N> before trusting N.
# See PREQ-7781.
MAX_ATTEMPTS="${MAX_ATTEMPTS:-50}"

claim_build_number() {
gh api --method POST -H "$GH_API_VERSION_HEADER" "$REFS_API_URL" -f "ref=refs/build-locks/$1" -f "sha=${GITHUB_SHA}" 2>&1

@gitar-bot gitar-bot Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Bug: Collision detection relies on fragile English error string

Collision handling keys off the substring "Reference already exists" (get_build_number.sh:44). This depends on gh/GitHub's exact human-readable 422 message; if the wording, casing, or localization ever changes, every collision falls through to the fatal error branch (line 45) and the atomic-claim mechanism breaks entirely under normal concurrency rather than retrying. Consider matching on the HTTP status (e.g. capture gh api exit and use --include/checking for 422) or on the stable status/code field so detection doesn't hinge on prose.

Was this helpful? React with 👍 / 👎

}

echo "Fetching build number from repository properties..."
PROPERTIES_API_URL="repos/${GITHUB_REPOSITORY}/properties/values"
BUILD_NUMBER=$(gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq '.[] | select(.property_name == "build_number") | .value')
echo "Current build number from repo: ${BUILD_NUMBER:=0}"
if ! [[ "$BUILD_NUMBER" =~ ^[0-9]+$ ]]; then
echo "::error title=Invalid build number::Build number '${BUILD_NUMBER}' is not a valid positive integer." >&2
exit 1
fi

BUILD_NUMBER=$((BUILD_NUMBER + 1))
# BUILD_NUMBER above is only a starting hint for where to search; it may be stale (e.g. a concurrent
# run claimed further ahead and hasn't updated the property yet). Correctness never depends on it
# being accurate, only on the ref-creation compare-and-swap below.
attempt=1
CANDIDATE=$((BUILD_NUMBER + 1))
while true; do
RESPONSE=$(claim_build_number "$CANDIDATE") && CLAIM_STATUS=0 || CLAIM_STATUS=$?

if [[ "$CLAIM_STATUS" -eq 0 ]]; then
echo "Claimed build number ${CANDIDATE} (refs/build-locks/${CANDIDATE})"
break
fi

if [[ "$RESPONSE" != *"Reference already exists"* ]]; then
echo "::error title=Build number claim failed::${RESPONSE}" >&2
exit 1
fi

Comment on lines +34 to +48

@gitar-bot gitar-bot Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Stale build_number hint can spuriously exhaust MAX_ATTEMPTS

The claim loop scans linearly from BUILD_NUMBER+1 and gives up after MAX_ATTEMPTS (default 50) consecutive collisions (get_build_number.sh:35-57). Since refs/build-locks/* are never pruned and the property PATCH is best-effort, if the hint lags the true frontier by more than MAX_ATTEMPTS (e.g. after repeated PATCH failures or a burst of >50 concurrent claims), every candidate in the window collides and the run fails even though a free number exists just beyond the window. Consider seeding the starting candidate from the highest existing build-lock ref, or exponentially jumping the candidate on repeated collisions instead of a fixed +1 linear scan.

Was this helpful? React with 👍 / 👎

if (( attempt >= MAX_ATTEMPTS )); then
echo "::error title=Build number race::Could not claim a build number after ${MAX_ATTEMPTS} attempts (concurrent claims)." >&2
exit 1
fi

echo "Build number ${CANDIDATE} already claimed; trying $((CANDIDATE + 1)) (attempt $((attempt + 1))/${MAX_ATTEMPTS})..."
CANDIDATE=$((CANDIDATE + 1))
attempt=$((attempt + 1))
done

# Best-effort: keep the custom property as a hint for the next run's starting point. Correctness
# never depends on this succeeding or being accurate.
gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \
-f "properties[][property_name]=build_number" \
-f "properties[][value]=${BUILD_NUMBER}"
echo "Incremented 'build_number' repository property to ${BUILD_NUMBER}"
echo "${BUILD_NUMBER}" > "$BUILD_NUMBER_FILE"
-f "properties[][value]=${CANDIDATE}" \
|| echo "::warning title=Build number hint not updated::Failed to update the build_number property; this does not affect correctness."

echo "${CANDIDATE}" > "$BUILD_NUMBER_FILE"
107 changes: 97 additions & 10 deletions spec/get_build_number_spec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
eval "$(shellspec - -c) exit 1"

export GITHUB_REPOSITORY="my org/my-repo"
export GITHUB_SHA="deadbeefcafef00dfeed"
TEMP_DIR="${SHELLSPEC_TMPBASE:-/tmp}"
export BUILD_NUMBER_FILE="${TEMP_DIR}/build_number.txt"

Expand All @@ -10,26 +11,24 @@ Mock gh
End

Describe 'get_build_number.sh'
It 'should increment and return the build number'
It 'should claim the next build number when there is no contention'
Mock gh
if [[ "$*" =~ "api --method PATCH" ]]; then
if [[ "$*" == *"git/refs"* ]]; then
echo "gh $*"
elif [[ "$*" =~ "properties/values" ]]; then
elif [[ "$*" == *"--method PATCH"* ]]; then
echo "gh $*"
elif [[ "$*" == *"properties/values"* ]]; then
echo '42'
else
echo "gh $*"
fi
End
# shellcheck disable=SC2317
# preserve() { %preserve BUILD_NUMBER; }
# AfterRun preserve
When run script get-build-number/get_build_number.sh
The line 1 should include "Fetching build number"
The line 2 should equal "Current build number from repo: 42"
The line 3 should include "43"
The output should include "Claimed build number 43"
The path "$BUILD_NUMBER_FILE" should be file
The contents of file "$BUILD_NUMBER_FILE" should equal "43"
# The variable BUILD_NUMBER should equal "43"
End

It 'should return an error if BUILD_NUMBER is invalid'
Expand All @@ -44,12 +43,100 @@ Describe 'get_build_number.sh'

It 'should handle empty build number'
Mock gh
if [[ "$*" == *"properties/values"* && "$*" != *"--method PATCH"* ]]; then
echo ''
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be success
The line 2 should equal "Current build number from repo: 0"
# Ignore empty line from second call to gh
The line 4 should include "1"
The output should include "Claimed build number 1"
The contents of file "$BUILD_NUMBER_FILE" should equal "1"
End

It 'should retry when a concurrent run already claimed the next number, then succeed'
export GH_REFS_CALLS_FILE="${TEMP_DIR}/gh_refs_calls_retry.txt"
rm -f "$GH_REFS_CALLS_FILE"
Mock gh
if [[ "$*" == *"git/refs"* ]]; then
count=$(($(cat "$GH_REFS_CALLS_FILE" 2>/dev/null || echo 0) + 1))
echo "$count" > "$GH_REFS_CALLS_FILE"
if [[ "$count" -eq 1 ]]; then
echo '{"message":"Reference already exists"}' >&2
exit 1
else
echo "gh $*"
fi
elif [[ "$*" == *"--method PATCH"* ]]; then
echo "gh $*"
elif [[ "$*" == *"properties/values"* ]]; then
echo '42'
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be success
The output should include "Build number 43 already claimed; trying 44"
The output should include "Claimed build number 44"
The path "$BUILD_NUMBER_FILE" should be file
The contents of file "$BUILD_NUMBER_FILE" should equal "44"
End

It 'should fail after exhausting retries under permanent contention'
export MAX_ATTEMPTS=3
Mock gh
if [[ "$*" == *"git/refs"* ]]; then
echo '{"message":"Reference already exists"}' >&2
exit 1
elif [[ "$*" == *"properties/values"* ]]; then
echo '42'
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be failure
The output should include "already claimed"
The stderr should include "::error title=Build number race::Could not claim a build number after 3 attempts"
End

It 'should fail immediately on an unexpected API error, not treat it as a collision'
Mock gh
if [[ "$*" == *"git/refs"* ]]; then
echo '{"message":"Internal Server Error"}' >&2
exit 1
elif [[ "$*" == *"properties/values"* ]]; then
echo '42'
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be failure
The output should include "Current build number from repo: 42"
The stderr should include "::error title=Build number claim failed::"
The stderr should include "Internal Server Error"
End

It 'should still succeed even if updating the build_number property hint fails'
Mock gh
if [[ "$*" == *"git/refs"* ]]; then
echo "gh $*"
elif [[ "$*" == *"--method PATCH"* ]]; then
exit 1
elif [[ "$*" == *"properties/values"* ]]; then
echo '42'
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be success
The output should include "Claimed build number 43"
The output should include "Build number hint not updated"
The contents of file "$BUILD_NUMBER_FILE" should equal "43"
End
End
Loading