Skip to content

[build] delete a PR's head branch when the PR is closed without merging - #18199

Open
titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:delete-closed-pr-branches
Open

titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:delete-closed-pr-branches

Conversation

@titusfortner

Copy link
Copy Markdown
Member

💥 What does this PR do?

  • Deletes SeleniumHQ branches when a PR is closed without merging (this matches the existing behavior for when closed with a merge)

🔧 Implementation Notes

  • Uses pull_request_target because pull_request doesn't run for PRs with merge conflicts, which most abandoned PRs have; it never checks out PR code, and the branch name reaches the shell only through an env var.
  • Nothing is lost: the closed PR keeps its commits and offers "Restore branch" if anyone wants to pick it back up.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5.5)
    • What was generated: the branch analysis, the workflow, and this description
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Branches from PRs closed before this lands (about 30) will need a one-time cleanup.

🔄 Types of changes

  • Cleanup (repo branch hygiene)

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_describe

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Oct 18, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Delete unmerged PR head branches when PRs close

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Delete repository-owned head branches when pull requests close without merging.
• Use a conflict-tolerant event and restrict deletion to eligible branches.
Diagram

graph TD
  Event["PR closed"] --> Gate{"Eligible branch?"} -->|yes| CLI["Delete step"] --> API["GitHub REST API"] --> Ref["Head branch ref"]
  Gate -->|no| Skip["No deletion"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Scheduled cleanup of closed PR branches
  • ➕ Could also remove branches from PRs closed before this workflow lands.
  • ➖ Requires scanning historical PRs and handling more API calls.
  • ➖ Leaves branches in place until the next scheduled run.

Recommendation: Keep the event-driven workflow for future closures: it is immediate and narrowly scoped. Handle previously closed PR branches with a separate one-time cleanup rather than complicating this workflow.

Files changed (1) +25 / -0

Other (1) +25 / -0
delete-closed-branch.ymlAdd guarded cleanup for unmerged PR branches +25/-0

Add guarded cleanup for unmerged PR branches

• Adds a pull_request_target workflow for closed PRs, including those with merge conflicts. It grants contents write permission and deletes the head ref only when the PR is unmerged and its branch belongs to this repository; it does not check out PR code.

.github/workflows/delete-closed-branch.yml

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Delete same-repository branches when PRs close without merging

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a workflow to delete same-repository head branches when PRs close without merging.
• Use pull_request_target to handle conflicted PRs without checking out PR code.
• Skip merged PRs, fork PRs, and runs in repository forks.
Diagram

graph TD
  event["Closed PR"] --> gate{"Eligible to delete?"} -->|yes| api["GitHub API"] --> branch["Head branch"]
  gate -->|no| skip["No deletion"]
Loading
High-Level Assessment

The event-driven workflow fits the goal: GitHub's existing post-merge branch deletion does not cover unmerged closures, while a scheduled cleanup would need to infer which branches are safe to remove. The same-repository checks and lack of PR-code checkout appropriately limit the privileged workflow.

Files changed (1) +25 / -0

Other (1) +25 / -0
delete-closed-branch.ymlDelete eligible PR head branches on closure +25/-0

Delete eligible PR head branches on closure

• Adds a pull_request_target workflow for closed PRs that deletes an unmerged head branch through the GitHub API only when it belongs to the current repository. It skips runs in forks, grants contents write permission, and passes the branch name through an environment variable without checking out PR code.

.github/workflows/delete-closed-branch.yml

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. New work on closed branches gets deleted 🐞 Bug ☼ Reliability
Description
The gh api step deletes the ref named in the close event without checking whether it still points
to pull_request.head.sha. If an update workflow reuses one of its fixed branch names while this
job is queued, the job can delete the newly pushed branch, including one already used for a new pull
request.
Code

.github/workflows/delete-closed-branch.yml[25]

+        run: gh api --method DELETE "repos/$REPO/git/refs/heads/$BRANCH"
Evidence
The delete request uses only the historical branch name from the event, while the repository's
update workflow pushes to fixed branch names and then opens or edits a pull request. The GitHub
endpoint deletes the reference supplied in the request, not a reference conditional on its former
commit.

.github/workflows/delete-closed-branch.yml[14-25]
.github/workflows/update-pr.yml[58-69]
.github/workflows/pin-browsers.yml[11-20]
🌐 The delete-reference endpoint deletes the supplied ref; its documented request has no expected-SHA parameter.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The close-event job can delete a branch that was updated or recreated after the pull request closed.
## Fix Focus Areas
- .github/workflows/delete-closed-branch.yml[20-25]
## Recommended Fix
Before deletion, fetch the current head ref and skip cleanup unless its SHA matches the closed pull request's head SHA. Also skip deletion if another open pull request uses that ref; handle an already-deleted ref as successful cleanup.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


Grey Divider

Context sources
✅ Web pages:
  +2 more
Review mode: Auto: ⚖️ Balanced: Workflow grants write access and deletes branches, creating security and availability risk.

Grey Divider

Tip of the day
💡 Did you know, you can move or hide resolved findings to focus on what still needs action

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
BRANCH: ${{ github.event.pull_request.head.ref }}
run: gh api --method DELETE "repos/$REPO/git/refs/heads/$BRANCH"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remediation recommended

1. New work on closed branches gets deleted 🐞 Bug ☼ Reliability

The gh api step deletes the ref named in the close event without checking whether it still points
to pull_request.head.sha. If an update workflow reuses one of its fixed branch names while this
job is queued, the job can delete the newly pushed branch, including one already used for a new pull
request.
Agent Prompt
## Issue description
The close-event job can delete a branch that was updated or recreated after the pull request closed.
## Fix Focus Areas
- .github/workflows/delete-closed-branch.yml[20-25]
## Recommended Fix
Before deletion, fetch the current head ref and skip cleanup unless its SHA matches the closed pull request's head SHA. Also skip deletion if another open pull request uses that ref; handle an already-deleted ref as successful cleanup.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

This branch has not been deployed

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

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants