Repository navigation
Fix PRs blocked by skipped build-and-test check - #1611
Merged
Merged
Conversation
Replace path filters on workflow triggers with a check-changes job that uses git diff to detect source changes. The build matrix is conditional on that check. A gate job (build-and-test-gate) always runs and reports the correct status: pass for docs-only PRs, pass/fail based on actual test results for code PRs. Branch protection should require "build-and-test-gate" instead of the individual build matrix jobs. https://claude.ai/code/session_01Y9hWTJfZiQuNXuQyTvw7Uq Signed-off-by: Claude <noreply@anthropic.com>
Contributor
📂 Previous Runs📜 Run @ 5762d79 (#22259708913)✅ Results of HolmesGPT evalsAutomatically triggered by commit 5762d79 on branch Results of HolmesGPT evals
📜 Run @ 77b752a (#22259212595)✅ Results of HolmesGPT evalsAutomatically triggered by commit 77b752a on branch Results of HolmesGPT evals
✅ Results of HolmesGPT evalsAutomatically triggered by commit 5b59ee2 on branch Results of HolmesGPT evals
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" Option 3: Add PR labels to include extra evals in automatic regression runs:
Examples: 🏷️ Valid tags
Commands: CLI: |
Contributor
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8b74708d
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8b74708d me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8b74708d
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8b74708dPatch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:8b74708dRobusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:8b74708d |
Contributor
WalkthroughAdds a runtime change detector job Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger as "Push / PR / workflow_dispatch"
participant Runner as "GitHub Actions Runner"
participant Check as "check-changes (job)"
participant Build as "build (job)"
participant Gate as "build-and-test-gate (job)"
Trigger->>Runner: start workflow
Runner->>Check: execute check-changes
Check-->>Runner: outputs should_test (true/false)
alt should_test == "true"
Runner->>Build: run build job (needs: check-changes)
Build-->>Runner: build result (success/failure)
end
Runner->>Gate: always run gate (depends on check-changes & build)
alt should_test == "false"
Gate-->>Runner: report success (docs-only / no build run)
else build failed
Gate-->>Runner: report failure
else build success
Gate-->>Runner: report success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-test.yaml:
- Line 18: Replace deprecated GitHub Action versions: update every occurrence of
"actions/checkout@v2" to "actions/checkout@v4" and every
"actions/setup-python@v2" to "actions/setup-python@v5" (these appear in the
workflow where the checkout/setup-python steps are defined, e.g., the lines
referencing actions/checkout and actions/setup-python); ensure the step names
and inputs remain unchanged and run a quick lint or actionlint check to confirm
no other required parameter changes for the newer major versions.
- Around line 30-32: Replace the git diff range that only inspects the last
commit (CHANGED_FILES=$(git diff --name-only HEAD~1..HEAD)) with a range using
the push event boundary (use github.event.before..HEAD) so all commits in the
push are considered; update the CHANGED_FILES assignment to use git diff
--name-only "${{ github.event.before }}..HEAD" and add a guard for the zero SHA
case (when github.event.before is all zeros) to fall back to setting
should_test=true or diffing the branch tip to ensure tests run for new branches.
- Around line 147-161: The build-and-test-gate currently treats a missing or
empty needs.check-changes.outputs.should_test as "docs-only" and exits zero,
masking failures in the check-changes job; update the gate step (job
build-and-test-gate) to first assert needs.check-changes.result == "success" (or
that needs.check-changes.outputs.should_test is explicitly set) and if not, echo
an error and exit 1, and then keep the existing should_test == "true" branch—use
the job/result symbol needs.check-changes.result and the output
needs.check-changes.outputs.should_test to implement these checks so a failing
check-changes job causes the gate to fail rather than silently pass.
- Upgrade actions/checkout to v4 and actions/setup-python to v5 - Use github.event.before..github.sha for push events to cover all commits in multi-commit pushes (with zero-SHA guard for new branches) - Add check-changes.result guard in gate job to fail if the check-changes job itself crashes instead of silently passing https://claude.ai/code/session_01Y9hWTJfZiQuNXuQyTvw7Uq Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build-and-test.yaml (1)
38-38: Consider two-dot..range for push events.On line 38 the three-dot
...range ("$BEFORE"...${{ github.sha }}) computes the diff from the merge-base ofBEFOREandHEADtoHEAD. For push eventsBEFOREis always on the same branch asHEAD, so in normal fast-forward pushes both are equivalent. But on a force-push (non-linear history)..more precisely captures "every file that changed between the old and new branch tip", which is the intended semantics for deciding whether to run tests.The previous review's suggested fix used
.., and the PR event on line 26 correctly uses...(merge-base is right for cross-branch comparisons). Using..here would make the distinction explicit and match the prior recommendation.♻️ Suggested change
- CHANGED_FILES=$(git diff --name-only "$BEFORE"...${{ github.sha }}) + CHANGED_FILES=$(git diff --name-only "$BEFORE"..${{ github.sha }})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-test.yaml at line 38, The git diff invocation that sets CHANGED_FILES uses a three-dot range (git diff --name-only "$BEFORE"...${{ github.sha }}) which uses the merge-base and can miss intended push-range semantics on non-linear force-pushes; update that command to use a two-dot range (git diff --name-only "$BEFORE"..${{ github.sha }}) so CHANGED_FILES reflects files changed between the old and new branch tips for push events, keeping the existing three-dot usage for PR comparisons unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-and-test.yaml:
- Line 38: The git diff invocation that sets CHANGED_FILES uses a three-dot
range (git diff --name-only "$BEFORE"...${{ github.sha }}) which uses the
merge-base and can miss intended push-range semantics on non-linear
force-pushes; update that command to use a two-dot range (git diff --name-only
"$BEFORE"..${{ github.sha }}) so CHANGED_FILES reflects files changed between
the old and new branch tips for push events, keeping the existing three-dot
usage for PR comparisons unchanged.
- Drop push-to-master trigger (redundant — PRs already gate on tests) - Remove the zero-SHA guard and push event handling (no longer needed) - Refactor path matching into SKIP_PATTERNS and ALWAYS_TEST arrays defined at the top of the script for easy maintenance https://claude.ai/code/session_01Y9hWTJfZiQuNXuQyTvw7Uq Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build-and-test.yaml (1)
46-51:ALWAYS_TESTmatching uses exact string comparison — consider usingcasefor consistency.The
SKIP_PATTERNSsection usescase/glob matching, butALWAYS_TESTuses[ "$file" = "$pattern" ]. If a glob pattern is ever added toALWAYS_TEST, it won't match. Usingcasehere too would make both arrays behave the same way.Suggested change
# Check always-test exceptions first for pattern in "${ALWAYS_TEST[@]}"; do - if [ "$file" = "$pattern" ]; then + case "$file" in + $pattern) SHOULD_TEST=true break 2 - fi + ;; + esac done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-test.yaml around lines 46 - 51, The ALWAYS_TEST loop currently uses exact string equality ([ "$file" = "$pattern" ]) so glob patterns won't match; change the inner comparison to use shell pattern matching with a case statement (matching $file against $pattern) so ALWAYS_TEST behaves like SKIP_PATTERNS, and ensure SHOULD_TEST is set and break 2 remains when a match is found (refer to variables ALWAYS_TEST, file, pattern, and the SHOULD_TEST flag).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-and-test.yaml:
- Around line 46-51: The ALWAYS_TEST loop currently uses exact string equality
([ "$file" = "$pattern" ]) so glob patterns won't match; change the inner
comparison to use shell pattern matching with a case statement (matching $file
against $pattern) so ALWAYS_TEST behaves like SKIP_PATTERNS, and ensure
SHOULD_TEST is set and break 2 remains when a match is found (refer to variables
ALWAYS_TEST, file, pattern, and the SHOULD_TEST flag).
aantn
enabled auto-merge (squash)
February 21, 2026 16:07
moshemorad
approved these changes
Feb 22, 2026
moshemorad
pushed a commit
that referenced
this pull request
Feb 22, 2026
Replace path filters on workflow triggers with a check-changes job that uses git diff to detect source changes. The build matrix is conditional on that check. A gate job (build-and-test-gate) always runs and reports the correct status: pass for docs-only PRs, pass/fail based on actual test results for code PRs. Branch protection should require "build-and-test-gate" instead of the individual build matrix jobs. https://claude.ai/code/session_01Y9hWTJfZiQuNXuQyTvw7Uq Signed-off-by: Claude <noreply@anthropic.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * CI now detects whether source changes require tests and conditionally runs the build only when needed. * Added an always-running gate job that aggregates check and build results for branch protection. * Workflow sequencing updated so build depends on the change check; tooling steps for checkout and Python setup were upgraded for improved reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
This was referenced Feb 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace path filters on workflow triggers with a check-changes job that
uses git diff to detect source changes. The build matrix is conditional
on that check. A gate job (build-and-test-gate) always runs and reports
the correct status: pass for docs-only PRs, pass/fail based on actual
test results for code PRs.
Branch protection should require "build-and-test-gate" instead of the
individual build matrix jobs.
https://claude.ai/code/session_01Y9hWTJfZiQuNXuQyTvw7Uq
Signed-off-by: Claude noreply@anthropic.com
Summary by CodeRabbit