Fix race condition in release workflow by removing unsafe resume options
Summary
Removes the ability to start the release workflow from build-artifacts or do-release steps to prevent builds from running without proper merge verification. This eliminates a race condition where builds could execute before the develop→main PR was verified and merged.
Problem
The previous workflow allowed starting from any step, including build-artifacts and do-release. When starting from these later steps, the wait-for-main-pr verification job would be skipped, but build jobs would still run because they accepted needs.wait-for-main-pr.result == 'skipped'. This created a safety gap where:
Builds could run without verifying the develop→main merge succeeded
No checks confirmed the PR passed status checks before building
Rollback wouldn't trigger properly if builds failed
Solution
Removed build-artifacts and do-release from start_from_step options - workflow can now only start from:
bump-version (default)
merge-develop-to-main
Updated all job conditions to remove references to the removed steps:
wait-for-main-pr - lines 424-432
All build jobs (build-windows, build-debian, build-arch, build-rhel) - require wait-for-main-pr.result == 'success' only
rollback-on-build-failure - lines 994-1006
do-release - lines 1071-1083
upload-s3 - lines 1148-1160
Outcomes ✅ Cannot bypass merge verification - No way to start workflow past the merge verification step ✅ Build jobs require successful PR verification - All builds check needs.wait-for-main-pr.result == 'success' ✅ Simpler workflow logic - Fewer edge cases and conditional branches to maintain ✅ Can still resume from merge step - Supports restarting from merge-develop-to-main if needed
Testing
YAML syntax validated with poetry run python -c "import yaml; yaml.safe_load(open('.github/workflows/release.yml'))"
All job conditions updated consistently
Removed all references to deprecated step options
Fix race condition in release workflow by removing unsafe resume options
Summary
Removes the ability to start the release workflow from build-artifacts or do-release steps to prevent builds from running without proper merge verification. This eliminates a race condition where builds could execute before the develop→main PR was verified and merged.
Problem
The previous workflow allowed starting from any step, including build-artifacts and do-release. When starting from these later steps, the wait-for-main-pr verification job would be skipped, but build jobs would still run because they accepted needs.wait-for-main-pr.result == 'skipped'. This created a safety gap where:
Builds could run without verifying the develop→main merge succeeded
No checks confirmed the PR passed status checks before building
Rollback wouldn't trigger properly if builds failed
Solution
Removed build-artifacts and do-release from start_from_step options - workflow can now only start from:
bump-version (default)
merge-develop-to-main
Updated all job conditions to remove references to the removed steps:
wait-for-main-pr - [lines 424-432](vscode-webview://1at842bsak2p6q97kddh3jg9gfr5ql764vt5714hl6ck9n1j1fj6/.github/workflows/release.yml#L424-L432)
All build jobs (build-windows, build-debian, build-arch, build-rhel) - require wait-for-main-pr.result == 'success' only
rollback-on-build-failure - [lines 994-1006](vscode-webview://1at842bsak2p6q97kddh3jg9gfr5ql764vt5714hl6ck9n1j1fj6/.github/workflows/release.yml#L994-L1006)
do-release - [lines 1071-1083](vscode-webview://1at842bsak2p6q97kddh3jg9gfr5ql764vt5714hl6ck9n1j1fj6/.github/workflows/release.yml#L1071-L1083)
upload-s3 - [lines 1148-1160](vscode-webview://1at842bsak2p6q97kddh3jg9gfr5ql764vt5714hl6ck9n1j1fj6/.github/workflows/release.yml#L1148-L1160)
Outcomes
✅ Cannot bypass merge verification - No way to start workflow past the merge verification step
✅ Build jobs require successful PR verification - All builds check needs.wait-for-main-pr.result == 'success'
✅ Simpler workflow logic - Fewer edge cases and conditional branches to maintain
✅ Can still resume from merge step - Supports restarting from merge-develop-to-main if needed
Testing
YAML syntax validated with poetry run python -c "import yaml; yaml.safe_load(open('.github/workflows/release.yml'))"
All job conditions updated consistently
Removed all references to deprecated step options
This PR addresses a critical race condition in the release workflow by removing the ability to start from build-artifacts or do-release steps. This ensures that builds cannot run without proper merge verification, which is an important security and reliability improvement.
Strengths
1. Security and Safety Improvements
Eliminates bypass vulnerability: Previously, starting from build-artifacts or do-release would skip the wait-for-main-pr verification job, allowing builds to run without confirming the develop to main merge succeeded.
Enforces proper workflow order: Now the workflow can only start from bump-version or merge-develop-to-main, ensuring critical verification steps cannot be skipped.
Consistent validation: All build jobs now require needs.wait-for-main-pr.result == success (previously allowed skipped), ensuring verification actually ran.
2. Code Quality
Consistent updates: All affected job conditions were updated uniformly across all build jobs (Windows, Debian, Arch, RHEL) and downstream jobs (rollback, do-release, upload-s3).
Reduced complexity: Fewer conditional branches make the workflow easier to understand and maintain.
Well-documented: The PR description clearly explains the problem, solution, and outcomes.
3. Testing and Validation
YAML syntax validation was performed.
Clear test plan documented in PR description.
Good use of git notes for rollback tracking (from previous commits in this branch).
Observations and Considerations
1. Line Formatting Change
There is a minor formatting change at .github/workflows/release.yml:426-430 where the condition was split across multiple lines for readability. This is a stylistic improvement.
2. Verification Job Dependency
The change from allowing both success or skipped to requiring only success is the core fix. This prevents builds from running when wait-for-main-pr is skipped. Critical improvement.
3. Workflow Flexibility Trade-off
Previous state: Users could resume from any step (useful for debugging/retries after build failures).
New state: Users can only start from bump-version or merge-develop-to-main.
Question: What happens if all builds succeed but do-release fails (e.g., network issue) or S3 upload fails but release succeeds?
Currently, you would need to re-run from merge-develop-to-main, which would re-merge (or fail if already merged) and rebuild all artifacts. Consider documenting this behavior or adding a workflow_dispatch option to manually trigger just the release creation step in edge cases.
4. Concurrency Controls
The workflow has good concurrency controls. However, if a workflow is running and someone starts a new one from merge-develop-to-main while the first is at build stage, both could potentially proceed. The cancel-in-progress: false prevents cancellation but does not prevent the second from starting. This is likely acceptable given the PR checks and merge requirements, but worth noting.
Recommendations
Minor Suggestions
Consider adding a comment in the workflow explaining why build-artifacts and do-release were removed, for future maintainers at lines 28-35.
Document recovery procedures for how to handle partial failures:
What to do if release creation fails after successful builds
How to manually create a release from existing artifacts
S3 upload retry procedures
Consider a manual-release workflow for edge cases where you have verified artifacts but need to retry just the release/upload steps.
Testing Recommendations
Before merging, consider testing:
Happy path: bump-version to merge to builds to release
Start from merge-develop-to-main (verify it works correctly)
Verify attempting to skip to non-existent build-artifacts is properly prevented by the UI
Confirm rollback still works correctly when builds fail
Overall Assessment
Verdict: Approve with minor suggestions
This is a well-thought-out security improvement that closes a significant gap in the release workflow. The changes are:
Consistent across all affected jobs
Well-documented in the PR description
Focused on a specific problem with a clear solution
Properly tested (YAML validation)
The trade-off of reduced flexibility is acceptable given the security benefits. The suggestions above are minor enhancements rather than blockers.
Code Quality Score: 9/10
Clear intent and execution
Consistent implementation
Could benefit from inline documentation
Security Impact: Positive
Closes verification bypass vulnerability
Enforces proper workflow order
Risk Level: Low
Changes are localized to workflow conditionals
No changes to actual build/release logic
Reduces code paths (simpler = safer)
Great work on identifying and fixing this race condition!
## Pull Request Review - PR #33
### Summary
This PR addresses a critical race condition in the release workflow by removing the ability to start from build-artifacts or do-release steps. This ensures that builds cannot run without proper merge verification, which is an important security and reliability improvement.
---
## Strengths
### 1. Security and Safety Improvements
- Eliminates bypass vulnerability: Previously, starting from build-artifacts or do-release would skip the wait-for-main-pr verification job, allowing builds to run without confirming the develop to main merge succeeded.
- Enforces proper workflow order: Now the workflow can only start from bump-version or merge-develop-to-main, ensuring critical verification steps cannot be skipped.
- Consistent validation: All build jobs now require needs.wait-for-main-pr.result == success (previously allowed skipped), ensuring verification actually ran.
### 2. Code Quality
- Consistent updates: All affected job conditions were updated uniformly across all build jobs (Windows, Debian, Arch, RHEL) and downstream jobs (rollback, do-release, upload-s3).
- Reduced complexity: Fewer conditional branches make the workflow easier to understand and maintain.
- Well-documented: The PR description clearly explains the problem, solution, and outcomes.
### 3. Testing and Validation
- YAML syntax validation was performed.
- Clear test plan documented in PR description.
- Good use of git notes for rollback tracking (from previous commits in this branch).
---
## Observations and Considerations
### 1. Line Formatting Change
There is a minor formatting change at .github/workflows/release.yml:426-430 where the condition was split across multiple lines for readability. This is a stylistic improvement.
### 2. Verification Job Dependency
The change from allowing both success or skipped to requiring only success is the core fix. This prevents builds from running when wait-for-main-pr is skipped. Critical improvement.
### 3. Workflow Flexibility Trade-off
Previous state: Users could resume from any step (useful for debugging/retries after build failures).
New state: Users can only start from bump-version or merge-develop-to-main.
Question: What happens if all builds succeed but do-release fails (e.g., network issue) or S3 upload fails but release succeeds?
Currently, you would need to re-run from merge-develop-to-main, which would re-merge (or fail if already merged) and rebuild all artifacts. Consider documenting this behavior or adding a workflow_dispatch option to manually trigger just the release creation step in edge cases.
### 4. Concurrency Controls
The workflow has good concurrency controls. However, if a workflow is running and someone starts a new one from merge-develop-to-main while the first is at build stage, both could potentially proceed. The cancel-in-progress: false prevents cancellation but does not prevent the second from starting. This is likely acceptable given the PR checks and merge requirements, but worth noting.
---
## Recommendations
### Minor Suggestions
1. Consider adding a comment in the workflow explaining why build-artifacts and do-release were removed, for future maintainers at lines 28-35.
2. Document recovery procedures for how to handle partial failures:
- What to do if release creation fails after successful builds
- How to manually create a release from existing artifacts
- S3 upload retry procedures
3. Consider a manual-release workflow for edge cases where you have verified artifacts but need to retry just the release/upload steps.
### Testing Recommendations
Before merging, consider testing:
1. Happy path: bump-version to merge to builds to release
2. Start from merge-develop-to-main (verify it works correctly)
3. Verify attempting to skip to non-existent build-artifacts is properly prevented by the UI
4. Confirm rollback still works correctly when builds fail
---
## Overall Assessment
Verdict: Approve with minor suggestions
This is a well-thought-out security improvement that closes a significant gap in the release workflow. The changes are:
- Consistent across all affected jobs
- Well-documented in the PR description
- Focused on a specific problem with a clear solution
- Properly tested (YAML validation)
The trade-off of reduced flexibility is acceptable given the security benefits. The suggestions above are minor enhancements rather than blockers.
### Code Quality Score: 9/10
- Clear intent and execution
- Consistent implementation
- Could benefit from inline documentation
### Security Impact: Positive
- Closes verification bypass vulnerability
- Enforces proper workflow order
### Risk Level: Low
- Changes are localized to workflow conditionals
- No changes to actual build/release logic
- Reduces code paths (simpler = safer)
---
Great work on identifying and fixing this race condition!
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fix race condition in release workflow by removing unsafe resume options
Summary
Removes the ability to start the release workflow from build-artifacts or do-release steps to prevent builds from running without proper merge verification. This eliminates a race condition where builds could execute before the develop→main PR was verified and merged.
Problem
The previous workflow allowed starting from any step, including build-artifacts and do-release. When starting from these later steps, the wait-for-main-pr verification job would be skipped, but build jobs would still run because they accepted needs.wait-for-main-pr.result == 'skipped'. This created a safety gap where:
Builds could run without verifying the develop→main merge succeeded
No checks confirmed the PR passed status checks before building
Rollback wouldn't trigger properly if builds failed
Solution
Removed build-artifacts and do-release from start_from_step options - workflow can now only start from:
bump-version (default)
merge-develop-to-main
Updated all job conditions to remove references to the removed steps:
wait-for-main-pr - lines 424-432
All build jobs (build-windows, build-debian, build-arch, build-rhel) - require wait-for-main-pr.result == 'success' only
rollback-on-build-failure - lines 994-1006
do-release - lines 1071-1083
upload-s3 - lines 1148-1160
Outcomes
✅ Cannot bypass merge verification - No way to start workflow past the merge verification step
✅ Build jobs require successful PR verification - All builds check needs.wait-for-main-pr.result == 'success'
✅ Simpler workflow logic - Fewer edge cases and conditional branches to maintain
✅ Can still resume from merge step - Supports restarting from merge-develop-to-main if needed
Testing
YAML syntax validated with poetry run python -c "import yaml; yaml.safe_load(open('.github/workflows/release.yml'))"
All job conditions updated consistently
Removed all references to deprecated step options
Pull Request Review - PR #33
Summary
This PR addresses a critical race condition in the release workflow by removing the ability to start from build-artifacts or do-release steps. This ensures that builds cannot run without proper merge verification, which is an important security and reliability improvement.
Strengths
1. Security and Safety Improvements
2. Code Quality
3. Testing and Validation
Observations and Considerations
1. Line Formatting Change
There is a minor formatting change at .github/workflows/release.yml:426-430 where the condition was split across multiple lines for readability. This is a stylistic improvement.
2. Verification Job Dependency
The change from allowing both success or skipped to requiring only success is the core fix. This prevents builds from running when wait-for-main-pr is skipped. Critical improvement.
3. Workflow Flexibility Trade-off
Previous state: Users could resume from any step (useful for debugging/retries after build failures).
New state: Users can only start from bump-version or merge-develop-to-main.
Question: What happens if all builds succeed but do-release fails (e.g., network issue) or S3 upload fails but release succeeds?
Currently, you would need to re-run from merge-develop-to-main, which would re-merge (or fail if already merged) and rebuild all artifacts. Consider documenting this behavior or adding a workflow_dispatch option to manually trigger just the release creation step in edge cases.
4. Concurrency Controls
The workflow has good concurrency controls. However, if a workflow is running and someone starts a new one from merge-develop-to-main while the first is at build stage, both could potentially proceed. The cancel-in-progress: false prevents cancellation but does not prevent the second from starting. This is likely acceptable given the PR checks and merge requirements, but worth noting.
Recommendations
Minor Suggestions
Consider adding a comment in the workflow explaining why build-artifacts and do-release were removed, for future maintainers at lines 28-35.
Document recovery procedures for how to handle partial failures:
Consider a manual-release workflow for edge cases where you have verified artifacts but need to retry just the release/upload steps.
Testing Recommendations
Before merging, consider testing:
Overall Assessment
Verdict: Approve with minor suggestions
This is a well-thought-out security improvement that closes a significant gap in the release workflow. The changes are:
The trade-off of reduced flexibility is acceptable given the security benefits. The suggestions above are minor enhancements rather than blockers.
Code Quality Score: 9/10
Security Impact: Positive
Risk Level: Low
Great work on identifying and fixing this race condition!