fix(devops): stop branch pushes from evicting queued main deploys - #4954
Conversation
All push-triggered deploy runs shared the single concurrency group 'staging'. GitHub keeps only one pending run per group - each new run cancels the previously pending one, even across branches - so a feature-branch push could evict a queued main deploy and main.openfront.dev silently stayed stale until the next merge. Scope the concurrency group to the deploy target instead: per branch for pushes, a dedicated group for the nightly, per host for manual dispatches (unchanged). The old global group was also accidentally serializing the staging host's shared state, so make the now-possible overlap safe in deploy.sh: give the remote update script a unique per-deploy filename (same pattern the env file already uses) instead of a fixed path concurrent deploys overwrote while one might be executing it, and run the host-side update (container swap, docker prune) under flock so only the CI-side image builds actually run in parallel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughDeployment concurrency now varies by event and target. Remote deployment scripts now use randomized filenames, host-wide locking, a 900-second wait limit, and cleanup after execution. ChangesDeployment concurrency
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy.sh`:
- Line 115: Replace the RANDOM-based naming in the deployment setup around
REMOTE_UPDATE_SCRIPT with collision-resistant, per-run filenames for the remote
update script and environment file. Prefer atomic remote mktemp allocation, or
reuse a unique run identifier consistently in both filenames, ensuring
concurrent deployments cannot overwrite one another before flock is acquired.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d167a8b-a0f2-4a3d-96bf-804b5ec8b9cc
📒 Files selected for processing (2)
.github/workflows/deploy.ymldeploy.sh
| # Randomize the remote script name so concurrent deployments (different | ||
| # branches share the staging host) don't overwrite each other's copy while | ||
| # one of them is executing it. | ||
| REMOTE_UPDATE_SCRIPT="$REMOTE_UPDATE_PATH/update-openfront-${SUBDOMAIN}-${RANDOM}.sh" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- deploy.sh relevant sections ---'
sed -n '90,190p' deploy.sh
printf '%s\n' '--- deployment references ---'
rg -n --glob '!node_modules' --glob '!dist' 'REMOTE_UPDATE_SCRIPT|REMOTE_UPDATE_PATH|SUBDOMAIN|deploy\.sh|flock' .github . 2>/dev/null | head -240Repository: openfrontio/OpenFrontIO
Length of output: 7430
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- deploy.sh validation and setup ---'
sed -n '1,75p' deploy.sh
printf '%s\n' '--- deploy workflow inputs and deployment step ---'
sed -n '40,110p' .github/workflows/deploy.yml
sed -n '165,190p' .github/workflows/deploy.yml
printf '%s\n' '--- release deployment contexts ---'
sed -n '60,95p' .github/workflows/release.yml
sed -n '120,145p' .github/workflows/release.yml
sed -n '175,195p' .github/workflows/release.yml
sed -n '228,248p' .github/workflows/release.yml
printf '%s\n' '--- standalone Bash RANDOM probe ---'
bash -c '
min=32768
max=-1
for i in $(seq 1 200000); do
n=$RANDOM
(( n < min )) && min=$n
(( n > max )) && max=$n
done
printf "observed_min=%s observed_max=%s documented_upper_bound=32767\n" "$min" "$max"
'Repository: openfrontio/OpenFrontIO
Length of output: 12712
🏁 Script executed:
#!/bin/bash
set -e
bash -c '
min=32768
max=-1
for ((i=0; i<200000; i++)); do
n=$RANDOM
(( n < min )) && min=$n
(( n > max )) && max=$n
done
printf "observed_min=%s observed_max=%s documented_upper_bound=32767\n" "$min" "$max"
'
python3 - <<'PY'
from math import comb
space = 32768
for concurrent in (2, 4, 8, 16):
no_collision = 1.0
for i in range(concurrent):
no_collision *= (space - i) / space
print(f"concurrent={concurrent} collision_probability={1-no_collision:.12%}")
PYRepository: openfrontio/OpenFrontIO
Length of output: 429
Use a collision-resistant deployment identifier.
$RANDOM can repeat within its 15-bit range. The script and environment file are uploaded before flock acquires the host lock, so concurrent deployments can overwrite each other’s files.
Use an atomic remote mktemp allocation, or include a unique per-run identifier in both filenames.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy.sh` at line 115, Replace the RANDOM-based naming in the deployment
setup around REMOTE_UPDATE_SCRIPT with collision-resistant, per-run filenames
for the remote update script and environment file. Prefer atomic remote mktemp
allocation, or reuse a unique run identifier consistently in both filenames,
ensuring concurrent deployments cannot overwrite one another before flock is
acquired.
Source: MCP tools
Description:
Merged PRs sometimes never reached main.openfront.dev. All push-triggered deploy runs shared the single concurrency group
staging, and GitHub keeps only one pending run per group — each new run cancels the previously pending one, even across branches. So a feature-branch push could evict a queued main deploy, and main.openfront.dev silently stayed stale until the next merge (the nightly doesn't help — it deploys to nightly.openfront.dev).Real occurrence (Aug 10): the deploys for #4916 (21:38 UTC) and #4913 (21:39 UTC) were both cancelled after being bumped out of the queue by later pushes; main.openfront.dev only caught up because #4923 happened to merge two minutes later.
Changes
.github/workflows/deploy.yml— scope the concurrency group to the deploy target instead of one shared group:staging-<branch>(so only a newer push to the same branch supersedes a queued deploy — which is the desired latest-wins behavior)staging-nightlydeploy.sh— the old global group was accidentally serializing shared state on the staging host, so make the now-possible overlap safe:update-openfront-<subdomain>-$RANDOM.sh, same pattern the env file already uses) instead of a fixed path that concurrent deploys overwrote while another might be executing it. Cleaned up after a successful run.flock -w 900, so the container swap anddocker image/container pruneinupdate.shstay serialized per host. Only the CI-side image builds (the slow part) run in parallel; the 15-min lock wait plus build fits inside the job's 30-min timeout, and the commit.txt readiness poll only starts after the swap completes.Not touched, but noting: the
VERSION_TAG: latestenv in the Deploy step is dead (build-deploy.shgenerates its own timestamp tag and shadows it), andbuild.shtags every build:latest— with parallel builds that tag is no longer guaranteed to be the newest push, though nothing in this repo consumes it for deploys.🤖 Generated with Claude Code