[build] add reusable commit-changes.yml workflow

[titusfortner]

User description

Repeated pattern with our bazel.yml approach. Bazel workflow generates a patch, then calling workflow does:

• checkout
• download patch
• apply patch
• commit change
• push commit

💥 What does this PR do?

Adds a reusable commit-changes.yml workflow to manage this flow

Refactors the following workflows to use it:

• ci-lint.yml (commit-fixes job)
• ci-renovate-rbe.yml
• pin-browsers.yml
• pre-release.yml (push-rust-version job)
• release.yml (update-version job)

🔧 Implementation Notes

This pattern was duplicated across 5 workflows with minor variations. The reusable workflow supports:

• Custom artifact name and commit message
• Optional target ref to checkout
• Optional push-branch for force-pushing to a different branch (selenium manager job)

💡 Additional Considerations

More tweaks to simplify incoming

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement

---

Description

• Introduces reusable commit-changes.yml workflow for standardized patch application

• Refactors 5 workflows to use centralized commit-changes workflow

• Supports custom artifact names, commit messages, and optional branch pushing

• Eliminates duplicated patch-apply-commit-push logic across workflows

---

Diagram Walkthrough

flowchart LR
  A["Artifact Generation<br/>bazel.yml"] -- "artifact-name" --> B["commit-changes.yml<br/>Reusable Workflow"]
  B -- "applies patch" --> C["Git Commit<br/>& Push"]
  D["ci-lint.yml"] -- "uses" --> B
  E["ci-renovate-rbe.yml"] -- "uses" --> B
  F["pin-browsers.yml"] -- "uses" --> B
  G["pre-release.yml"] -- "uses" --> B
  H["release.yml"] -- "uses" --> B

Loading

File Walkthrough

	Relevant files
Enhancement	commit-changes.yml New reusable workflow for patch management                              .github/workflows/commit-changes.yml New reusable workflow that handles patch application, git configuration, and commit/push operations Accepts inputs for artifact name, commit message, optional ref, and optional push-branch Supports force-push to alternate branches via push-branch parameter Outputs changes-committed flag to indicate success/failure Supports optional SELENIUM_CI_TOKEN secret for authenticated operations +72/-0    pin-browsers.yml Refactor to use commit-changes and gh CLI                                .github/workflows/pin-browsers.yml Refactors pull-request job into two separate jobs: push-changes and create-pr push-changes job now uses commit-changes.yml with force-push to pinned-browser-updates branch create-pr job uses GitHub CLI to create PR, checking for existing PRs to avoid duplicates Replaces peter-evans/create-pull-request action with native gh pr create command +30/-29
Cleanup	ci-lint.yml Refactor to use commit-changes workflow                                    .github/workflows/ci-lint.yml Replaces 31 lines of inline patch-apply-commit-push logic with call to commit-changes.yml Simplifies commit-fixes job to use reusable workflow with artifact-name and commit-message inputs Moves fork check to job-level condition instead of step-level check +6/-37    ci-renovate-rbe.yml Refactor to use commit-changes workflow                                    .github/workflows/ci-renovate-rbe.yml Replaces 18 lines of inline patch-apply-commit-push logic with commit-changes.yml call Simplifies commit-repins job configuration Removes manual git configuration and error handling steps +4/-22    pre-release.yml Refactor to use commit-changes workflow                                    .github/workflows/pre-release.yml Replaces 27 lines of inline patch-apply-commit-push logic in push-rust-version job with commit-changes.yml call Adds force-push to rust-release-${{ inputs.version }} branch via push-branch parameter Passes SELENIUM_CI_TOKEN secret to reusable workflow Minor comment fix: "Run rust jobs" instead of "Rust jobs" +8/-24    release.yml Refactor to use commit-changes workflow                                    .github/workflows/release.yml Replaces 27 lines of inline patch-apply-commit-push logic in update-version job with commit-changes.yml call Simplifies version reset workflow by delegating to reusable workflow Passes SELENIUM_CI_TOKEN secret for authenticated git operations +6/-23

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Untrusted patch push Description: The reusable workflow downloads an artifact-produced changes.patch and applies it (git apply --index changes.patch) before pushing with contents: write, which can allow a compromised or attacker-influenced patch artifact to commit and push arbitrary repository changes (including .github/workflows/*) and, when push-branch is set, force-push them to a target branch. commit-changes.yml [41-63] Referred Code - name: Checkout uses: actions/checkout@v4 with: ref: ${{ inputs.ref || github.ref }} token: ${{ secrets.SELENIUM_CI_TOKEN || github.token }} - name: Download patch uses: actions/download-artifact@v4 with: name: ${{ inputs.artifact-name }} continue-on-error: true - name: Apply and commit id: commit run: | if [ -f changes.patch ] && [ -s changes.patch ]; then git apply --index changes.patch git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" git commit -m "$COMMIT_MESSAGE" if [ -n "$PUSH_BRANCH" ]; then git push origin HEAD:"$PUSH_BRANCH" --force else ... (clipped 2 lines)
	Force-push risk Description: Allowing push-branch to trigger git push origin HEAD:"$PUSH_BRANCH" --force can enable destructive overwrites of the remote branch history if misconfigured or invoked with an unintended branch name, potentially bypassing normal safeguards depending on branch protection settings and token privileges. commit-changes.yml [59-62] Referred Code if [ -n "$PUSH_BRANCH" ]; then git push origin HEAD:"$PUSH_BRANCH" --force else git push
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: 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: 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 can silently proceed after patch download/apply/commit/push failures (e.g., continue-on-error: true on download and no explicit handling of git apply, git commit, or git push exit codes), reducing reliability and diagnosability. Referred Code - name: Download patch uses: actions/download-artifact@v4 with: name: ${{ inputs.artifact-name }} continue-on-error: true - name: Apply and commit id: commit run: | if [ -f changes.patch ] && [ -s changes.patch ]; then git apply --index changes.patch git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" git commit -m "$COMMIT_MESSAGE" if [ -n "$PUSH_BRANCH" ]; then git push origin HEAD:"$PUSH_BRANCH" --force else git push fi echo "::notice::Changes committed and pushed" echo "committed=true" >> "$GITHUB_OUTPUT" else ... (clipped 3 lines) 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: Unvalidated force-push target: The reusable workflow accepts push-branch and performs a force push to the provided branch without validating/allowlisting the target, which could be risky depending on who can call the workflow and with what inputs. Referred Code push-branch: description: Branch to push to (defaults to current branch, uses force push) required: false type: string default: '' outputs: changes-committed: description: Whether changes were committed and pushed value: ${{ jobs.commit.outputs.committed }} secrets: SELENIUM_CI_TOKEN: required: false jobs: commit: name: Commit Changes runs-on: ubuntu-latest outputs: committed: ${{ steps.commit.outputs.committed }} permissions: contents: write ... (clipped 24 lines) 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]

PR Code Suggestions ✨

Latest suggestions up to 4e296c3

Category	Suggestion	Impact
Security	Restrict workflow to intended repo Add an if condition to the commit job to ensure the workflow only runs within the SeleniumHQ/selenium repository, enhancing security. .github/workflows/commit-changes.yml [32-46] jobs: commit: name: Commit Changes + if: github.repository == 'SeleniumHQ/selenium' runs-on: ubuntu-latest outputs: committed: ${{ steps.commit.outputs.committed }} permissions: contents: write actions: read steps: - name: Checkout uses: actions/checkout@v4 with: ref: ${{ inputs.ref || github.ref }} token: ${{ secrets.SELENIUM_CI_TOKEN || github.token }} Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is a valuable security hardening measure that restricts the powerful reusable workflow to its intended repository, SeleniumHQ/selenium, preventing potential misuse in other contexts.	Medium
Possible issue	Push explicitly to a branch To prevent git push from failing in detached HEAD states, explicitly push to the checked-out ref name instead of relying on implicit upstream configuration. .github/workflows/commit-changes.yml [53-92] - name: Apply and commit id: commit run: | if [ "$DOWNLOAD_OUTCOME" != "success" ]; then echo "::notice::Artifact not found (may not have been uploaded)" echo "committed=false" >> "$GITHUB_OUTPUT" exit 0 fi if [ -f changes.patch ] && [ -s changes.patch ]; then if ! git apply --index changes.patch; then echo "::error::Failed to apply patch" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" if ! git commit -m "$COMMIT_MESSAGE"; then echo "::error::Failed to commit changes" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi if [ -n "$PUSH_BRANCH" ]; then if ! git push origin HEAD:"$PUSH_BRANCH" --force; then echo "::error::Failed to push to $PUSH_BRANCH" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi else - if ! git push; then + if ! git push origin HEAD:"$REF_NAME"; then echo "::error::Failed to push" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi fi echo "::notice::Changes committed and pushed" echo "committed=true" >> "$GITHUB_OUTPUT" else echo "::notice::No changes to commit" echo "committed=false" >> "$GITHUB_OUTPUT" fi + env: + DOWNLOAD_OUTCOME: ${{ steps.download.outcome }} + COMMIT_MESSAGE: ${{ inputs.commit-message }} + PUSH_BRANCH: ${{ inputs.push-branch }} + REF_NAME: ${{ inputs.ref || github.ref_name }} Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that git push can fail in a detached HEAD state, but the proposed fix of pushing to github.ref_name is not universally correct, as it would fail when trying to push to a tag.	Medium
More

---

Previous suggestions

✅ Suggestions up to commit 5e09c2c

Category	Suggestion	Impact
Security	Avoid unsafe force pushes Replace git push --force with the safer git push --force-with-lease to prevent overwriting remote changes in case of race conditions. .github/workflows/commit-changes.yml [67-79] if [ -n "$PUSH_BRANCH" ]; then - if ! git push origin HEAD:"$PUSH_BRANCH" --force; then + if ! git push origin HEAD:"$PUSH_BRANCH" --force-with-lease; then echo "::error::Failed to push to $PUSH_BRANCH" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi else if ! git push; then echo "::error::Failed to push" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi fi Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out the risk of using git push --force and proposes using --force-with-lease as a safer alternative to prevent race conditions and accidental overwrites, which is a critical improvement for an automated CI workflow.	Medium
Possible issue	✅ Allow artifact downloads permission Suggestion Impact: The workflow job permissions were updated to include `actions: read`, matching the suggestion to allow reliable artifact downloads. code diff: permissions: contents: write + actions: read Add actions: read permission to the commit job to ensure the download-artifact step can reliably fetch artifacts in restricted environments. .github/workflows/commit-changes.yml [38-39] permissions: contents: write + actions: read [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that actions: read permission might be necessary for actions/download-artifact@v4 in repositories with restricted permissions, improving the workflow's robustness and preventing silent failures.	Low

✅ Suggestions up to commit 8d46d3a

Category	Suggestion	Impact
Possible issue	✅ Fail on missing artifact Suggestion Impact: The commit added explicit failure handling and clearer error messages for `git apply`, `git commit`, and `git push` (exiting with errors and setting `committed=false`), aligning with the suggestion’s goal of not masking failures. However, it did not remove `continue-on-error: true` from the artifact download step nor add an explicit "missing changes.patch" check. code diff: if [ -f changes.patch ] && [ -s changes.patch ]; then - git apply --index changes.patch + if ! git apply --index changes.patch; then + echo "::error::Failed to apply patch" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" - git commit -m "$COMMIT_MESSAGE" + if ! git commit -m "$COMMIT_MESSAGE"; then + echo "::error::Failed to commit changes" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi if [ -n "$PUSH_BRANCH" ]; then - git push origin HEAD:"$PUSH_BRANCH" --force + if ! git push origin HEAD:"$PUSH_BRANCH" --force; then + echo "::error::Failed to push to $PUSH_BRANCH" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi else - git push + if ! git push; then + echo "::error::Failed to push" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi Remove continue-on-error: true from the artifact download step and add an explicit check for the patch file's existence to avoid masking failures and provide clearer error messages. .github/workflows/commit-changes.yml [46-88] - name: Download patch uses: actions/download-artifact@v4 with: name: ${{ inputs.artifact-name }} - continue-on-error: true - name: Apply and commit id: commit run: | - if [ -f changes.patch ] && [ -s changes.patch ]; then + if [ ! -f changes.patch ]; then + echo "::error::Expected changes.patch artifact but it was not downloaded" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi + + if [ -s changes.patch ]; then if ! git apply --index changes.patch; then echo "::error::Failed to apply patch" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" if ! git commit -m "$COMMIT_MESSAGE"; then echo "::error::Failed to commit changes" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi if [ -n "$PUSH_BRANCH" ]; then if ! git push origin HEAD:"$PUSH_BRANCH" --force; then echo "::error::Failed to push to $PUSH_BRANCH" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi else if ! git push; then echo "::error::Failed to push" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi fi echo "::notice::Changes committed and pushed" echo "committed=true" >> "$GITHUB_OUTPUT" else echo "::notice::No changes to commit" echo "committed=false" >> "$GITHUB_OUTPUT" fi env: COMMIT_MESSAGE: ${{ inputs.commit-message }} PUSH_BRANCH: ${{ inputs.push-branch }} [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that continue-on-error: true can mask failures, and improving the robustness of this new reusable workflow by providing clearer failure states is a valuable improvement.	Medium
Learned best practice	✅ Validate inputs and fail fast Suggestion Impact: The commit implemented fail-fast/guarded command execution by wrapping `git apply`, `git commit`, and `git push` in `if ! ...; then` blocks that emit GitHub Actions errors and exit non-zero. However, it did not add `set -euo pipefail`, did not validate/trim `COMMIT_MESSAGE`, and did not change the PUSH_BRANCH check to use `${PUSH_BRANCH:-}`. code diff: if [ -f changes.patch ] && [ -s changes.patch ]; then - git apply --index changes.patch + if ! git apply --index changes.patch; then + echo "::error::Failed to apply patch" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" - git commit -m "$COMMIT_MESSAGE" + if ! git commit -m "$COMMIT_MESSAGE"; then + echo "::error::Failed to commit changes" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi if [ -n "$PUSH_BRANCH" ]; then - git push origin HEAD:"$PUSH_BRANCH" --force + if ! git push origin HEAD:"$PUSH_BRANCH" --force; then + echo "::error::Failed to push to $PUSH_BRANCH" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi else - git push + if ! git push; then + echo "::error::Failed to push" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi fi Validate that COMMIT_MESSAGE is non-empty (and optionally trim it) and enable strict shell options to prevent unexpected behavior from empty inputs or failing commands. .github/workflows/commit-changes.yml [51-85] - name: Apply and commit id: commit run: | + set -euo pipefail + if [ -f changes.patch ] && [ -s changes.patch ]; then + if [ -z "${COMMIT_MESSAGE//[[:space:]]/}" ]; then + echo "::error::Commit message is required" + echo "committed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi + if ! git apply --index changes.patch; then echo "::error::Failed to apply patch" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" if ! git commit -m "$COMMIT_MESSAGE"; then echo "::error::Failed to commit changes" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi - if [ -n "$PUSH_BRANCH" ]; then + if [ -n "${PUSH_BRANCH:-}" ]; then if ! git push origin HEAD:"$PUSH_BRANCH" --force; then echo "::error::Failed to push to $PUSH_BRANCH" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi else if ! git push; then echo "::error::Failed to push" echo "committed=false" >> "$GITHUB_OUTPUT" exit 1 fi fi echo "::notice::Changes committed and pushed" echo "committed=true" >> "$GITHUB_OUTPUT" else echo "::notice::No changes to commit" echo "committed=false" >> "$GITHUB_OUTPUT" fi [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (workflow inputs/env) by trimming/checking presence before use.	Low

✅ Suggestions up to commit 83eac7c

Category	Suggestion	Impact
Possible issue	Grant permissions to create PRs Add permissions: { pull-requests: write } to the create-pr job to allow it to create pull requests. .github/workflows/pin-browsers.yml [31-54] create-pr: name: Create Pull Request needs: push-changes if: github.event.repository.fork == false && needs.push-changes.outputs.changes-committed == 'true' runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write steps: - name: Checkout uses: actions/checkout@v4 - name: Create Pull Request 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" else 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 Merge after verify the new browser versions properly passing the tests and no bugs need to be filed" fi Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the create-pr job is missing the necessary pull-requests: write permission, which would cause the gh pr create command to fail. This is a critical fix for the workflow's functionality.	Medium
	Safer force pushes to branches Use git push --force-with-lease instead of --force to prevent accidentally overwriting concurrent changes on the remote branch. .github/workflows/commit-changes.yml [60-64] if [ -n "$PUSH_BRANCH" ]; then - git push origin HEAD:"$PUSH_BRANCH" --force + git push origin HEAD:"$PUSH_BRANCH" --force-with-lease else git push fi Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential race condition and proposes using --force-with-lease for a safer force push, which is a best practice in CI/CD pipelines to prevent data loss.	Medium
Learned best practice	✅ Declare required artifact permissions Suggestion Impact: The workflow job permissions were updated to include `actions: read`, matching the suggestion to avoid relying on implicit artifact access permissions. code diff: permissions: contents: write + actions: read Add actions: read to the job permissions because the workflow downloads artifacts; this avoids relying on implicit/default permissions that may differ across repos/settings. .github/workflows/commit-changes.yml [38-39] permissions: contents: write + actions: read Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Request least privileges explicitly; ensure required permissions (like artifact read) are declared for jobs that download artifacts.	Low
	✅ Avoid brittle multiline CLI quoting Suggestion Impact: The commit incorporated the suggested robustness improvement to the `gh pr list` jq expression by adding `// empty` so the `existing` variable becomes empty instead of `null` when no PR exists. It did not implement the main heredoc/`--body-file` change. code diff: - existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number') + existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty') Avoid embedding a multi-line --body string directly in the command; write the body via a heredoc and pass it using --body-file to prevent quoting/indentation issues. .github/workflows/pin-browsers.yml [42-54] 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" else + cat > pr-body.txt <<'EOF' +This is an automated pull request to update pinned browsers and drivers + +Merge after verify the new browser versions properly passing the tests and no bugs need to be filed +EOF 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 - - Merge after verify the new browser versions properly passing the tests and no bugs need to be filed" + --body-file pr-body.txt fi Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Prefer robust input handling at integration boundaries; avoid brittle multi-line shell quoting when building API/CLI payloads.	Low

✅ Suggestions up to commit af68a41

Category	Suggestion	Impact
Security	Fix command injection in commit To prevent command injection, pass the commit message to git commit via standard input using echo "$COMMIT_MESSAGE" | git commit --file - instead of using the -m flag. .github/workflows/commit-changes.yml [58] -git commit -m "$COMMIT_MESSAGE" +echo "$COMMIT_MESSAGE" | git commit --file - Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a command injection vulnerability and provides a standard, effective mitigation, significantly improving the security of the new workflow.	High
Possible issue	✅ Fail on patch application errors Suggestion Impact: The workflow step's run block was updated to include `set -e`, ensuring the script exits on errors such as a failed `git apply`. code diff: run: | + set -e if [ -f changes.patch ] && [ -s changes.patch ]; then Add set -e to the beginning of the run block to ensure the script exits immediately if any command, such as git apply, fails. .github/workflows/commit-changes.yml [54-69] -if [ -f changes.patch ] && [ -s changes.patch ]; then - git apply --index changes.patch - git config --local user.email "selenium-ci@users.noreply.github.com" - git config --local user.name "Selenium CI Bot" - git commit -m "$COMMIT_MESSAGE" - […] -else - echo "::notice::No changes to commit" - echo "committed=false" >> "$GITHUB_OUTPUT" -fi +run: | + set -e + if [ -f changes.patch ] && [ -s changes.patch ]; then + git apply --index changes.patch + […] + else + echo "::notice::No changes to commit" + echo "committed=false" >> "$GITHUB_OUTPUT" + fi [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the script would silently continue if git apply fails; adding set -e is a crucial change to ensure the job fails immediately on error.	Medium
	Allow workflow to error on missing patch Add a fail-if-no-changes boolean input to the reusable workflow to allow callers to specify whether the job should fail if no patch is found, preserving the behavior of the original workflows. .github/workflows/commit-changes.yml [3-23] on: workflow_call: inputs: artifact-name: - description: Name of artifact containing changes.patch - required: true - type: string + … commit-message: - description: Commit message - required: true - type: string + … ref: … push-branch: … + fail-if-no-changes: + description: Whether to fail the job if no changes.patch is found + required: false + type: boolean + default: false +… +steps: + - name: Apply and commit + id: commit + run: | + set -e + if [ -f changes.patch ] && [ -s changes.patch ]; then + git apply --index changes.patch + […] + echo "committed=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::No changes to commit" + echo "committed=false" >> "$GITHUB_OUTPUT" + if [ "${FAIL_IF_NO_CHANGES}" = "true" ]; then + echo "::error::No patch applied"; exit 1 + fi + fi + env: + COMMIT_MESSAGE: ${{ inputs.commit-message }} + PUSH_BRANCH: ${{ inputs.push-branch }} + FAIL_IF_NO_CHANGES: ${{ inputs.fail-if-no-changes }} Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a semantic difference between the old and new workflows and proposes a flexible solution to restore the original behavior where needed, improving the reusability and correctness of the new workflow.	Medium
General	Enable full git fetch Add fetch-depth: 0 to the actions/checkout step to ensure the full git history is fetched, which is necessary for force-pushing. .github/workflows/commit-changes.yml [41-45] - name: Checkout uses: actions/checkout@v4 with: ref: ${{ inputs.ref || github.ref }} token: ${{ secrets.SELENIUM_CI_TOKEN || github.token }} + fetch-depth: 0 Suggestion importance[1-10]: 6 __ Why: Adding fetch-depth: 0 ensures the full git history is available, preventing potential errors during the git push --force operation and increasing the workflow's robustness.	Low
	Exit early if PR exists Modify the script to exit with a success code immediately after finding an existing pull request, rather than silently finishing the step. .github/workflows/pin-browsers.yml [43-54] -existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number') -if [ -n "$existing" ]; then - echo "::notice::PR #$existing already exists" -else - 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 +if gh pr view --json number --jq .number pinned-browser-updates > /dev/null 2>&1; then + echo "::notice::PR from pinned-browser-updates already exists." + exit 0 +fi +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 Merge after verify the new browser versions properly passing the tests and no bugs need to be filed" -fi Suggestion importance[1-10]: 5 __ Why: The suggestion improves the script's clarity by exiting explicitly if a pull request already exists, making the workflow's behavior more transparent and robust.	Low
Learned best practice	✅ Validate workflow inputs before use Suggestion Impact: The commit adopted part of the suggestion by adding a shell safety setting (`set -e`) at the start of the "Apply and commit" step. It did not implement the proposed input trimming/validation nor `set -euo pipefail`. code diff: @@ -51,6 +51,7 @@ - name: Apply and commit id: commit run: | + set -e if [ -f changes.patch ] && [ -s changes.patch ]; then Validate/sanitize COMMIT_MESSAGE and PUSH_BRANCH (non-empty, trimmed, and restricted charset) before using them, to avoid unexpected behavior or refspec injection. .github/workflows/commit-changes.yml [51-72] - name: Apply and commit id: commit run: | + set -euo pipefail + + COMMIT_MESSAGE="${COMMIT_MESSAGE#"${COMMIT_MESSAGE%%[![:space:]]*}"}" + COMMIT_MESSAGE="${COMMIT_MESSAGE%"${COMMIT_MESSAGE##*[![:space:]]}"}" + if [ -z "$COMMIT_MESSAGE" ]; then + echo "::error::commit-message must be non-empty" + exit 1 + fi + + if [ -n "${PUSH_BRANCH:-}" ] && ! [[ "$PUSH_BRANCH" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "::error::push-branch contains invalid characters" + exit 1 + fi + if [ -f changes.patch ] && [ -s changes.patch ]; then git apply --index changes.patch git config --local user.email "selenium-ci@users.noreply.github.com" git config --local user.name "Selenium CI Bot" git commit -m "$COMMIT_MESSAGE" - if [ -n "$PUSH_BRANCH" ]; then + if [ -n "${PUSH_BRANCH:-}" ]; then git push origin HEAD:"$PUSH_BRANCH" --force else git push fi echo "::notice::Changes committed and pushed" echo "committed=true" >> "$GITHUB_OUTPUT" else echo "::notice::No changes to commit" echo "committed=false" >> "$GITHUB_OUTPUT" fi env: COMMIT_MESSAGE: ${{ inputs.commit-message }} PUSH_BRANCH: ${{ inputs.push-branch }} [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (workflow inputs/env) before using them in shell commands.	Low
	✅ Guard against null CLI output Suggestion Impact: Updated the `gh pr list` jq expression to `.[0].number // empty`, ensuring an empty string is returned when no PR exists before the `-n` check. code diff: - existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number') + existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty') Make the gh pr list parsing robust by returning an empty string when no PR exists (instead of null) before checking -n. .github/workflows/pin-browsers.yml [39-54] - name: Create Pull Request env: GH_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }} run: | - existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number') + existing=$(gh pr list --head pinned-browser-updates --json number --jq '.[0].number // empty') if [ -n "$existing" ]; then echo "::notice::PR #$existing already exists" else 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 Merge after verify the new browser versions properly passing the tests and no bugs need to be filed" fi [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (CLI/JSON outputs) before using values.	Low

[Copilot]

Pull request overview

This PR introduces a reusable workflow commit-changes.yml to eliminate code duplication across multiple CI workflows that follow a common pattern: generate changes, download a patch artifact, apply the patch, commit, and push.

Changes:

• Added new reusable workflow .github/workflows/commit-changes.yml with configurable inputs for artifact name, commit message, git ref, and push branch
• Refactored 5 existing workflows to use the new reusable workflow: release.yml, pre-release.yml, pin-browsers.yml, ci-renovate-rbe.yml, and ci-lint.yml
• Simplified pin-browsers.yml to use gh CLI for PR creation instead of peter-evans/create-pull-request action

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
.github/workflows/commit-changes.yml	New reusable workflow that handles downloading patch artifacts, applying changes, committing, and pushing to a branch with configurable parameters
.github/workflows/release.yml	Refactored update-version job to use the new reusable workflow
.github/workflows/pre-release.yml	Refactored push-rust-version job to use the new reusable workflow; updated comment on line 33
.github/workflows/pin-browsers.yml	Split logic into push-changes (using reusable workflow) and create-pr jobs; switched from peter-evans/create-pull-request to gh CLI
.github/workflows/ci-renovate-rbe.yml	Refactored commit-repins job to use the new reusable workflow
.github/workflows/ci-lint.yml	Refactored commit-fixes job to use the new reusable workflow; moved fork check to job-level condition

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

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