feat: implement issue #425 — dev-lead: Incomplete issue resolution - missing code reviews and CI failures

.github/workflows/force-deploy-pr-review.yml

@@ -13,24 +13,24 @@ jobs:
       matrix:
         repo: [bmad-bgreat-suite, google-app-scripts]
       max-parallel: 1
-    
+
     steps:
       - uses: actions/checkout@v4
-      
+
       - name: Deploy to ${{ matrix.repo }}
         run: |
           mkdir -p temp_repo
           cd temp_repo
-          
+
           git clone https://${{ secrets.GITHUB_TOKEN }}@github.com/petry-projects/${{ matrix.repo }}.git
           cd ${{ matrix.repo }}
-          
+
           git config user.name "github-actions[bot]"
           git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
-          
+
           mkdir -p .github/workflows
           cp ../../.github/workflows/pr-review.yml .github/workflows/
-          
+
           git add .github/workflows/pr-review.yml
           if git commit -m "deploy: add pr-review.yml workflow"; then
             git push origin main && echo "✅ Deployed" || echo "⚠️ Push failed"

.github/workflows/pr-review-reusable.yml

@@ -9,6 +9,10 @@ on:
         required: false
       COPILOT_GITHUB_TOKEN:
         required: false
+      DON_PETRY_BOT_GH_PAT:
+        required: false
+      GH_PAT:
+        required: false
 
 permissions: {}
 
@@ -19,3 +23,5 @@ jobs:
       CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
       GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }}
       COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}
+      DON_PETRY_BOT_GH_PAT: ${{ secrets.DON_PETRY_BOT_GH_PAT }}
+      GH_PAT: ${{ secrets.GH_PAT }}

.github/workflows/pr-review.yml

@@ -9,6 +9,10 @@ on:
         required: false
       COPILOT_GITHUB_TOKEN:
         required: false
+      DON_PETRY_BOT_GH_PAT:
+        required: false
+      GH_PAT:
+        required: false
 
   # ── Event-driven triggers ────────────────────────────────────────────────
   # These replace the former time-based schedule. The review engine's own

prompts/dev-lead/fix-bot-comment.md

@@ -1,4 +1,4 @@
-<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, ACTOR, COMMENT_BODY, HEAD_SHA -->
+<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, ACTOR, COMMENT_BODY, HEAD_SHA, CI_STATUS_JSON, ALL_REVIEWS_JSON -->
 # Dev-Lead Agent: Fix Bot Comment Issues
 You are the dev-lead agent for the `${REPO}` repository. Your task is to address issues raised by an automated code analysis bot on a pull request.
 
@@ -15,6 +15,24 @@ You are the dev-lead agent for the `${REPO}` repository. Your task is to address
 ${COMMENT_BODY}
 ```
 
+## PR State (Holistic Assessment)
+
+Before acting on the comment above, review the full PR state so you never declare "no-changes" while the PR is blocked.
+
+**CI check results:**
+
+```json
+${CI_STATUS_JSON}
+```
+
+**All review states:**
+
+```json
+${ALL_REVIEWS_JSON}
+```
+
+Treat any check with `conclusion` = `"failure"`, `"timed_out"`, `"cancelled"`, `"action_required"`, `"stale"`, or `"startup_failure"` and any review with `state` = `"CHANGES_REQUESTED"` as **Tier 1 blockers** — address them in addition to the bot comment below. Only declare "no-changes" when zero Tier 1 blockers exist.
+
 ## Task
 
 Analyze the bot's findings and address each actionable issue:

prompts/dev-lead/fix-reviews.md

@@ -1,4 +1,4 @@
-<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, OPEN_THREADS_JSON, BASE_REF, TRIGGERING_REVIEWER -->
+<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, OPEN_THREADS_JSON, BASE_REF, TRIGGERING_REVIEWER, CI_STATUS_JSON, ALL_REVIEWS_JSON -->
 # Dev-Lead Agent: Fix Review Comments
 
 You are the dev-lead agent for the `${REPO}` repository. Your task is to address open review threads on a pull request.
@@ -22,6 +22,26 @@ ${OPEN_THREADS_JSON}
 
 Work through each phase in order.
 
+### Phase 0 — Holistic Assessment (do this first)
+
+Before addressing individual threads, assess the full PR state so you never declare "no-changes" while the PR is still blocked.
+
+**CI check results:**
+
+```json
+${CI_STATUS_JSON}
+```
+
+Identify any checks with `conclusion` = `"failure"`, `"timed_out"`, `"cancelled"`, `"action_required"`, `"stale"`, or `"startup_failure"`. These are **Tier 1 blockers** — fixing them is explicitly in-scope even if no review thread specifically asks for it.
+
+**All review states:**
+
+```json
+${ALL_REVIEWS_JSON}
+```
+
+Identify any entries with `state` = `"CHANGES_REQUESTED"`. Each one is a **Tier 1 blocker**. Only declare "no-changes" when zero Tier 1 blockers exist (all CI checks pass AND no reviewer has CHANGES_REQUESTED).
+
 ### Phase 1 — Address Threads
 
 For each open review thread:
@@ -67,7 +87,7 @@ Read every changed line as if you are the reviewer seeing the response:
 - Resolve every thread you fix; resolve outdated threads without a corresponding code change
 - Only resolve threads whose `author.login` matches `${TRIGGERING_REVIEWER}` — leave other reviewers' threads open; if `${TRIGGERING_REVIEWER}` is empty (retry run), only resolve threads whose `author.login` ends with `[bot]` — leave human reviewers' threads open
 - Do not resolve threads you are skipping due to ambiguity — leave those open and note them in your output
-- Do not make changes beyond what the review threads request
+- Do not make changes beyond what the review threads request, except that fixing Tier-1 blockers (failure/timed_out/cancelled/action_required/stale/startup_failure CI checks and CHANGES_REQUESTED reviews) is always in-scope
 - If a review thread is ambiguous, apply the most conservative interpretation
 - Do not commit or push — the CI workflow handles git operations after you finish
 

prompts/dev-lead/review-changes.md

@@ -1,4 +1,4 @@
-<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, PR_TITLE, PR_DESCRIPTION, OPEN_THREADS_JSON -->
+<!-- VARIABLES: PR_NUMBER, PR_URL, REPO, PR_TITLE, PR_DESCRIPTION, OPEN_THREADS_JSON, CI_STATUS_JSON, ALL_REVIEWS_JSON -->
 # Dev-Lead Agent: Human Pull Request Review Response
 
 You are the dev-lead agent for the `${REPO}` repository. A human reviewer has submitted a pull request review requesting changes. Your task is to address all open review threads.
@@ -27,6 +27,26 @@ ${OPEN_THREADS_JSON}
 
 Work through each phase in order. Human reviewer feedback is high-priority — implement exactly what is asked.
 
+### Phase 0 — Holistic Assessment (do this first)
+
+Before addressing review threads, assess the full PR state so you never declare "no-changes" while the PR is still blocked.
+
+**CI check results:**
+
+```json
+${CI_STATUS_JSON}
+```
+
+Identify any checks with `conclusion` = `"failure"`, `"timed_out"`, `"cancelled"`, `"action_required"`, `"stale"`, or `"startup_failure"`. These are **Tier 1 blockers** — fix them before anything else.
+
+**All review states:**
+
+```json
+${ALL_REVIEWS_JSON}
+```
+
+Identify any entries with `state` = `"CHANGES_REQUESTED"`. Each is a **Tier 1 blocker**. Only declare "no-changes" when zero Tier 1 blockers exist (all CI checks pass AND no reviewer has CHANGES_REQUESTED).
+
 ### Phase 1 — Address Threads
 
 For each open review thread:

scripts/dev-lead-fix-reviews.sh

@@ -259,6 +259,64 @@ resolve_actor_outdated_threads() {
   echo "::notice::resolve_actor_outdated_threads: resolved ${resolved_count} outdated thread(s) on PR #${PR_NUMBER}"
 }
 
+# fetch_pr_context: exports CI_STATUS_JSON and ALL_REVIEWS_JSON for holistic assessment.
+# Called before engine invocation in fix-reviews, fix-bot-comment, and review-changes
+# so the agent can identify Tier-1 blockers (failing CI + CHANGES_REQUESTED reviews)
+# and never wrongly declare "no-changes" while the PR is still blocked.
+fetch_pr_context() {
+  # CI check results: requires HEAD_SHA. Gracefully degrade to empty array when not set
+  # (e.g., review-changes in dry-run where the PR API call is skipped).
+  CI_STATUS_JSON="[]"
+  if [ -n "${HEAD_SHA:-}" ]; then
+    CI_STATUS_JSON=$(gh api --paginate "repos/${REPO}/commits/${HEAD_SHA}/check-runs?per_page=100" \
+      2>/dev/null \
+      | jq -s '[.[].check_runs[] | {name:.name, status:.status, conclusion:.conclusion, details_url:.details_url}]' \
+      2>/dev/null || echo "[]")
+    # Also include legacy commit statuses (Jenkins, external CI, etc.) that use
+    # the separate statuses API rather than check-runs. Merge into CI_STATUS_JSON
+    # so the agent sees a unified picture of all required status checks.
+    local statuses_json
+    # The statuses API returns the full history per context (newest first).
+    # Dedupe by context so a stale failure overwritten by a later success does not
+    # appear as a Tier-1 blocker: group_by preserves input order within each group,
+    # so `first` picks the newest entry for each context.
+    statuses_json=$(gh api --paginate "repos/${REPO}/commits/${HEAD_SHA}/statuses?per_page=100" \
+      2>/dev/null \
+      | jq -s '[ [.[].[] | select(.context != null)] | group_by(.context)[] | first | {name:.context, status:(if .state == "pending" then "in_progress" else "completed" end), conclusion:(if .state == "success" then "success" elif .state == "failure" or .state == "error" then "failure" else null end), details_url:.target_url} ]' \
+      2>/dev/null || echo "[]")
+    CI_STATUS_JSON=$(printf '%s\n%s' "$CI_STATUS_JSON" "$statuses_json" \
+      | jq -s 'add // []' 2>/dev/null || echo "$CI_STATUS_JSON")
+  fi
+  export CI_STATUS_JSON
+
+  # All PR reviews with state — deduplicated per reviewer (latest review per user only).
+  # Uses --paginate so PRs with more than 100 reviews are fully covered.
+  ALL_REVIEWS_JSON=$(gh api --paginate "repos/${REPO}/pulls/${PR_NUMBER}/reviews?per_page=100" \
+    2>/dev/null \
+    | jq -s '[ [.[].[] | select(.user != null)] | group_by(.user.login)[] | sort_by(.id) | last | {id:.id, user:.user.login, state:.state, submitted_at:.submitted_at} ]' \
+    2>/dev/null || echo "[]")
+  export ALL_REVIEWS_JSON
+}
+
+# has_tier1_blockers: returns 0 (true) if CI_STATUS_JSON or ALL_REVIEWS_JSON contain
+# Tier-1 blockers: CI checks with a non-success conclusion (failure, timed_out,
+# cancelled, action_required, stale, startup_failure) or CHANGES_REQUESTED reviews.
+# Used to gate post_no_changes — never post a terminal no-changes marker while blockers
+# exist, so the retry cron can re-attempt on the same SHA.
+has_tier1_blockers() {
+  local failing_checks changes_requested
+  failing_checks=$(printf '%s' "${CI_STATUS_JSON:-[]}" | \
+    jq '[.[] | select(.conclusion != null and (
+          .conclusion == "failure" or .conclusion == "timed_out" or
+          .conclusion == "cancelled" or .conclusion == "action_required" or
+          .conclusion == "stale" or .conclusion == "startup_failure"
+        ))] | length' 2>/dev/null || echo "0")
+  changes_requested=$(printf '%s' "${ALL_REVIEWS_JSON:-[]}" | \
+    jq '[.[] | select(.state == "CHANGES_REQUESTED")] | length' \
+    2>/dev/null || echo "0")
+  [ "${failing_checks:-0}" -gt 0 ] || [ "${changes_requested:-0}" -gt 0 ]
+}
+
 # try_enable_auto_merge: enables auto-merge (squash) on the PR if reviewDecision is
 # APPROVED and auto-merge is not already set. Safe to call speculatively — checks
 # eligibility first and is idempotent if auto-merge is already on.
@@ -525,6 +583,7 @@ case "$INTENT_TYPE" in
       -F owner="${REPO%%/*}" -F repo="${REPO##*/}" -F pr="$PR_NUMBER" \
       --jq '.data.repository.pullRequest.reviewThreads.nodes | map(select(.isResolved == false))' 2>/dev/null || echo "[]")
     export OPEN_THREADS_JSON
+    fetch_pr_context
     rc=0
     build_and_run "fix-reviews" || rc=$?
     [ "$rc" -eq 2 ] && handle_rate_limit "fix-reviews"
@@ -535,7 +594,11 @@ case "$INTENT_TYPE" in
       else
         notify_coderabbit_resolve
         resolve_actor_outdated_threads "fix-reviews"
-        post_no_changes "fix-reviews"
+        if has_tier1_blockers; then
+          echo "::warning::Tier-1 blockers still present (failing CI or CHANGES_REQUESTED reviews) — skipping no-changes marker to allow retries"
+        else
+          post_no_changes "fix-reviews"
+        fi
       fi
       try_enable_auto_merge
     fi
@@ -544,6 +607,7 @@ case "$INTENT_TYPE" in
   fix-bot-comment)
     export PR_NUMBER PR_URL="https://github.com/${REPO}/pull/${PR_NUMBER}"
     export REPO ACTOR="${ACTOR:-}" COMMENT_BODY="${COMMENT_BODY:-}" HEAD_SHA
+    fetch_pr_context
     rc=0
     build_and_run "fix-bot-comment" || rc=$?
     [ "$rc" -eq 2 ] && handle_rate_limit "fix-bot-comment"
@@ -554,7 +618,11 @@ case "$INTENT_TYPE" in
       else
         notify_coderabbit_resolve
         resolve_actor_outdated_threads "fix-bot-comment"
-        post_no_changes "fix-bot-comment"
+        if has_tier1_blockers; then
+          echo "::warning::Tier-1 blockers still present (failing CI or CHANGES_REQUESTED reviews) — skipping no-changes marker to allow retries"
+        else
+          post_no_changes "fix-bot-comment"
+        fi
       fi
       try_enable_auto_merge
     fi
@@ -596,6 +664,7 @@ case "$INTENT_TYPE" in
       -F owner="${REPO%%/*}" -F repo="${REPO##*/}" -F pr="$PR_NUMBER" \
       --jq '.data.repository.pullRequest.reviewThreads.nodes | map(select(.isResolved == false))' 2>/dev/null || echo "[]")
     export OPEN_THREADS_JSON BASE_REF="${BASE_REF:-main}"
+    fetch_pr_context
     rc=0
     build_and_run "review-changes" || rc=$?
     [ "$rc" -eq 2 ] && handle_rate_limit "review-changes"
@@ -606,7 +675,11 @@ case "$INTENT_TYPE" in
       else
         notify_coderabbit_resolve
         resolve_actor_outdated_threads "review-changes"
-        post_reviews_terminal "review-changes" "no-changes" "No changes were needed for this PR."
+        if has_tier1_blockers; then
+          echo "::warning::Tier-1 blockers still present (failing CI or CHANGES_REQUESTED reviews) — skipping no-changes marker to allow retries"
+        else
+          post_reviews_terminal "review-changes" "no-changes" "No changes were needed for this PR."
+        fi
       fi
       try_enable_auto_merge
     fi