Skip to content

Fix race condition and versioning in beta deployment #202

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

Open
nschimme wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix race condition and versioning in beta deployment #202

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
80 changes: 63 additions & 17 deletions .github/workflows/deploy-beta.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ on:
types:
- completed

concurrency:
group: deploy-beta
cancel-in-progress: true

jobs:
check-builds:
runs-on: ubuntu-latest
Expand All @@ -23,6 +27,19 @@ jobs:
const commitSha = context.payload.workflow_run.head_sha;
console.log(`Checking build status for commit: ${commitSha}`);

// Ensure this is the latest commit on master
const { data: branch } = await github.rest.repos.getBranch({
owner: context.repo.owner,
repo: context.repo.repo,
branch: 'master'
});

if (branch.commit.sha !== commitSha) {
console.log(`Commit ${commitSha} is not the head of master (${branch.commit.sha}). Skipping deployment to avoid downgrading.`);
core.setOutput('all_builds_successful', 'false');
return;
}

let allSuccessful = true;
const missingOrFailed = [];
const successfulRunIds = {};
Expand Down Expand Up @@ -152,29 +169,58 @@ jobs:
ls -R ${{ github.workspace }}/release_artifacts
echo "--- End of artifact listing ---"

- name: Delete Previous Pre-release
- name: Update Beta Release
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_REPOSITORY: ${{ github.repository }}
RELEASE_FILES: ${{ steps.download_artifacts.outputs.release_files }}
COMMIT_SHA: ${{ github.event.workflow_run.head_sha }}
run: |
set -e
RELEASE_TAG="beta"
if gh release view "$RELEASE_TAG" --repo "$GITHUB_REPOSITORY" > /dev/null 2>&1; then
echo "Beta release '$RELEASE_TAG' exists. Deleting the release..."
gh release delete "$RELEASE_TAG" --repo "$GITHUB_REPOSITORY" --yes || { echo "Failed to delete release: $RELEASE_TAG"; } # Add error handling
gh api -X DELETE repos/$GITHUB_REPOSITORY/git/refs/tags/$RELEASE_TAG || true
echo "Updating Beta release for commit: $COMMIT_SHA"

# Check if release exists
if ! gh release view "$RELEASE_TAG" > /dev/null 2>&1; then
echo "Creating new Beta release..."
gh release create "$RELEASE_TAG" --target "$COMMIT_SHA" --title "MMapper Beta" --notes "Latest development build. Back up your map before using." --prerelease
else
echo "Beta release '$RELEASE_TAG' does not exist. No need to delete."
echo "Updating existing Beta release..."
gh release edit "$RELEASE_TAG" --target "$COMMIT_SHA" --title "MMapper Beta" --notes "Latest development build. Back up your map before using."
fi

- name: Update Beta Pre-release
uses: softprops/action-gh-release@v3
with:
tag_name: beta
name: MMapper Beta
prerelease: true
draft: false
generate_release_notes: true
body: "Latest development build. Back up your map before using.\n\n"
files: ${{ steps.download_artifacts.outputs.release_files }}
token: ${{ secrets.GITHUB_TOKEN }}
# 1. Get list of existing assets before upload
echo "Fetching existing assets..."
existing_assets=$(gh release view "$RELEASE_TAG" --json assets --jq '.assets[].name' || echo "")

# 2. Upload new assets
echo "Uploading new assets..."
# Convert comma-separated string to array for gh command
IFS=',' read -r -a files_array <<< "$RELEASE_FILES"
if [ ${#files_array[@]} -eq 0 ]; then
echo "No files found to upload!"
exit 1
fi
gh release upload "$RELEASE_TAG" "${files_array[@]}" --clobber
Comment on lines +198 to +204

@sourcery-ai sourcery-ai Bot Apr 15, 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.

issue: Avoid treating an empty RELEASE_FILES string as a single empty filename when building files_array.

With IFS=',' read -r -a files_array <<< "$RELEASE_FILES", an empty RELEASE_FILES still produces files_array of length 1 containing an empty string. That means the length check never fails and gh release upload is called with an empty argument.

Consider handling the empty case before splitting, e.g.:

if [ -z "$RELEASE_FILES" ]; then
  echo "No files found to upload!"
  exit 1
fi
IFS=',' read -r -a files_array <<< "$RELEASE_FILES"

or otherwise filter out empty entries before calling gh release upload so you don’t pass empty paths.


# 3. Get names of newly uploaded assets
new_asset_names=()
for f in "${files_array[@]}"; do
new_asset_names+=("$(basename "$f")")
done

# 4. Delete assets that were there but aren't in the new set
echo "Cleaning up obsolete assets..."
for old_asset in $existing_assets; do
keep=false
for new_asset in "${new_asset_names[@]}"; do
if [[ "$old_asset" == "$new_asset" ]]; then
keep=true
break
fi
done
if [ "$keep" = false ]; then
echo "Deleting obsolete asset: $old_asset"
gh release delete-asset "$RELEASE_TAG" "$old_asset" --yes
fi
done
Comment on lines +223 to +226

@sourcery-ai sourcery-ai Bot Apr 15, 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.

suggestion (bug_risk): Make obsolete-asset deletion tolerant of per-asset failures to avoid aborting the whole update.

With set -e enabled, any failure in gh release delete-asset for a single asset will abort the whole step and can leave the beta release in a partially updated state. Since this is a cleanup step, it should tolerate per-asset failures.

Consider ignoring deletion failures, for example:

echo "Deleting obsolete asset: $old_asset"
if ! gh release delete-asset "$RELEASE_TAG" "$old_asset" --yes; then
  echo "Warning: failed to delete obsolete asset: $old_asset" >&2
fi

or by appending || echo "Warning: ..." so set -e doesn’t terminate the job on these non-critical errors.

Suggested change
echo "Deleting obsolete asset: $old_asset"
gh release delete-asset "$RELEASE_TAG" "$old_asset" --yes
fi
done
echo "Deleting obsolete asset: $old_asset"
if ! gh release delete-asset "$RELEASE_TAG" "$old_asset" --yes; then
echo "Warning: failed to delete obsolete asset: $old_asset" >&2
fi
fi
done

Loading