Route Falcor CI through dedicated runner#11754
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR adds the ChangesFalcor CI bridge
CI filter quoting
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e898b82b-177d-4280-bfef-da8f71b46c0a
📒 Files selected for processing (3)
.github/actionlint.yaml.github/workflows/ci-falcor-test.yml.github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6dae6e12-80cd-4dd0-a616-d931291d5717
📒 Files selected for processing (3)
.github/scripts/relay-external-ci.py.github/workflows/ci-falcor-test.yml.github/workflows/ci.yml
| - name: Run external CI | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| EXTERNAL_CI_API_BASE_URL: ${{ secrets.FALCOR_CI_API_BASE_URL }} | ||
| EXTERNAL_CI_PROJECT: ${{ secrets.FALCOR_CI_PROJECT }} | ||
| EXTERNAL_CI_REF: ${{ secrets.FALCOR_CI_REF }} | ||
| EXTERNAL_CI_TOKEN: ${{ secrets.FALCOR_CI_TOKEN }} | ||
| EXTERNAL_CI_VARIABLES_JSON: ${{ secrets.FALCOR_CI_VARIABLES_JSON }} | ||
| EXTERNAL_CI_ARTIFACT_NAME: slang-tests-windows-x86_64-cl-release | ||
| run: | | ||
| set -euo pipefail | ||
| cd FalcorBin/tests | ||
| python ./testing/run_image_tests.py --config windows-vs2022-Release --run-only | ||
| python3 .github/scripts/relay-external-ci.py |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Set the relay timeout below the 100-minute job timeout.
The called script defaults to 7200 seconds, but this job times out after 100 minutes. Without an override, GitHub can terminate the job before the relay reports a controlled timeout.
Proposed fix
EXTERNAL_CI_TOKEN: ${{ secrets.FALCOR_CI_TOKEN }}
EXTERNAL_CI_VARIABLES_JSON: ${{ secrets.FALCOR_CI_VARIABLES_JSON }}
EXTERNAL_CI_ARTIFACT_NAME: slang-tests-windows-x86_64-cl-release
+ EXTERNAL_CI_TIMEOUT_SECONDS: "5400"
run: |
set -euo pipefail
python3 .github/scripts/relay-external-ci.py📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run external CI | |
| env: | |
| GITHUB_TOKEN: ${{ github.token }} | |
| EXTERNAL_CI_API_BASE_URL: ${{ secrets.FALCOR_CI_API_BASE_URL }} | |
| EXTERNAL_CI_PROJECT: ${{ secrets.FALCOR_CI_PROJECT }} | |
| EXTERNAL_CI_REF: ${{ secrets.FALCOR_CI_REF }} | |
| EXTERNAL_CI_TOKEN: ${{ secrets.FALCOR_CI_TOKEN }} | |
| EXTERNAL_CI_VARIABLES_JSON: ${{ secrets.FALCOR_CI_VARIABLES_JSON }} | |
| EXTERNAL_CI_ARTIFACT_NAME: slang-tests-windows-x86_64-cl-release | |
| run: | | |
| set -euo pipefail | |
| cd FalcorBin/tests | |
| python ./testing/run_image_tests.py --config windows-vs2022-Release --run-only | |
| python3 .github/scripts/relay-external-ci.py | |
| - name: Run external CI | |
| env: | |
| GITHUB_TOKEN: ${{ github.token }} | |
| EXTERNAL_CI_API_BASE_URL: ${{ secrets.FALCOR_CI_API_BASE_URL }} | |
| EXTERNAL_CI_PROJECT: ${{ secrets.FALCOR_CI_PROJECT }} | |
| EXTERNAL_CI_REF: ${{ secrets.FALCOR_CI_REF }} | |
| EXTERNAL_CI_TOKEN: ${{ secrets.FALCOR_CI_TOKEN }} | |
| EXTERNAL_CI_VARIABLES_JSON: ${{ secrets.FALCOR_CI_VARIABLES_JSON }} | |
| EXTERNAL_CI_ARTIFACT_NAME: slang-tests-windows-x86_64-cl-release | |
| EXTERNAL_CI_TIMEOUT_SECONDS: "5400" | |
| run: | | |
| set -euo pipefail | |
| python3 .github/scripts/relay-external-ci.py |
| git fetch origin "${{ github.base_ref }}" | ||
| BASE=origin/${{ github.base_ref }} | ||
| else | ||
| BASE=HEAD^1 | ||
| fi | ||
|
|
||
| shouldRun=true | ||
| if files="$(git diff --name-only $BASE...HEAD)"; then | ||
| if files="$(git diff --name-only "${BASE}...HEAD")"; then |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass github.base_ref through env before using it in Bash.
Lines 27-28 still expand the GitHub expression directly into the shell script. Move it into an environment variable and quote that variable in Bash.
Proposed fix
- id: filter
+ env:
+ BASE_REF: ${{ github.base_ref }}
run: |
# This step prevents subsequent steps from running if only documentation was changed
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
- git fetch origin "${{ github.base_ref }}"
- BASE=origin/${{ github.base_ref }}
+ git fetch origin "$BASE_REF"
+ BASE="origin/${BASE_REF}"
else
BASE=HEAD^1
fiAs per path instructions, .github/workflows/** changes require verifying correct trigger conditions and proper secret handling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| git fetch origin "${{ github.base_ref }}" | |
| BASE=origin/${{ github.base_ref }} | |
| else | |
| BASE=HEAD^1 | |
| fi | |
| shouldRun=true | |
| if files="$(git diff --name-only $BASE...HEAD)"; then | |
| if files="$(git diff --name-only "${BASE}...HEAD")"; then | |
| - id: filter | |
| env: | |
| BASE_REF: ${{ github.base_ref }} | |
| run: | | |
| # This step prevents subsequent steps from running if only documentation was changed | |
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | |
| git fetch origin "$BASE_REF" | |
| BASE="origin/${BASE_REF}" | |
| else | |
| BASE=HEAD^1 | |
| fi | |
| shouldRun=true | |
| if files="$(git diff --name-only "${BASE}...HEAD")"; then |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 27-27: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 28-28: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
Sources: Path instructions, Linters/SAST tools
|
Heads-up from CI-health monitoring (not a review): the |
2 participants
Summary:
Validation: