[build] retrigger CI after auto-format commits by titusfortner · Pull Request #17000 · SeleniumHQ/selenium

titusfortner · 2026-01-25T04:31:53Z

User description

A PR had a lint issue that the autofix job autofixed, except there was another failure, and the way GitHub
does commits from GITHUB_TOKEN, it does not send the commit added / PR updated event, so the jobs do not rerun,
so the failures were hidden, which is not what we want.

💥 What does this PR do?

Use SELENIUM_CI_TOKEN to commit the changes so auto-fix pushes trigger a full CI run
Adds loop prevention to skip auto-formatting if the last commit was already from "Selenium CI Bot"

🔧 Implementation Notes

Added a check-bot-commit job that:

Runs after format fails (for PRs on non-forks)
Checks if the last commit author is "Selenium CI Bot"
Outputs should-commit=true/false

The commit-fixes job now only runs if should-commit == 'true'.

💡 Additional Considerations

Requires SELENIUM_CI_TOKEN secret to be configured with appropriate permissions. Falls back to current behavior if not available.

🔄 Types of changes

Bug fix (backwards compatible)

PR Type

Bug fix, Enhancement

Description

Adds loop prevention to skip auto-formatting if last commit from bot
Uses SELENIUM_CI_TOKEN to trigger full CI run on auto-fix commits
Implements check-bot-commit job to detect previous bot commits
Prevents infinite commit loops in auto-format workflow

Diagram Walkthrough

flowchart LR
  format["format job fails"]
  check["check-bot-commit job"]
  commit["commit-fixes job"]
  format -- "always runs after failure" --> check
  check -- "checks last commit author" --> commit
  commit -- "only runs if should-commit=true" --> trigger["triggers full CI run"]

File Walkthrough

Relevant files

Configuration changes

ci-lint.yml `Add bot commit detection and token-based CI retrigger` .github/workflows/ci-lint.yml Added new `check-bot-commit` job that detects if last commit was from Selenium CI Bot Job outputs `should-commit` flag to prevent infinite commit loops Modified `commit-fixes` job to depend on `check-bot-commit` instead of `format` Added `SELENIUM_CI_TOKEN` secret pass-through to `commit-changes` workflow	+27/-2

qodo-code-review · 2026-01-25T04:32:16Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	CI secret exfiltration Description: Passing the high-privilege `SELENIUM_CI_TOKEN` into the `commit-fixes` job (which has `contents: write`) could enable token exfiltration or unintended pushes if the called workflow (`./.github/workflows/commit-changes.yml`) executes any repository-controlled code (e.g., scripts from the PR branch) that can read environment/outputs and leak the secret. ci-lint.yml [90-103] Referred Code commit-fixes: name: Commit fixes needs: check-bot-commit if: needs.check-bot-commit.outputs.should-commit == 'true' permissions: contents: write actions: read uses: ./.github/workflows/commit-changes.yml with: artifact-name: format-changes commit-message: "Auto-format code" ref: ${{ github.event.pull_request.head.ref }} secrets: SELENIUM_CI_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }}
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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation 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: Token usage review: The workflow passes `SELENIUM_CI_TOKEN` into a reusable workflow and its downstream handling/permissions are not visible in this diff, so secret exposure and least-privilege adherence cannot be fully verified. Referred Code secrets: SELENIUM_CI_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }} 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 · 2026-01-25T04:33:23Z

PR Code Suggestions ✨

Latest suggestions up to bd8d060

Category	Suggestion	Impact
Possible issue	✅ ~~Checkout an immutable PR SHA~~ Suggestion Impact: The workflow checkout ref was changed from github.event.pull_request.head.ref to github.event.pull_request.head.sha, ensuring the action checks out the exact PR commit. code diff: @@ -75,7 +75,7 @@ - name: Checkout uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.ref }} + ref: ${{ github.event.pull_request.head.sha }} To prevent a race condition, check out the immutable PR head SHA (`github.event.pull_request.head.sha`) instead of the mutable branch ref (`github.event.pull_request.head.ref`). .github/workflows/ci-lint.yml [75-78] - name: Checkout uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.ref }} + ref: ${{ github.event.pull_request.head.sha }} `[Suggestion processed]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a race condition where checking out the branch `ref` could lead to analyzing a different commit than the one that failed the format check, thus making the bot-commit check unreliable. Using the commit `sha` ensures the correct commit is analyzed.	Medium
Incremental ^[*]	Skip committing on unknown author Add a check to handle cases where the last commit author's email cannot be determined, preventing the workflow from incorrectly proceeding to commit fixes. .github/workflows/ci-lint.yml [82-83] -LAST_AUTHOR_EMAIL=$(git log -1 --format='%ae') +LAST_AUTHOR_EMAIL="$(git log -1 --format='%ae' \|\| true)" +if [ -z "$LAST_AUTHOR_EMAIL" ]; then + echo "::warning::Unable to determine last commit author email; skipping commit-fixes" + echo "should-commit=false" >> "$GITHUB_OUTPUT" + exit 0 +fi if [ "$LAST_AUTHOR_EMAIL" = "selenium-ci@users.noreply.github.com" ]; then Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a potential bug where an empty author email would cause an unwanted commit, and provides a robust fix by adding necessary error handling.	Medium
Incremental ^[*]	Normalize author email before compare Normalize the author email to lowercase and trim whitespace before comparison to make the check more robust. .github/workflows/ci-lint.yml [82-83] -LAST_AUTHOR_EMAIL=$(git log -1 --format='%ae') +LAST_AUTHOR_EMAIL="$(git show -s --format='%ae' HEAD \| tr '[:upper:]' '[:lower:]' \| xargs)" if [ "$LAST_AUTHOR_EMAIL" = "selenium-ci@users.noreply.github.com" ]; then Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves the robustness of the script by normalizing the email string before comparison, which is a good defensive programming practice against potential environment inconsistencies.	Low
General	Gate fixes on format failure Make the `commit-fixes` job explicitly dependent on the `format` job's failure, in addition to the `check-bot-commit` output, to improve robustness. .github/workflows/ci-lint.yml [90-93] commit-fixes: name: Commit fixes - needs: check-bot-commit - if: needs.check-bot-commit.outputs.should-commit == 'true' + needs: [format, check-bot-commit] + if: needs.format.result == 'failure' && needs.check-bot-commit.outputs.should-commit == 'true' Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion improves the workflow's robustness and readability by making the trigger conditions for the `commit-fixes` job explicit, preventing potential issues if dependency logic changes in the future.	Low
Update

Previous suggestions

✅ Suggestions up to commit 7175fad

Category	Suggestion	Impact
General	✅ ~~Use case-insensitive author name comparison~~ Suggestion Impact: Instead of implementing a case-insensitive author name comparison, the workflow was changed to check the last commit author's email address against the bot's noreply email, which achieves the same robustness goal via a different approach. code diff: - LAST_AUTHOR=$(git log -1 --format='%an') - if [ "$LAST_AUTHOR" = "Selenium CI Bot" ]; then + LAST_AUTHOR_EMAIL=$(git log -1 --format='%ae') + if [ "$LAST_AUTHOR_EMAIL" = "selenium-ci@users.noreply.github.com" ]; then To improve robustness, modify the shell script to use a case-insensitive comparison when checking the commit author's name. .github/workflows/ci-lint.yml [81-88] run: \| LAST_AUTHOR=$(git log -1 --format='%an') - if [ "$LAST_AUTHOR" = "Selenium CI Bot" ]; then + if [ "$(echo "$LAST_AUTHOR" \| tr '[:upper:]' '[:lower:]')" = "selenium ci bot" ]; then echo "::notice::Last commit was from Selenium CI Bot - skipping commit-fixes" echo "should-commit=false" >> "$GITHUB_OUTPUT" else echo "should-commit=true" >> "$GITHUB_OUTPUT" fi `[Suggestion processed]` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that a case-sensitive comparison for the author name is brittle and proposes a more robust, case-insensitive check, improving the reliability of the workflow.	Low
Learned best practice	✅ ~~Harden author check validation~~ Suggestion Impact: The workflow was updated to check the last commit author by email (%ae) instead of display name (%an), aligning with the suggestion’s goal of using a more stable identifier. However, the additional hardening (explicit bash shell, strict mode, trimming, and empty-author handling) was not implemented. code diff: - name: Check last commit author id: check run: \| - LAST_AUTHOR=$(git log -1 --format='%an') - if [ "$LAST_AUTHOR" = "Selenium CI Bot" ]; then + LAST_AUTHOR_EMAIL=$(git log -1 --format='%ae') + if [ "$LAST_AUTHOR_EMAIL" = "selenium-ci@users.noreply.github.com" ]; then echo "::notice::Last commit was from Selenium CI Bot - skipping commit-fixes" echo "should-commit=false" >> "$GITHUB_OUTPUT" else Make the step more robust by enabling strict shell mode, trimming/validating the retrieved author, and preferring a stable identifier (e.g., author email) rather than a display name. .github/workflows/ci-lint.yml [79-88] - name: Check last commit author id: check + shell: bash run: \| - LAST_AUTHOR=$(git log -1 --format='%an') - if [ "$LAST_AUTHOR" = "Selenium CI Bot" ]; then + set -euo pipefail + + LAST_AUTHOR_EMAIL="$(git log -1 --format='%ae' \| tr -d '\r' \| xargs \|\| true)" + if [ -z "$LAST_AUTHOR_EMAIL" ]; then + echo "::warning::Could not determine last commit author email; skipping auto-commit to avoid loops" + echo "should-commit=false" >> "$GITHUB_OUTPUT" + elif [ "$LAST_AUTHOR_EMAIL" = "selenium-ci-bot@example.com" ]; then echo "::notice::Last commit was from Selenium CI Bot - skipping commit-fixes" echo "should-commit=false" >> "$GITHUB_OUTPUT" else echo "should-commit=true" >> "$GITHUB_OUTPUT" fi `[Suggestion processed]` Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (GitHub Actions + shell) by trimming/checking presence and avoiding brittle identity checks before using values.	Low

Copilot

Pull request overview

This PR fixes an issue where auto-format commits from the CI bot didn't trigger subsequent CI runs, hiding test failures. The solution uses a Personal Access Token (SELENIUM_CI_TOKEN) for commits and implements loop prevention to avoid infinite commit cycles.

Changes:

Added SELENIUM_CI_TOKEN to commit-fixes job to trigger CI on bot commits
Implemented check-bot-commit job to detect and prevent infinite loops
Restructured job dependencies: format → check-bot-commit → commit-fixes

[build] retrigger CI after auto-format commits

7175fad

titusfortner requested a review from Copilot January 25, 2026 04:31

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

Copilot started reviewing on behalf of titusfortner January 25, 2026 04:32 View session

qodo-code-review bot added the Review effort 2/5 label Jan 25, 2026

Copilot AI reviewed Jan 25, 2026

View reviewed changes

titusfortner added 2 commits January 24, 2026 22:43

[build] use author email for bot commit detection

bd8d060

[build] use immutable SHA for checkout

8daa6df

titusfortner merged commit 07dd181 into trunk Jan 25, 2026
25 checks passed

titusfortner deleted the ci_autoformat_retrigger branch January 25, 2026 05:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[build] retrigger CI after auto-format commits#17000

[build] retrigger CI after auto-format commits#17000
titusfortner merged 3 commits intotrunkfrom
ci_autoformat_retrigger

titusfortner commented Jan 25, 2026 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Jan 25, 2026 •

edited

Loading

Uh oh!

qodo-code-review bot commented Jan 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

titusfortner commented Jan 25, 2026 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

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

Uh oh!

PR Compliance Guide 🔍

Uh oh!

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

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

titusfortner commented Jan 25, 2026 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Jan 25, 2026 •

edited

Loading

qodo-code-review bot commented Jan 25, 2026 •

edited

Loading