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

[julien-carsique-sonarsource]

Summary

• Replaces the verify-after-write + retry approach from PREQ-7781 Detect and retry concurrent build_number increments #335 (closed) with a real atomic claim.
• gitar-bot's review on PREQ-7781 Detect and retry concurrent build_number increments #335 correctly found verify-after-write doesn't close the race: two runs that both read N and both PATCH N+1 both pass verification and claim the same number.
• Git ref creation is a genuine compare-and-swap (POST .../git/refs fails with 422 if the ref already exists). get_build_number.sh now claims a number by creating refs/build-locks/<N> — exclusively and deterministically.

Design

• build_number property is read only as a starting hint; correctness never depends on it being accurate.
• Loop: try to create refs/build-locks/<candidate>. Success = claimed. "Reference already exists" = collision, try the next candidate immediately. Any other API error is fatal (unchanged from before).
• After claiming, the property is PATCHed to the claimed value, best-effort, purely to keep the next run's starting hint fresh.
• refs/build-locks/* accumulate indefinitely; pruning is a follow-up, out of scope here (doesn't affect correctness).

Blocking dependency

Needs the Vault-issued build-number bot token to gain contents: write (to create refs) — it currently only has repository_custom_properties: write. That permission is defined once, shared by every repo using this pattern, so granting it widens that token's scope org-wide. See companion PR: SonarSource/re-terraform-aws-vault#9518, and PREQ-7781 for the tradeoff writeup.

Do not merge before the Vault PR is reviewed/merged and Mate Molnar has weighed in — this PR is non-functional without it (ref creation will 403).

Test plan

• Rewrote spec/get_build_number_spec.sh: no-contention claim, single-collision retry, exhausted retries under permanent contention, non-collision API errors treated as fatal, hint-update failures don't fail the run.
• All 7 examples pass (shellspec spec/get_build_number_spec.sh --shell bash)
• 100% line coverage on get_build_number.sh (kcov)
• shellcheck clean

[hashicorp-vault-sonar-prod]

PREQ-7781

[gitar-bot]

💡 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 👍 / 👎}

[gitar-bot]

💡 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 👍 / 👎}

[gitar-bot]

CI failed: 4 CI jobs failed across the test suites with a 403 Forbidden 'Resource not accessible by integration' error when attempting to claim build numbers via Git refs.

Overview

All 4 analyzed CI log failures point to the same root cause: the GitHub token lacks sufficient permissions (contents: write) to create or update Git references when running the newly introduced atomic build number claiming mechanism (get_build_number.sh).

Failures

Git Reference Creation Failure during Build Number Claim (confidence: high)

• Type: authentication
• Affected jobs: 92631533249, 92631537395, 92631525290, 92631765512
• Related to change: yes
• Root cause: The new atomic build number mechanism attempts to create or update Git references via the GitHub CLI/API, but the workflow or integration token lacks write permissions (contents: write) on the repository.
• Suggested fix: Update the GitHub Actions workflow configuration to include explicit permissions (permissions: contents: write) or ensure the token supplied has write access to manage Git references.

Summary

• Change-related failures: 4 jobs failed due to missing write permissions required by the new atomic build number claiming feature (PREQ-7781).
• Infrastructure/flaky failures: 0 infrastructure or flaky test failures.
• Recommended action: Add permissions: contents: write to the affected GitHub Actions workflow jobs so the integration can create and update Git refs successfully.

Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Replaces the verify-and-retry build number allocation with atomic Git ref creation to eliminate race conditions, backed by comprehensive ShellSpec test coverage. Consider tightening the collision error string matching and addressing potential exhaustion of retry attempts from stale hints.

💡 Bug: Collision detection relies on fragile English error string

📄 get-build-number/get_build_number.sh:20 📄 get-build-number/get_build_number.sh:44

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.

💡 Edge Case: Stale build_number hint can spuriously exhaust MAX_ATTEMPTS

📄 get-build-number/get_build_number.sh:34-48 📄 get-build-number/get_build_number.sh:61-64

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.

🤖 Prompt for agents

Code Review: Replaces the verify-and-retry build number allocation with atomic Git ref creation to eliminate race conditions, backed by comprehensive ShellSpec test coverage. Consider tightening the collision error string matching and addressing potential exhaustion of retry attempts from stale hints.

1. 💡 Bug: Collision detection relies on fragile English error string
   Files: get-build-number/get_build_number.sh:20, get-build-number/get_build_number.sh:44

   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.

2. 💡 Edge Case: Stale build_number hint can spuriously exhaust MAX_ATTEMPTS
   Files: get-build-number/get_build_number.sh:34-48, get-build-number/get_build_number.sh:61-64

   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.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}