fix: promote tagged release commit to main and stabilize release checks#584
fix: promote tagged release commit to main and stabilize release checks#584
Conversation
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Claude finished @lourou's task —— View job Code Review Complete ✓Tasks
SummaryThis PR makes significant improvements to the release pipeline with well-thought-out safety guardrails. The changes are solid overall, but I've identified several issues ranging from critical bugs to best practice improvements. Critical Issues: 1 bug that will cause workflow failures Critical Issues1. Bug: Summary step will fail when git command unavailable (promote-to-main.yml:98)Severity: Critical — will cause workflow failure RELEASE_SHA_SHORT=$(git rev-parse --short "$RELEASE_SHA")Problem: This runs in the Fix: - name: Summary
run: |
TAG="${{ steps.version.outputs.tag }}"
if [[ "$TAG" == *-* ]]; then
echo "## Promotion Skipped" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
echo "Prerelease **$TAG** — no promotion to main." >> $GITHUB_STEP_SUMMARY
else
RESULT="${{ steps.promote_main.outputs.result }}"
RELEASE_SHA="${{ steps.resolve.outputs.release_sha }}"
RELEASE_SHA_SHORT=$(echo "$RELEASE_SHA" | cut -c1-7) # Use string manipulation instead of git
if [ "$RESULT" = "up_to_date" ]; then
echo "## No Promotion Needed" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
echo "main is already at release commit **$RELEASE_SHA_SHORT** for tag **$TAG**." >> $GITHUB_STEP_SUMMARY
else
echo "## main Promoted" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
echo "main fast-forwarded to release commit **$RELEASE_SHA_SHORT** for tag **$TAG**." >> $GITHUB_STEP_SUMMARY
fi
fiHigh Priority Issues2. Logic Issue: Tag resolution from workflow_run event (promote-to-main.yml:22)Severity: High — may not work as expected in all cases TAG="${{ github.event.workflow_run.head_branch }}"Problem: Recommended Fix: - name: Extract tag from workflow_run
id: version
run: |
# workflow_run doesn't directly expose the tag, but we can extract it from the display title
# or alternatively query the workflow run for its triggering ref
RUN_ID="${{ github.event.workflow_run.id }}"
TAG=$(gh api "/repos/${{ github.repository }}/actions/runs/${RUN_ID}" --jq '.head_branch')
# Verify it looks like a tag (not a branch)
if [[ ! "$TAG" =~ ^[0-9]+\.[0-9]+\.[0-9]+([-.].*)?$ ]]; then
echo "::error::Expected a version tag but got: $TAG"
exit 1
fi
echo "tag=$TAG" >> $GITHUB_OUTPUT
# ... rest of logicAlternative approach: Pass the tag explicitly via workflow outputs from 3. Logic Issue: SwiftLint change detection may miss files (swiftlint.yml:28)Severity: High — may skip linting when it shouldn't if git diff --name-only "origin/${{ github.base_ref }}...${{ github.sha }}" | grep -E '\.swift$' > /dev/null; thenProblem: The three-dot Recommended: Use two-dot syntax for clarity: git diff --name-only "origin/${{ github.base_ref }}..${{ github.sha }}" | grep -E '\.swift$'Both work in most cases, but 4. Security: Token permissions overly broad (promote-to-main.yml:17)Severity: Medium-High — violates least-privilege principle permissions:
contents: writeCurrent: Grants write to all repository content. Best Practice: GitHub Actions should use minimal permissions. While Recommendation: Document in a comment why broad permissions are needed: permissions:
contents: write # Required to push directly to main branchMedium Priority Issues5. Robustness: Race condition in concurrent workflow runs (auto-release.yml:8-10)Severity: Medium — unlikely but possible concurrency:
group: release-pipeline
cancel-in-progress: falseProblem: All release workflows share the Current handling: The Recommendation: This is already well-handled with the concurrency group. Consider adding a comment to explain the race protection: concurrency:
group: release-pipeline
cancel-in-progress: false # Queue instead of cancel to ensure every tag gets processed6. Robustness: Git fetch may be stale (promote-to-main.yml:51)Severity: Medium — could cause subtle issues git fetch origin --tags --forceProblem: Fetches tags but not refs. Later, on line 68, you Recommendation: Fetch both tags and full history: git fetch origin --tags --force
git fetch origin main dev # Ensure we have recent historyOr use: git fetch origin --tags --force --unshallow || true7. Robustness: Fast-forward push could fail on race (promote-to-main.yml:83)Severity: Medium — could fail if main is updated during the workflow git push origin "$RELEASE_SHA":refs/heads/mainProblem: If another workflow (or person) pushes to main between the ancestor check (line 78) and the push (line 83), this will fail. The workflow assumes it has exclusive access to main via the concurrency group, which is correct, but there's no explicit check. Recommendation: Add # First set remote tracking
git fetch origin main
git branch -f main "$RELEASE_SHA"
git push --force-with-lease=main:$MAIN_SHA origin main
# Or simpler - use git push with explicit refspec and --force-with-lease
git push --force-with-lease origin "$RELEASE_SHA":refs/heads/mainWait — on second thought,
Verdict: The current approach is acceptable given the concurrency controls. No change needed, but consider adding a comment: # Safe to push directly because:
# 1. Concurrency group prevents races
# 2. Verified fast-forward above
# 3. GitHub Actions is in bypass list for main branch ruleset
git push origin "$RELEASE_SHA":refs/heads/mainLow Priority / Best Practices8. Documentation: Unclear legend placement (docs/release-process.md:22-24)Issue: The "Legend" appears after the flowchart, but it explains symbols used in the flowchart. Readers have to read ahead to understand the diagram. Recommendation: Move the legend immediately before or as part of the Overview section: ## Overview
Releases are fully automated via GitHub Actions. A single `make release` command triggers the entire pipeline.
**Version Types:**
- `X.Y.Z` — Production release (promotes to `main`, deploys to App Store)
- `X.Y.Z-dev.N` — Prerelease (no promotion, optional TestFlight distribution)
[flowchart here]9. Documentation: Missing failure scenario (docs/release-process.md:126-128)Issue: The troubleshooting section mentions tag resolution failure, but doesn't explain the most common cause: Recommendation: Add a note: **`promote-to-main.yml` fails with "Could not resolve commit for tag"**
The tag referenced by the Auto Release run is missing or not yet visible in the clone. This can happen if:
1. The tag was deleted manually after creation
2. The workflow_run event raced with tag propagation (rare)
**Fix:** Re-run the workflow after confirming the tag exists remotely with `git ls-remote --tags origin`.Positive Observations
Testing RecommendationsBefore merging, verify these scenarios:
Summary of Recommendations
|

Summary
This PR revises the release pipeline to keep automation while tightening safety and CI reliability.
What changed
promote-to-main.ymland changed promotion to use the exact tagged release commit (not the movingdevtip).mainis ancestor of that SHA before fast-forwardswiftlint.ymlso required checks remain stable:main/dev.swiftfiles changedrelease-pipelineconcurrency toauto-release.yml.create-release.ymlrelease-tag-on-merge.ymlauto-release.ymldocs/release-process.mdto match actual behavior end-to-end.Why
devif new commits land after tagging.One-time repo setup still required
For direct CI fast-forward of
main, add GitHub Actions to themainruleset bypass list:Settings → Rules → Rulesets → main ruleset → Bypass list.