feat(dev-lead): retry failed initial issue implementations, never silent (#781)

.github/workflows/dev-lead-reusable.yml

@@ -46,6 +46,7 @@ permissions: {}
 #                                                                 avoids issue#/PR# collision)
 #   issues / issue_comment on issue  -> dev-lead-issue-<n>       (issue lane)
 #   repository_dispatch (ci-failure) -> dev-lead-pr-<n>          (client_payload.pr_number)
+#   repository_dispatch (issue-retry)-> dev-lead-issue-<n>       (client_payload.issue_number)
 #   anything else                    -> dev-lead-run-<run_id>    (unique; never cancelled)
 #
 # cancel-in-progress is false: a newer same-lane event queues behind the active
@@ -58,6 +59,7 @@ concurrency:
       || (github.event.issue.pull_request && format('dev-lead-pr-{0}', github.event.issue.number))
       || (github.event.issue.number && format('dev-lead-issue-{0}', github.event.issue.number))
       || (github.event.client_payload.pr_number && format('dev-lead-pr-{0}', github.event.client_payload.pr_number))
+      || (github.event.client_payload.issue_number && format('dev-lead-issue-{0}', github.event.client_payload.issue_number))
       || format('dev-lead-run-{0}', github.run_id)
     }}
   cancel-in-progress: false

.github/workflows/dev-lead.yml

@@ -32,7 +32,7 @@ on:
   check_run:
     types: [completed]
   repository_dispatch:
-    types: [dev-lead-ci-failure, dev-lead-reviews-retry]
+    types: [dev-lead-ci-failure, dev-lead-reviews-retry, dev-lead-issue-retry]
 
 permissions: {}
 

.gitleaks.toml

@@ -132,3 +132,51 @@ description = "Suppress the example expired-JWT fixture in the gitleaks-config s
 condition = "AND"
 paths = ['''^tests/test_gitleaks_config\.bats$''']
 regexes = ['''eyJhbGciOiJIUzI1NiIs''']
+
+# Engine fallback and fix-issue unit tests originally used a 40-char classic-PAT-length
+# placeholder "ghp_classicPATplaceholder0000000000000000" in commit eb7fd4c to trigger
+# the copilot-skip path in engine.sh (ghp_* prefix → copilot fallback skipped). Commit
+# 7cbe053 replaced it with the shorter "ghp_stub" (below entropy threshold).
+# gitleaks v8.24.3 does not honour condition="AND" when both `commits` and `regexes`
+# are specified together, so the commit-scoped entry below is inert. The path-based
+# entries that follow cover both the historical placeholder (eb7fd4c) and the current
+# stub; .gitleaksignore fingerprints are retained as a belt-and-suspenders fallback.
+[[allowlists]]
+description = "Suppress classic-PAT-length placeholder in Phase 1 engine-failure tests (commit eb7fd4c)"
+targetRules = ["github-pat"]
+condition = "AND"
+commits = ["eb7fd4c955378d274794e38ee9b942912ce79c6f"]
+regexes = ['''ghp_classicPATplaceholder0000000000000000''']
+
+# Path-based entries suppress both the historical 40-char placeholder (commit eb7fd4c)
+# and the current shorter stub (ghp_stub, below entropy threshold) in these test files.
+# Each allowlist uses a single regex so that gitleaks' condition="AND" logic (which ANDs
+# across condition types AND within the regexes array) can satisfy the regex condition
+# with one match.
+[[allowlists]]
+description = "Suppress ghp_stub placeholder in test_engine_fallback.bats (current commits)"
+targetRules = ["github-pat"]
+condition = "AND"
+paths = ['''^tests/dev-lead/unit/test_engine_fallback\.bats$''']
+regexes = ['''ghp_stub''']
+
+[[allowlists]]
+description = "Suppress historical classic-PAT placeholder in test_engine_fallback.bats (commit eb7fd4c)"
+targetRules = ["github-pat"]
+condition = "AND"
+paths = ['''^tests/dev-lead/unit/test_engine_fallback\.bats$''']
+regexes = ['''ghp_classicPATplaceholder0000000000000000''']
+
+[[allowlists]]
+description = "Suppress ghp_stub placeholder in test_fix_issue.bats (current commits)"
+targetRules = ["github-pat"]
+condition = "AND"
+paths = ['''^tests/dev-lead/unit/test_fix_issue\.bats$''']
+regexes = ['''ghp_stub''']
+
+[[allowlists]]
+description = "Suppress historical classic-PAT placeholder in test_fix_issue.bats (commit eb7fd4c)"
+targetRules = ["github-pat"]
+condition = "AND"
+paths = ['''^tests/dev-lead/unit/test_fix_issue\.bats$''']
+regexes = ['''ghp_classicPATplaceholder0000000000000000''']

.gitleaksignore

@@ -0,0 +1,17 @@
+# Fingerprints of known false-positive gitleaks findings.
+# Each line: {commit}:{file}:{rule}:{line}
+#
+# commit eb7fd4c (Phase 1 engine-failure): introduced COPILOT_GITHUB_TOKEN set to
+# a 40-char test placeholder matching the ghp_* classic-PAT pattern (used to
+# trigger the copilot-skip path in engine.sh). These three findings are test fixtures,
+# not real credentials. The commit-scoped [[allowlists]] entry in .gitleaks.toml
+# targets the same commit+regex but is not suppressed by gitleaks v8.24.3 when
+# condition="AND" is combined with both commits and regexes; fingerprint ignores
+# are used here as the reliable fallback.
+eb7fd4c955378d274794e38ee9b942912ce79c6f:tests/dev-lead/unit/test_fix_issue.bats:github-pat:485
+eb7fd4c955378d274794e38ee9b942912ce79c6f:tests/dev-lead/unit/test_engine_fallback.bats:github-pat:276
+eb7fd4c955378d274794e38ee9b942912ce79c6f:tests/dev-lead/unit/test_engine_fallback.bats:github-pat:323
+# commit 334fac2 (initial .gitleaksignore): the comment at line 5 quoted the
+# 40-char classic-PAT-length test placeholder verbatim; commit a547c50 rewrote
+# it to avoid the literal. Fingerprint suppresses the historical commit's finding.
+334fac2ca4528d821b7511e1a0f8e5051a2aca65:.gitleaksignore:github-pat:5

docs/dev-lead/spec.md

@@ -109,6 +109,8 @@ Every GitHub webhook event that reaches `dev-lead.yml` is classified into exactl
 | `issues` labeled `dev-lead` | Any | `issue` | `dev-lead-fix-issue.sh` |
 | `check_run` completed, failure | Not a `dev-lead /` check | _relay only_ | `ci-relay` job |
 | `repository_dispatch` `dev-lead-ci-failure` | Dispatched by `ci-relay` | `fix-ci` | `dev-lead-fix-ci.sh` |
+| `repository_dispatch` `dev-lead-reviews-retry` | Dispatched by `dev-lead-retry` cron | _(payload `intent_type`)_ | `dev-lead-fix-reviews.sh` |
+| `repository_dispatch` `dev-lead-issue-retry` | Dispatched by `dev-lead-retry` cron (#781) | `issue` | `dev-lead-fix-issue.sh` |
 | All others | — | `skip` | (no-op) |
 
 ### 4.2 Intent definitions
@@ -436,6 +438,22 @@ Reads the specific comment body as the user's instruction. Executes it as an age
 6. **Wait for CI** — fix any failures (calls `dev-lead-fix-ci.sh` logic inline)
 7. **Tag CODEOWNERS** — post comment tagging relevant owners when CI is green
 
+**Engine-failure handling (`handle_engine_failure`, #781).** When `run_writer_with_fallback`
+fails (before a PR exists), the handler never exits silently. It classifies the cause from the
+`/tmp/dev-lead-failure-reason` sidecar written by `engine.sh` (`rate-limited` | `missing-binary`
+| `engine-error`), computes the attempt number from prior `<!-- dev-lead-issue <N> ... -->`
+markers, and posts a comment with the cause, a run deep-link, and a redacted ~40-line tail of the
+session output. Decision matrix (`MAX_ATTEMPTS=3`):
+
+| Cause | `attempt < MAX` | `attempt ≥ MAX` |
+|---|---|---|
+| `missing-binary` | `dev-lead:needs-human` label + comment (deterministic infra — no retry) | _(same)_ |
+| `rate-limited` / `engine-error` | retryable marker `<!-- dev-lead-issue <N> status=<failed\|rate-limited> attempt=<K> reason=<r> run=<id> [reset=ISO] -->` + comment | `dev-lead:needs-human` label + "retries exhausted" comment |
+
+Exit codes are unchanged (`2` rate-limit, `1` otherwise). The retryable marker is what the
+`dev-lead-retry` cron scans for to re-dispatch a `dev-lead-issue-retry`; an issue carrying the
+`dev-lead:needs-human` label is skipped by the cron.
+
 **Standards injection (always prepended to prompt):**
 
 The prompt includes instructions to:
@@ -600,6 +618,24 @@ claude → gemini → copilot → abort (post "rate limited" comment on PR)
 
 This mirrors the `review-batch.sh` `session_engine_fallback()` pattern.
 
+When **all** engines are exhausted, `run_writer_with_fallback` records the cause class to
+`/tmp/dev-lead-failure-reason` (`rate-limited` | `missing-binary` | `engine-error`), and
+`run_writer` persists the redacted session output to the run step summary on every exit path
+(including the rate-limit early return). Downstream handlers read the sidecar to classify the
+failure rather than collapsing everything into a generic exit code.
+
+### 10.1a Failed initial issue implementations (#781)
+
+A first issue implementation that fails at the engine step has no PR to attach a retry marker to,
+so before #781 it stalled silently. Now `dev-lead-fix-issue.sh` posts a cause-bearing
+`<!-- dev-lead-issue <N> status=<failed|rate-limited> attempt=<K> ... -->` marker (see §7.5), and
+the `dev-lead-retry` cron's `scan_repo_issues` enumerates open `dev-lead`-labeled issues, finds
+the newest such marker, and re-dispatches a `dev-lead-issue-retry` (honouring the rate-limit
+`reset=` window and the `attempt < MAX_ATTEMPTS` ceiling, skipping issues that already have an open
+PR or the `dev-lead:needs-human` label). After `MAX_ATTEMPTS` (3) the issue is labeled
+`dev-lead:needs-human` and left for a human — the same terminal escalation used for deterministic
+`missing-binary` infra failures.
+
 ### 10.2 CI still failing after MAX_CI_CYCLES
 
 The handler posts a comment on the PR:

scripts/dev-lead-fix-issue.sh

@@ -11,13 +11,163 @@ REPO="${REPO:-${GITHUB_REPOSITORY:-}}"
 DEV_LEAD_DRY_RUN="${DEV_LEAD_DRY_RUN:-false}"
 export PROMPTS_DIR="${PROMPTS_DIR:-prompts/dev-lead}"
 
+# Machine-readable marker the retry cron (dev-lead-retry.sh) scans for to requeue
+# a failed initial issue implementation. Mirrors the PR-side conventions in
+# dev-lead-fix-ci.sh / dev-lead-fix-reviews.sh:
+#   <!-- dev-lead-issue <N> status=<failed|rate-limited> attempt=<K> reason=<r> run=<id> [reset=ISO] -->
+ISSUE_MARKER_PREFIX="<!-- dev-lead-issue "
+# Total attempts (initial + retries) before escalating to a human. Matches the
+# MAX_ATTEMPTS=3 convention used by auto-rebase-retry.sh.
+MAX_ATTEMPTS="${MAX_ATTEMPTS:-3}"
+# Label applied when an issue can no longer be auto-retried; the retry cron
+# skips issues carrying it.
+NEEDS_HUMAN_LABEL="${NEEDS_HUMAN_LABEL:-dev-lead:needs-human}"
+
 check_existing_pr() {
   local existing
   existing=$(gh api "repos/${REPO}/pulls?state=open" \
     --jq "[.[] | select(.head.ref | startswith(\"dev-lead/issue-${ISSUE_NUMBER}\"))] | length" 2>/dev/null || echo "0")
   [ "$existing" -gt 0 ]
 }
 
+# count_prior_attempts: highest attempt= recorded in any prior dev-lead-issue
+# marker on this issue (0 if none). The next attempt is this value + 1.
+# --paginate keeps the count accurate on issues with >30 comments.
+count_prior_attempts() {
+  local prefix="${ISSUE_MARKER_PREFIX}${ISSUE_NUMBER} "
+  gh api --paginate "repos/${REPO}/issues/${ISSUE_NUMBER}/comments?per_page=100" 2>/dev/null \
+    | jq -s -r --arg p "$prefix" '
+        [ .[] | .[]? | .body // ""
+          | select(contains($p))
+          | capture("attempt=(?<a>[0-9]+)").a
+          | tonumber
+        ] | max // 0' 2>/dev/null \
+    || echo 0
+}
+
+# html_escape: escape &, <, > so a redacted snippet cannot break the wrapping
+# <pre>/<details> block in the issue comment (mirrors engine.sh:run_writer).
+html_escape() {
+  sed 's|&|\&amp;|g; s|<|\&lt;|g; s|>|\&gt;|g'
+}
+
+# session_snippet: redacted ~40-line tail of the engine session output, wrapped
+# in a collapsible <details> block. The source file is already secret-scrubbed
+# by engine.sh:run_writer, so no further redaction is needed here.
+session_snippet() {
+  [ -s /tmp/dev-lead-session-output.txt ] || return 0
+  printf '<details><summary>Last 40 lines of engine output (redacted)</summary>\n\n<pre>\n'
+  tail -n 40 /tmp/dev-lead-session-output.txt | html_escape
+  printf '\n</pre>\n</details>'
+}
+
+# ensure_needs_human_label: idempotently create then apply the escalation label.
+# gh issue edit --add-label fails if the label does not exist, so create first.
+ensure_needs_human_label() {
+  gh label create "$NEEDS_HUMAN_LABEL" --repo "$REPO" \
+    --color B60205 --description "dev-lead could not complete this issue; needs human attention" \
+    2>/dev/null || true
+  gh issue edit "$ISSUE_NUMBER" --repo "$REPO" --add-label "$NEEDS_HUMAN_LABEL" 2>/dev/null || true
+}
+
+# handle_engine_failure <engine_rc>
+# Replaces the previous silent `exit 1` (and unifies the rate-limit branch).
+# Classifies the cause via the /tmp/dev-lead-failure-reason sidecar written by
+# engine.sh, surfaces it on the issue (marker + human-readable comment + run
+# link + redacted snippet), and decides retry vs. human escalation. Never
+# returns — exits 2 for rate-limit, 1 otherwise (exit semantics unchanged).
+handle_engine_failure() {
+  local engine_rc="$1"
+  rm -f "$prompt_file"
+
+  # Cause class from the engine layer; default to engine-error if the sidecar is
+  # absent (e.g. an older engine.sh). exit 2 always means rate-limited.
+  local reason="engine-error"
+  if [ -s /tmp/dev-lead-failure-reason ]; then
+    reason=$(tr -d '[:space:]' < /tmp/dev-lead-failure-reason)
+  fi
+  [ -n "$reason" ] || reason="engine-error"
+  [ "$engine_rc" -eq 2 ] && reason="rate-limited"
+
+  local reset=""
+  [ -f /tmp/dev-lead-rate-limit-reset ] && reset=$(cat /tmp/dev-lead-rate-limit-reset)
+
+  local attempt
+  attempt=$(( $(count_prior_attempts) + 1 ))
+  local run_url="${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-$REPO}/actions/runs/${GITHUB_RUN_ID:-}"
+  local snippet
+  snippet=$(session_snippet)
+
+  # Deterministic infra failure (missing engine binary): retrying cannot help —
+  # escalate to a human immediately, no retry marker.
+  if [ "$reason" = "missing-binary" ]; then
+    echo "::error::Engine binary missing while implementing issue #${ISSUE_NUMBER} — escalating to human"
+    ensure_needs_human_label
+    gh issue comment "$ISSUE_NUMBER" --repo "$REPO" --body "<!-- dev-lead-issue ${ISSUE_NUMBER} status=needs-human attempt=${attempt} reason=${reason} run=${GITHUB_RUN_ID:-} -->
+## Dev-Lead: cannot implement issue #${ISSUE_NUMBER} — needs human attention
+
+A required engine binary was missing in the runner environment while implementing this issue. This is an **infrastructure/configuration** problem, not a transient error, so automatic retry is disabled.
+
+- **Cause:** \`${reason}\`
+- **Run:** ${run_url}
+
+${snippet}
+
+Please check the runner/engine configuration, then re-apply the \`dev-lead\` label to retry." 2>/dev/null || true
+    exit 1
+  fi
+
+  # Retries exhausted: escalate to a human, no further retry marker.
+  if [ "$attempt" -ge "$MAX_ATTEMPTS" ]; then
+    echo "::error::Engine failed to implement issue #${ISSUE_NUMBER} (reason=${reason}); retries exhausted (attempt ${attempt}/${MAX_ATTEMPTS}) — escalating to human"
+    ensure_needs_human_label
+    gh issue comment "$ISSUE_NUMBER" --repo "$REPO" --body "<!-- dev-lead-issue ${ISSUE_NUMBER} status=needs-human attempt=${attempt} reason=${reason} run=${GITHUB_RUN_ID:-} -->
+## Dev-Lead: cannot implement issue #${ISSUE_NUMBER} — needs human attention
+
+Automatic retries are exhausted (**attempt ${attempt} of ${MAX_ATTEMPTS}**). The most recent failure was \`${reason}\`.
+
+- **Run:** ${run_url}
+
+${snippet}
+
+Please review the failures above. To retry anyway, remove the \`${NEEDS_HUMAN_LABEL}\` label and re-apply the \`dev-lead\` label." 2>/dev/null || true
+    [ "$engine_rc" -eq 2 ] && exit 2
+    exit 1
+  fi
+
+  # Retryable (rate-limited or engine-error, attempt < MAX): post the marker the
+  # retry cron scans for, plus a human-readable comment with the cause.
+  local status="failed"
+  [ "$reason" = "rate-limited" ] && status="rate-limited"
+  local reset_attr=""
+  [ -n "$reset" ] && reset_attr=" reset=${reset}"
+  local marker="${ISSUE_MARKER_PREFIX}${ISSUE_NUMBER} status=${status} attempt=${attempt} reason=${reason} run=${GITHUB_RUN_ID:-}${reset_attr} -->"
+
+  local cause_line reset_line=""
+  if [ "$reason" = "rate-limited" ]; then
+    echo "::warning::All engines rate-limited while implementing issue #${ISSUE_NUMBER} (attempt ${attempt}/${MAX_ATTEMPTS}) — will retry automatically"
+    cause_line="All engines were **rate-limited**."
+    [ -n "$reset" ] && reset_line="
+- **Limit resets:** ${reset}"
+  else
+    echo "::error::Engine failed to implement issue #${ISSUE_NUMBER} (reason=${reason}, attempt ${attempt}/${MAX_ATTEMPTS}) — will retry automatically"
+    cause_line="The engine failed (\`${reason}\` — e.g. a timeout or transient engine error)."
+  fi
+
+  gh issue comment "$ISSUE_NUMBER" --repo "$REPO" --body "${marker}
+## Dev-Lead: issue #${ISSUE_NUMBER} implementation failed — will retry
+
+${cause_line} This was **attempt ${attempt} of ${MAX_ATTEMPTS}**; dev-lead will retry automatically. You can also re-apply the \`dev-lead\` label to retry now.
+
+- **Cause:** \`${reason}\`
+- **Run:** ${run_url}${reset_line}
+
+${snippet}" 2>/dev/null || true
+
+  [ "$engine_rc" -eq 2 ] && exit 2
+  exit 1
+}
+
 main() {
   if [ -z "$ISSUE_NUMBER" ]; then
     echo "::error::ISSUE_NUMBER is required"
@@ -74,21 +224,11 @@ main() {
 
   local engine_rc=0
   run_writer_with_fallback "$prompt_file" "fix-issue" || engine_rc=$?
-  if [ "$engine_rc" -eq 2 ]; then
-    echo "::warning::All engines rate-limited — cannot implement issue #${ISSUE_NUMBER}; re-apply the label to retry"
-    local reset_msg=""
-    if [ -f /tmp/dev-lead-rate-limit-reset ]; then
-      local reset_time
-      reset_time=$(cat /tmp/dev-lead-rate-limit-reset)
-      [ -n "$reset_time" ] && reset_msg=" The limit is expected to reset at ${reset_time}."
-    fi
-    gh issue comment "$ISSUE_NUMBER" --repo "$REPO" --body "<!-- dev-lead-rate-limited -->All engines are currently rate-limited.${reset_msg} Please re-apply the \`dev-lead\` label when the rate limit clears to retry." 2>/dev/null || true
-    rm -f "$prompt_file"
-    exit 2
-  elif [ "$engine_rc" -ne 0 ]; then
-    echo "::error::Engine failed to implement issue #${ISSUE_NUMBER}"
-    rm -f "$prompt_file"
-    exit 1
+  if [ "$engine_rc" -ne 0 ]; then
+    # Unified failure handler: classifies the cause, surfaces it on the issue
+    # (marker + comment + run link + redacted snippet), and decides retry vs.
+    # human escalation. Never returns — exits 2 (rate-limit) or 1 (otherwise).
+    handle_engine_failure "$engine_rc"
   fi
 
   # git status --porcelain catches untracked files that git diff misses.