Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 19, 2026

User description

Our current usage of Bazel is inconsistent across workflows. We want one entry point to bazel from GitHub so everything
is consistent. The pattern is Bazel does things and stores artifacts that calling workflows consume
as needed in subsequent jobs.

💥 What does this PR do?

Migrates GitHub Actions workflows to use the centralized bazel.yml reusable workflow pattern

Workflows migrated:

  • release.yml - Build packages job
  • update-documentation.yml - Documentation generation
  • pre-release.yml - Rust version generation, matrix-based update generation
  • ci.yml - Check targets job with separate read-targets job
  • ci-lint.yml - Format job
  • ci-renovate-rbe.yml - Repin job
  • nightly.yml - use artifact-name/artifact-path instead of nightly-release-files

Changes to bazel.yml:

  • Added ref input for checking out specific tags/branches
  • Added artifact-path input for uploading specific files (vs git diff patches)
  • Removed nightly-release-files and Release Nightly step

🔧 Implementation Notes

Separates concerns: Bazel operations run in a consistent environment with proper caching,
while git operations run in lightweight jobs that download artifacts.

For ci.yml, Bazel query results are written to a file artifact instead of GITHUB_OUTPUT
because reusable workflows can't pass outputs back directly.
A lightweight read-targets job downloads and reads the artifact.

💡 Additional Considerations

This consolidation makes it easier to update Bazel setup/caching in one place and maintain consistent build environments.
I have additional PRs that will make use of this consistent pattern.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement, Tests


Description

  • Consolidates Bazel workflow execution into centralized bazel.yml reusable workflow

  • Migrates 7 workflows to use consistent artifact-based pattern for build outputs

  • Replaces GitHub output mechanism with file artifacts for cross-job data passing

  • Separates Bazel operations from git operations into distinct lightweight jobs


Diagram Walkthrough

flowchart LR
  A["Individual Workflows<br/>release, ci, pre-release, etc."] -->|"call"| B["bazel.yml<br/>Reusable Workflow"]
  B -->|"runs Bazel<br/>with caching"| C["Build Environment"]
  C -->|"uploads artifacts"| D["Artifact Storage"]
  D -->|"downloads"| E["Lightweight Git Jobs<br/>commit, push, release"]
  E -->|"git operations"| F["Repository"]
Loading

File Walkthrough

Relevant files
Enhancement
9 files
check-bazel-targets.sh
Replace GitHub output with file artifact for targets         
+5/-9     
bazel.yml
Add ref and artifact-path inputs, remove nightly-release-files
+20/-22 
ci-lint.yml
Migrate format job to use bazel.yml reusable workflow       
+26/-58 
ci-renovate-rbe.yml
Migrate repin job to bazel.yml with separate commit job   
+46/-39 
ci.yml
Split check targets into bazel.yml and lightweight read-targets job
+33/-31 
nightly.yml
Migrate grid job to bazel.yml, separate release into new job
+25/-5   
pre-release.yml
Migrate all update jobs to bazel.yml with matrix-based generation
+126/-128
release.yml
Split build and release into separate bazel.yml and git jobs
+59/-45 
update-documentation.yml
Migrate docs generation to bazel.yml with separate commit job
+32/-26 

@titusfortner titusfortner requested a review from Copilot January 19, 2026 21:13
@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Jan 19, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 19, 2026

PR Compliance Guide 🔍

(Compliance updated until commit 62af24c)

Below is a summary of compliance checks for this PR:

Security Compliance
Artifact exfiltration

Description: The new artifact-path input is passed directly to actions/upload-artifact (path: ${{
inputs.artifact-path || 'changes.patch' }}), which can allow a caller to upload arbitrary
runner files (e.g., /home/runner/**, tool caches, or other sensitive build outputs) and
potentially exfiltrate secrets or credentials if this reusable workflow is ever invoked
from a context where an attacker can influence inputs.
bazel.yml [64-264]

Referred Code
artifact-name:
  description: Name of artifact to upload
  required: false
  type: string
  default: ''
artifact-path:
  description: Path/glob of files to upload (if empty, uploads git diff as changes.patch)
  required: false
  type: string
  default: ''
rerun-with-debug:
  description: Rerun failing tests with debugging enabled
  required: false
  type: boolean
  default: false
gpg-sign:
  description: Import GPG key for signing (Java releases)
  required: false
  type: boolean
  default: false
fetch-depth:


 ... (clipped 180 lines)
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:
Unvalidated ref input: The reusable workflow checks out ${{ inputs.ref }} without any allowlist/validation, so
compliance depends on ensuring only trusted callers can pass ref and fetch-depth.

Referred Code
  with:
    ref: ${{ inputs.ref || github.ref }}
    fetch-depth: ${{ inputs.fetch-depth }}
- name: Pull latest changes from head ref for PRs

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

Previous compliance checks

Compliance check up to commit 26be08a
Security Compliance
Open tmate access

Description: The workflow enables mxschmitt/action-tmate@v3 with limit-access-to-actor: false, which
can allow unintended interactive runner access (and potential secret/environment exposure)
to anyone who obtains the tmate connection details when debugging is enabled.
bazel.yml [243-248]

Referred Code
- name: Start SSH session
  if: failure() && runner.debug == '1'
  uses: mxschmitt/action-tmate@v3
  with:
    limit-access-to-actor: false
- name: Save git diff
Untrusted patch apply

Description: The job downloads an artifact changes.patch and runs git apply then git push, so a
compromised/incorrectly-scoped artifact could inject unintended repository changes that
get pushed automatically (similar artifact-to-commit flows also appear in other workflows
in this PR).
ci-lint.yml [79-88]

Referred Code
run: |
  if [ -s changes.patch ]; then
    git apply changes.patch
    rm changes.patch
    git config --local user.name "Selenium CI Bot"
    git config --local user.email "selenium-ci@users.noreply.github.com"
    git add -A
    git commit -m "Auto-format code"
    git push
    echo "::notice::Auto-formatted and pushed. New CI run will start."
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:
Masked command failures: The workflow swallows potential git apply and git commit failures by using || echo ...,
which can hide real errors (e.g., patch conflicts) and lead to silent misbehavior.

Referred Code
- name: Download version reset patch
  uses: actions/download-artifact@v4
  with:
    name: version-reset
- name: Apply patch
  run: git apply --index changes.patch || echo "No changes to apply"
- name: Prep git
  run: |
    git config --local user.email "selenium-ci@users.noreply.github.com"
    git config --local user.name "Selenium CI Bot"
- name: Push version changes
  run: |
    git commit -m "[build] Reset versions to nightly after ${{ needs.prepare.outputs.tag }} release" || echo "No changes to commit"
    git push

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 workflow inputs: The new workflow inputs ref and artifact-path are consumed directly (checkout ref and
uploaded file globs) without validation/allowlisting, which may enable unintended refs or
file exfiltration depending on who can call the reusable workflow.

Referred Code
ref:
  description: Git ref to checkout (branch, tag, or SHA)
  required: false
  type: string
  default: ''
run:
  description: Bazel command to run
  required: true
  type: string
os:
  description: One of ubuntu, windows or macos
  required: false
  type: string
  default: ubuntu
browser:
  description: One of chrome, firefox, or edge
  required: false
  type: string
  default: ''
browser-version:
  description: One of stable, latest, latest-beta or latest-devedition


 ... (clipped 73 lines)

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

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

This PR consolidates GitHub Actions workflows to use a centralized bazel.yml reusable workflow, improving consistency and maintainability of Bazel operations across the repository. The migration separates concerns by running Bazel operations in a consistent environment while git operations run in lightweight jobs that consume artifacts.

Changes:

  • Centralized Bazel workflow with new ref and artifact-path inputs for flexible checkout and artifact handling
  • Migrated 7 workflows to use the reusable pattern (release, pre-release, nightly, ci, ci-lint, ci-renovate-rbe, update-documentation)
  • Modified check-bazel-targets.sh to write targets to a file instead of GITHUB_OUTPUT for artifact-based communication

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/bazel.yml Added ref input for specific checkouts, replaced nightly-release-files with flexible artifact-path, improved artifact upload logic
scripts/github-actions/check-bazel-targets.sh Changed output from GITHUB_OUTPUT to file-based (bazel-targets.txt) for artifact consumption
.github/workflows/ci.yml Split check job into check (Bazel query) and read-targets (artifact reader) jobs
.github/workflows/ci-lint.yml Migrated format checking to use centralized bazel.yml, separated protected file checking
.github/workflows/ci-renovate-rbe.yml Migrated repin operations to centralized workflow with multi-line run commands
.github/workflows/nightly.yml Separated grid build from release, moved release creation to dedicated job
.github/workflows/pre-release.yml Restructured to use matrix-based parallel generation jobs with centralized patch application
.github/workflows/release.yml Split into prepare/build/create-release jobs, added reset-version generation job
.github/workflows/update-documentation.yml Separated into parse/generate-docs/commit-docs jobs using artifact flow

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 19, 2026

PR Code Suggestions ✨

Latest suggestions up to c819f09

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid failing on missing artifacts

In the commit-fixes job, make the download-artifact step non-fatal by adding
continue-on-error: true and adjust the subsequent step to handle cases where the
artifact is missing. This prevents masking the original failure cause from the
format job.

.github/workflows/ci-lint.yml [56-95]

 commit-fixes:
   name: Commit fixes
   needs: format
   if: failure() && github.event_name == 'pull_request'
   runs-on: ubuntu-latest
   permissions:
     contents: write
     actions: read
   steps:
     - name: Check Permissions
       if: github.event.pull_request.head.repo.fork == true
       run: |
         echo "::error::Code needs formatting. Run ./scripts/format.sh locally and push changes."
         exit 1
     - name: Checkout PR
       uses: actions/checkout@v4
       with:
         ref: ${{ github.event.pull_request.head.ref }}
     - name: Download format changes
       uses: actions/download-artifact@v4
+      continue-on-error: true
       with:
         name: format-changes
     - name: Apply and commit fixes
       id: apply
       run: |
-        if [ -s changes.patch ]; then
+        if [ -f changes.patch ] && [ -s changes.patch ]; then
           git apply --index changes.patch
           rm changes.patch
           git config --local user.name "Selenium CI Bot"
           git config --local user.email "selenium-ci@users.noreply.github.com"
           git commit -m "Auto-format code"
           echo "has_changes=true" >> "$GITHUB_OUTPUT"
         else
-          echo "::notice::No formatting changes needed."
+          echo "::notice::No format patch artifact found; skipping auto-commit."
+          echo "has_changes=false" >> "$GITHUB_OUTPUT"
         fi
     - name: Push fixes
       if: steps.apply.outputs.has_changes == 'true'
       run: |
         git push
         echo "::notice::Auto-formatted and pushed. New CI run will start."
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the commit-fixes job could fail and mask the root cause if the format job fails for reasons other than formatting, thus not producing the expected artifact. Adding continue-on-error makes the workflow more robust and improves debuggability.

Medium
Upload only when files exist

In the bazel.yml workflow, modify the if condition for the Upload artifact step
to use hashFiles to ensure an artifact is only uploaded if the specified
artifact-path contains files or if changes.patch exists and is not empty.

.github/workflows/bazel.yml [257-264]

 - name: Upload artifact
-  if: always() && inputs.artifact-name != ''
+  if: |
+    always() && inputs.artifact-name != '' && (
+      (inputs.artifact-path != '' && hashFiles(inputs.artifact-path) != '') ||
+      (inputs.artifact-path == '' && hashFiles('changes.patch') != '')
+    )
   uses: actions/upload-artifact@v5
   with:
     name: ${{ inputs.artifact-name }}
     path: ${{ inputs.artifact-path || 'changes.patch' }}
     retention-days: 6
     if-no-files-found: ${{ inputs.artifact-path != '' && 'error' || 'warn' }}
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential failure point where a downstream job might fail if an expected artifact is not created. Conditionally running the upload step based on file existence makes the reusable workflow more robust and prevents downstream failures.

Medium
Incremental [*]
Include untracked files in patch

Before running git diff, stage untracked files using git add -N . to ensure they
are included in the generated changes.patch.

.github/workflows/bazel.yml [256]

-run: git diff --binary > changes.patch
+run: |
+  git add -N .
+  git diff --binary > changes.patch
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that git diff misses untracked files and proposes using git add -N . to include them in the patch, which prevents changes from being silently lost.

Medium
Avoid empty commits after patch

After applying a patch with git apply --index, check for staged changes using
git diff --cached --quiet before attempting to commit, to avoid errors from
empty commits.

.github/workflows/ci-renovate-rbe.yml [58-66]

 if [ -f changes.patch ] && [ -s changes.patch ]; then
   git apply --index changes.patch
+  if git diff --cached --quiet; then
+    echo "No changes to commit"
+    exit 0
+  fi
   git config --local user.email "selenium-ci@users.noreply.github.com"
   git config --local user.name "Selenium CI Bot"
   git commit -m 'Repin dependencies'
   git push
 else
   echo "No changes to commit"
 fi
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that git apply might not produce changes, which would cause git commit to fail, and proposes a robust check using git diff --cached --quiet to prevent the error.

Medium
  • More

Previous suggestions

✅ Suggestions up to commit 62af24c
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix artifact path nesting
Suggestion Impact:The commit removed the explicit `path: docs/api` from the artifact download step, which effectively causes the artifact to be downloaded to the default workspace directory (equivalent to `path: .`), addressing the nested directory issue.

code diff:

@@ -95,7 +95,6 @@
         uses: actions/download-artifact@v4
         with:
           name: documentation
-          path: docs/api
       - name: Setup git

In .github/workflows/update-documentation.yml, change the download path for the
documentation artifact from docs/api to . to prevent creating a nested and
incorrect directory structure.

.github/workflows/update-documentation.yml [94-98]

 - name: Download documentation
   uses: actions/download-artifact@v4
   with:
     name: documentation
-    path: docs/api
+    path: .

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix. The upload-artifact action preserves the full path, so downloading to a subdirectory creates a nested, incorrect path structure (docs/api/docs/api/...), which would break the documentation update logic.

High
Prevent nested dist directories
Suggestion Impact:The workflow no longer downloads the nightly-grid artifact into build/dist (the explicit 'path: build/dist' line was removed), which effectively avoids creating a nested build/dist/build/dist structure and aligns with downloading into the default location.

code diff:

@@ -192,7 +192,6 @@
         uses: actions/download-artifact@v4
         with:
           name: nightly-grid
-          path: build/dist
       - name: Create nightly release

In .github/workflows/nightly.yml, change the download path for the nightly-grid
artifact from build/dist to . to avoid creating a nested directory structure and
ensure the release step finds the files.

.github/workflows/nightly.yml [191-195]

 - name: Download grid packages
   uses: actions/download-artifact@v4
   with:
     name: nightly-grid
-    path: build/dist
+    path: .

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix. The upload-artifact action preserves the full path, so downloading to build/dist would create a nested build/dist/build/dist/ path. This would cause the subsequent release step to fail as it would not find any files to upload.

High
Avoid doubled output directories
Suggestion Impact:The commit removed the explicit `path: build/dist` from the download-artifact step, causing the artifact to be downloaded to the default location (workspace, effectively like `.`), which addresses the doubled output directory issue the suggestion targeted.

code diff:

         uses: actions/download-artifact@v4
         with:
           name: release-packages
-          path: build/dist

In .github/workflows/release.yml, change the download path for the
release-packages artifact from build/dist to . to prevent a nested directory
issue that would cause the release creation to fail.

.github/workflows/release.yml [72-76]

 - name: Download release packages
   uses: actions/download-artifact@v4
   with:
     name: release-packages
-    path: build/dist
+    path: .

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix. The upload-artifact action preserves the full path, so downloading to build/dist would create a nested build/dist/build/dist/ path. This would cause the ncipollo/release-action to fail as it would not find the packages for the draft release.

High
Incremental [*]
Capture complete patch diffs
Suggestion Impact:The workflow was changed to use `git diff --binary > changes.patch`, partially addressing the suggestion (binary handling). However, it did not add `git diff --cached --binary` nor the empty-file removal check.

code diff:

       - name: Save git diff
         if: always() && inputs.artifact-name != '' && inputs.artifact-path == ''
-        run: git diff > changes.patch
+        run: git diff --binary > changes.patch

Modify the Save git diff step to capture both staged and unstaged changes,
handle binary files, and avoid creating empty patch files. Update the run
command to git diff --binary > changes.patch; git diff --cached --binary >>
changes.patch; [ -s changes.patch ] || rm -f changes.patch.

.github/workflows/bazel.yml [254-258]

 - name: Save git diff
   if: always() && inputs.artifact-name != '' && inputs.artifact-path == ''
-  run: git diff > changes.patch
+  run: |
+    git diff --binary > changes.patch
+    git diff --cached --binary >> changes.patch
+    [ -s changes.patch ] || rm -f changes.patch
 - name: Upload artifact
   if: always() && inputs.artifact-name != ''

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that git diff alone is insufficient as it misses staged changes, which are used by downstream jobs (git apply --index). The proposed change to include git diff --cached and --binary is a crucial fix for the correctness of the new reusable workflow.

Medium
✅ Suggestions up to commit 28ce7c2
CategorySuggestion                                                                                                                                    Impact
Possible issue
Upload artifacts even on failure
Suggestion Impact:The workflow conditions for "Save git diff" and "Upload artifact" were updated to include always(), ensuring these steps run even if earlier steps fail. (The upload action version was also bumped to v5.)

code diff:

       - name: Save git diff
-        if: inputs.artifact-name != '' && inputs.artifact-path == ''
+        if: always() && inputs.artifact-name != '' && inputs.artifact-path == ''
         run: git diff > changes.patch
       - name: Upload artifact
-        if: inputs.artifact-name != ''
-        uses: actions/upload-artifact@v4
+        if: always() && inputs.artifact-name != ''
+        uses: actions/upload-artifact@v5

Add always() to the conditions for saving and uploading artifacts in bazel.yml
to ensure they run even if the main run step fails.

.github/workflows/bazel.yml [254-264]

 - name: Save git diff
-  if: inputs.artifact-name != '' && inputs.artifact-path == ''
+  if: always() && inputs.artifact-name != '' && inputs.artifact-path == ''
   run: git diff > changes.patch
 - name: Upload artifact
-  if: inputs.artifact-name != ''
+  if: always() && inputs.artifact-name != ''
   uses: actions/upload-artifact@v4
   with:
     name: ${{ inputs.artifact-name }}
     path: ${{ inputs.artifact-path || 'changes.patch' }}
     retention-days: 6
     if-no-files-found: ${{ inputs.artifact-path != '' && 'error' || 'warn' }}

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical flaw where artifacts are not generated if the main run step fails, which breaks several dependent jobs like auto-formatting. Adding always() is essential for the new workflow design to function as intended.

High
Avoid failing on missing artifacts

In the commit-fixes job, add continue-on-error: true to the artifact download
step and adjust the subsequent script to handle cases where the artifact is
missing.

.github/workflows/ci-lint.yml [56-96]

 commit-fixes:
   name: Commit fixes
   needs: format
   if: failure() && github.event_name == 'pull_request'
   runs-on: ubuntu-latest
   permissions:
     contents: write
     actions: read
   steps:
     - name: Check Permissions
       if: github.event.pull_request.head.repo.fork == true
       run: |
         echo "::error::Code needs formatting. Run ./scripts/format.sh locally and push changes."
         exit 1
     - name: Checkout PR
       uses: actions/checkout@v4
       with:
         ref: ${{ github.event.pull_request.head.ref }}
     - name: Download format changes
       uses: actions/download-artifact@v4
       with:
         name: format-changes
+      continue-on-error: true
     - name: Apply and commit fixes
       id: apply
       run: |
-        if [ -s changes.patch ]; then
+        if [ -f changes.patch ] && [ -s changes.patch ]; then
           git apply changes.patch
           rm changes.patch
           git config --local user.name "Selenium CI Bot"
           git config --local user.email "selenium-ci@users.noreply.github.com"
           git add -A
           git commit -m "Auto-format code"
           echo "has_changes=true" >> "$GITHUB_OUTPUT"
         else
-          echo "::notice::No formatting changes needed."
+          echo "::notice::No formatting patch artifact found; nothing to apply."
+          echo "has_changes=false" >> "$GITHUB_OUTPUT"
         fi
     - name: Push fixes
       if: steps.apply.outputs.has_changes == 'true'
       run: |
         git push
         echo "::notice::Auto-formatted and pushed. New CI run will start."
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the download-artifact step will fail if the artifact doesn't exist, halting the job. Adding continue-on-error: true and checking for the patch file's existence makes the commit-fixes job more robust and resilient to this scenario.

Medium
Learned
best practice
Safely escape injected context strings

Use toJSON(...) to safely escape commit messages before putting them into a bash
variable, and normalize whitespace so unexpected quotes/newlines in messages
can’t break the script.

.github/workflows/ci-renovate-rbe.yml [19-38]

 run: |
-  COMMITS="${{ join(github.event.commits.*.message, ' ') }}"
+  COMMITS_JSON='${{ toJSON(join(github.event.commits.*.message, " ")) }}'
+  COMMITS="${COMMITS_JSON%\"}"; COMMITS="${COMMITS#\"}"
+  COMMITS="${COMMITS//\\n/ }"; COMMITS="${COMMITS//\\r/ }"
   if [[ "$COMMITS" == *"[java]"* ]]; then
     RULES_JVM_EXTERNAL_REPIN=1 bazel run @maven//:pin
   fi
   if [[ "$COMMITS" == *"[rust]"* ]]; then
     CARGO_BAZEL_REPIN=true bazel run @crates//:all
   fi
   if [[ "$COMMITS" == *"[js]"* ]]; then
     bazel run -- @pnpm//:pnpm install --dir "$PWD" --lockfile-only
   fi
   if [[ "$COMMITS" == *"[dotnet]"* ]]; then
     ./dotnet/update-deps.sh
   fi
   if [[ "$COMMITS" == *"[py]"* ]]; then
     bazel run //py:requirements.update
   fi
   if [[ "$COMMITS" == *"[rb]"* ]]; then
     ./go rb:pin
   fi
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Add validation/escaping at GitHub Actions integration boundaries; avoid directly injecting untrusted context strings into shell scripts without safe encoding.

Low
✅ Suggestions up to commit 26be08a
CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle empty bazel targets file correctly
Suggestion Impact:The workflow step was changed to conditionally write the multiline targets output only when bazel-targets.txt is non-empty, and otherwise write `targets=` to GITHUB_OUTPUT, preventing an empty newline output.

code diff:

@@ -44,11 +44,15 @@
       - name: Read targets
         id: read
         run: |
-          {
-            echo "targets<<EOF"
-            cat bazel-targets.txt
-            echo "EOF"
-          } >> "$GITHUB_OUTPUT"
+          if [ -s bazel-targets.txt ]; then
+            {
+              echo "targets<<EOF"
+              cat bazel-targets.txt
+              echo "EOF"
+            } >> "$GITHUB_OUTPUT"
+          else
+            echo "targets=" >> "$GITHUB_OUTPUT"
+          fi

In the read-targets job, check if bazel-targets.txt is empty. If so, set the
targets output to an empty string to avoid it containing a newline, which could
cause issues in downstream jobs.

.github/workflows/ci.yml [33-51]

 read-targets:
   name: Read Targets
   needs: check
   runs-on: ubuntu-latest
   outputs:
     targets: ${{ steps.read.outputs.targets }}
   steps:
     - name: Download targets
       uses: actions/download-artifact@v4
       with:
         name: check-targets
     - name: Read targets
       id: read
       run: |
-        {
-          echo "targets<<EOF"
-          cat bazel-targets.txt
-          echo "EOF"
-        } >> "$GITHUB_OUTPUT"
+        if [ -s bazel-targets.txt ]; then
+          {
+            echo "targets<<EOF"
+            cat bazel-targets.txt
+            echo "EOF"
+          } >> "$GITHUB_OUTPUT"
+        else
+          echo "targets=" >> "$GITHUB_OUTPUT"
+        fi

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that an empty bazel-targets.txt file leads to a job output with a newline, which can cause issues in downstream jobs. The proposed fix ensures the output is a truly empty string, which is a critical improvement for the correctness of the workflow logic.

Medium
Prevent job failure if no changes
Suggestion Impact:The workflow was changed to check whether changes.patch exists and is non-empty before applying it and performing git commit/push. The implementation differs from the suggestion by combining steps and by failing the job when the patch is missing/empty (exit 1) instead of skipping commit/push.

code diff:

+      - name: Apply and push
+        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 "update selenium manager version and rust changelog"
+            git push origin HEAD:rust-release-${{ inputs.version }} --force
+          else
+            echo "::error::No changes to apply for rust version update"
+            exit 1
+          fi

In the push-rust-version job, add a condition to check if changes.patch is
non-empty before attempting to apply it and commit, preventing job failure when
there are no changes.

.github/workflows/pre-release.yml [43-66]

 push-rust-version:
   name: Push Rust Version
   needs: generate-rust-version
   runs-on: ubuntu-latest
   steps:
     - name: Checkout repo
       uses: actions/checkout@v4
       with:
         token: ${{ secrets.SELENIUM_CI_TOKEN }}
         fetch-depth: 0
     - name: Download rust version patch
       uses: actions/download-artifact@v4
       with:
         name: rust-version
     - name: Apply patch
-      run: git apply --index changes.patch
+      run: |
+        if [ -s changes.patch ]; then
+          git apply --index changes.patch
+          echo "CHANGES_APPLIED=true" >> "$GITHUB_ENV"
+        fi
     - name: Prep git
+      if: env.CHANGES_APPLIED == 'true'
       run: |
         git config --local user.email "selenium-ci@users.noreply.github.com"
         git config --local user.name "Selenium CI Bot"
     - name: Push changes
+      if: env.CHANGES_APPLIED == 'true'
       run: |
         git commit -m "update selenium manager version and rust changelog"
         git push origin HEAD:rust-release-${{ inputs.version }} --force

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: This is a valid improvement for workflow robustness, preventing a potential failure if no version changes are generated. This pattern of checking for a non-empty patch is used elsewhere in the PR, making this a consistent and valuable addition.

Medium
General
Fetch full git history
Suggestion Impact:The workflow was updated to support configuring actions/checkout's fetch-depth via a new reusable-workflow input, and the checkout step now uses that input. This enables callers to set fetch-depth to 0 for full history, addressing the underlying need, though it does not hardcode fetch-depth: 0 by default.

code diff:

@@ -81,6 +81,11 @@
         required: false
         type: boolean
         default: false
+      fetch-depth:
+        description: Number of commits to fetch (0 for full history)
+        required: false
+        type: number
+        default: 1
 
 jobs:
   bazel:
@@ -99,6 +104,7 @@
         uses: actions/checkout@v4
         with:
           ref: ${{ inputs.ref || github.ref }}
+          fetch-depth: ${{ inputs.fetch-depth }}

Add fetch-depth: 0 to the actions/checkout step in the bazel.yml reusable
workflow to ensure the full git history is available for subsequent git
operations.

.github/workflows/bazel.yml [98-101]

 - name: Checkout source tree
   uses: actions/checkout@v4
   with:
     ref: ${{ inputs.ref || github.ref }}
+    fetch-depth: 0

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: This is a critical fix for the reusable workflow, as several calling workflows rely on git diff or commit ranges which require the full git history. Without fetch-depth: 0, these operations would fail or produce incorrect results.

Medium
Learned
best practice
Avoid swallowing patch-apply errors
Suggestion Impact:The workflow was changed to conditionally run `git apply --index changes.patch` only when the patch file exists and is non-empty, and removed the unconditional `|| echo "No changes to apply"` that would mask real git apply failures. If no patch is present, it now emits a notice and skips apply/commit/push.

code diff:

+      - name: Apply and push
         run: |
-          git config --local user.email "selenium-ci@users.noreply.github.com"
-          git config --local user.name "Selenium CI Bot"
-      - name: Push version changes
-        run: |
-          git commit -m "[build] Reset versions to nightly after ${{ needs.prepare.outputs.tag }} release" || echo "No changes to commit"
-          git push
+          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 "[build] Reset versions to nightly after ${{ needs.prepare.outputs.tag }} release"
+            git push
+          else
+            echo "::notice::No version changes to apply"
+          fi

Replace the unconditional || echo ... with an explicit file existence/size check
so real git apply failures fail the job instead of being silently ignored.

.github/workflows/release.yml [189-190]

 - name: Apply patch
-  run: git apply --index changes.patch || echo "No changes to apply"
+  shell: bash
+  run: |
+    if [ ! -f changes.patch ] || [ ! -s changes.patch ]; then
+      echo "No changes to apply"
+      exit 0
+    fi
+    git apply --index changes.patch

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Add explicit validation/availability guards at integration boundaries (e.g., downloaded artifacts/files) and avoid swallowing errors from critical steps.

Low

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated 14 comments.

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

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

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@titusfortner titusfortner merged commit 421cf91 into trunk Jan 20, 2026
29 checks passed
@titusfortner titusfortner deleted the bazel_yml branch January 20, 2026 04:14
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 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants