From f541bcb073bbaba8668c3ae10cb7b2b7e36ec441 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 17 Mar 2026 10:18:39 +0000 Subject: [PATCH] fix: address AI review findings for CI pipeline and duplicate detection - Fix anti-pattern regex 2 to match quoted form '"$CI_STATE" != "success"' (was r'\$CI_STATE\s*!=\s*"success"', now r'"?\$CI_STATE"?\s*!=\s*"success"') - Update both anti-pattern messages to say 'extract ci_passed() to lib/' instead of implying it already exists as a shared helper in dev-poll.sh - Add explicit 'when: event: [push, pull_request]' trigger block to ci.yml - Add '-r' to xargs in shellcheck step to handle zero .sh files gracefully - Fix operator precedence bug in review-poll.sh:62: scope the OR clause with braces so CI_STATE=pending bypass only applies when WOODPECKER_REPO_ID=0 Co-Authored-By: Claude Sonnet 4.6 --- .woodpecker/ci.yml | 5 ++++- .woodpecker/detect-duplicates.py | 6 +++--- review/review-poll.sh | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.woodpecker/ci.yml b/.woodpecker/ci.yml index 7592551..85310bd 100644 --- a/.woodpecker/ci.yml +++ b/.woodpecker/ci.yml @@ -5,11 +5,14 @@ # 1. shellcheck — lint all .sh files (warnings+errors) # 2. duplicate-detection — report copy-pasted code blocks (non-blocking) +when: + event: [push, pull_request] + steps: - name: shellcheck image: koalaman/shellcheck-alpine:stable commands: - - find . -name "*.sh" -not -path "./.git/*" -print0 | xargs -0 shellcheck --severity=warning + - find . -name "*.sh" -not -path "./.git/*" -print0 | xargs -0 -r shellcheck --severity=warning - name: duplicate-detection image: python:3-alpine diff --git a/.woodpecker/detect-duplicates.py b/.woodpecker/detect-duplicates.py index 4124a40..caf6012 100644 --- a/.woodpecker/detect-duplicates.py +++ b/.woodpecker/detect-duplicates.py @@ -27,11 +27,11 @@ MIN_FILES = int(os.environ.get("DUP_MIN_FILES", "2")) ANTI_PATTERNS = [ ( r'"\$CI_STATE"\s*=\s*"success"', - 'Hardcoded CI_STATE="success" check — use ci_passed() from dev-poll.sh instead', + 'Hardcoded CI_STATE="success" check — extract ci_passed() to lib/ and call it here', ), ( - r'\$CI_STATE\s*!=\s*"success"', - 'Hardcoded CI_STATE!="success" check — use ci_passed() from dev-poll.sh instead', + r'"?\$CI_STATE"?\s*!=\s*"success"', + 'Hardcoded CI_STATE!="success" check — extract ci_passed() to lib/ and call it here', ), ( r'WOODPECKER_REPO_ID\s*=\s*[1-9][0-9]*', diff --git a/review/review-poll.sh b/review/review-poll.sh index a586b30..c24eb4b 100755 --- a/review/review-poll.sh +++ b/review/review-poll.sh @@ -59,7 +59,7 @@ while IFS= read -r line; do # Skip if CI is running/failed. Allow "success" or no CI configured (empty/pending with no pipelines) if [ "$CI_STATE" != "success" ]; then # Projects without CI (woodpecker_repo_id=0) treat empty/pending as pass - if [ "${WOODPECKER_REPO_ID:-2}" = "0" ] && [ "$CI_STATE" = "" ] || [ "$CI_STATE" = "pending" ]; then + if [ "${WOODPECKER_REPO_ID:-2}" = "0" ] && { [ "$CI_STATE" = "" ] || [ "$CI_STATE" = "pending" ]; }; then : # no CI configured, proceed to review else log " #${PR_NUM} CI=${CI_STATE}, skip"