Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 24, 2026

User description

These end up sitting out there. Shouldn't be any reason not to merge them if everything is passing.

💥 What does this PR do?

  • Once tests pass, the PR will merge and delete branch

💡 Additional Considerations

Anything else we want to auto-commit? 😀

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Auto-merge browser version update PRs after tests pass

  • Capture PR number from creation for merge operation

  • Delete branch automatically upon successful merge


Diagram Walkthrough

flowchart LR
  A["Check existing PR"] --> B{"PR exists?"}
  B -->|Yes| C["Use existing PR number"]
  B -->|No| D["Create new PR and capture number"]
  C --> E["Auto-merge PR with delete-branch"]
  D --> E
Loading

File Walkthrough

Relevant files
Enhancement
pin-browsers.yml
Add auto-merge logic for browser update PRs                           

.github/workflows/pin-browsers.yml

  • Modified PR creation to capture the PR number for subsequent merge
    operation
  • Added auto-merge step that merges the PR and deletes the branch after
    tests pass
  • Handles both existing and newly created PRs with unified merge logic
+5/-2     

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Jan 24, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 24, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Auto-merge privilege risk

Description: The workflow now auto-merges and deletes the branch via gh pr merge "$pr" --auto --merge
--delete-branch using a repository secret token (secrets.SELENIUM_CI_TOKEN), which can
bypass intended human review/approval gates and becomes a meaningful supply-chain risk if
the automation branch (pinned-browser-updates) or workflow execution context is ever
compromised (e.g., an attacker able to influence that branch/PR content could get changes
merged automatically once checks pass).
pin-browsers.yml [40-55]

Referred Code
env:
  GH_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }}
run: |
  existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty')
  if [ -n "$existing" ]; then
    echo "::notice::PR #$existing already exists"
    pr="$existing"
  else
    pr=$(gh pr create \
      --head pinned-browser-updates \
      --base trunk \
      --title "[dotnet][rb][java][js][py] Automated Browser Version Update" \
      --body $'This is an automated pull request to update pinned browsers and drivers\n\nMerge after verify the new browser versions properly passing the tests and no bugs need to be filed' \
      --json number --jq '.number')
  fi
  gh pr merge "$pr" --auto --merge --delete-branch
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing failure checks: The workflow does not validate that pr is non-empty or that gh pr create succeeded before
attempting gh pr merge, which can fail unclearly and lacks actionable context.

Referred Code
existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty')
if [ -n "$existing" ]; then
  echo "::notice::PR #$existing already exists"
  pr="$existing"
else
  pr=$(gh pr create \
    --head pinned-browser-updates \
    --base trunk \
    --title "[dotnet][rb][java][js][py] Automated Browser Version Update" \
    --body $'This is an automated pull request to update pinned browsers and drivers\n\nMerge after verify the new browser versions properly passing the tests and no bugs need to be filed' \
    --json number --jq '.number')
fi
gh pr merge "$pr" --auto --merge --delete-branch

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing merge audit: The workflow performs critical actions (auto-merge and branch deletion) without explicitly
logging the merge outcome/details beyond default GitHub Actions output, which may be
insufficient for required audit reconstruction.

Referred Code
existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty')
if [ -n "$existing" ]; then
  echo "::notice::PR #$existing already exists"
  pr="$existing"
else
  pr=$(gh pr create \
    --head pinned-browser-updates \
    --base trunk \
    --title "[dotnet][rb][java][js][py] Automated Browser Version Update" \
    --body $'This is an automated pull request to update pinned browsers and drivers\n\nMerge after verify the new browser versions properly passing the tests and no bugs need to be filed' \
    --json number --jq '.number')
fi
gh pr merge "$pr" --auto --merge --delete-branch

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 24, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate PR number before merge

Add a check to verify that the $pr variable is not empty before attempting to
execute the gh pr merge command.

.github/workflows/pin-browsers.yml [46-55]

         pr="$existing"
         pr=$(gh pr create \
           --head pinned-browser-updates \
           --base trunk \
           --title "[dotnet][rb][java][js][py] Automated Browser Version Update" \
           --body $'This is an automated pull request to update pinned browsers and drivers\n\nMerge after verify the new browser versions properly passing the tests and no bugs need to be filed' \
           --json number --jq '.number')
+        if [ -z "$pr" ]; then
+          echo "::error::Failed to get or create PR number"
+          exit 1
+        fi
         gh pr merge "$pr" --auto --merge --delete-branch
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a critical flaw where the script would fail if the gh pr create command fails and the pr variable is empty. Adding a check to ensure pr is not empty before merging prevents the workflow step from failing.

Medium
  • Update

Copy link
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

Adds automation so the scheduled “Pin Browsers” workflow will enable auto-merge on the pinned-browser-updates PR (and delete the branch) once required checks pass, reducing manual maintenance for routine browser version bumps.

Changes:

  • Capture the PR number when creating a new pinned-browser-updates PR (or reuse the existing PR number).
  • Enable auto-merge for that PR and delete the branch after merge.

--body $'This is an automated pull request to update pinned browsers and drivers\n\nMerge after verify the new browser versions properly passing the tests and no bugs need to be filed' \
--json number --jq '.number')
fi
gh pr merge "$pr" --auto --merge --delete-branch
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

gh pr merge is using --merge, but this repo’s documented convention is to squash PRs into trunk (see AGENTS.md:71). Using merge commits may fail if merge commits are disabled in repo settings and it also breaks the expected history style. Consider switching to --squash (or omit the method flag if squash is the repo default).

Copilot uses AI. Check for mistakes.
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 Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants