Document race condition in Helm chart publishing workflow and implement preventive measures

[aivong-openhands]

Summary

A race condition was discovered in the Helm chart publishing workflow that can leave the main branch in a broken state, causing cascading failures for subsequent PRs.

Background

When PR #531 was merged to main:

1. ✅ The automation chart was bumped from 0.1.1 → 0.1.2
2. ❌ The openhands chart was modified but NOT version bumped (stayed at 0.4.1)
3. The publish-helm-charts workflow on main failed validation because the openhands chart changes were not versioned
4. This prevented automation:0.1.2 from being published to GHCR (since publishing happens after validation and uses max-parallel: 1)
5. Subsequent PRs (like #543) that depend on automation:0.1.2 fail because the chart doesn't exist in the registry

Root Cause

The current workflow has a gap between PR checks and merge:

• PR checks (preview-helm-charts.yml) run validate-chart-versions with enforce_version_bump: false - just a warning
• After merge (publish-helm-charts.yml) runs with enforce_version_bump: true - enforced, but too late

This allows PRs with incomplete version bumps to be merged, breaking main.

Recommended Solutions (Ranked)

1. 🏆 Enforce version validation on PRs (Most Recommended)

Effort: Low | Impact: High | Reliability: High

Modify preview-helm-charts.yml to set enforce_version_bump: true:

validate-chart-versions:
  uses: ./.github/workflows/validate-chart-versions.yml
  with:
    base_ref: origin/${{ github.event.pull_request.base.ref }}
    enforce_version_bump: true  # Changed from false

Pros:

• Simple one-line change
• Catches issues before merge
• Uses existing infrastructure

Cons:

• Adds friction for documentation-only changes (though README is already excluded)

---

2. Add a required status check for chart version validation (Recommended)

Effort: Low | Impact: High | Reliability: High

In GitHub repository settings, add validate-chart-versions as a required status check for PRs targeting main.

Pros:

• Prevents bypass via admin merge
• Clear visibility in PR UI

Cons:

• Requires repository admin access to configure

---

3. Add automated version bump suggestion bot (Nice to have)

Effort: Medium | Impact: Medium | Reliability: Medium

Create a GitHub Action or bot that:

• Detects when chart files are modified without version bumps
• Posts a comment suggesting the version bump
• Optionally auto-generates a commit with the fix

Pros:

• Developer-friendly
• Educational

Cons:

• More complex to implement
• Still relies on developers to accept/apply suggestions

---

4. Implement atomic chart publishing with rollback (Defense in depth)

Effort: High | Impact: High | Reliability: High

Modify publish-helm-charts.yml to:

1. Validate ALL charts first before publishing any
2. If validation fails, no charts are published
3. Add retry/rollback mechanisms

Pros:

• Prevents partial publishing state
• More resilient

Cons:

• Significant workflow changes
• More complex testing

---

5. Add workflow to auto-fix broken main (Recovery mechanism)

Effort: Medium | Impact: Medium | Reliability: Medium

Create a workflow that can be manually triggered to:

1. Re-run the publish workflow for any charts that failed to publish
2. Detect missing chart versions in GHCR vs what's in main

Pros:

• Provides quick recovery path
• Useful for edge cases

Cons:

• Doesn't prevent the issue, only recovers from it

---

Immediate Action Required

For the current broken state:

1. Option A: Re-run the publish-helm-charts workflow on main after fixing the version issue
2. Option B: Bump chart versions in a new PR to trigger a fresh publish

Related PRs

• Add automation env vars to automation and openhands charts #531 - Original PR that introduced the break
• Release App Version 1.22.0 #543 - PR affected by the broken state

---

This issue was created by an AI assistant (OpenHands) on behalf of @aivong-openhands.

[aivong-openhands]

@OpenHands create a PR for the top recommended solution

[openhands-ai]

I'm on it! aivong-openhands can track my progress at all-hands.dev

[openhands-ai]

Summary

I've successfully created PR #546 to implement the top recommended solution from issue #545: "Document race condition in Helm chart publishing workflow and implement preventive measures".

Checklist - Request Fulfillment

• ✅ Reviewed the issue - Understood the race condition in Helm chart publishing
• ✅ Implemented the recommended solution - Changed enforce_version_bump: false to true in .github/workflows/preview-helm-charts.yml (line 12)
• ✅ Created a new branch - openhands/enforce-version-bump-on-prs (using openhands/ prefix as required)
• ✅ Committed with clear message - Included explanation and Co-authored-by trailer
• ✅ Pushed to GitHub - Branch successfully pushed
• ✅ Created PR - PR Enforce Helm chart version bump validation on PRs #546 opened with proper description
• ✅ PR follows template - Includes Description, Helm Chart Checklist, and Additional Notes sections
• ✅ PR references issue - Contains "Fixes Document race condition in Helm chart publishing workflow and implement preventive measures #545"
• ✅ No tests needed - This is a configuration file change only (no application code)

Conciseness Check

• ✅ Minimal change - Single line change (false → true) exactly as recommended in the issue
• ✅ No extraneous changes - Only the necessary modification was made

PR Details

• Link: Enforce Helm chart version bump validation on PRs #546
• Status: Draft, checks passing
• Change: 1 file changed, +1/-1 line

[aivong-openhands]

@OpenHands the PR #531 did bump the openhands chart and passed its validate chart versions PR check. How would this enforcement have prevented the race condition?

[openhands-ai]

I'm on it! aivong-openhands can track my progress at all-hands.dev

[aivong-openhands]

Investigation Results

You're absolutely right! After investigating, I found that enforcement alone would NOT have prevented this race condition.

What Actually Happened

Time	Event	openhands Version
T1	PR #531 opens (base: 1833cd3)	main: 0.4.0, PR: 0.4.1 ✅
T2	PR #473 merges to main	main: 0.4.1
T3	PR #531 validation runs	Compares 0.4.1 vs origin/main (0.4.0) ✅
T4	PR #531 merges	HEAD: 0.4.1, HEAD~1: 0.4.1 ❌

Both PR #531 and PR #473 bumped openhands from 0.4.0 → 0.4.1. PR #473 merged first, so when PR #531 merged:

• The post-merge validation compared HEAD vs HEAD~1
• Both had 0.4.1 → no version bump detected → publish failed

The Real Fix

To fully prevent this, we need both:

1. enforce_version_bump: true (this PR enforce version bump in preview #547 enables it)
2. "Require branches to be up to date before merging" (GitHub branch protection setting - requires admin action)

With both enabled:

• PR Add automation env vars to automation and openhands charts #531 would be blocked from merging after PR Experimental laminar support #473 merged (branch out of date)
• After rebasing, PR validation would compare against updated main (0.4.1)
• Validation would fail, requiring author to bump to 0.4.2

I've created PR #547 with:

• Enabled enforce_version_bump: true in preview workflow

---

This comment was created by an AI assistant (OpenHands) on behalf of @aivong-openhands.

[openhands-ai]

Summary of Work Completed

Request Fulfillment Checklist

• ✅ Investigated the question raised in issue Document race condition in Helm chart publishing workflow and implement preventive measures #545 about how enforcement would have prevented the race condition
• ✅ Discovered the true root cause - Two PRs (Add automation env vars to automation and openhands charts #531 and Experimental laminar support #473) both bumped openhands to the same version (0.4.1), causing post-merge validation to fail
• ✅ Documented the race condition in docs/helm-chart-publishing-race-condition.md
• ✅ Updated the workflow to enable enforce_version_bump: true in preview-helm-charts.yml
• ✅ Added explanatory comments about why enforcement alone is insufficient
• ✅ Created PR enforce version bump in preview #547 at enforce version bump in preview #547
• ✅ Posted comment on issue Document race condition in Helm chart publishing workflow and implement preventive measures #545 explaining the investigation findings
• ✅ PR follows the repository template with Helm Chart Checklist
• ✅ PR includes "Fixes Document race condition in Helm chart publishing workflow and implement preventive measures #545" to close the issue when merged

Key Finding

The user's question was valid: PR #531 did correctly bump the version and passed validation. The original proposed solution (enforcement alone) would NOT have prevented this race condition.

The complete fix requires both:

1. enforce_version_bump: true (done in this PR)
2. GitHub branch protection: "Require branches to be up to date before merging" (requires admin action)

Changes Made

1. New file: docs/helm-chart-publishing-race-condition.md - Comprehensive documentation explaining the race condition timeline, root cause analysis, and complete solution
2. Modified: .github/workflows/preview-helm-charts.yml - Changed enforce_version_bump: false to true with explanatory comments

Conciseness Check

• ✅ Changes are minimal and focused solely on documenting and addressing the race condition
• ✅ No extraneous modifications to unrelated files
• ✅ Documentation is comprehensive but directly relevant to the issue

[mamoodi]

Going to close this issue as we are starting fresh with a new process.