Skip to content

Gate CI test jobs via change scanning instead of workflow-level paths-ignore #8979

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SID-6921 wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Gate CI test jobs via change scanning instead of workflow-level paths-ignore #8979

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69ba220
Gate CI test jobs using changed-file analysis instead of paths-ignore
SID-6921 Jul 3, 2026
e602050
Fix workflow: add permissions, disable persist-credentials, use env v…
SID-6921 Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions .github/workflows/cicd_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,11 @@ on:
- dev
- main
- releasing/*
paths-ignore: # skip if only docs are modified
- '**.md'
- '**.rst'
- 'docs/**'
pull_request:
branches:
- dev
- main
- releasing/*
paths-ignore: # skip if only docs are modified
- '**.md'
- '**.rst'
- 'docs/**'

concurrency:
# automatically cancel the previously triggered workflows when there's a newer version
Expand Down Expand Up @@ -52,7 +44,59 @@ env:
# When support is dropped for a version it is important to update these as appropriate.

jobs:
changes:
runs-on: ubuntu-latest
permissions:
contents: read
outputs:
run_tests: ${{ steps.diff.outputs.run_tests }}
steps:
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- uses: actions/checkout@v6
with:
fetch-depth: 0
Comment on lines +54 to +56
Comment thread
coderabbitai[bot] marked this conversation as resolved.
persist-credentials: false
- id: diff
name: Determine if non-doc changes exist
env:
BASE_REF: ${{ github.base_ref }}
EVENT_NAME: ${{ github.event_name }}
EVENT_BEFORE: ${{ github.event.before }}
EVENT_SHA: ${{ github.sha }}
shell: bash
run: |
set -euo pipefail

# Default to running tests on any failure (fail-open for safety)
trap 'echo "run_tests=true" >> "$GITHUB_OUTPUT"' ERR

if [[ "$EVENT_NAME" == "pull_request" ]]; then
base_ref="origin/$BASE_REF"
git fetch origin "$BASE_REF" --depth=1
changed_files=$(git diff --name-only "$base_ref"..."$EVENT_SHA")
else
before="$EVENT_BEFORE"
if [[ -z "$before" || "$before" == "0000000000000000000000000000000000000000" ]]; then
echo "run_tests=true" >> "$GITHUB_OUTPUT"
exit 0
fi
changed_files=$(git diff --name-only "$before"..."$EVENT_SHA")
fi

run_tests=false
while IFS= read -r file; do
[[ -z "$file" ]] && continue
if [[ "$file" == docs/* || "$file" == *.md || "$file" == *.rst ]]; then
continue
fi
run_tests=true
break
done <<< "$changed_files"

echo "run_tests=$run_tests" >> "$GITHUB_OUTPUT"
Comment on lines +66 to +95

@coderabbitai coderabbitai Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

git ls-files .github/workflows && echo '---' && sed -n '1,220p' .github/workflows/cicd_tests.yml

Repository: Project-MONAI/MONAI

Length of output: 8407

🌐 Web query:

GitHub Actions skipped job required status check branch protection treated as success or failure

💡 Result:

GitHub treats a skipped job as a successful status for the purposes of branch protection [1][2][3]. When a job is skipped—for example, due to a conditional if expression—it reports its status as "Success," meaning it will not block a pull request from merging even if that job is configured as a required status check [1][2][4]. However, the behavior differs depending on why the job was skipped: 1. Job-level conditionals: If a job within a workflow is skipped because its if condition evaluated to false, it is treated as a success [5][2]. This can lead to unintended results if you rely on that job to validate your code [6]. 2. Workflow-level filtering: If an entire workflow is skipped due to path filtering, branch filtering, or a commit message, the associated checks will remain in a "Pending" state [7][2][8]. Because they never report a status, they will block the pull request from merging if they are configured as required [7][2][8]. 3. Dependency chains: If a job is skipped because a prerequisite job failed, it is also treated as a success [6][2]. To prevent this, you can use the always() conditional expression in your job's if field to ensure the job runs (and reports a failure) even if its dependencies failed [6][2][8]. For required checks that depend on conditions, it is generally recommended to avoid using path or branch filtering if the workflow is required for merging [8]. Instead, consider using the always() function to ensure that downstream jobs explicitly report a failure status if required upstream tasks do not succeed [6][2].

Citations:

🏁 Script executed:

rg -n "needs: changes|timeout-minutes:|if: needs\.changes\.outputs\.run_tests" .github/workflows/cicd_tests.yml

Repository: Project-MONAI/MONAI

Length of output: 520

Fail open when diff detection breaks .github/workflows/cicd_tests.yml:58-84
git fetch/git diff under set -euo pipefail can fail the changes job, which skips every needs: changes test job. Skipped required checks pass, so a transient diff error can let untested changes merge. Default run_tests=true on diff failures and add a timeout.

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 47-84: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

[error] 62-62: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

[error] 63-63: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/cicd_tests.yml around lines 58 - 84, The changes-detection
step in cicd_tests.yml can fail closed under set -euo pipefail when git fetch or
git diff errors, which prevents the changes job from setting a safe default.
Update the shell logic around the changed_files computation so any
diff-detection failure in the current changes job path falls back to
run_tests=true instead of exiting, and add a timeout to the fetch/diff
operation; keep the fix localized to the workflow step that uses
github.event_name, git fetch, git diff, and the run_tests output.


static-checks: # Perform static type and other checks using runtests.sh
needs: changes
if: needs.changes.outputs.run_tests == 'true'
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down Expand Up @@ -84,6 +128,8 @@ jobs:
$(pwd)/runtests.sh --build --${{ matrix.opt }} -j $(nproc --all)

min-dep: # Test with minumum dependencies installed for different OS, Python, and PyTorch combinations
needs: changes
if: needs.changes.outputs.run_tests == 'true'
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -167,6 +213,8 @@ jobs:
NGC_TEAM: ${{ secrets.NGC_TEAM }}

full-dep: # Test with full dependencies installed for different OS runners
needs: changes
if: needs.changes.outputs.run_tests == 'true'
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -238,6 +286,8 @@ jobs:
shell: bash

packaging: # Test package generation
needs: changes
if: needs.changes.outputs.run_tests == 'true'
runs-on: ubuntu-latest
env:
QUICKTEST: True
Expand Down
Loading