feat: implement issue #947 — P1 (#860 follow-up): don't re-invoke rate-limited advisory engines — one exhaustion notice per engine per run

scripts/engine.sh

@@ -1255,12 +1255,49 @@ run_writer() {
   return "$rc"
 }
 
+# ── Run-scoped engine-exhaustion registry (issue #947) ────────────────────────
+# Once an engine is found rate-limited / unavailable during this run (process),
+# it is recorded here so later run_writer_with_fallback calls in the SAME run do
+# NOT re-invoke it. The #860 runaway re-walked claude → copilot → gemini on every
+# dev-lead-fix-ci cycle, re-burning quota on an already-exhausted engine and
+# re-posting the same usage-limit notice. Keyed by engine name → reason
+# (rate-limited | missing-binary). Guarded so a re-source of engine.sh in the
+# same process (e.g. review-batch's copilot pre-flight) does not wipe live state.
+if ! declare -p _ENGINE_EXHAUSTED_REASON >/dev/null 2>&1; then
+  declare -gA _ENGINE_EXHAUSTED_REASON
+fi
+
+# reset_engine_exhaustion — clear the registry. Exhaustion is intentionally
+# run-scoped, so production code never calls this; it exists for test isolation.
+reset_engine_exhaustion() {
+  _ENGINE_EXHAUSTED_REASON=()
+}
+
+# _engine_is_exhausted <engine> — 0 if already marked exhausted this run.
+_engine_is_exhausted() {
+  [ -n "${1:-}" ] && [ -n "${_ENGINE_EXHAUSTED_REASON["$1"]:-}" ]
+}
+
+# _mark_engine_exhausted <engine> <reason>
+# Record the engine as exhausted for the rest of the run and emit EXACTLY ONE
+# notice (at most one notice per engine per run). No-op on repeat marks so a
+# multi-cycle caller cannot produce a notice storm.
+_mark_engine_exhausted() {
+  local engine="${1:-}" reason="${2:-rate-limited}"
+  [ -z "$engine" ] && return 0
+  [ -n "${_ENGINE_EXHAUSTED_REASON["$engine"]:-}" ] && return 0
+  _ENGINE_EXHAUSTED_REASON["$engine"]="$reason"
+  echo "::warning::[engine-exhaustion] ${engine} marked exhausted for this run (${reason}) — it will not be re-invoked" >&2
+}
+
 # run_writer_with_fallback <prompt_file> [intent_type]
 # Tries primary engine, falls back through claude → copilot → gemini on rate-limit.
 # intent_type is passed to model_for_intent() so each engine uses the appropriate
 # tier model for the given task complexity (e.g. haiku for triage, sonnet for writes).
 # Only rate-limit (exit 2) and missing-binary (exit 127) trigger fallback;
 # other failures propagate immediately.
+# An engine found rate-limited/unavailable in an earlier call this run is skipped
+# (not re-invoked) — see the exhaustion registry above (#947).
 run_writer_with_fallback() {
   local prompt_file="$1"
   local intent="${2:-}"
@@ -1296,9 +1333,23 @@ run_writer_with_fallback() {
       continue
     fi
 
+    # Run-scoped exhaustion (#947): an engine already found rate-limited/
+    # unavailable earlier in this run is not re-invoked. Reflect its recorded
+    # reason into the aggregate so the final return code is unchanged, and emit
+    # no repeat notice (the one-time notice fired when it was first marked).
+    if _engine_is_exhausted "$engine"; then
+      case "${_ENGINE_EXHAUSTED_REASON["$engine"]}" in
+        missing-binary) any_missing=1 ;;
+        *)              any_rate_limited=1 ;;
+      esac
+      echo "  [engine] $engine already exhausted this run — skipping (not re-invoked)" >&2
+      continue
+    fi
+
     if ! check_provider_headroom "$engine"; then
       echo "::warning::$engine at/above usage threshold — trying next engine" >&2
       any_rate_limited=1
+      _mark_engine_exhausted "$engine" "rate-limited"
       continue
     fi
 
@@ -1324,8 +1375,13 @@ run_writer_with_fallback() {
       #           binary rather than a retryable quota error so that infra failures
       #           surface loudly instead of being masked as rate-limit retries).
       echo "::warning::$engine unavailable (exit $rc), trying next engine" >&2
-      [ "$rc" -eq 2 ] && any_rate_limited=1
-      [ "$rc" -eq 127 ] && any_missing=1
+      if [ "$rc" -eq 2 ]; then
+        any_rate_limited=1
+        _mark_engine_exhausted "$engine" "rate-limited"
+      else
+        any_missing=1
+        _mark_engine_exhausted "$engine" "missing-binary"
+      fi
       continue
     fi
     # A non-fallback engine error (137/143=signal, generic non-zero) — propagate

tests/dev-lead/unit/test_engine_fallback.bats

@@ -387,3 +387,114 @@ STUB
 
   rm -f "$GITHUB_STEP_SUMMARY"
 }
+
+# ── run-scoped engine exhaustion (#947) ───────────────────────────────────────
+# Once an engine is found rate-limited/unavailable during a run, it must NOT be
+# re-invoked on later run_writer_with_fallback calls in the same run (the #860
+# fix-ci-cycle re-burn), and it must emit AT MOST ONE exhaustion notice per
+# engine per run. These tests call the function twice+ in the SAME shell (no
+# bats `run`, which forks a subshell) so the process-scoped registry persists.
+
+@test "exhaustion: rate-limited engine is not re-invoked on a later call in the same run" {
+  local claude_record gemini_record
+  claude_record="$(mktemp "$STUB_BIN_DIR/claude_record.XXXXXX")"
+  gemini_record="$(mktemp "$STUB_BIN_DIR/gemini_record.XXXXXX")"
+  _make_recording_stub "claude" 2 "$claude_record"   # rate-limited (exit 2)
+  _make_recording_stub "gemini" 0 "$gemini_record"   # succeeds
+  export GEMINI_API_KEY="test-key"
+  export COPILOT_GITHUB_TOKEN="ghp_stub"              # ghp_* → copilot skipped
+  _source_engine "claude"
+
+  local rc1=0 rc2=0
+  run_writer_with_fallback "$TEST_PROMPT" || rc1=$?
+  run_writer_with_fallback "$TEST_PROMPT" || rc2=$?
+
+  [ "$rc1" -eq 0 ]
+  [ "$rc2" -eq 0 ]
+  # claude invoked once (call 1), then skipped as exhausted on call 2
+  [ "$(wc -l < "$claude_record")" -eq 1 ]
+  # gemini served both calls
+  [ "$(wc -l < "$gemini_record")" -eq 2 ]
+}
+
+@test "exhaustion: at most one exhaustion notice per engine across calls in a run" {
+  _make_stub "claude" 2
+  _make_stub "gemini" 0
+  export GEMINI_API_KEY="test-key"
+  export COPILOT_GITHUB_TOKEN="ghp_stub"
+  _source_engine "claude"
+
+  local errlog
+  errlog="$(mktemp "$STUB_BIN_DIR/errlog.XXXXXX")"
+  run_writer_with_fallback "$TEST_PROMPT" 2>>"$errlog" || true
+  run_writer_with_fallback "$TEST_PROMPT" 2>>"$errlog" || true
+  run_writer_with_fallback "$TEST_PROMPT" 2>>"$errlog" || true
+
+  # Exactly one "marked exhausted" notice for claude across all three calls.
+  local n
+  n=$(grep -c 'claude marked exhausted' "$errlog" || true)
+  [ "$n" -eq 1 ]
+}
+
+@test "exhaustion: all-engines-exhausted second call still returns 2 (rate-limited)" {
+  _make_stub "claude" 2
+  _make_stub "gemini" 2
+  export GEMINI_API_KEY="test-key"
+  export COPILOT_GITHUB_TOKEN="stub-token"
+  cat > "$STUB_BIN_DIR/gh" <<'GHEOF'
+#!/usr/bin/env bash
+case "$*" in
+  *"copilot"*) echo "rate limit exceeded"; exit 1 ;;
+  *) echo "{}" ;;
+esac
+GHEOF
+  chmod +x "$STUB_BIN_DIR/gh"
+  _source_engine "claude"
+
+  local rc1=0 rc2=0
+  run_writer_with_fallback "$TEST_PROMPT" || rc1=$?
+  run_writer_with_fallback "$TEST_PROMPT" || rc2=$?
+
+  [ "$rc1" -eq 2 ]
+  [ "$rc2" -eq 2 ]
+  [ "$(cat /tmp/dev-lead-failure-reason)" = "rate-limited" ]
+}
+
+@test "exhaustion: reset_engine_exhaustion clears the registry" {
+  local claude_record
+  claude_record="$(mktemp "$STUB_BIN_DIR/claude_record.XXXXXX")"
+  _make_recording_stub "claude" 2 "$claude_record"
+  _make_stub "gemini" 0
+  export GEMINI_API_KEY="test-key"
+  export COPILOT_GITHUB_TOKEN="ghp_stub"
+  _source_engine "claude"
+
+  run_writer_with_fallback "$TEST_PROMPT" || true    # claude recorded (1), exhausted
+  reset_engine_exhaustion
+  run_writer_with_fallback "$TEST_PROMPT" || true    # claude re-invoked after reset
+
+  [ "$(wc -l < "$claude_record")" -eq 2 ]
+}
+
+@test "exhaustion: headroom-threshold breach exhausts engine permanently" {
+  _make_stub "gemini" 0
+  export GEMINI_API_KEY="test-key"
+  export COPILOT_GITHUB_TOKEN="ghp_stub"
+  _source_engine "claude"
+  # Override headroom check: claude is always at/above threshold; others have headroom
+  check_provider_headroom() { [ "$1" = "claude" ] && return 1; return 0; }
+
+  local errlog
+  errlog="$(mktemp "$STUB_BIN_DIR/errlog.XXXXXX")"
+  local rc1=0 rc2=0
+  run_writer_with_fallback "$TEST_PROMPT" 2>>"$errlog" || rc1=$?
+  run_writer_with_fallback "$TEST_PROMPT" 2>>"$errlog" || rc2=$?
+
+  # gemini serves both calls despite claude's headroom failure
+  [ "$rc1" -eq 0 ]
+  [ "$rc2" -eq 0 ]
+  # Exactly one exhaustion notice for claude across both calls
+  local n
+  n=$(grep -c 'claude marked exhausted' "$errlog" || true)
+  [ "$n" -eq 1 ]
+}