Skip to content

fix: increase label retry window to prevent race condition with opened-event required-label checks#28896

Merged
pelikhan merged 3 commits intomainfrom
copilot/fix-required-label-checks
Apr 28, 2026
Merged

fix: increase label retry window to prevent race condition with opened-event required-label checks#28896
pelikhan merged 3 commits intomainfrom
copilot/fix-required-label-checks

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 28, 2026

GitHub's REST-to-GraphQL bridge can take 30–60+ seconds to resolve a newly-created PR's node ID, causing issues.addLabels to fail with a transient unprocessable error. The previous retry ceiling (~26 s total) was insufficient to beat the 30-second wait many pull_request_target workflows use before checking required labels, resulting in failed CI on the opened event even though the label is eventually applied.

Changes

  • LABEL_MAX_RETRIES 3 → 5 — extends to 1 initial + 5 retries = 6 total attempts
  • Explicit LABEL_MAX_DELAY_MS = 30000 — previously fell back to the default 10 s cap; new retry schedule is 6 s → 12 s → 24 s → 30 s → 30 s (~102 s max vs. ~26 s before)
  • Actionable warning hint — failure message now suggests triggering required-label checks on the labeled event as a fallback for repositories where this race condition persists
  • Test updated — retry-exhaustion test expects 6 total addLabels calls (was 4)

…d-event required-label checks

- LABEL_MAX_RETRIES: 3 → 5 (now 1 initial + 5 retries = 6 total attempts)
- Add explicit LABEL_MAX_DELAY_MS = 30000 (was defaulting to 10 000 ms)
- New retry schedule: 6 s, 12 s, 24 s, 30 s, 30 s — max ~102 s vs previous ~26 s
- Add actionable hint in warning message suggesting the labeled event as an alternative
- Update test to expect 6 total addLabels calls (was 4)

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a98000a7-2ad2-432f-8905-f57672c1b5fe

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix late label application issue for required-label checks fix: increase label retry window to prevent race condition with opened-event required-label checks Apr 28, 2026
Copilot AI requested a review from pelikhan April 28, 2026 10:32
@pelikhan pelikhan marked this pull request as ready for review April 28, 2026 10:37
Copilot AI review requested due to automatic review settings April 28, 2026 10:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends the label-application retry window when creating pull requests to better handle GitHub’s transient REST→GraphQL node-id resolution delay, reducing CI failures on opened workflows that require labels.

Changes:

  • Increased LABEL_MAX_RETRIES from 3 to 5 for label operations.
  • Added an explicit LABEL_MAX_DELAY_MS = 30000 cap to the label retry backoff configuration.
  • Updated the retry-exhaustion unit test expectations to match the new retry count and timing commentary.
Show a summary per file
File Description
actions/setup/js/create_pull_request.cjs Expands and caps label retry backoff behavior; adds a warning hint about opened vs labeled event checks.
actions/setup/js/create_pull_request.test.cjs Updates test expectations for the increased number of addLabels attempts and revised delay schedule commentary.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread actions/setup/js/create_pull_request.cjs Outdated
@github-actions github-actions Bot mentioned this pull request Apr 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 95/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
should retry addLabels on race condition and warn after all retries exhausted actions/setup/js/create_pull_request.test.cjs:1182 ✅ Design Minor: call-count assertion is implementation-adjacent, but it directly validates the retry configuration being changed

Analysis

The single modified test updates the retry exhaustion scenario to reflect the new LABEL_MAX_RETRIES = 5 (up from 3), which means 6 total calls instead of 4. The test covers:

  1. result.success === true — verifies PR creation still succeeds when label-add retries are exhausted (behavioral ✅)
  2. result.fallback_used === undefined — verifies no silent fallback to an issue (behavioral ✅)
  3. addLabels called 6 times — verifies the retry count configuration is honoured (implementation-adjacent, but directly testing the change being made ✅)
  4. core.warning with expected message — verifies the warning is surfaced to the user (behavioral ✅)

Mocking is limited to the GitHub API (global.github.rest.issues.addLabels, global.core), which is the accepted pattern for external I/O in this codebase. The timer-based retry delay is exercised via vi.useFakeTimers() / vi.runAllTimersAsync().

The minor 5-point deduction is for the toHaveBeenCalledTimes(6) assertion, which ties the test to the internal retry count constant rather than an observable outcome. This is acceptable in this case because the retry count is precisely what the PR is changing, but be aware that future refactoring of the retry mechanism could break this assertion without a behavioral regression.


Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 1 test (vitest)

Verdict

Check passed. 0% of new/modified tests are implementation tests (threshold: 30%). No coding-guideline violations detected.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25048060386

🧪 Test quality analysis by Test Quality Sentinel · ● 386.4K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 95/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). No coding-guideline violations detected.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pelikhan pelikhan merged commit 6b17302 into main Apr 28, 2026
7 checks passed
@pelikhan pelikhan deleted the copilot/fix-required-label-checks branch April 28, 2026 10:44
@github-actions
Copy link
Copy Markdown
Contributor

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (25048342287)

Generated by Smoke CI for issue #28896 ·

@github-actions
Copy link
Copy Markdown
Contributor

Comment Memory

CI lights the path\nGreen checks bloom at dawn\nQuiet bots still sing

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Generated by Smoke CI for issue #28896 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR labels can be applied too late for opened-event required-label checks

3 participants