Replace hardcoded bazel-selenium references with dynamic path resolution

[titusfortner]

User description

🔗 Related Issues

None - discovered while working in a git worktree

💥 What does this PR do?

Removes hardcoded bazel-selenium references that break in git worktrees or differently-named checkouts.

• .gitignore: Replace individual bazel-* entries with /bazel-* glob pattern
• README.md: Update Ruby paths to use bazel-bin/external/... instead of bazel-selenium/external/...
• dotnet/private/docfx.bzl: Resolve execution root dynamically via bazel-bin symlink
• dotnet/update-deps.sh: Use bazel info output_base to find external dependencies

🔧 Implementation Notes

The bazel-selenium symlink name is derived from the workspace name and doesn't exist in worktrees or renamed checkouts. The fixes use stable Bazel paths:

• bazel-bin symlink always exists and points to <exec_root>/bazel-out/<config>/bin
• bazel info output_base returns the correct path regardless of workspace name
• /bazel-* in gitignore catches all Bazel convenience symlinks at the repo root

💡 Additional Considerations

None

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Replace hardcoded bazel-selenium references with dynamic path resolution

• Use bazel-bin symlink to resolve execution root in docfx scripts

• Use bazel info output_base for external dependencies in update-deps.sh

• Update Ruby documentation paths to use bazel-bin instead of bazel-selenium

---

Diagram Walkthrough

flowchart LR
  A["Hardcoded bazel-selenium paths"] -->|docfx.bzl| B["Resolve via bazel-bin symlink"]
  A -->|update-deps.sh| C["Resolve via bazel info output_base"]
  A -->|README.md| D["Update to bazel-bin paths"]
  B --> E["Unix/Windows scripts work in worktrees"]
  C --> F["External deps found correctly"]
  D --> G["Documentation accurate"]

Loading

File Walkthrough

	Relevant files
Bug fix	docfx.bzl Dynamic execution root resolution for docfx scripts            dotnet/private/docfx.bzl Replace hardcoded bazel-selenium paths with dynamic EXEC_ROOT resolution Unix template: resolve execution root by traversing from bazel-bin symlink Windows template: resolve execution root via bazel-bin junction with setlocal scope Use resolved EXEC_ROOT for both dotnet and docfx tool paths +11/-4    update-deps.sh Dynamic external dependencies resolution via bazel info    dotnet/update-deps.sh Replace hardcoded bazel-selenium/external with dynamic bazel info output_base Validate OUTPUT_BASE exists before constructing external directory path Use EXTERNAL_DIR variable for cleaner path construction Add error suppression for bazel info and find commands +4/-2
Documentation	README.md Update Ruby development paths to use bazel-bin                      README.md Update Ruby debugger path from bazel-selenium/external/bundle/bin/rdbg to bazel-bin/external/bundle/bin/rdbg Update RubyMine interpreter path from bazel-selenium/external/rules_ruby++ruby+ruby/dist/bin/ruby to bazel-bin/external/rules_ruby++ruby+ruby/dist/bin/ruby Maintain consistency with dynamic path resolution approach +2/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: Unhandled bazel failure: bazel info output_base failures are suppressed and can produce an empty EXTERNAL_DIR that becomes /external, leading to incorrect dependency discovery without an actionable error. Referred Code EXTERNAL_DIR=$(cd "$REPO_ROOT" && bazel info output_base 2>/dev/null)/external if [[ -d "$EXTERNAL_DIR" ]]; then DOTNET_DIR=$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1) 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 204f8c3

Category	Suggestion	Impact
Possible issue	✅ Fix execution root path resolution Suggestion Impact: The commit corrected EXEC_ROOT resolution by switching to `bazel info execution_root` (and similarly for Windows), addressing the same underlying problem of incorrect path calculation, though via a different implementation than the suggestion's bazel-bin realpath parsing. code diff: -# Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) -EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +EXEC_ROOT=$(bazel info execution_root) exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" """ @@ -44,12 +43,10 @@ _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" -rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) -cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." -set EXEC_ROOT=%CD% -cd /d "%BUILD_WORKSPACE_DIRECTORY%" +for /f "tokens=*" %%i in ('bazel info execution_root') do set "EXEC_ROOT=%%i" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* +endlocal """ Fix the logic for calculating the EXEC_ROOT path in the Unix shell script template. The current implementation is incorrect and will fail to find the required executables. dotnet/private/docfx.bzl [38-41] -# Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) -EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +# Resolve Bazel execution root from the real bazel-bin path. +BIN_REAL="$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin" && pwd -P)" +OUTPUT_BASE="${BIN_REAL%%/bazel-out/*}" +WS_NAME="$(basename "$BUILD_WORKSPACE_DIRECTORY")" +EXEC_ROOT="${OUTPUT_BASE}/execroot/${WS_NAME}" exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw in the PR's logic for resolving the Bazel execution root, which would lead to incorrect paths and script failure, and provides a robust and correct fix.	High
Update

---

Previous suggestions

✅ Suggestions up to commit 55602bb

Category	Suggestion	Impact
High-level	Reconsider replacing a specialized GitHub Action The pin-browsers.yml workflow's replacement of the peter-evans/create-pull-request action with a custom script is a regression, as it loses the ability to update existing pull requests. It is recommended to revert to the specialized action for more robust functionality. Examples: .github/workflows/pin-browsers.yml [19-54] push-changes: name: Push Changes needs: update if: github.event.repository.fork == false uses: ./.github/workflows/commit-changes.yml with: artifact-name: pinned-browsers commit-message: "Update pinned browser versions" push-branch: pinned-browser-updates secrets: ... (clipped 26 lines) Solution Walkthrough: Before: # .github/workflows/pin-browsers.yml push-changes: uses: ./.github/workflows/commit-changes.yml with: push-branch: pinned-browser-updates ... create-pr: needs: push-changes if: needs.push-changes.outputs.changes-committed == 'true' run: | existing=$(gh pr list --head pinned-browser-updates ...) if [ -n "$existing" ]; then echo "::notice::PR #$existing already exists" else gh pr create --head pinned-browser-updates ... fi After: # .github/workflows/pin-browsers.yml pull-request: needs: update runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 with: name: pinned-browsers - uses: peter-evans/create-pull-request@v6 with: token: ${{ secrets.SELENIUM_CI_TOKEN }} commit-message: "Update pinned browser versions" branch: "pinned-browser-updates" title: "[...] Automated Browser Version Update" ... Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant functional regression in the pin-browsers.yml workflow, where the new implementation fails to update existing pull requests, unlike the peter-evans/create-pull-request action it replaces.	High
General	Add fallback and quoting for external path Add a fallback to the original external directory path if bazel info output_base fails, and quote variable expansions. dotnet/update-deps.sh [6-8] -EXTERNAL_DIR=$(cd "$REPO_ROOT" && bazel info output_base 2>/dev/null)/external +EXTERNAL_DIR="$(cd "$REPO_ROOT" && bazel info output_base 2>/dev/null)/external" if [[ -d "$EXTERNAL_DIR" ]]; then - DOTNET_DIR=$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1) + DOTNET_DIR="$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1)" +elif [[ -d "$REPO_ROOT/bazel-selenium/external" ]]; then + DOTNET_DIR="$(find "$REPO_ROOT/bazel-selenium/external" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1)" +fi Suggestion importance[1-10]: 6 __ Why: The suggestion improves the script's robustness by adding a fallback mechanism to find the .NET directory, which was part of the original logic removed in the PR.	Low
Learned best practice	✅ Validate external command output Suggestion Impact: The commit adds an explicit warning path when `bazel info output_base` fails, indicating the script is now handling that failure case more deliberately before falling back to system dotnet. While it doesn't show the full `OUTPUT_BASE` guarding logic from the suggestion, it directly addresses the same concern of reacting safely to failed/empty bazel output. code diff: +else + echo "Warning: bazel info output_base failed; falling back to system dotnet" >&2 Guard against missing/failed bazel or empty output_base so you don’t accidentally probe /external or a malformed path; only form EXTERNAL_DIR when bazel info succeeds and returns a non-empty path. dotnet/update-deps.sh [6-13] -EXTERNAL_DIR=$(cd "$REPO_ROOT" && bazel info output_base 2>/dev/null)/external -if [[ -d "$EXTERNAL_DIR" ]]; then - DOTNET_DIR=$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1) - if [[ -n "$DOTNET_DIR" && -x "$DOTNET_DIR/dotnet" ]]; then - DOTNET="$DOTNET_DIR/dotnet" - echo "Using bazel-managed dotnet: $DOTNET" +OUTPUT_BASE="$(cd "$REPO_ROOT" && command -v bazel >/dev/null 2>&1 && bazel info output_base 2>/dev/null || true)" +if [[ -n "${OUTPUT_BASE}" ]]; then + EXTERNAL_DIR="${OUTPUT_BASE}/external" + if [[ -d "$EXTERNAL_DIR" ]]; then + DOTNET_DIR=$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1) + if [[ -n "$DOTNET_DIR" && -x "$DOTNET_DIR/dotnet" ]]; then + DOTNET="$DOTNET_DIR/dotnet" + echo "Using bazel-managed dotnet: $DOTNET" + fi fi fi Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add validation/guards for integration-boundary inputs (e.g., environment/tool outputs) before constructing paths from them.	Low
	✅ Add robust path existence checks Suggestion Impact: Instead of adding explicit bazel-bin/EXEC_ROOT existence checks, the commit changed the approach to resolving EXEC_ROOT by calling `bazel info execution_root` on both Unix and Windows. This removes reliance on `bazel-bin` being present and avoids the original fragile path derivation that motivated the suggested guards. code diff: @@ -35,8 +35,7 @@ _UNIX_TEMPLATE = """#!/usr/bin/env bash set -euo pipefail cd "$BUILD_WORKSPACE_DIRECTORY" -# Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) -EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +EXEC_ROOT=$(bazel info execution_root) exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" """ @@ -44,12 +43,10 @@ _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" -rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) -cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." -set EXEC_ROOT=%CD% -cd /d "%BUILD_WORKSPACE_DIRECTORY%" +for /f "tokens=*" %%i in ('bazel info execution_root') do set "EXEC_ROOT=%%i" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* +endlocal Add explicit existence checks and a clear error message if bazel-bin isn’t present or the derived EXEC_ROOT is empty/invalid, to avoid confusing failures when run outside Bazel or before a build. dotnet/private/docfx.bzl [35-53] _UNIX_TEMPLATE = """#!/usr/bin/env bash set -euo pipefail cd "$BUILD_WORKSPACE_DIRECTORY" +if [ ! -e "$BUILD_WORKSPACE_DIRECTORY/bazel-bin" ]; then + echo "bazel-bin not found; run a Bazel build first." >&2 + exit 1 +fi # Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +if [ -z "${EXEC_ROOT}" ] || [ ! -d "${EXEC_ROOT}" ]; then + echo "Failed to resolve Bazel execution root." >&2 + exit 1 +fi exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" """ _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" +if not exist "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin" ( + echo bazel-bin not found; run a Bazel build first. 1>&2 + exit /b 1 +) rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." set EXEC_ROOT=%CD% +if not exist "%EXEC_ROOT%" ( + echo Failed to resolve Bazel execution root. 1>&2 + exit /b 1 +) cd /d "%BUILD_WORKSPACE_DIRECTORY%" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* """ Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add validation/guards at integration boundaries when deriving filesystem paths from Bazel/workspace state.	Low
Possible issue	Quote patch file checks Quote the changes.patch file path in the if condition to handle potential spaces or special characters in filenames. .github/workflows/commit-changes.yml [55] -if [ -f changes.patch ] && [ -s changes.patch ]; then +if [ -f "changes.patch" ] && [ -s "changes.patch" ]; then Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that quoting file paths in shell tests is a good practice for robustness, although it's a minor issue in this specific context.	Low

[Copilot]

Pull request overview

This PR aims to replace hardcoded bazel-selenium references with dynamic path resolution to support git worktrees and differently-named checkouts. However, it also includes substantial GitHub Actions workflow refactoring that is unrelated to the stated purpose.

Changes:

• Replaced hardcoded bazel-selenium paths with dynamic resolution using bazel info output_base and bazel-bin symlinks
• Simplified .gitignore to use /bazel-* pattern instead of individual entries
• Created a new reusable GitHub Actions workflow for committing changes and refactored five workflow files to use it

Reviewed changes

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

Show a summary per file

File	Description
.gitignore	Replaced individual bazel-* entries with /bazel-* glob pattern to catch all Bazel convenience symlinks
README.md	Updated Ruby development paths to use bazel-bin/external/... instead of bazel-selenium/external/...
dotnet/private/docfx.bzl	Implemented dynamic execution root resolution via bazel-bin symlink traversal
dotnet/update-deps.sh	Changed to use bazel info output_base for finding external dependencies
.github/workflows/commit-changes.yml	Created new reusable workflow for applying and committing patch artifacts
.github/workflows/release.yml	Refactored to use new reusable commit-changes workflow
.github/workflows/pre-release.yml	Refactored to use new reusable commit-changes workflow
.github/workflows/pin-browsers.yml	Refactored to use new reusable commit-changes workflow and improved PR creation logic
.github/workflows/ci-renovate-rbe.yml	Refactored to use new reusable commit-changes workflow
.github/workflows/ci-lint.yml	Refactored to use new reusable commit-changes workflow and improved fork handling

[titusfortner]

this branch was rebased off wrong branch. fixed now.

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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 edge checks: The new execution-root resolution assumes bazel-bin exists and has the expected ../../.. structure, but adds no explicit validation or actionable error message if the symlink/junction is missing or layout differs. Referred Code # Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

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

[qodo-code-review]

Persistent suggestions updated to latest commit 204f8c3

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Improve path resolution in Windows script Suggestion Impact: The Windows template was updated to set EXEC_ROOT via a for /f loop (now using `bazel info execution_root`) and `endlocal` was added, matching the suggestion’s intent of more robust path resolution and proper environment scoping. code diff: _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" -rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) -cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." -set EXEC_ROOT=%CD% -cd /d "%BUILD_WORKSPACE_DIRECTORY%" +for /f "tokens=*" %%i in ('bazel info execution_root') do set "EXEC_ROOT=%%i" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* +endlocal """ Refactor the Windows batch script to use a for loop for more robust path resolution and add endlocal to properly manage environment variables. dotnet/private/docfx.bzl [44-53] _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) -cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." -set EXEC_ROOT=%CD% -cd /d "%BUILD_WORKSPACE_DIRECTORY%" +for /f "delims=" %%i in ("%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\..") do set "EXEC_ROOT=%%~fi" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* +endlocal """ [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: This is a good suggestion that improves the Windows batch script by using a more idiomatic and robust method for path resolution and by correctly scoping environment variable changes with endlocal.	Low
	Warn on bazel info failure Add a warning message to inform the user if bazel info output_base fails and the script falls back to the system dotnet. dotnet/update-deps.sh [6-9] OUTPUT_BASE=$(cd "$REPO_ROOT" && bazel info output_base 2>/dev/null) +if [[ -z "$OUTPUT_BASE" ]]; then + echo "Warning: bazel info output_base failed, defaulting to system dotnet" +fi if [[ -n "$OUTPUT_BASE" && -d "$OUTPUT_BASE/external" ]]; then EXTERNAL_DIR="$OUTPUT_BASE/external" DOTNET_DIR=$(find "$EXTERNAL_DIR" -maxdepth 1 -name "rules_dotnet++dotnet+dotnet_*" -type d 2>/dev/null | head -1) Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves user experience by adding a warning message when a command fails silently, which helps in debugging why the script falls back to the system dotnet.	Low
Learned best practice	✅ Guard and validate exec-root resolution Suggestion Impact: The patch updates both Unix and Windows templates to resolve EXEC_ROOT via `bazel info execution_root` instead of traversing from bazel-bin. However, it does not add the suggested guards/validation for empty/non-existent paths. code diff: cd "$BUILD_WORKSPACE_DIRECTORY" -# Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) -EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +EXEC_ROOT=$(bazel info execution_root) exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" """ @@ -44,12 +43,10 @@ _WINDOWS_TEMPLATE = """@echo off setlocal cd /d "%BUILD_WORKSPACE_DIRECTORY%" -rem Resolve execution root from bazel-bin junction (bin -> config -> bazel-out -> exec_root) -cd /d "%BUILD_WORKSPACE_DIRECTORY%\\bazel-bin\\..\\..\\.." -set EXEC_ROOT=%CD% -cd /d "%BUILD_WORKSPACE_DIRECTORY%" +for /f "tokens=*" %%i in ('bazel info execution_root') do set "EXEC_ROOT=%%i" "%EXEC_ROOT%\\{dotnet}" exec ^ "%EXEC_ROOT%\\{docfx}" {config} %* Avoid assuming the bazel-bin/../../.. traversal is valid; resolve via bazel info execution_root and validate it’s non-empty and exists before use. dotnet/private/docfx.bzl [38-41] -# Resolve execution root from bazel-bin symlink (bin -> config -> bazel-out -> exec_root) -EXEC_ROOT=$(cd "$BUILD_WORKSPACE_DIRECTORY/bazel-bin/../../.." && pwd -P) +EXEC_ROOT="$(cd "$BUILD_WORKSPACE_DIRECTORY" && bazel info execution_root 2>/dev/null || true)" +if [[ -z "$EXEC_ROOT" || ! -d "$EXEC_ROOT" ]]; then + echo "Failed to resolve Bazel execution root" >&2 + exit 1 +fi exec "$EXEC_ROOT/{dotnet}" exec \ "$EXEC_ROOT/{docfx}" {config} "$@" [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., filesystem/exec-root resolution) instead of assuming paths exist.	Low
More

[Copilot]

Pull request overview

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