Fix CI safety check to evaluate latest workflow attempt

[justin808]

Summary

Fixes the ensure-master-docs-safety GitHub Action to check the latest attempt of workflow runs instead of the overall run conclusion. This prevents false positives when workflows are manually re-run and succeed.

Problem

The safety check was blocking docs-only commits from skipping CI when previous master commits had workflows marked as "failed", even if those workflows had been successfully re-run. This happened because:

1. GitHub marks a workflow run as "failed" even after a manual rerun succeeds
2. The run.conclusion field is never updated to "success" when reruns succeed
3. The safety check was only looking at run.conclusion, not the actual latest attempt

This created a situation where:

• Commit A fails a workflow
• The workflow is manually re-run and succeeds
• Commit B (docs-only) is blocked because commit A's workflow is still marked as "failed"
• This continues indefinitely, blocking all subsequent commits

Solution

Modified the action to:

1. Fetch the jobs for each workflow run via the GitHub API
2. Find the maximum run_attempt number to identify the latest attempt
3. Filter jobs to only those from the latest attempt
4. Check if any jobs in the latest attempt have failed conclusions
5. Only block docs-only commits if the latest attempt has actual failures

This allows the safety check to correctly recognize when failures have been resolved via manual reruns, while still preventing docs-only skips when there are genuine unresolved failures.

Test Plan

• Code changes reviewed for correctness
• Linting passes locally
• CI passes on this PR (will verify the fix works)
• After merge, verify that docs-only commits are no longer blocked by previously-fixed workflow failures

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores
  • Improved the accuracy of documentation safety checks by enhancing how workflow failures are detected and assessed, ensuring more precise identification of build issues during the latest workflow attempts.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The workflow failure detection logic in the ensure-master-docs-safety action has been refactored. Instead of relying on the overall run conclusion, it now evaluates the latest job attempt for each workflow run, determining failure status based on individual job conclusions within that attempt.

Changes

Cohort / File(s)	Summary
Workflow failure detection refactor .github/actions/ensure-master-docs-safety/action.yml	Replaced run.conclusion-based failure checking with per-job latest-attempt assessment. Now fetches all jobs per run, identifies the latest run_attempt, and evaluates job conclusions from that attempt. Marks runs as failing if any job in the latest attempt has conclusions in {failure, timed_out, cancelled, action_required}. Treats runs with no jobs as incomplete.

Sequence Diagram(s)

sequenceDiagram
    participant Action as Ensure Docs Safety
    participant API as GitHub API
    participant Check as Job Assessment
    
    Action->>API: Fetch latest workflow runs
    activate API
    API-->>Action: Workflow run list
    deactivate API
    
    loop For each run
        Action->>API: Fetch all jobs for run
        activate API
        API-->>Action: Job list with run_attempt
        deactivate API
        
        Note over Action: Identify latest run_attempt
        
        rect rgb(200, 220, 255)
            Note over Check: NEW: Analyze latest attempt jobs only
            Action->>Check: Evaluate job conclusions<br/>(failure|timed_out|cancelled|action_required)
        end
        
        alt Any job failed in latest attempt
            Action->>Action: Mark run as FAILED
        else No jobs found
            Action->>Action: Mark run as INCOMPLETE
        else All jobs succeeded
            Action->>Action: Mark run as SUCCESS
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • Logic correctness for latest attempt identification and filtering
  • Handling of edge cases (missing jobs, no attempts, API response variations)
  • Async/await patterns in API call sequencing
  • Verification that all failure states {failure, timed_out, cancelled, action_required} are appropriately captured

Suggested labels

bug, review-needed

Poem

🐰 A rabbit hops through workflow logs,
Finding truth in each job's fate—
No more guessing by the run's old fog,
Just the latest attempt, accurate!
Failures caught with clearer sight,
Docs stay safe, day and night. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix CI safety check to evaluate latest workflow attempt' directly and accurately summarizes the main change: updating the CI safety check to evaluate the latest workflow attempt instead of overall run conclusion.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/fix-ci-safety-loop

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review

I've reviewed PR #2062 and the changes look solid overall. Here's my detailed feedback:

✅ Strengths

1. Correct Problem Identification: The fix properly addresses a real GitHub API limitation where run.conclusion doesn't update after successful reruns.

2. Well-Documented Logic: Excellent inline comments explaining the rationale (lines 74-76, 107-109).

3. Preserves Existing Safety Checks: The failure condition list remains comprehensive (failure, timed_out, cancelled, action_required).

4. Maintains Early Exit Logic: Still handles edge cases like no workflows found (line 63-66) and incomplete runs (line 92-105).

🔍 Potential Issues & Suggestions

1. API Rate Limiting Concern (Medium Priority)

Issue: The new code makes an additional API call per workflow run (line 114-119). For repos with many workflows, this could hit rate limits.

Recommendation: Add error handling for rate limit responses with try/catch block around the API call.

2. Pagination Not Handled for Jobs (High Priority)

Issue: Line 118 uses per_page: 100, but some workflow runs may have >100 jobs (especially matrix builds).

Recommendation: Use pagination with github.paginate.iterator similar to how workflow runs are fetched, or at minimum check jobsResponse.data.total_count and log a warning if it exceeds 100.

3. Edge Case: Empty Jobs Array (Low Priority)

Issue: Line 123-127 treats no jobs as incomplete/failing. This might be overly conservative for workflows that were cancelled before any jobs started.

Consideration: Should this check run.status === 'completed' to distinguish between "no jobs yet" vs "completed with no jobs"? This may be acceptable as-is, but worth documenting the rationale.

4. Array.prototype.map with Potential Empty Array

Issue: Line 130 could fail if jobs array exists but all jobs lack run_attempt field.

Recommendation: Filter out undefined values before calling Math.max, or add a check to ensure run_attempt exists on jobs.

🎯 Testing Recommendations

Since this is critical CI infrastructure:

1. Manual Testing: Test with a workflow that has:

   • Initial failure → manual rerun → success
   • Multiple reruns (attempt 3+)
   • Matrix jobs with >100 total jobs
   • Cancelled runs with no jobs

2. Consider Adding Tests: While GitHub Actions are hard to unit test, consider adding example workflow run scenarios in comments to document expected behavior.

📊 Performance Impact

• Before: 1 API call (list workflow runs)
• After: 1 + N API calls (where N = unique workflows on previous commit)
• Typical Impact: For ~10 workflows, this is acceptable
• Mitigation: Consider caching if this becomes an issue

🔒 Security Considerations

No security concerns identified. The code:

• Uses authenticated GitHub API via github-script action
• Doesn't expose sensitive data
• Only reads public workflow information

📝 Minor Nit

Line 158: The error message still shows run.conclusion which might now be misleading since you're checking job conclusions. Consider updating the message to clarify that failing jobs in the latest attempt were detected.

---

Summary

The fix is fundamentally sound and addresses the core issue. The main concerns are:

1. Must add: Pagination for jobs API (high priority)
2. Should add: Error handling for rate limits (medium priority)
3. Nice to have: Edge case handling for empty arrays (low priority)

Great work documenting the problem and solution! The PR description is exemplary. 🎉

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

.github/actions/ensure-master-docs-safety/action.yml (1)

157-159: Consider clarifying error message to reflect job-level determination.

The error message references run.conclusion, but the failure determination is now based on individual job conclusions from the latest attempt. For clarity, consider whether the message should indicate which specific jobs failed or note that it's checking the latest attempt's jobs. This will reduce confusion for developers investigating why their docs-only commit was blocked.

For example, you might include a summary like: "...failed workflows (from latest rerun attempt):" or collect job names from latestJobs that actually failed.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between beb70f0 and 8a7e317.

📒 Files selected for processing (1)

• .github/actions/ensure-master-docs-safety/action.yml (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: claude-review

🔇 Additional comments (3)

.github/actions/ensure-master-docs-safety/action.yml (3)

107-150: ✓ Core logic correctly implements latest-attempt checking.

The refactor properly addresses the issue: fetching jobs, identifying the latest attempt via Math.max(run_attempt), and checking job conclusions from that specific attempt. The handling of empty jobs (lines 123–127) prevents Math.max edge cases, and the conclusion checks (failure, timed_out, cancelled, action_required) reasonably categorize blocking failures vs. non-blocking outcomes (skipped, neutral).

---

114-119: No error handling on API calls—verify intent.

The github.rest.actions.listJobsForWorkflowRun call has no try-catch. If the API call fails, the entire action fails, which may be intentional as a fail-safe (conservatively block docs-only skips on uncertainty). Confirm this is the desired behavior, or add handling for transient API failures.

---

73-76: Excellent explanatory comments.

The new comments clearly document why fetching the latest attempt is necessary and explain the GitHub API quirk where run.conclusion persists even after reruns. This helps future maintainers understand the design decision.

[claude]

Code Review: PR #2062

Summary

This PR effectively fixes a critical issue where the CI safety check was incorrectly blocking docs-only commits due to stale workflow failure states. The solution correctly addresses the root cause by checking the latest attempt of workflow runs instead of the overall run conclusion.

---

✅ Strengths

1. Correct Problem Analysis: The PR description clearly identifies the GitHub API behavior where run.conclusion doesn't update after successful reruns.

2. Well-Documented Code: Excellent inline comments explaining:

   • Why fetching jobs is necessary (lines 74-76, 107-109)
   • What each failure conclusion means (lines 136-142)
   • The logic for finding latest attempts (line 129)

3. Proper API Usage: Correctly uses github.rest.actions.listJobsForWorkflowRun to get job-level details.

4. Maintains Conservative Approach: Correctly treats edge cases (no jobs found) as failures, which is the safe default.

---

⚠️ Issues & Recommendations

🐛 Critical: Pagination Missing for Jobs API

Location: Line 114-119

Issue: The listJobsForWorkflowRun call uses per_page: 100 but doesn't handle pagination. Workflow runs with >100 jobs will have incomplete data, potentially missing failures in jobs beyond the first page.

Impact: False negatives - a failure in job #101 would be missed, allowing docs-only skips when they shouldn't be allowed.

Fix: Use pagination iterator similar to the workflow runs fetch (lines 35-61).

🔍 Performance: Multiple API Calls in Loop

Location: Lines 112-150

Issue: The code makes one API call per workflow run sequentially. For repos with many workflows, this could be slow and hit rate limits.

Current: O(n) API calls where n = number of unique workflows
Typical case: 3-5 workflows = 3-5 sequential API calls
Edge case: 10+ workflows could cause noticeable delays

Impact: Minor performance concern, but acceptable for most cases. GitHub Actions rate limit is 1000 req/hr, so this is unlikely to be a problem in practice.

Potential optimization (if needed in future):

• Batch requests using Promise.all() for parallel execution
• Cache job results if the same workflow is checked multiple times

🧪 Test Coverage

Missing: This PR doesn't include automated tests for the new logic.

Recommended test cases:

1. Workflow with successful rerun should NOT block docs-only skip
2. Workflow with failed rerun should block docs-only skip
3. Workflow with multiple reruns - only latest attempt matters
4. Workflow with >100 jobs (pagination test)
5. Workflow with no jobs (edge case)

Note: Testing GitHub Actions can be challenging. Consider:

• Integration test by triggering actual workflow runs
• Unit test by mocking GitHub API responses
• Manual verification (which was likely done)

📝 Minor: Inconsistent Error Message

Location: Line 158

Issue: The error message shows run.conclusion which might not accurately reflect the latest attempt's status anymore.

Suggestion: Update message to clarify it's based on latest attempt analysis rather than run.conclusion value.

---

🔒 Security Considerations

✅ No security concerns identified. The code:

• Uses authenticated GitHub API calls (implicit via github-script action)
• Doesn't expose sensitive data
• Properly validates inputs (checking for empty SHA, null checks)
• Uses GitHub's official Octokit client

---

🎯 Performance Considerations

API Rate Limits:

• Current: ~5 API calls per docs-only commit check (1 for runs + 1-4 for jobs)
• GitHub Actions limit: 1000 requests/hour
• Verdict: Acceptable. Would need 200+ docs-only commits/hour to hit limits.

Execution Time:

• Sequential API calls: ~1-2 seconds per workflow
• Typical case (3-5 workflows): 3-10 seconds total
• Verdict: Minor delay but acceptable for safety check

---

📋 Checklist Review

From PR description:

• ✅ Code changes reviewed for correctness
• ✅ Linting passes locally
• ⏳ CI passes on this PR (merged, so presumably passed)
• ⏳ Post-merge verification pending

---

🎓 Best Practices Assessment

Excellent:

• ✅ Clear problem statement with reproduction steps
• ✅ Thorough inline documentation
• ✅ Conservative failure handling
• ✅ Maintains existing safety guarantees

Could be improved:

• ⚠️ Add pagination for job fetching
• ⚠️ Add automated tests
• ⚠️ Update error message for clarity

---

🏁 Final Verdict

Overall Quality: ⭐⭐⭐⭐ (4/5)

This is a well-implemented fix that correctly solves the identified problem. The code is readable, well-documented, and handles most edge cases appropriately.

Primary concern: Missing pagination for jobs API could cause false negatives in edge cases.

Recommendation: Consider a follow-up PR to add pagination for complete robustness, though the current implementation should work fine for typical React on Rails CI workflows.

---

📚 References

• GitHub Actions API - List jobs for workflow run
• GitHub Actions - Re-running workflows

---

Review conducted using repository's CLAUDE.md guidelines