Skip to content

Improve PR labels#2458

Merged
Dreamsorcerer merged 4 commits into
mainfrom
sam/improve-pr-labels
Jun 11, 2026
Merged

Improve PR labels#2458
Dreamsorcerer merged 4 commits into
mainfrom
sam/improve-pr-labels

Conversation

@Dreamsorcerer

@Dreamsorcerer Dreamsorcerer commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator
  • Fix a race condition that could add the label just after a push (based on the status of the previous push).
  • Don't add the label to draft PRs.
  • Fix permission errors in some cases.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves the pr-label.yml workflow by fixing two issues: a race condition that could apply the ready-to-merge label based on stale CI results from a previous push, and incorrect labeling of draft PRs.

  • The race condition fix re-fetches each PR's current HEAD SHA in the workflow_run path and skips label updates when it doesn't match the run's SHA, preventing old CI results from tagging a PR that has already moved on to a newer commit.
  • Draft PR support is added via new converted_to_draft and ready_for_review event triggers; both the pull_request_target and workflow_run paths now gate label application on !pr.draft.
  • The ciCompleteOk helper is extracted as a shared function and updated to use listWorkflowRuns with per_page: 1, ensuring that an in-progress re-run for the same SHA correctly suppresses the label rather than falling through to an older completed run.

Confidence Score: 5/5

The changes are targeted and well-reasoned fixes to real labeling issues; the worst-case outcome of any residual timing window is a missed or extra label, not data loss or a security problem.

Both fixes (SHA-staleness guard in the workflow_run path and draft-state awareness) are straightforward and correctly implemented. The ciCompleteOk refactor with per_page:1 also closes a pre-existing gap where an in-progress re-run would be skipped in favour of an older completed result. No logic paths have been regressed.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/pr-label.yml Adds draft-PR awareness and a SHA-staleness guard; refactors ciCompleteOk as a shared helper using per_page:1 to correctly block on in-progress re-runs. Logic is sound with minor residual TOCTOU windows already noted in prior review threads.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Event fires] --> B{event type?}

    B -- pull_request_target --> C{action?}
    C -- converted_to_draft\nor synchronize --> D[setLabel false\nremove ready-to-merge]
    C -- ready_for_review --> E[Fetch fresh PR state]
    E --> F[ciCompleteOk pr.head.sha]
    F --> G{ciOk AND not draft?}
    G -- yes --> H[setLabel true\nadd ready-to-merge]
    G -- no --> I[setLabel false]

    B -- workflow_run --> J[ciCompleteOk run.head_sha\nper_page=1 - latest run only]
    J --> K[Resolve PR numbers\nfrom payload or commit SHA]
    K --> L[For each PR: fetch fresh state]
    L --> M{pr.head.sha == run.head_sha?}
    M -- no stale event --> N[skip - log and continue]
    M -- yes --> O{ciOk AND not draft?}
    O -- yes --> P[add label]
    O -- no --> Q[remove label]
Loading

Reviews (3): Last reviewed commit: "Fix" | Re-trigger Greptile

Comment thread .github/workflows/pr-label.yml
Comment thread .github/workflows/pr-label.yml
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Flag Coverage Δ
OS-ubuntu-24.04-arm 64.93% <ø> (+<0.01%) ⬆️
OS-ubuntu-latest 65.83% <ø> (+<0.01%) ⬆️
Py-3.10 65.82% <ø> (+<0.01%) ⬆️
Py-3.11 65.82% <ø> (-0.01%) ⬇️
Py-3.12 65.82% <ø> (ø)
Py-3.13 65.82% <ø> (+<0.01%) ⬆️
Py-3.14 65.84% <ø> (+<0.01%) ⬆️
Py-3.14t 65.82% <ø> (-0.01%) ⬇️
SelfHosted-Large 30.64% <ø> (?)
SelfHosted-Linux 38.38% <ø> (ø)
SelfHosted-macOS 37.58% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 17 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 10, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Jun 10, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 10, 2026
@Dreamsorcerer Dreamsorcerer merged commit f38a562 into main Jun 11, 2026
31 of 42 checks passed
@Dreamsorcerer Dreamsorcerer deleted the sam/improve-pr-labels branch June 11, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants