[build] require ci rbe to pass to merge pin browsers by titusfortner · Pull Request #17431 · SeleniumHQ/selenium

titusfortner · 2026-05-10T08:27:49Z

The problem is that right now the pin browser PRs are being auto-merged even when they fail. The way GitHub manages what "pass all tests" is very strict. When we say nothing can get merged that doesn't pass CI-RBE tests, then no one can push directly to trunk even with admin permissions.

💥 What does this PR do?

Whenever ci-rbe workflow passes, the automerge workflow will run to see if it is a PR by ci-selenium and if it has an automerge label, and merges it.

🔧 Implementation Notes

We could poll to wait for all passing checks, but that seems wasteful
I added logic so that while it won't wait for other checks, it won't merge if another check has already come back as failed.

🤖 AI assistance

No substantial AI assistance used
AI assisted (complete below)
- Tool(s): claude
- What was generated: syntax
- I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

Bug fix (backwards compatible)

qodo-code-review · 2026-05-10T08:28:02Z

Review Summary by Qodo

Implement CI-RBE gated auto-merge for pin browser PRs

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Create automerge workflow triggered by CI-RBE completion
• Validate PR eligibility before auto-merging pin browser updates
• Check for failed checks to prevent merging broken PRs
• Replace direct merge with automerge label application

Diagram

flowchart LR
  A["CI-RBE Workflow Completes"] -->|success event| B["Automerge Workflow Triggered"]
  B --> C["Check PR Eligibility"]
  C -->|open, correct SHA, automerge label| D["Check for Failed Checks"]
  D -->|no failures| E["Enable Auto-Merge"]
  D -->|failures found| F["Skip Merge"]
  C -->|ineligible| F
  G["Pin Browsers Workflow"] -->|add automerge label| H["PR Ready for Auto-Merge"]

File Changes

1. .github/workflows/automerge.yml ✨ Enhancement +46/-0

New automerge workflow gated by CI-RBE

• New workflow file triggered on CI-RBE completion
• Validates PR eligibility: open state, correct SHA, automerge label
• Checks for failed or cancelled check runs before merging
• Executes auto-merge with squash and branch deletion if all conditions pass

.github/workflows/automerge.yml

2. .github/workflows/pin-browsers.yml 🐞 Bug fix +1/-1

Switch to label-based merge triggering
• Replace direct auto-merge with automerge label application
• Defers merge decision to automerge workflow
• Allows CI-RBE validation before actual merge
.github/workflows/pin-browsers.yml

qodo-code-review · 2026-05-10T08:28:03Z

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0)

1. Missed failures due pagination 🐞 Bug ☼ Reliability

Description

The automerge job queries the Checks API without pagination, so it can miss failures beyond the
first page and still enable auto-merge even though some checks already failed.

Code

.github/workflows/automerge.yml[R39-41]

+        run: |
+          failed=$(gh api "repos/$REPO/commits/$SHA/check-runs" --jq 'any(.check_runs[]; .conclusion == "failure" or .conclusion == "cancelled")')
+          echo "failed=$failed" >> "$GITHUB_OUTPUT"

Evidence
The workflow only inspects the first page of check-runs for the commit SHA, but this repo’s CI
fan-out can produce many check runs for a single SHA, so failures may exist on later pages and won’t
be considered.
.github/workflows/automerge.yml[36-41]
.github/workflows/ci.yml[20-122]
Best Practice: GitHub REST API pagination

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 automerge workflow checks `check-runs` for a commit SHA without pagination, so it may miss failed checks that appear on later pages and incorrectly proceed to enable auto-merge.

### Issue Context
This repo’s CI workflow fans out into many jobs/reusable workflows, increasing the likelihood that check runs exceed the default page size.

### Fix Focus Areas
- .github/workflows/automerge.yml[36-41]

### Suggested approach
- Use `gh api --paginate` and aggregate pages before applying `jq`, e.g.:
 - `gh api --paginate "repos/$REPO/commits/$SHA/check-runs?per_page=100" | jq -s 'any(.[].check_runs[]; ... )'`
- Alternatively, use a PR-centric command that already handles pagination (e.g., `gh pr checks`) if it provides the needed signal.

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

2. Label timing blocks automerge 🐞 Bug ☼ Reliability

Description

pin-browsers.yml now only applies the "automerge" label; if a pinned-browser PR already exists and
CI - RBE already succeeded before the label is added, the workflow_run-triggered automerge workflow
will not run and the PR can remain unmerged.

Code

.github/workflows/pin-browsers.yml[58]
+          gh pr edit "$pr" --add-label automerge

Evidence

Automerge runs only on completion of the "CI - RBE" workflow, while pin-browsers applies the label
after detecting an existing PR. If CI - RBE already completed successfully for that existing PR’s
current HEAD, no new workflow_run event is generated by adding the label, so the automerge job never
executes.

.github/workflows/pin-browsers.yml[46-58]
.github/workflows/automerge.yml[3-7]

Agent prompt

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

### Issue description
When `pin-browsers.yml` finds an existing PR, it adds the `automerge` label but does not guarantee an additional `CI - RBE` completion event will occur afterward. If `CI - RBE` already passed for the current head SHA, the automerge workflow won’t run and the PR can get stuck.

### Issue Context
This is most likely during rollout/transition (existing open PRs) or any scenario where the label is applied after CI has already completed.

### Fix Focus Areas
- .github/workflows/pin-browsers.yml[46-58]
- .github/workflows/automerge.yml[3-7]

### Suggested approach (pick one)
1) **Trigger automerge evaluation on labeling**
  - Add `pull_request: { types: [labeled] }` (and/or `pull_request_target` if appropriate) to `automerge.yml`, and re-check eligibility + CI status before enabling auto-merge.

2) **One-shot evaluation in pin-browsers**
  - After adding the label, query whether `CI - RBE` already succeeded for the PR head SHA; if yes and no failed checks exist, run `gh pr merge ... --auto --match-head-commit` immediately (single query, no polling).

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

3. Uncaught failing conclusions 🐞 Bug ≡ Correctness

Description

The automerge workflow only blocks on check-run conclusions "failure" and "cancelled", so it can
still enable auto-merge when checks have already completed in other failure-like states (e.g.,
timed_out, action_required).

Code

.github/workflows/automerge.yml[40]

+          failed=$(gh api "repos/$REPO/commits/$SHA/check-runs" --jq 'any(.check_runs[]; .conclusion == "failure" or .conclusion == "cancelled")')

Evidence
The jq filter explicitly checks only two conclusion values; any other terminal non-success
conclusions will be ignored and won’t prevent enabling auto-merge.
.github/workflows/automerge.yml[39-41]
Best Practice: GitHub Checks API (check runs) conclusion values

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 current filter only treats `failure` and `cancelled` as blocking. Other terminal conclusions that indicate a bad state (like `timed_out` and `action_required`) are ignored.

### Issue Context
Your PR description says you "won't merge if another check has already come back as failed"—timeouts and action_required are also already-known bad outcomes.

### Fix Focus Areas
- .github/workflows/automerge.yml[39-41]

### Suggested approach
- Treat these as blocking at minimum: `failure`, `cancelled`, `timed_out`, `action_required`.
- Consider whether to treat any non-null conclusion other than `success`/`skipped`/`neutral` as blocking (depending on your desired strictness).

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

4. Branch PR lookup implicit 🐞 Bug ⚙ Maintainability

Description

The automerge workflow uses the workflow_run head branch name to identify the PR; using the PR
number from the workflow_run payload would be more explicit and less fragile across edge cases.

Code

.github/workflows/automerge.yml[R20-33]

+    env:
+      GH_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }}
+      BRANCH: ${{ github.event.workflow_run.head_branch }}
+      REPO: ${{ github.repository }}
+      SHA: ${{ github.event.workflow_run.head_sha }}
+    steps:
+      - name: Check pull request eligibility
+        id: eligible
+        run: |
+          eligible=$(gh pr view "$BRANCH" --repo "$REPO" --json headRefName,headRefOid,labels,state --jq '
+            .state == "OPEN" and
+            .headRefOid == env.SHA and
+            any(.labels[].name; . == "automerge")
+          ')

Evidence
The PR is resolved via gh pr view "$BRANCH" where BRANCH comes from workflow_run.head_branch.
Using the PR number directly would make the merge target unambiguous and simplify the script.
.github/workflows/automerge.yml[20-33]
Best Practice: GitHub Actions workflow_run payload

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 workflow currently uses branch name to locate the PR. This works in common cases but is less explicit than using the PR number attached to the workflow_run event.

### Issue Context
Using a PR number avoids ambiguity and makes the intent clearer.

### Fix Focus Areas
- .github/workflows/automerge.yml[20-33]

### Suggested approach
- Set `PR_NUMBER` from `github.event.workflow_run.pull_requests[0].number` (guard for empty arrays).
- Use `gh pr view "$PR_NUMBER" ...` and `gh pr merge "$PR_NUMBER" ...` instead of passing the branch name.

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

Copilot

Pull request overview

Adds a dedicated auto-merge mechanism for the pinned browser update PRs so they only become merge-eligible after CI - RBE completes successfully, preventing auto-merges when RBE fails.

Changes:

Update the pinned browsers workflow to label the created PR with automerge instead of enabling auto-merge immediately.
Add a new automerge workflow triggered by successful CI - RBE workflow_run events that enables auto-merge for eligible PRs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
.github/workflows/pin-browsers.yml	Switches from immediate `gh pr merge --auto` to applying an `automerge` label on the bot-created PR.
.github/workflows/automerge.yml	New workflow that runs after successful CI-RBE completion and enables auto-merge for labeled PRs from `selenium-ci`.

diemol · 2026-05-10T08:37:52Z

+        if: steps.eligible.outputs.eligible == 'true'
+        id: checks
+        run: |
+          failed=$(gh api "repos/$REPO/commits/$SHA/check-runs" --jq 'any(.check_runs[]; .conclusion == "failure" or .conclusion == "cancelled")')


I think having .conclusion != "success" is better than checking for other states. After changing this, we should be good to merge.

qodo-code-review · 2026-05-10T08:32:14Z

+        run: |
+          failed=$(gh api "repos/$REPO/commits/$SHA/check-runs" --jq 'any(.check_runs[]; .conclusion == "failure" or .conclusion == "cancelled")')
+          echo "failed=$failed" >> "$GITHUB_OUTPUT"


1. Missed failures due pagination 🐞 Bug ☼ Reliability

The automerge job queries the Checks API without pagination, so it can miss failures beyond the first page and still enable auto-merge even though some checks already failed.

Agent Prompt

### Issue description The automerge workflow checks `check-runs` for a commit SHA without pagination, so it may miss failed checks that appear on later pages and incorrectly proceed to enable auto-merge. ### Issue Context This repo’s CI workflow fans out into many jobs/reusable workflows, increasing the likelihood that check runs exceed the default page size. ### Fix Focus Areas - .github/workflows/automerge.yml[36-41] ### Suggested approach - Use `gh api --paginate` and aggregate pages before applying `jq`, e.g.: - `gh api --paginate "repos/$REPO/commits/$SHA/check-runs?per_page=100" | jq -s 'any(.[].check_runs[]; ... )'` - Alternatively, use a PR-centric command that already handles pagination (e.g., `gh pr checks`) if it provides the needed signal.

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

qodo-code-review · 2026-05-10T08:32:14Z

            pr="${pr_url##*/}"
          fi
-          gh pr merge "$pr" --auto --squash --delete-branch
+          gh pr edit "$pr" --add-label automerge


2. Label timing blocks automerge 🐞 Bug ☼ Reliability

pin-browsers.yml now only applies the "automerge" label; if a pinned-browser PR already exists and CI - RBE already succeeded before the label is added, the workflow_run-triggered automerge workflow will not run and the PR can remain unmerged.

Agent Prompt

### Issue description When `pin-browsers.yml` finds an existing PR, it adds the `automerge` label but does not guarantee an additional `CI - RBE` completion event will occur afterward. If `CI - RBE` already passed for the current head SHA, the automerge workflow won’t run and the PR can get stuck. ### Issue Context This is most likely during rollout/transition (existing open PRs) or any scenario where the label is applied after CI has already completed. ### Fix Focus Areas - .github/workflows/pin-browsers.yml[46-58] - .github/workflows/automerge.yml[3-7] ### Suggested approach (pick one) 1) **Trigger automerge evaluation on labeling** - Add `pull_request: { types: [labeled] }` (and/or `pull_request_target` if appropriate) to `automerge.yml`, and re-check eligibility + CI status before enabling auto-merge. 2) **One-shot evaluation in pin-browsers** - After adding the label, query whether `CI - RBE` already succeeded for the PR head SHA; if yes and no failed checks exist, run `gh pr merge ... --auto --match-head-commit` immediately (single query, no polling).

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

titusfortner · 2026-05-10T08:49:49Z

I've changed my mind; we may want this approach for something later, but for now we just won't auto-merge and that's fine.

diemol · 2026-05-10T09:03:34Z

👍

[build] require ci rbe to pass to merge pin browsers

736948e

Copilot AI review requested due to automatic review settings May 10, 2026 08:27

selenium-ci added the B-build Includes scripting, bazel and CI integrations label May 10, 2026

Copilot started reviewing on behalf of titusfortner May 10, 2026 08:28 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

qodo-code-review Bot reviewed May 10, 2026

View reviewed changes

titusfortner closed this May 10, 2026

titusfortner mentioned this pull request May 10, 2026

[build] do not auto-merge browser updates #17432

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[build] require ci rbe to pass to merge pin browsers#17431

[build] require ci rbe to pass to merge pin browsers#17431
titusfortner wants to merge 1 commit into
trunkfrom
automerge

titusfortner commented May 10, 2026

Uh oh!

qodo-code-review Bot commented May 10, 2026

Uh oh!

qodo-code-review Bot commented May 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

diemol May 10, 2026

Uh oh!

qodo-code-review Bot May 10, 2026

Uh oh!

qodo-code-review Bot May 10, 2026

Uh oh!

titusfortner commented May 10, 2026

Uh oh!

diemol commented May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

titusfortner commented May 10, 2026

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review Bot commented May 10, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

diemol May 10, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

titusfortner commented May 10, 2026

Uh oh!

diemol commented May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qodo-code-review Bot commented May 10, 2026 •

edited

Loading