fix(ci): prevent infinite loop in release workflow - #164
Conversation
Added `if: github.actor != 'github-actions[bot]'` condition to prevent the release workflow from triggering itself when semantic-release creates commits. This fixes an infinite loop where the bot's release commits would trigger the workflow again. Also added comprehensive tests to validate workflow configuration and prevent regression. Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds bot-exclusion guard and 15m job timeouts to release and CI workflows, replaces combined install steps with explicit jq/yq setup, and introduces bats-based and shell-based tests to validate GitHub Actions workflow files and conventions. Changes
Sequence Diagram(s)(omitted — changes do not introduce a new multi-component control flow requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
🤖 Fix all issues with AI agents
In `@tests/github_workflows.bats`:
- Around line 35-43: The setup() function currently calls itself causing
infinite recursion; remove the self-call inside setup() or replace it with the
correct parent/helper setup function from your helper file (do not call setup()
from within setup()); locate the setup() definition and either delete the inner
"setup" call or change it to the actual helper function name (e.g., common_setup
or the setup function exported by bats_helper) after verifying that helper's
function name.
- Around line 22-33: The helpers workflow_has_trigger and job_has_if_condition
are unreliable because they rely on yaml_get/yq exit codes which return 0 for
null results; update both functions to either call yq eval with the -e flag (so
it errors on null) or explicitly test for non-empty output from yq (e.g.,
capture yq eval ".on.${trigger_type}" or ".jobs.${job_name}.if" and check [[ -n
... ]]) instead of redirecting to /dev/null and relying on the exit status;
ensure the changes are applied to the workflow_has_trigger and
job_has_if_condition functions so they correctly detect missing paths.
In `@tests/run_workflow_tests.sh`:
- Around line 1-6: Add strict error handling to the script by enabling "set -euo
pipefail" immediately after the shebang (#!/usr/bin/env bash) so the script
exits on errors, treats unset variables as failures, and fails on pipeline
errors; ensure this change applies to the top of the file that defines
WORKFLOW_DIR, RELEASE_WORKFLOW, and CI_WORKFLOW so the rest of the script runs
under strict mode.
🧹 Nitpick comments (4)
tests/run_workflow_tests.sh (4)
8-12: UnusedYELLOWvariable.The
YELLOWcolor variable is defined but never used in the script. Consider removing it or using it for warning messages.♻️ Proposed fix
# Colors RED='\033[0;31m' GREEN='\033[0;32m' -YELLOW='\033[1;33m' NC='\033[0m' # No Color
117-126: Error messages should be sent to stderr.Per coding guidelines, error messages should use
>&2redirect. This applies to lines 118, 121, and 124.♻️ Proposed fix
if [ "$if_condition" = "null" ] || [ -z "$if_condition" ]; then - echo " Error: No if condition found" + echo " Error: No if condition found" >&2 test_10_pass=false elif ! echo "$if_condition" | grep -q 'github-actions\[bot\]'; then - echo " Error: if condition does not check for github-actions[bot]" + echo " Error: if condition does not check for github-actions[bot]" >&2 test_10_pass=false elif ! echo "$if_condition" | grep -q '!='; then - echo " Error: if condition does not use inequality operator" + echo " Error: if condition does not use inequality operator" >&2 test_10_pass=false fi
62-70: Consider checking foryqdependency before use.The script uses
yqfor YAML parsing but doesn't verify it's installed. Ifyqis unavailable, tests will fail with unclear error messages.♻️ Proposed fix - add dependency check after line 6
# Check dependencies if ! command -v yq &> /dev/null; then echo "Error: yq is required but not installed" >&2 exit 1 fi
172-174: Consider explicit success exit.Per coding guidelines, scripts should return exit code 0 on success explicitly.
♻️ Proposed fix
if [ $FAIL -gt 0 ]; then exit 1 fi + +exit 0
| workflow_has_trigger() { | ||
| local workflow_file="$1" | ||
| local trigger_type="$2" | ||
| yaml_get "$workflow_file" ".on.${trigger_type}" &>/dev/null | ||
| } | ||
|
|
||
| # Helper: Check if job has 'if' condition | ||
| job_has_if_condition() { | ||
| local workflow_file="$1" | ||
| local job_name="$2" | ||
| yaml_get "$workflow_file" ".jobs.${job_name}.if" &>/dev/null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify yq exit code behavior for non-existent YAML paths
echo "name: test" > /tmp/test.yaml
echo "Testing yq exit codes..."
# Test existing path
yq eval '.name' /tmp/test.yaml > /dev/null 2>&1
echo "Existing path exit code: $?"
# Test non-existing path
yq eval '.nonexistent' /tmp/test.yaml > /dev/null 2>&1
echo "Non-existing path exit code: $?"
# Test deeply nested non-existing path
yq eval '.on.push' /tmp/test.yaml > /dev/null 2>&1
echo "Nested non-existing path exit code: $?"
# Also test what output is returned
echo ""
echo "Output for non-existent path:"
yq eval '.nonexistent' /tmp/test.yaml
rm /tmp/test.yamlRepository: baleen37/claude-plugins
Length of output: 283
🏁 Script executed:
# Find and examine the test file and yaml_get function
find . -name "github_workflows.bats" -type fRepository: baleen37/claude-plugins
Length of output: 96
🏁 Script executed:
# Look for yaml_get function definition
rg "yaml_get" --type bash -B 2 -A 5Repository: baleen37/claude-plugins
Length of output: 95
🏁 Script executed:
cat ./tests/github_workflows.batsRepository: baleen37/claude-plugins
Length of output: 3776
🏁 Script executed:
# Search for yaml_get definition
rg "yaml_get\s*\(\)" -A 10Repository: baleen37/claude-plugins
Length of output: 602
🏁 Script executed:
# Search for yaml_get anywhere
rg "yaml_get" -A 3 -B 1Repository: baleen37/claude-plugins
Length of output: 2632
🏁 Script executed:
find .github/workflows -type f -name "*.yml" -o -name "*.yaml"Repository: baleen37/claude-plugins
Length of output: 121
🏁 Script executed:
# Check if the workflow files referenced in tests exist
ls -la .github/workflows/ 2>/dev/null || echo "Directory not found"Repository: baleen37/claude-plugins
Length of output: 352
🏁 Script executed:
cat .github/workflows/ci.ymlRepository: baleen37/claude-plugins
Length of output: 611
🏁 Script executed:
cat .github/workflows/release.ymlRepository: baleen37/claude-plugins
Length of output: 4089
The helpers are unreliable because yq eval returns exit code 0 for both existing and non-existent paths.
workflow_has_trigger() and job_has_if_condition() redirect output to /dev/null and rely on exit codes to determine success. However, yq eval returns 0 (success) even when a path doesn't exist—it simply returns null. This means tests like workflow_has_trigger "$RELEASE_WORKFLOW" "nonexistent_trigger" would incorrectly pass.
The actual workflows have the expected paths, so these tests work in practice, but the logic is fundamentally flawed. Use yq eval with the -e flag (error on null) or check the actual output instead of just the exit code:
workflow_has_trigger() {
local workflow_file="$1"
local trigger_type="$2"
yq eval -e ".on.${trigger_type}" "$workflow_file" >/dev/null 2>&1
}Alternatively, explicitly check for non-null output:
workflow_has_trigger() {
local workflow_file="$1"
local trigger_type="$2"
[[ -n "$(yq eval ".on.${trigger_type}" "$workflow_file" 2>/dev/null)" ]]
}🤖 Prompt for AI Agents
In `@tests/github_workflows.bats` around lines 22 - 33, The helpers
workflow_has_trigger and job_has_if_condition are unreliable because they rely
on yaml_get/yq exit codes which return 0 for null results; update both functions
to either call yq eval with the -e flag (so it errors on null) or explicitly
test for non-empty output from yq (e.g., capture yq eval ".on.${trigger_type}"
or ".jobs.${job_name}.if" and check [[ -n ... ]]) instead of redirecting to
/dev/null and relying on the exit status; ensure the changes are applied to the
workflow_has_trigger and job_has_if_condition functions so they correctly detect
missing paths.
| #!/usr/bin/env bash | ||
| # Manual workflow test runner | ||
|
|
||
| WORKFLOW_DIR=".github/workflows" | ||
| RELEASE_WORKFLOW="${WORKFLOW_DIR}/release.yml" | ||
| CI_WORKFLOW="${WORKFLOW_DIR}/ci.yml" |
There was a problem hiding this comment.
Missing set -euo pipefail for proper error detection.
Per coding guidelines, shell scripts should use set -euo pipefail at the beginning for proper error detection. This ensures the script fails fast on errors, unset variables, and pipeline failures.
🔧 Proposed fix
#!/usr/bin/env bash
# Manual workflow test runner
+set -euo pipefail
WORKFLOW_DIR=".github/workflows"
RELEASE_WORKFLOW="${WORKFLOW_DIR}/release.yml"
CI_WORKFLOW="${WORKFLOW_DIR}/ci.yml"📝 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.
| #!/usr/bin/env bash | |
| # Manual workflow test runner | |
| WORKFLOW_DIR=".github/workflows" | |
| RELEASE_WORKFLOW="${WORKFLOW_DIR}/release.yml" | |
| CI_WORKFLOW="${WORKFLOW_DIR}/ci.yml" | |
| #!/usr/bin/env bash | |
| # Manual workflow test runner | |
| set -euo pipefail | |
| WORKFLOW_DIR=".github/workflows" | |
| RELEASE_WORKFLOW="${WORKFLOW_DIR}/release.yml" | |
| CI_WORKFLOW="${WORKFLOW_DIR}/ci.yml" |
🤖 Prompt for AI Agents
In `@tests/run_workflow_tests.sh` around lines 1 - 6, Add strict error handling to
the script by enabling "set -euo pipefail" immediately after the shebang
(#!/usr/bin/env bash) so the script exits on errors, treats unset variables as
failures, and fails on pipeline errors; ensure this change applies to the top of
the file that defines WORKFLOW_DIR, RELEASE_WORKFLOW, and CI_WORKFLOW so the
rest of the script runs under strict mode.
- Add timeout-minutes: 15 to prevent hanging workflows - Use jq-action instead of apt-get for faster installation - Separate yq installation into dedicated step - Split npm dependencies installation Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 17-20: Replace the floating "latest" yq download with a pinned
release and verify its checksum: change the URL in the Install yq step to use
the v4.50.1 artifact (yq_linux_amd64) and add a checksum verification step that
downloads the release's checksums file and validates the binary SHA256 before
chmod +x; ensure the workflow fails if the checksum does not match so the
installation step (Install yq) only proceeds with a verified binary.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
14-16: Consider pinning third-party action to commit SHA.Using tag-based version references (
@v2) for third-party actions is convenient but less secure than pinning to a full commit SHA. Tags can be moved or deleted by the repository owner.🔒 Proposed fix to pin action to SHA
- name: Install jq - uses: sergeysova/jq-action@v2 + uses: sergeysova/jq-action@a3f0d4ff59cc1dddf023fc0b325dd75b10deec58 # v2
| - name: Install yq | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y jq python3 | ||
| wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/local/bin/yq | ||
| chmod +x /usr/local/bin/yq |
There was a problem hiding this comment.
❓ Verification inconclusive
mikefarah yq latest stable release version GitHub checksum
- Latest stable release (GitHub):
v4.50.1(released 2025-12-14) [1] - GitHub “checksums” asset SHA-256:
352de970780b7015d8271b4c2c998b898c9cc980ea9240262288d50cd2299767[1] - Where the per-binary checksums are: in the release asset named
checksumsforv4.50.1(it lists SHA-256s for each downloadable artifact, e.g.,yq_linux_amd64,yq_darwin_arm64, etc.) [1]
Sources: [1]
Pin yq version and verify checksum for reproducibility and security.
Using /latest/ in the download URL makes builds non-reproducible and vulnerable to unexpected breakages if yq releases a breaking change. Additionally, there's no integrity check on the downloaded binary, which is a supply chain security concern.
The approach of pinning a specific version and verifying the checksum is correct. Update the version to the latest stable (v4.50.1) and obtain the correct checksum for the yq_linux_amd64 binary from the official release checksums asset.
🔒 Proposed fix with version pinning and checksum verification
- name: Install yq
run: |
- wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/local/bin/yq
+ YQ_VERSION="v4.50.1"
+ YQ_CHECKSUM="<checksum-from-official-release>"
+ wget -q "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O /usr/local/bin/yq
+ echo "${YQ_CHECKSUM} /usr/local/bin/yq" | sha256sum -c -
chmod +x /usr/local/bin/yqReplace <checksum-from-official-release> with the SHA256 value from the release's checksums asset.
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 17 - 20, Replace the floating "latest"
yq download with a pinned release and verify its checksum: change the URL in the
Install yq step to use the v4.50.1 artifact (yq_linux_amd64) and add a checksum
verification step that downloads the release's checksums file and validates the
binary SHA256 before chmod +x; ensure the workflow fails if the checksum does
not match so the installation step (Install yq) only proceeds with a verified
binary.
Root cause: The setup() function in github_workflows.bats was calling itself (setup -> setup), causing an infinite loop in non-TTY environments like GitHub Actions CI. Solution: - Remove the duplicate setup() function entirely - Create ensure_yq() helper function for yq dependency check - Call ensure_yq() only in tests that require yq (8 tests) This prevents the recursive setup() call while maintaining the same test functionality. Tests that don't use yq (grep-based tests) will run without the dependency check. Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
This PR fixes a critical bug in the release workflow that caused infinite loops when semantic-release created commits.
Problem
The release workflow was triggering itself whenever semantic-release (running as
github-actions[bot]) created release commits. This created an infinite loop:Solution
Added
if: github.actor != 'github-actions[bot]'condition to the release job. This prevents the workflow from running when triggered by the bot's own commits.Changes
Modified
.github/workflows/release.yml:if: github.actor != 'github-actions[bot]'to the release jobAdded
tests/github_workflows.bats:Added
tests/run_workflow_tests.sh:Test plan
github-actions[bot]run_workflow_tests.shscript🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.