bug(dev-lead): rate-limited runs are silently lost — no retry, and rate limits miscounted as exhaustion failures

[don-petry]

Problem

When any engine invocation (Claude, Gemini, Copilot) hits a rate limit and run_writer_with_fallback exhausts all three fallbacks, the workflow exits 1 and the original triggering event is permanently lost. There is no queue, no retry, no deferred execution. The PR stays broken.

This affected at least PRs #80, #126, #175 yesterday — all got status=failed markers from rate limits, not from real engine errors.

---

Root cause

1. Rate limits are not distinguished from real failures

run_writer_with_fallback exits 2 on rate limit, but fix-ci.sh maps any non-zero exit from the engine to post_summary "failed":

# dev-lead-fix-ci.sh L147-161
if [ "$engine_rc" -ne 0 ]; then
  post_summary "failed" "Engine invocation failed (exit ${engine_rc})"
  fail_count=$(count_recent_failures)
  if [ "$fail_count" -ge "$MAX_FAIL_ATTEMPTS" ]; then
    post_exhaustion "$reason"
  fi
  exit 1
fi

A rate-limit exit (2) and a real engine error (1) both result in status=failed. The exhaustion guard then counts rate-limit failures toward the MAX_FAIL_ATTEMPTS threshold (default: 2), which can lock a PR out of all future automated help — even when the rate limit has long since reset.

Observed on PR #80: 5 status=failed markers accumulated across 24h, mostly from rate limits. Exhaustion was posted at 20:20 UTC on 2026-05-15.

2. The triggering event is one-shot with no queue

All intents that require engine work (fix-ci, fix-reviews, fix-bot-comment, human) are triggered by transient GitHub events (check_run completed, pull_request_review submitted, issue_comment created). Once the workflow run fails:

• The event payload is gone
• GitHub does not re-fire the event
• No retry is scheduled
• fix-ci at least leaves a status=failed marker, but nothing reads it to retry
• fix-reviews / fix-bot-comment / human leave no marker at all — the work is silently discarded

3. The fix-reviews path has zero recovery

dev-lead-fix-reviews.sh calls run_writer_with_fallback and propagates its exit code directly (line 32). No comment, no marker, no log artifact beyond the failed Actions run. The bot review that triggered the run is unaddressed with no trace.

---

Scope

All 8 repos are affected. Any org-level rate limit event (shared Claude subscription) will cascade across repos simultaneously — the worst case is the entire fleet hitting rate limits at the same time, losing all in-flight work at once.

---

Options considered

Option A — Separate status=rate-limited from status=failed (low effort, partial fix)

Change fix-ci.sh to detect whether the non-zero exit from run_writer_with_fallback was a rate limit (exit 2) vs. a real engine failure (exit 1), and post status=rate-limited instead of status=failed. Exclude status=rate-limited from the exhaustion count_recent_failures query.

Pros: Fixes the exhaustion miscounting immediately. Simple change, low risk.
Cons: Work is still lost. Just quieter about it.
Effort: ~1h

---

Option B — In-workflow sleep-and-retry (wrong tool for the job)

Parse the "resets N:XXpm" string from the engine error, sleep until then, retry in the same runner.

Pros: No new infrastructure.
Cons: Rate-limit windows can be 6+ hours (observed: reset at 23:20 UTC from a 17:05 hit). The job timeout-minutes: 30 would expire. Even if extended, burning a runner for 6 hours of sleep to save one LLM call is wasteful and blocks the per-PR concurrency slot. Not viable.

---

Option C — workflow_run self-trigger on failure (too coarse)

Add a workflow_run trigger that fires when dev-lead completes with failure conclusion.

Pros: Immediate, automatic.
Cons: Doesn't know why it failed or what work to retry. Any non-rate-limit failure would trigger an infinite retry loop. Needs complex state to distinguish rate-limit failures from real ones. The trigger also fires for the ci-relay job (which never calls an engine), adding noise. High risk of runaway loops.

---

Option D — Scheduled retry scanner (recommended for fix-ci)

A new lightweight workflow dev-lead-retry.yml runs on a schedule (e.g., every 2 hours), scans all open PRs across all 8 repos for status=rate-limited markers on the current HEAD SHA, and re-dispatches repository_dispatch dev-lead-ci-failure for each. This reuses the existing ci-relay dispatch infrastructure and the existing idempotency check — no duplicate work if the rate limit already cleared and a new push re-triggered the run.

Pros:

• Reuses existing repository_dispatch + idempotency infrastructure unchanged
• Clean separation of concerns (retry logic is its own workflow)
• Testable in dry-run mode
• Handles org-wide rate limit bursts gracefully (all PRs picked up on next tick)
• The staggered cron pattern from pr-review.yml (4x/hour) is available precedent if faster recovery is wanted

Cons:

• Up to 2-hour delay between rate limit clearing and retry (acceptable for a background agent)
• Requires cross-repo read access via GH_PAT_WORKFLOWS (already available)
• Does not cover fix-reviews / fix-bot-comment / human (see Option E)
• If rate limits haven't cleared by the time the cron fires, it re-dispatches and immediately fails again → adds another status=rate-limited marker. Mitigation: parse reset time from marker body and skip if reset time is in the future.

Effort: ~1 day

---

Option E — status=rate-limited marker for fix-reviews + retry cron (recommended for fix-reviews)

Pros: Full coverage of all intent types.
Cons: Adds a new dispatch event type. fix-reviews has no existing marker infrastructure to build on. Moderate implementation complexity.
Effort: ~1 day (after Option D)

---

Recommended implementation plan

Phase 1 (quick win — fixes exhaustion miscounting):

• In fix-ci.sh: detect exit 2 from run_writer_with_fallback explicitly; post status=rate-limited instead of status=failed; do not count toward exhaustion threshold
• In count_recent_failures: filter to status=failed only (already is, but add explicit comment)
• Update unit tests

Phase 2 (retry for fix-ci):

• Add dev-lead-retry.yml — scheduled every 2 hours, scans open PRs across all 8 repos for status=rate-limited on current HEAD SHA, re-dispatches dev-lead-ci-failure
• Parse rate-limit reset time from marker body (if recorded) to skip retries before the window clears
• Add E2E scenario: 07-rate-limit-retry

Phase 3 (retry for fix-reviews, fix-bot-comment, human):

• Add rate-limit marker to fix-reviews.sh
• Extend retry cron to scan for and re-dispatch review retries

---

Related

• PR fix(dev-lead): PR-level exhaustion guard prevents token waste on large PRs #188 (exhaustion guard) — the guard is correct but the inputs are wrong; rate limits should not count as exhaustion events
• Context doc: docs/dev-lead/monitor-and-close-prompt.md — mentions workflow_run trigger as a future consideration (Option C above — not recommended)
• Tracking issue: Phase 7: Shadow period — monitor claude.yml vs dev-lead.yml (ends ~2026-05-29) #180 (shadow period) — this should be resolved before Phase 7 completion

[don-petry]

Analysis: Improvements to the Plan

After reading the actual scripts (engine.sh, dev-lead-fix-ci.sh, dev-lead-fix-reviews.sh), several gaps are worth addressing before implementation begins.

---

1. Critical: run_writer rate-limit detection is broken (not in the plan)

The plan treats "rate-limit returns exit 2" as a given, but it doesn't today. In engine.sh, run_writer checks for rate limits by reading /tmp/dev-lead-writer-stderr:

if [ \"\$rc\" -ne 0 ] && output_check=\$(cat /tmp/dev-lead-writer-stderr 2>/dev/null) && is_rate_limited \"\$output_check\"; then
  return 2
fi

claude --print writes to stdout, not to that file — nothing ever writes /tmp/dev-lead-writer-stderr. So is_rate_limited never fires, run_writer returns exit 1 (claude's real exit), and run_writer_with_fallback propagates immediately without trying gemini or copilot.

This is confirmed by the actual run log for PR #175 (run 25941760151): "You've hit your limit · resets 11:20pm (UTC)" → exit code 1. The fallback never ran.

Fix (should be Phase 0 / top of Phase 1): Capture claude's stdout into a temp file, tee back to job stdout, and feed it to is_rate_limited:

local _tmp rc=0
_tmp=\$(mktemp)
timeout \"\$ACTION_TIMEOUT_SEC\" claude --print ... < \"\$prompt_file\" | tee \"\$_tmp\" || rc=\${PIPESTATUS[0]}
if [ \"\$rc\" -ne 0 ] && is_rate_limited \"\$(cat \"\$_tmp\")\"; then
  rm -f \"\$_tmp\"; return 2
fi
rm -f \"\$_tmp\"

Without this, Phase 1's "detect exit 2" change in fix-ci.sh never fires for real rate-limit events.

---

2. Idempotency check would block retries (plan gap)

The plan says the retry cron "reuses existing idempotency infrastructure unchanged." It won't work. check_idempotency in fix-ci.sh does:

if echo \"\$comments\" | grep -qF \"\${MARKER_PREFIX}\${HEAD_SHA}\"; then
  return 0  # skip
fi

A status=rate-limited marker contains \${MARKER_PREFIX}\${HEAD_SHA}, so the retried run would match, print "Already handled — skipping," and exit 0. The idempotency check must be taught to treat status=rate-limited as retriable, not done.

Fix: Match only terminal statuses (applied, failed, no-changes) in the SHA-level check; explicitly pass through rate-limited.

---

3. Reset time capture belongs in Phase 1, not Phase 2

Phase 2 depends on parsing reset time from the marker body to skip retries before the window clears. If Phase 1 writes the marker without the reset time, Phase 2 either ships incomplete or must handle missing timestamps as a special case.

These belong together: when writing status=rate-limited, parse the resets H:MMam/pm (UTC) string from the engine output (already matched by is_rate_limited) and embed an ISO timestamp in the marker body. This is a one-time addition with no added risk.

---

4. Cross-repo cascade risk in the retry cron

All 8 repos share an org-level Claude subscription. If the cron fires and re-dispatches all repos simultaneously, they collectively hit the same rate limit again immediately — the same scenario that caused the original burst. The retry cron needs staggered dispatch (e.g., 30s between repos). The offset-by-index approach from pr-review.yml's 4×/hour staggering is a ready model.

---

5. Rate-limited marker accumulation without dedup

A 6-hour rate-limit window with a 2-hour cron produces 3 firings before clearing. Each firing dispatches → hits rate limit again → posts another status=rate-limited marker. A PR could accumulate 3–4 identical markers, polluting the comment thread and making any comment-count-based logic fragile.

Before posting a rate-limited marker, check if one already exists for the same SHA. Skip or update rather than append. The existing check_idempotency pattern (grep PR comments) is the right model.

---

6. All five intent types in fix-reviews.sh are affected

The issue body mentions fix-reviews, fix-bot-comment, and human. But dev-lead-fix-reviews.sh handles five intent types:

Intent	User-triggered?	Silent loss today?
fix-reviews	no (bot review)	yes
fix-bot-comment	no (bot comment)	yes
human	yes	yes — worst UX
human-pr	yes	yes — worst UX
rebase	no (conflict detection)	yes

human and human-pr are the most severe: a user explicitly requested action and received nothing, not even acknowledgment. rebase is lower urgency (conflict state is visible in the PR), but a rate-limited rebase attempt still leaves the PR in a broken state with no trace.

All five must be covered in Phase 3 marker additions and retry dispatch. The issue's Phase 3 scope should be updated to enumerate all five explicitly.

---

7. User-triggered intents need a visible notification (independent of retry)

For human and human-pr, even if retry infrastructure isn't built until Phase 3, the user deserves an immediate acknowledgment rather than silence. A post_comment call in fix-reviews.sh on rate-limit exit — "Rate limited, will retry automatically" — is a ~5-line fix that can ship with Phase 1 and requires no retry infrastructure. This should be in Phase 1.

---

8. Cleanup of already-misattributed exhausted PRs

After Phase 1 fixes the exhaustion miscounting, PRs already blocked by wrongly-posted status=exhausted markers remain blocked indefinitely. The plan has no cleanup step. At minimum, document which PRs need exhaustion comment deletion. PR #80 is known (5 status=failed markers, exhaustion posted 2026-05-15 20:20 UTC). Others may exist across the 8-repo fleet.

---

Revised phase breakdown

Phase 0 — prerequisite (unlocks everything else):

• Fix run_writer stdout capture so is_rate_limited actually fires and fallback engines are tried

Phase 1 — quick wins:

• fix-ci.sh: detect exit 2 as status=rate-limited; embed parsed reset timestamp in marker body
• check_idempotency: allow re-execution through rate-limited markers (only block on terminal statuses)
• count_recent_failures: confirm it filters to status=failed only (add explicit comment)
• fix-reviews.sh: on rate-limit exit from any intent type, post a status=rate-limited marker (fix-ci format, adapted) — this is the minimum for Phase 3 to build on
• fix-reviews.sh: for human and human-pr specifically, post a user-visible "Rate limited, will retry" PR comment
• Unit tests for all changes
• One-time cleanup: identify and clear wrongly-posted exhaustion markers across the 8-repo fleet

Phase 2 — retry for fix-ci:

• dev-lead-retry.yml: scheduled cron; staggered dispatch across repos (30s between repos); skip if reset time (from marker) is in the future; dedup before dispatching (don't re-dispatch if a rate-limited marker was already retried this cycle)
• E2E scenario 07-rate-limit-retry

Phase 3 — retry for all fix-reviews intents:

• Extend retry cron to scan for and re-dispatch all five intent types: fix-reviews, fix-bot-comment, human, human-pr, rebase
• New dispatch event type (or extend existing with intent metadata)
• Unit tests and E2E scenario