Skip to content

PREQ-7781 Detect and retry concurrent build_number increments #335

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

Closed
julien-carsique-sonarsource wants to merge 1 commit into from
Closed

PREQ-7781 Detect and retry concurrent build_number increments #335

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
55 changes: 41 additions & 14 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 @@ -6,19 +6,46 @@ set -euo pipefail
: "${GITHUB_REPOSITORY:?}"
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"
# The custom-properties API has no conditional/atomic update (no If-Match support), so
# concurrent runs (e.g. GitHub Stacked PRs) can race on the read-increment-write cycle and
# claim the same number. Verify after writing and retry on a detected collision to close
# most of that window; see PREQ-7781.
MAX_ATTEMPTS="${MAX_ATTEMPTS:-10}"

get_property_value() {
gh api -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" --jq '.[] | select(.property_name == "build_number") | .value'
}

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))
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"

attempt=1
while true; do
BUILD_NUMBER=$(get_property_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
Comment on lines +26 to +29

NEXT_BUILD_NUMBER=$((BUILD_NUMBER + 1))
gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \
-f "properties[][property_name]=build_number" \
-f "properties[][value]=${NEXT_BUILD_NUMBER}"

CONFIRMED_BUILD_NUMBER=$(get_property_value)
if [[ "$CONFIRMED_BUILD_NUMBER" == "$NEXT_BUILD_NUMBER" ]]; then
echo "Incremented 'build_number' repository property to ${NEXT_BUILD_NUMBER}"
echo "${NEXT_BUILD_NUMBER}" > "$BUILD_NUMBER_FILE"
exit 0
fi
Comment on lines +31 to +41

@gitar-bot gitar-bot Bot Aug 6, 2026 •
edited
Loading

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: Verify-after-write misses symmetric same-value race

When two runs both read N and both PATCH N+1, each read-back returns N+1 and CONFIRMED == NEXT for both, so both pass verification and claim the same number — precisely the 'Multiple builds found' scenario the PR aims to fix. The check only catches a concurrent run that writes a different value between our PATCH and our read-back; it does not detect two runs writing the identical next value. This narrows but does not close the reported window. Consider adding random jitter before the initial read, or a post-write delay before verification so a competing writer's value can diverge, and document that this remains best-effort until the BUILD-11797 atomic-store migration.

Delay before verification so a same-value competing write is more likely to be overwritten and detected (still best-effort).:

NEXT_BUILD_NUMBER=$((BUILD_NUMBER + 1))
gh api --method PATCH -H "$GH_API_VERSION_HEADER" "$PROPERTIES_API_URL" \
  -f "properties[][property_name]=build_number" \
  -f "properties[][value]=${NEXT_BUILD_NUMBER}"

# Give a competing writer time to overwrite with a different value so the
# symmetric "both wrote N+1" collision becomes detectable on read-back.
sleep "0.$((RANDOM % 900 + 100))"
CONFIRMED_BUILD_NUMBER=$(get_property_value)

Was this helpful? React with 👍 / 👎

Comment on lines +31 to +41

if (( attempt >= MAX_ATTEMPTS )); then
echo "::error title=Build number race::Could not obtain a unique build number after ${MAX_ATTEMPTS} attempts; concurrent runs kept overwriting each other." >&2
exit 1
fi

echo "Concurrent update detected (expected ${NEXT_BUILD_NUMBER}, found ${CONFIRMED_BUILD_NUMBER}); retrying (attempt $((attempt + 1))/${MAX_ATTEMPTS})..."
sleep "0.$((RANDOM % 900 + 100))"
attempt=$((attempt + 1))
done
79 changes: 73 additions & 6 deletions spec/get_build_number_spec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,29 @@ End

Describe 'get_build_number.sh'
It 'should increment and return the build number'
export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_1.txt"
rm -f "$GH_GET_CALLS_FILE"
Mock gh
if [[ "$*" =~ "api --method PATCH" ]]; then
echo "gh $*"
elif [[ "$*" =~ "properties/values" ]]; then
echo '42'
count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1))
echo "$count" > "$GH_GET_CALLS_FILE"
if [[ "$count" -eq 1 ]]; then
echo '42'
else
echo '43'
fi
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 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 @@ -43,13 +47,76 @@ Describe 'get_build_number.sh'
End

It 'should handle empty build number'
export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_3.txt"
rm -f "$GH_GET_CALLS_FILE"
Mock gh
echo ''
if [[ "$*" =~ "api --method PATCH" ]]; then
echo "gh $*"
elif [[ "$*" =~ "properties/values" ]]; then
count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1))
echo "$count" > "$GH_GET_CALLS_FILE"
if [[ "$count" -eq 1 ]]; then
echo ''
else
echo '1'
fi
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"
End

It 'should retry when a concurrent run wins the race, then succeed'
export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_4.txt"
rm -f "$GH_GET_CALLS_FILE"
Mock gh
Comment on lines +74 to +77
if [[ "$*" =~ "api --method PATCH" ]]; then
echo "gh $*"
elif [[ "$*" =~ "properties/values" ]]; then
count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1))
echo "$count" > "$GH_GET_CALLS_FILE"
case "$count" in
1) echo '42' ;; # our initial read
2) echo '99' ;; # a concurrent run raced ahead of our write
3) echo '99' ;; # our retry read picks up their value
*) echo '100' ;; # our second write is now confirmed
esac
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be success
The output should include "Concurrent update detected (expected 43, found 99)"
The output should include "Incremented 'build_number' repository property to 100"
The path "$BUILD_NUMBER_FILE" should be file
The contents of file "$BUILD_NUMBER_FILE" should equal "100"
End

It 'should fail after exhausting retries if the race never resolves'
export MAX_ATTEMPTS=3
export GH_GET_CALLS_FILE="${TEMP_DIR}/gh_get_calls_5.txt"
rm -f "$GH_GET_CALLS_FILE"
Mock gh
if [[ "$*" =~ "api --method PATCH" ]]; then
echo "gh $*"
elif [[ "$*" =~ "properties/values" ]]; then
# Every read disagrees with what we just wrote, simulating permanent contention.
count=$(($(cat "$GH_GET_CALLS_FILE" 2>/dev/null || echo 0) + 1))
echo "$count" > "$GH_GET_CALLS_FILE"
echo "$((count * 100))"
else
echo "gh $*"
fi
End
When run script get-build-number/get_build_number.sh
The status should be failure
The output should include "Concurrent update detected"
The stderr should include "::error title=Build number race::Could not obtain a unique build number after 3 attempts"
End
End
Loading