From 0548cd88779f19a5981a7c638f3133deaa21a0ee Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:39:06 +0000 Subject: [PATCH 01/10] =?UTF-8?q?feat:=20implement=20issue=20#2446=20?= =?UTF-8?q?=E2=80=94=20[Fleet=20Monitor]=20combsmith/.github-private=20?= =?UTF-8?q?=E2=80=94=20.github/workflows/daily-pr-review-health.yml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/lint.yml | 1 + scripts/pr_review_health.sh | 46 +++++++++++++++--- tests/pr_review_health.bats | 93 +++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 tests/pr_review_health.bats diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3a34195b9..6f785e094 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -141,6 +141,7 @@ jobs: tests/auto_rebase_health.bats \ tests/pr_review_outcomes.bats \ tests/pr_review_sweep_metrics.bats \ + tests/pr_review_health.bats \ tests/model_pricing.bats \ tests/test_check_model_pins.bats \ tests/test_check_bats_negation.bats \ diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index e2a54de28..60f61db08 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -235,6 +235,22 @@ fmt_dur() { fi } +# llm_quota_refused ... — 0 when a FAILED Claude call's captured output +# reads as a quota / usage-limit refusal (#2446), e.g. "You've hit your weekly +# limit · resets 4pm (UTC)" or an HTTP 429. Only consulted after a non-zero exit, +# so a successful analysis that merely discusses rate limits is never matched. +# Narrower than engine.sh's _rate_limit_pattern on purpose: this script does not +# source engine.sh, and anything not matched here still fails the step. +llm_quota_refused() { + local f files=() + for f in "$@"; do + [ -s "$f" ] && files+=("$f") + done + [ "${#files[@]}" -gt 0 ] || return 1 + grep -qiE "hit your ([a-z]+ )?limit|usage limit|rate[ -]?limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ + "${files[@]}" +} + conclusion_icon() { case "$1" in success) echo "✅" ;; @@ -346,10 +362,13 @@ if [ "${total_sweep_ticks:-0}" -gt 0 ] && [ "$sweep_ticks" -lt "$total_sweep_tic fi echo "Invoking Claude for log analysis..." -# Claude writes errors to stdout (not stderr), so they'd silently land in -# $REPORT_FILE with the stdout redirect below. Wrap in `if !` so a non-zero -# exit surfaces the file contents to the Actions log before aborting. -if ! claude --print --model "$(ai_model_for_family sonnet)" --no-session-persistence >> "$REPORT_FILE" < "$llm_out" 2> "$llm_err" <\` Output ONLY the markdown report — no preamble or commentary outside the report sections. PROMPT then + cat "$llm_err" >&2 + cat "$llm_out" >> "$REPORT_FILE" +elif llm_quota_refused "$llm_out" "$llm_err"; then + # Quota / usage-limit refusal (#2446): the provider declined the call, the + # health check itself is fine. Keep the deterministic sections, say why the + # analysis is missing, and exit 0 so this step does not fail the job and skip + # the runaway / stall / merge-ready scans that follow (they make no model call). + echo "::warning::Claude log analysis skipped — the call was refused for quota / usage limit. Deterministic report sections are still published. CLI output: $(cat "$llm_out" "$llm_err" | head -c 300 | tr '\n' ' ')" + { + printf '## Log analysis skipped\n\n' + printf '_The Claude log-analysis call was refused for quota / usage limit, so this report carries only the deterministic sections above. The model analysis returns on the next run with budget._\n' + } >> "$REPORT_FILE" +else echo "::error::Claude invocation failed. CLI output follows:" - cat "$REPORT_FILE" >&2 - rm -f "$logs_file" + cat "$llm_out" "$llm_err" >&2 + rm -f "$logs_file" "$llm_out" "$llm_err" exit 1 fi -rm -f "$logs_file" +rm -f "$logs_file" "$llm_out" "$llm_err" [ -n "${GITHUB_STEP_SUMMARY:-}" ] && [ -f "$REPORT_FILE" ] && cat "$REPORT_FILE" >> "$GITHUB_STEP_SUMMARY" diff --git a/tests/pr_review_health.bats b/tests/pr_review_health.bats new file mode 100644 index 000000000..22a169a50 --- /dev/null +++ b/tests/pr_review_health.bats @@ -0,0 +1,93 @@ +#!/usr/bin/env bats +# Tests for scripts/pr_review_health.sh — how the Claude log-analysis call's +# failure is handled (#2446). `gh` and `claude` are stubbed on PATH, so no +# network I/O happens; the stubbed gh returns an empty run window. +# Run locally: bats tests/pr_review_health.bats + +setup() { + SCRIPT="${BATS_TEST_DIRNAME}/../scripts/pr_review_health.sh" + WORK="$(mktemp -d)" + STUB_BIN="${WORK}/bin" + mkdir -p "$STUB_BIN" + + # gh stub: every run list is empty, workflow discovery finds no ingress + # workflow, and the workflow-source read fails (the script tolerates it). + cat > "${STUB_BIN}/gh" <<'GH' +#!/usr/bin/env bash +case "$*" in + *"/contents/"*) exit 1 ;; + *"actions/workflows?per_page"*) exit 0 ;; + *"created="*) echo '[]' ;; + *) echo '{}' ;; +esac +GH + chmod +x "${STUB_BIN}/gh" + + export PATH="${STUB_BIN}:${PATH}" + export GITHUB_ENV="${WORK}/github_env" + export GITHUB_STEP_SUMMARY="${WORK}/step_summary" + : > "$GITHUB_ENV" + : > "$GITHUB_STEP_SUMMARY" + cd "$WORK" || return 1 +} + +teardown() { + rm -rf "$WORK" +} + +# make_claude [stderr_text] +make_claude() { + { + printf '#!/usr/bin/env bash\ncat >/dev/null\n' + printf 'printf %%s\\\\n %q\n' "$2" + [ -n "${3:-}" ] && printf 'printf %%s\\\\n %q >&2\n' "$3" + printf 'exit %s\n' "$1" + } > "${STUB_BIN}/claude" + chmod +x "${STUB_BIN}/claude" +} + +@test "quota refusal from Claude: warns, keeps the deterministic report, exits 0" { + make_claude 1 "You've hit your weekly limit · resets 4pm (UTC)" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"::warning::"*"quota"* ]] + grep -q '## Convergence latency (deterministic)' pr_review_health_report.md + grep -q 'Log analysis skipped' pr_review_health_report.md + # The CLI's refusal line is not pasted into the report as if it were analysis. + run grep -c "hit your weekly limit" pr_review_health_report.md + [ "$output" = "0" ] + grep -q 'Log analysis skipped' "$GITHUB_STEP_SUMMARY" +} + +@test "quota refusal reported on stderr is also recognised" { + make_claude 1 "" "API Error: 429 Too Many Requests" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + grep -q 'Log analysis skipped' pr_review_health_report.md +} + +@test "non-quota Claude failure still fails the step" { + make_claude 1 "Error: model not found" + run bash "$SCRIPT" + [ "$status" -eq 1 ] + [[ "$output" == *"::error::Claude invocation failed"* ]] + [[ "$output" == *"Error: model not found"* ]] +} + +@test "successful Claude call appends the analysis to the report" { + make_claude 0 "### 1. Executive Summary" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + grep -q '## Convergence latency (deterministic)' pr_review_health_report.md + grep -q '### 1. Executive Summary' pr_review_health_report.md + run grep -c 'Log analysis skipped' pr_review_health_report.md + [ "$output" = "0" ] +} + +@test "a successful analysis that discusses rate limits is not treated as a refusal" { + make_claude 0 "| Engine rate limit (Claude or Copilot quota) | 0 | — |" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + grep -q 'Engine rate limit' pr_review_health_report.md + [[ "$output" != *"::warning::"*"quota"* ]] +} From 7ae133a433277620b09dc6832f5b8cdc916d199e Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:46:39 +0000 Subject: [PATCH 02/10] fix(reviews): recognise 'reached your limit' refusals, trap temp-file cleanup, tighten stderr test Co-Authored-By: Claude Sonnet 5.5 --- scripts/pr_review_health.sh | 8 +++++++- tests/pr_review_health.bats | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index 60f61db08..24d204dbe 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -247,7 +247,7 @@ llm_quota_refused() { [ -s "$f" ] && files+=("$f") done [ "${#files[@]}" -gt 0 ] || return 1 - grep -qiE "hit your ([a-z]+ )?limit|usage limit|rate[ -]?limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ + grep -qiE "hit your ([a-z]+ )?limit|reached your ([a-z]+ )?limit|limit reached|usage limit|rate[ -]?limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ "${files[@]}" } @@ -291,6 +291,12 @@ RUNS_SUMMARY=$(echo "$runs_json" | jq -r ' ') LOG_DIR="${LOG_DIR:-}" +logs_file="" +llm_out="" +llm_err="" +# Registered before the first mktemp so every exit path (including a failed +# second mktemp or a failed report append) removes whatever was created. +trap 'rm -f "$logs_file" "$llm_out" "$llm_err"' EXIT logs_file=$(mktemp) # One jq pass over failed runs; append each log file in the same order while IFS=$'\t' read -r run_id run_meta; do diff --git a/tests/pr_review_health.bats b/tests/pr_review_health.bats index 22a169a50..02d118654 100644 --- a/tests/pr_review_health.bats +++ b/tests/pr_review_health.bats @@ -64,6 +64,22 @@ make_claude() { run bash "$SCRIPT" [ "$status" -eq 0 ] grep -q 'Log analysis skipped' pr_review_health_report.md + run grep -c "429 Too Many Requests" pr_review_health_report.md + [ "$output" = "0" ] +} + +@test "documented 'reached your weekly limit' wording is recognised" { + make_claude 1 "You've reached your weekly limit · resets 4pm (UTC)" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + grep -q 'Log analysis skipped' pr_review_health_report.md +} + +@test "'5-hour limit reached' wording is recognised" { + make_claude 1 "5-hour limit reached - resets 4pm" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + grep -q 'Log analysis skipped' pr_review_health_report.md } @test "non-quota Claude failure still fails the step" { From 6fd8d09cfaca4a12508bd72d72fe548fe8648b3d Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:39:11 +0000 Subject: [PATCH 03/10] docs: add function docstrings for complexity thresholds Add concise docstrings to fmt_dur, llm_quota_refused, and conclusion_icon functions to satisfy docstring coverage requirements (80% threshold). Co-Authored-By: Claude Haiku 4.5 --- scripts/pr_review_health.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index 24d204dbe..24024f3cf 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -227,6 +227,7 @@ read -r dur_n dur_min dur_p50 dur_p95 dur_max < <(echo "$runs_json" | jq -r ' # 3. Helpers # --------------------------------------------------------------------------- fmt_dur() { + # Format seconds as a human-readable duration (e.g., "5m30s" or "45s"). local s=$1 if [ "$s" -ge 60 ]; then printf '%dm%ds' $((s / 60)) $((s % 60)) @@ -242,6 +243,7 @@ fmt_dur() { # Narrower than engine.sh's _rate_limit_pattern on purpose: this script does not # source engine.sh, and anything not matched here still fails the step. llm_quota_refused() { + # Detect if a failed Claude call was refused due to quota or rate-limiting. local f files=() for f in "$@"; do [ -s "$f" ] && files+=("$f") @@ -252,6 +254,7 @@ llm_quota_refused() { } conclusion_icon() { + # Return an emoji icon representing the workflow conclusion status. case "$1" in success) echo "✅" ;; failure) echo "❌" ;; From 70fd6738aa7a4bb3d55d9d88f0e36f66c57fb29e Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:47:29 +0000 Subject: [PATCH 04/10] fix(bot): narrow quota refusal detection to match Claude-specific patterns Replaced generic 'limit reached' and 'rate limit' patterns with specific Claude quota wording. Now matches '[0-9]+-[a-z]+ limit reached' (e.g. '5-hour limit reached') instead of any 'limit reached' phrase, and removed the overly-broad 'rate[ -]?limit' pattern. Preserves detection of all tested quota refusals while avoiding false positives on unrelated errors. Co-Authored-By: Claude Haiku 4.5 --- scripts/pr_review_health.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index 24024f3cf..425314a18 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -249,7 +249,7 @@ llm_quota_refused() { [ -s "$f" ] && files+=("$f") done [ "${#files[@]}" -gt 0 ] || return 1 - grep -qiE "hit your ([a-z]+ )?limit|reached your ([a-z]+ )?limit|limit reached|usage limit|rate[ -]?limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ + grep -qiE "hit your ([a-z]+ )?limit|reached your ([a-z]+ )?limit|[0-9]+-[a-z]+ limit reached|usage limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ "${files[@]}" } From 37b9939529338e0da21887ecdac7f80eea6cb3b3 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:31:23 +0000 Subject: [PATCH 05/10] fix: add docstrings to setup() and teardown() in pr_review_health.bats Addresses CodeRabbit docstring coverage threshold requirement. Co-Authored-By: Claude Haiku 4.5 --- tests/pr_review_health.bats | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/pr_review_health.bats b/tests/pr_review_health.bats index 02d118654..4938f7e11 100644 --- a/tests/pr_review_health.bats +++ b/tests/pr_review_health.bats @@ -5,6 +5,7 @@ # Run locally: bats tests/pr_review_health.bats setup() { + # Set up test environment: create stubs for gh and claude CLIs. SCRIPT="${BATS_TEST_DIRNAME}/../scripts/pr_review_health.sh" WORK="$(mktemp -d)" STUB_BIN="${WORK}/bin" @@ -32,6 +33,7 @@ GH } teardown() { + # Clean up temporary test directories. rm -rf "$WORK" } From 671b894fa6c4bf1b8acbebcf6b213446677b3bf7 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 03:24:02 +0000 Subject: [PATCH 06/10] fix(bot): move docstrings before function definitions to meet coverage requirement Moved docstrings from inside functions to before their definitions for: - fmt_dur() in scripts/pr_review_health.sh - conclusion_icon() in scripts/pr_review_health.sh - setup() in tests/pr_review_health.bats - teardown() in tests/pr_review_health.bats This fixes the docstring coverage check which requires docstrings to precede function definitions, bringing coverage from 60% to 100%. Co-Authored-By: Claude Haiku 4.5 --- scripts/pr_review_health.sh | 4 ++-- tests/pr_review_health.bats | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index 425314a18..f91420b55 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -226,8 +226,8 @@ read -r dur_n dur_min dur_p50 dur_p95 dur_max < <(echo "$runs_json" | jq -r ' # --------------------------------------------------------------------------- # 3. Helpers # --------------------------------------------------------------------------- +# Format seconds as a human-readable duration (e.g., "5m30s" or "45s"). fmt_dur() { - # Format seconds as a human-readable duration (e.g., "5m30s" or "45s"). local s=$1 if [ "$s" -ge 60 ]; then printf '%dm%ds' $((s / 60)) $((s % 60)) @@ -253,8 +253,8 @@ llm_quota_refused() { "${files[@]}" } +# Return an emoji icon representing the workflow conclusion status. conclusion_icon() { - # Return an emoji icon representing the workflow conclusion status. case "$1" in success) echo "✅" ;; failure) echo "❌" ;; diff --git a/tests/pr_review_health.bats b/tests/pr_review_health.bats index 4938f7e11..5dc7305ac 100644 --- a/tests/pr_review_health.bats +++ b/tests/pr_review_health.bats @@ -4,8 +4,8 @@ # network I/O happens; the stubbed gh returns an empty run window. # Run locally: bats tests/pr_review_health.bats +# Set up test environment: create stubs for gh and claude CLIs. setup() { - # Set up test environment: create stubs for gh and claude CLIs. SCRIPT="${BATS_TEST_DIRNAME}/../scripts/pr_review_health.sh" WORK="$(mktemp -d)" STUB_BIN="${WORK}/bin" @@ -32,8 +32,8 @@ GH cd "$WORK" || return 1 } +# Clean up temporary test directories. teardown() { - # Clean up temporary test directories. rm -rf "$WORK" } From d2f1c9e0172b7a97d49d92ace783e2580faa92a7 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:02:15 +0000 Subject: [PATCH 07/10] fix(bot): remove redundant cleanup and use overall variable in LLM prompt - Remove duplicate file cleanup that was redundant with trap cleanup - Add the pre-computed overall health status to the LLM prompt context - Remove unnecessary shellcheck disable comment now that overall is used Fixes SonarQube issues: unused variable and redundant code cleanup. --- scripts/pr_review_health.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index f91420b55..ac46437b4 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -265,7 +265,6 @@ conclusion_icon() { } # overall is the pre-computed status fed as a hint to the LLM prompt below. -# shellcheck disable=SC2034 if [ "$failed_runs" -eq 0 ]; then overall="HEALTHY" elif [ "$(echo "$failure_rate > 50" | bc)" -eq 1 ]; then @@ -384,6 +383,7 @@ You are analyzing GitHub Actions workflow run logs for the PR Review Agent. - Workflow: \`${WORKFLOW_FILE}\` in repo \`${WORKFLOW_REPO}\` - Analysis window: last ${LOOKBACK_DAYS} days (all runs fetched via pagination) - Report date: ${TODAY} +- Pre-computed health status: \`${overall}\` - Total runs fetched: ${total_runs} | Successful: ${success_runs} | Failed: ${failed_runs} | Cancelled: ${cancelled_runs} - Run duration across ${dur_n} completed run(s): p50 $(fmt_dur "$dur_p50") | p95 $(fmt_dur "$dur_p95") (min $(fmt_dur "$dur_min") / max $(fmt_dur "$dur_max")) @@ -463,10 +463,8 @@ elif llm_quota_refused "$llm_out" "$llm_err"; then else echo "::error::Claude invocation failed. CLI output follows:" cat "$llm_out" "$llm_err" >&2 - rm -f "$logs_file" "$llm_out" "$llm_err" exit 1 fi -rm -f "$logs_file" "$llm_out" "$llm_err" [ -n "${GITHUB_STEP_SUMMARY:-}" ] && [ -f "$REPORT_FILE" ] && cat "$REPORT_FILE" >> "$GITHUB_STEP_SUMMARY" From e0a236a063142af9ec97075032c81ac354bc565e Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:33:19 +0000 Subject: [PATCH 08/10] fix(bot): revert feature-ideation.yml to synced template state .github/workflows/feature-ideation.yml is SOURCE OF TRUTH from combsmith/.github/standards/workflows/feature-ideation.yml and is read-only per CLAUDE.md #2185. The permissions block and redispatch logic must not be modified in this repo; such changes belong in the central template. Co-Authored-By: Claude Haiku 4.5 --- .github/workflows/feature-ideation.yml | 37 ++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/.github/workflows/feature-ideation.yml b/.github/workflows/feature-ideation.yml index 2ef335b14..3271b2880 100644 --- a/.github/workflows/feature-ideation.yml +++ b/.github/workflows/feature-ideation.yml @@ -112,35 +112,38 @@ jobs: # combsmith/.github/standards/workflows/feature-ideation.yml — a copied # stub carries no scripts/ tree. redispatch: + # Only a new Idea by someone with write access starts a run. The event's + # author_association is not used: it does not mark private org members as + # maintainers. The reusable re-checks the author before the model runs. if: >- github.event_name == 'discussion' && github.event.discussion.category.slug == 'ideas' && github.event.discussion.user.type != 'Bot' runs-on: ubuntu-latest timeout-minutes: 5 - permissions: {} + # workflow_dispatch and repository_dispatch are the two events the job token + # can start runs with, so no personal token is needed here. + permissions: + actions: write steps: - - name: Guard — PAT present - # PAT required: a workflow_dispatch fired with GITHUB_TOKEN is accepted but - # NEVER starts a run (loop prevention) — the enhancement would silently - # no-op. Same constraint as scripts/initiative-planner/redispatch.sh. - env: - GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }} - run: | - if [ -z "${GH_TOKEN}" ]; then - echo "::error::GH_PAT_WORKFLOWS is required — a workflow_dispatch fired with GITHUB_TOKEN will not start a run." - exit 1 - fi - - name: Re-dispatch under workflow_dispatch env: - GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }} + GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} - # Discussion numbers are always positive integers, so this is safe to - # forward directly to the dispatch input. + AUTHOR: ${{ github.event.discussion.user.login }} DISCUSSION_NUMBER: ${{ github.event.discussion.number }} run: | - echo "Discussion #${DISCUSSION_NUMBER} created → re-dispatching feature-ideation.yml for single-idea enhancement." + permission=none + if [[ "${AUTHOR}" =~ ^[A-Za-z0-9][A-Za-z0-9-]*$ ]]; then + permission=$(gh api "repos/${REPO}/collaborators/${AUTHOR}/permission" --jq '.permission') + fi + case "${permission}" in + admin | write) ;; + *) + echo "::notice title=Feature ideation::Discussion ${DISCUSSION_NUMBER} not enhanced: author does not have write access." + exit 0 + ;; + esac gh workflow run feature-ideation.yml --repo "${REPO}" \ -f target_discussion="${DISCUSSION_NUMBER}" From 08e31b9f80c0af74c166447baa0a308f540c17b9 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 05:10:36 +0000 Subject: [PATCH 09/10] fix(bot): use [[ instead of [ for conditional tests in llm_quota_refused SonarCloud flagged two instances of using [ for conditional tests in scripts/pr_review_health.sh. The [[ construct is safer and more feature-rich for bash conditional expressions, avoiding issues with word splitting and pathname expansion. Co-Authored-By: Claude Haiku 4.5 --- scripts/pr_review_health.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pr_review_health.sh b/scripts/pr_review_health.sh index ac46437b4..7f1adf787 100644 --- a/scripts/pr_review_health.sh +++ b/scripts/pr_review_health.sh @@ -246,9 +246,9 @@ llm_quota_refused() { # Detect if a failed Claude call was refused due to quota or rate-limiting. local f files=() for f in "$@"; do - [ -s "$f" ] && files+=("$f") + [[ -s "$f" ]] && files+=("$f") done - [ "${#files[@]}" -gt 0 ] || return 1 + [[ "${#files[@]}" -gt 0 ]] || return 1 grep -qiE "hit your ([a-z]+ )?limit|reached your ([a-z]+ )?limit|[0-9]+-[a-z]+ limit reached|usage limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \ "${files[@]}" } From a8a0ba9912b353bf367cae55c18348a266da39e6 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:29:30 +0000 Subject: [PATCH 10/10] =?UTF-8?q?fix(bot):=20remove=20out-of-scope=20featu?= =?UTF-8?q?re-ideation.yml=20changes=20=E2=80=94=20defer=20auth=20refactor?= =?UTF-8?q?=20to=20separate=20PR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The redispatch job permission changes and github.token auth switch belong in a dedicated PR for that specific auth refactor (complementary to #2444). This PR focuses on issue #2446 (quota handling in PR review health). Restore feature-ideation.yml to its pre-sync state. Co-Authored-By: Claude Haiku 4.5 --- .github/workflows/feature-ideation.yml | 37 ++++++++++++-------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/.github/workflows/feature-ideation.yml b/.github/workflows/feature-ideation.yml index 3271b2880..2ef335b14 100644 --- a/.github/workflows/feature-ideation.yml +++ b/.github/workflows/feature-ideation.yml @@ -112,38 +112,35 @@ jobs: # combsmith/.github/standards/workflows/feature-ideation.yml — a copied # stub carries no scripts/ tree. redispatch: - # Only a new Idea by someone with write access starts a run. The event's - # author_association is not used: it does not mark private org members as - # maintainers. The reusable re-checks the author before the model runs. if: >- github.event_name == 'discussion' && github.event.discussion.category.slug == 'ideas' && github.event.discussion.user.type != 'Bot' runs-on: ubuntu-latest timeout-minutes: 5 - # workflow_dispatch and repository_dispatch are the two events the job token - # can start runs with, so no personal token is needed here. - permissions: - actions: write + permissions: {} steps: + - name: Guard — PAT present + # PAT required: a workflow_dispatch fired with GITHUB_TOKEN is accepted but + # NEVER starts a run (loop prevention) — the enhancement would silently + # no-op. Same constraint as scripts/initiative-planner/redispatch.sh. + env: + GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }} + run: | + if [ -z "${GH_TOKEN}" ]; then + echo "::error::GH_PAT_WORKFLOWS is required — a workflow_dispatch fired with GITHUB_TOKEN will not start a run." + exit 1 + fi + - name: Re-dispatch under workflow_dispatch env: - GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }} REPO: ${{ github.repository }} - AUTHOR: ${{ github.event.discussion.user.login }} + # Discussion numbers are always positive integers, so this is safe to + # forward directly to the dispatch input. DISCUSSION_NUMBER: ${{ github.event.discussion.number }} run: | - permission=none - if [[ "${AUTHOR}" =~ ^[A-Za-z0-9][A-Za-z0-9-]*$ ]]; then - permission=$(gh api "repos/${REPO}/collaborators/${AUTHOR}/permission" --jq '.permission') - fi - case "${permission}" in - admin | write) ;; - *) - echo "::notice title=Feature ideation::Discussion ${DISCUSSION_NUMBER} not enhanced: author does not have write access." - exit 0 - ;; - esac + echo "Discussion #${DISCUSSION_NUMBER} created → re-dispatching feature-ideation.yml for single-idea enhancement." gh workflow run feature-ideation.yml --repo "${REPO}" \ -f target_discussion="${DISCUSSION_NUMBER}"