[build] consolidate nightly workflow by titusfortner · Pull Request #17005 · SeleniumHQ/selenium

titusfortner · 2026-01-25T20:13:24Z

💥 What does this PR do?

Significantly simplify the nightly workflow; follow-on to pre-release.yml and release.yml simplifications

Adding a prepare job to centralize input parsing and outputs (language, artifact-name, artifact-path, release-in-progress)
Removing separate per-language jobs (ruby, python, java, dotnet, javascript, grid) in favor of a single nightly-release job
Removing 'grid' as a separate language option since java:release already includes grid via java:package

🔧 Implementation Notes

There's no need for a separate grid job - the grid artifacts are included when java runs

💡 Additional Considerations

We'll see how it works tonight

🔄 Types of changes

Cleanup (formatting, renaming)

- Reduce from 254 to 116 lines by removing per-language jobs - Add prepare job to centralize input parsing and outputs - Remove 'grid' as separate option (java:release includes grid via java:package) - Use single nightly-release job with ./go {lang}:release nightly - release-grid runs only when artifact-name is set (all/java) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

qodo-code-review · 2026-01-25T20:13:37Z

User description

💥 What does this PR do?

Significantly simplify the nightly workflow; follow-on to pre-release.yml and release.yml simplifications

Adding a prepare job to centralize input parsing and outputs (language, artifact-name, artifact-path, release-in-progress)
Removing separate per-language jobs (ruby, python, java, dotnet, javascript, grid) in favor of a single nightly-release job
Removing 'grid' as a separate language option since java:release already includes grid via java:package

🔧 Implementation Notes

There's no need for a separate grid job - the grid artifacts are included when java runs

💡 Additional Considerations

We'll see how it works tonight

🔄 Types of changes

Cleanup (formatting, renaming)

PR Type

Enhancement

Description

Consolidate nightly workflow from 254 to 116 lines
Replace per-language jobs with single nightly-release job
Add prepare job to centralize input parsing and outputs
Remove grid as separate language option (included via java:release)
Simplify failure notification to single on-failure job

Diagram Walkthrough

flowchart LR
  A["Workflow Inputs"] --> B["prepare job"]
  B --> C["Parse language & artifacts"]
  B --> D["Check release ruleset"]
  C --> E["nightly-release job"]
  D --> E
  E --> F["release-grid job"]
  E --> G["on-failure job"]
  F --> G

File Walkthrough

Relevant files

Configuration changes

nightly.yml `Consolidate nightly workflow jobs and simplify structure` .github/workflows/nightly.yml Removed `version` input parameter and `grid` language option Added `prepare` job to centralize input parsing and artifact configuration Consolidated six per-language jobs (ruby, python, java, dotnet, javascript, grid) into single `nightly-release` job Removed six per-language failure notification jobs in favor of unified `on-failure` job Updated `release-grid` to depend on `prepare` and `nightly-release` with conditional execution	+34/-172

qodo-code-review · 2026-01-25T20:13:50Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Command injection Description: The workflow propagates `inputs.language` (including via `workflow_call`) into `needs.prepare.outputs.language` without validation and then uses it to build a shell command (`./go ${{ needs.prepare.outputs.language }}:release nightly`), which could enable command/argument injection if an untrusted caller supplies a crafted value outside the `workflow_dispatch` choice list. nightly.yml [41-74] Referred Code - name: Parse inputs id: parse env: LANGUAGE: ${{ inputs.language }} run: \| LANG="${LANGUAGE:-all}" echo "language=$LANG" >> "$GITHUB_OUTPUT" if [[ "$LANG" == "all" \|\| "$LANG" == "java" ]]; then echo "artifact-name=nightly-grid" >> "$GITHUB_OUTPUT" echo "artifact-path=build/dist/." >> "$GITHUB_OUTPUT" fi - name: Check if release ruleset is active id: check env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: \| set -euo pipefail if ! ENFORCEMENT=$(gh api /repos/${{ github.repository }}/rulesets/11911909 --jq '.enforcement' 2>/dev/null); then echo "::warning::Failed to check release ruleset, assuming release in progress" echo "release-in-progress=true" >> "$GITHUB_OUTPUT" exit 0 ... (clipped 13 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: 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 output defaults: The `prepare` step only sets `artifact-name` and `artifact-path` for `all`/`java`, so other languages may pass empty values to `nightly-release` and cause runtime failures depending on how `./.github/workflows/bazel.yml` consumes these inputs. Referred Code - name: Parse inputs id: parse env: LANGUAGE: ${{ inputs.language }} run: \| LANG="${LANGUAGE:-all}" echo "language=$LANG" >> "$GITHUB_OUTPUT" if [[ "$LANG" == "all" \|\| "$LANG" == "java" ]]; then echo "artifact-name=nightly-grid" >> "$GITHUB_OUTPUT" echo "artifact-path=build/dist/." >> "$GITHUB_OUTPUT" fi - name: Check if release ruleset is active id: check env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: \| set -euo pipefail if ! ENFORCEMENT=$(gh api /repos/${{ github.repository }}/rulesets/11911909 --jq '.enforcement' 2>/dev/null); then echo "::warning::Failed to check release ruleset, assuming release in progress" echo "release-in-progress=true" >> "$GITHUB_OUTPUT" exit 0 ... (clipped 14 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 · 2026-01-25T20:15:13Z

PR Code Suggestions ✨

Latest suggestions up to a315e68

Category	Suggestion	Impact
Possible issue	Restore modified manifest after publish Restore the original `javascript/selenium-webdriver/package.json` file after the publish step to avoid permanent mutations that could affect subsequent non-nightly releases. rake_tasks/node.rake [91-100] -if nightly - puts 'Updating Node version to nightly...' - Rake::Task['node:version'].invoke('nightly') - setup_github_npm_auth unless dry_run -else - setup_npm_auth unless dry_run +original_package_json = nil +package_json_path = 'javascript/selenium-webdriver/package.json' + +begin + if nightly + puts 'Updating Node version to nightly...' + Rake::Task['node:version'].invoke('nightly') + unless dry_run + original_package_json = File.read(package_json_path) + setup_github_npm_auth + end + else + setup_npm_auth unless dry_run + end + + puts dry_run ? 'Running Node package dry-run...' : 'Running Node package release...' + target = '//javascript/selenium-webdriver:selenium-webdriver.publish' +ensure + File.write(package_json_path, original_package_json) if original_package_json end -puts dry_run ? 'Running Node package dry-run...' : 'Running Node package release...' -target = '//javascript/selenium-webdriver:selenium-webdriver.publish' - Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion identifies a critical bug where `package.json` is permanently modified, which would leave the repository in a dirty state and cause subsequent non-nightly releases to fail or publish incorrectly.	High
Incremental ^[*]	Modify package.json via JSON Modify `package.json` by parsing it as JSON and updating the relevant keys, rather than using brittle string substitutions. rake_tasks/node.rake [45-49] package_json = 'javascript/selenium-webdriver/package.json' -content = File.read(package_json) -content = content.gsub('https://registry.npmjs.org/', 'https://npm.pkg.github.com') -content = content.gsub('"name": "selenium-webdriver"', '"name": "@seleniumhq/selenium-webdriver"') -File.write(package_json, content) +json = JSON.parse(File.read(package_json)) +json['name'] = '@seleniumhq/selenium-webdriver' +json['publishConfig'] \|\|= {} +json['publishConfig']['registry'] = 'https://npm.pkg.github.com' +File.write(package_json, JSON.pretty_generate(json) + "\n") Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is an excellent suggestion that replaces a fragile string replacement on a JSON file with a robust method of parsing, modifying, and re-serializing the JSON, preventing potential future breakages.	Medium
Incremental ^[*]	Prevent duplicate npmrc entries To prevent duplicate entries in `.npmrc`, modify the script to remove any existing GitHub Packages configuration before appending the new settings. rake_tasks/node.rake [37-42] if File.exist?(npmrc) - File.open(npmrc, 'a') { \|f\| f.puts("\n#{npmrc_content}") } + existing = File.read(npmrc).lines.reject { \|line\| + line.start_with?('//npm.pkg.github.com/:_authToken=') \|\| + line.start_with?('@seleniumhq:registry=') \|\| + line.start_with?('always-auth=') + }.join + File.write(npmrc, existing.rstrip + "\n\n#{npmrc_content}\n") else File.write(npmrc, "#{npmrc_content}\n") end File.chmod(0o600, npmrc) Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that appending to `.npmrc` can lead to duplicate entries, and proposes a more robust, idempotent approach by cleaning up old entries before adding new ones.	Low
More

Previous suggestions

Suggestions up to commit ef6cb2d

Category	Suggestion	Impact
High-level	Use a matrix strategy instead Instead of consolidating all language releases into a single, sequential job, use a dynamic matrix strategy. This will restore parallel execution for the 'all' languages option, reducing runtime and providing better failure isolation with per-language job statuses. Examples: .github/workflows/nightly.yml [31-75] prepare: name: Prepare runs-on: ubuntu-latest if: github.event.repository.fork == false outputs: language: ${{ steps.parse.outputs.language }} artifact-name: ${{ steps.parse.outputs.artifact-name }} artifact-path: ${{ steps.parse.outputs.artifact-path }} release-in-progress: ${{ steps.check.outputs.release-in-progress }} steps: ... (clipped 35 lines) Solution Walkthrough: Before: jobs: prepare: outputs: language: ${{ steps.parse.outputs.language }} steps: - id: parse run: \| LANG="${LANGUAGE:-all}" echo "language=$LANG" >> "$GITHUB_OUTPUT" ... nightly-release: needs: prepare ... run: ./go ${{ needs.prepare.outputs.language }}:release nightly After: jobs: prepare: outputs: matrix: ${{ steps.parse.outputs.matrix }} steps: - id: parse run: \| if [[ "${LANGUAGE:-all}" == "all" ]]; then echo 'matrix=["java", "ruby", "python", ...]' >> "$GITHUB_OUTPUT" else echo 'matrix=["${LANGUAGE}"]' >> "$GITHUB_OUTPUT" fi nightly-release: needs: prepare strategy: matrix: language: ${{ fromJson(needs.prepare.outputs.matrix) }} ... run: ./go ${{ matrix.language }}:release nightly Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a major regression in the PR, the loss of parallel execution, and proposes a superior matrix-based solution that maintains simplification while restoring parallelism and improving job monitoring.	High
Possible issue	Fix go command syntax Correct the `go` command syntax in the `nightly-release` job from `release nightly` to `release[nightly]` to match the expected format. .github/workflows/nightly.yml [72] -run: ./go ${{ needs.prepare.outputs.language }}:release nightly +run: ./go ${{ needs.prepare.outputs.language }}:release[nightly] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a syntax error in the refactored `go` command that would cause the `nightly-release` job to fail for all languages. Fixing this is critical for the workflow to function as intended.	High
Possible issue	Fix broken Slack notification title In the `on-failure` job, remove `release-grid` from the `needs` list to prevent the Slack notification title from being broken when `release-grid` is skipped. .github/workflows/nightly.yml [99-115] on-failure: name: Notify on Failure runs-on: ubuntu-latest - needs: [prepare, nightly-release, release-grid] + needs: [prepare, nightly-release] if: failure() steps: - uses: actions/checkout@v4 - name: Slack Notification uses: rtCamp/action-slack-notify@v2 env: SLACK_ICON_EMOJI: ":rotating_light:" SLACK_COLOR: failure SLACK_CHANNEL: selenium-tlc SLACK_USERNAME: GitHub Workflows SLACK_TITLE: Nightly ${{ needs.prepare.outputs.language }} Release failed MSG_MINIMAL: actions url SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a subtle bug in GitHub Actions logic where a skipped job in `needs` can cause output access to fail, leading to uninformative notifications. The fix is correct and significantly improves the reliability of failure reporting.	Medium
General	Avoid using a hardcoded ruleset ID Replace the hardcoded ruleset ID `11911909` in the GitHub API call with a dynamic lookup by the ruleset name "Release In Progress Access" to improve robustness. .github/workflows/nightly.yml [56-63] run: \| set -euo pipefail - if ! ENFORCEMENT=$(gh api /repos/${{ github.repository }}/rulesets/11911909 --jq '.enforcement' 2>/dev/null); then + RULESET_ID=$(gh api /repos/${{ github.repository }}/rulesets --jq '.[] \| select(.name == "Release In Progress Access") \| .id' 2>/dev/null) + if [[ -z "$RULESET_ID" ]]; then + echo "::warning::Could not find 'Release In Progress Access' ruleset. Assuming release is in progress." + echo "release-in-progress=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + if ! ENFORCEMENT=$(gh api /repos/${{ github.repository }}/rulesets/${RULESET_ID} --jq '.enforcement' 2>/dev/null); then echo "::warning::Failed to check release ruleset, assuming release in progress" echo "release-in-progress=true" >> "$GITHUB_OUTPUT" exit 0 fi echo "release-in-progress=$([[ "$ENFORCEMENT" == "active" ]] && echo 'true' \|\| echo 'false')" >> "$GITHUB_OUTPUT" Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using a hardcoded ID is brittle and proposes a more robust solution by fetching the ruleset ID by name, which improves the workflow's maintainability.	Medium
Learned best practice	Validate and normalize workflow inputs Validate and normalize `inputs.language` (trim + allowlist) and always set `artifact-name`/`artifact-path` outputs (possibly empty) to avoid accidental command injection and missing-output edge cases. .github/workflows/nightly.yml [41-51] - name: Parse inputs id: parse env: LANGUAGE: ${{ inputs.language }} run: \| - LANG="${LANGUAGE:-all}" + set -euo pipefail + + LANG="$(echo "${LANGUAGE:-all}" \| xargs)" + case "$LANG" in + all\|java\|ruby\|python\|dotnet\|javascript) ;; + ) echo "::error::Unsupported language: $LANG"; exit 1 ;; + esac + echo "language=$LANG" >> "$GITHUB_OUTPUT" + + # default outputs (empty means "not applicable") + echo "artifact-name=" >> "$GITHUB_OUTPUT" + echo "artifact-path=" >> "$GITHUB_OUTPUT" + if [[ "$LANG" == "all" \|\| "$LANG" == "java" ]]; then echo "artifact-name=nightly-grid" >> "$GITHUB_OUTPUT" echo "artifact-path=build/dist/.*" >> "$GITHUB_OUTPUT" fi Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (GitHub Actions inputs/env) before using them in shell commands and outputs.	Low
Learned best practice	Use outputs instead of hardcoding Use the computed `artifact-name`/`artifact-path` outputs instead of hardcoding `nightly-grid`, and make the `if` guard explicit to avoid accidental truthiness/empty-string behavior. .github/workflows/nightly.yml [77-89] release-grid: name: Release Grid needs: [prepare, nightly-release] - if: needs.prepare.outputs.artifact-name + if: needs.prepare.outputs.artifact-name != '' runs-on: ubuntu-latest permissions: contents: write steps: - name: Download grid packages uses: actions/download-artifact@v4 with: - name: nightly-grid + name: ${{ needs.prepare.outputs.artifact-name }} path: build/dist Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Reduce duplication and avoid hardcoding shared values by using a single source of truth (outputs) across jobs.	Low

Copilot

Pull request overview

This PR simplifies the nightly GitHub Actions workflow by consolidating multiple per-language jobs into a single release job with a preparatory input-parsing step, and by removing the dedicated “grid” language option.

Changes:

Added a prepare job to parse inputs and expose shared outputs (language, artifact info, release-in-progress flag).
Replaced per-language nightly jobs with a single nightly-release job that runs ./go <language>:release nightly.
Removed the separate grid language option and consolidated failure notifications into one on-failure job.

.github/workflows/nightly.yml

Move the nightly-specific npm configuration from the workflow into the rake task. This configures: - GitHub Packages registry and auth token via ~/.npmrc - Package name change to @seleniumhq/selenium-webdriver - Registry URL change to npm.pkg.github.com Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

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

rake_tasks/node.rake

titusfortner requested a review from Copilot January 25, 2026 20:13

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 20:13 View session

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

Copilot AI reviewed Jan 25, 2026

View reviewed changes

.github/workflows/nightly.yml Show resolved Hide resolved

.github/workflows/nightly.yml Show resolved Hide resolved

titusfortner requested a review from Copilot January 25, 2026 23:38

Copilot started reviewing on behalf of titusfortner January 25, 2026 23:38 View session

Copilot AI reviewed Jan 25, 2026

View reviewed changes

rake_tasks/node.rake Show resolved Hide resolved

titusfortner merged commit 371a719 into trunk Jan 25, 2026
32 checks passed

titusfortner deleted the consolidate-nightly-workflow branch January 25, 2026 23:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[build] consolidate nightly workflow#17005

[build] consolidate nightly workflow#17005
titusfortner merged 2 commits intotrunkfrom
consolidate-nightly-workflow

titusfortner commented Jan 25, 2026

Uh oh!

qodo-code-review bot commented Jan 25, 2026

File Walkthrough

Uh oh!

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

edited

Loading

Uh oh!

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

edited

Loading

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

titusfortner commented Jan 25, 2026

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review bot commented Jan 25, 2026

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

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

edited

Loading

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

edited

Loading