feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry)

scripts/dev-lead-fix-issue.sh

@@ -125,6 +125,55 @@ Please check the runner/engine configuration, then re-apply the \`dev-lead\` lab
     exit 1
   fi
 
+  # Stage timeout (exit 124): a first-class, NON-retryable condition. A
+  # same-budget retry would just time out again, so — like missing-binary —
+  # escalate to a human on the FIRST occurrence with NO retry marker (the retry
+  # cron skips dev-lead:needs-human). The comment states the tier, elapsed
+  # seconds, and budget, plus split/raise guidance. This branch precedes the
+  # attempt-ceiling / retry logic below on purpose. (#1018)
+  if [ "$reason" = "timeout" ]; then
+    local tier="deep/action" budget="unknown" elapsed="unknown" raise_var="DEEP_TIMEOUT_SEC"
+    [ -s /tmp/dev-lead-timeout-tier ]    && tier=$(tr -d '[:space:]' < /tmp/dev-lead-timeout-tier)
+    [ -s /tmp/dev-lead-timeout-budget ]  && budget=$(tr -d '[:space:]' < /tmp/dev-lead-timeout-budget)
+    [ -s /tmp/dev-lead-timeout-elapsed ] && elapsed=$(tr -d '[:space:]' < /tmp/dev-lead-timeout-elapsed)
+    case "$tier" in
+      action|writer) raise_var="ACTION_TIMEOUT_SEC" ;;
+      deep)          raise_var="DEEP_TIMEOUT_SEC" ;;
+    esac
+
+    # Surface completed-but-unpushed work (#1003): if the engine produced commits
+    # or staged/uncommitted changes before a later phase timed out, say so — so
+    # the timeout isn't a silent black hole. Be honest about the limitation: the
+    # branch is local to this ephemeral runner and was NOT pushed, so it is not
+    # recoverable from this run; the session snippet is context only, not
+    # restorable output.
+    local work_note=""
+    if [ -n "$(git status --porcelain 2>/dev/null)" ] || \
+       { [ -n "${pre_engine_sha:-}" ] && [ "$(git rev-parse HEAD 2>/dev/null)" != "$pre_engine_sha" ]; }; then
+      work_note="
+
+> **Note: the engine had produced changes before the timeout.** They were on branch \`${branch:-unknown}\` on the runner and were **not pushed**, so they are **not recoverable** from this run (the runner is ephemeral). The redacted session snippet below shows what it attempted — treat it as context, not restorable output. Re-apply \`dev-lead\` after splitting the issue or raising the budget to regenerate the work."
+    fi
+
+    echo "::error::Stage timeout (exit 124) while implementing issue #${ISSUE_NUMBER} at the ${tier} tier (elapsed=${elapsed}s, budget=${budget}s) — escalating to human (no same-budget retry)"
+    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 stage **timed out** (exit 124) while implementing this issue. A timeout is treated as a first-class, **non-retryable** condition — retrying with the same budget would just time out again — so this escalates for human review on the **first** occurrence instead of wasting a same-budget retry.
+
+- **Cause:** \`${reason}\`
+- **Tier:** ${tier}
+- **Elapsed:** ${elapsed}s
+- **Budget:** ${budget}s
+- **Run:** ${run_url}
+
+This hit the max ${tier} budget. **Split this issue** into smaller, independently-implementable pieces, or **raise \`vars.${raise_var}\`**, then re-apply the \`dev-lead\` label.${work_note}
+
+${snippet}" 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"

scripts/engine.sh

@@ -1272,7 +1272,13 @@ run_writer_with_fallback() {
   # mirroring the existing /tmp/dev-lead-rate-limit-reset sidecar. Downstream
   # handlers (dev-lead-fix-issue.sh) read it to classify the failure and pick
   # the right retry behavior. This is purely additive — no control-flow change.
-  rm -f /tmp/dev-lead-failure-reason
+  # The timeout-* sidecars (tier/budget/elapsed) are written only on a per-tier
+  # timeout (#1018); clear any stale copies here so a later non-timeout failure
+  # never surfaces a previous run's timeout context.
+  rm -f /tmp/dev-lead-failure-reason \
+        /tmp/dev-lead-timeout-tier \
+        /tmp/dev-lead-timeout-budget \
+        /tmp/dev-lead-timeout-elapsed
 
   for e in claude copilot gemini; do
     [ "$e" != "$REVIEW_ENGINE" ] && engines+=("$e")
@@ -1303,8 +1309,10 @@ run_writer_with_fallback() {
     set_engine_config
     local model
     model="$(model_for_intent "$intent")"
-    local rc=0
+    local rc=0 _t_start _t_end
+    _t_start=$(date +%s)
     run_writer "$prompt_file" "$model" || rc=$?
+    _t_end=$(date +%s)
     export REVIEW_ENGINE="$saved"
     # Restore original config for subsequent PRs in the same session
     set_engine_config
@@ -1320,9 +1328,24 @@ run_writer_with_fallback() {
       [ "$rc" -eq 127 ] && any_missing=1
       continue
     fi
-    # A non-fallback engine error (e.g. 124=timeout kill, 137/143=signal,
-    # generic non-zero) — propagate immediately, classifying it as engine-error.
-    printf 'engine-error\n' > /tmp/dev-lead-failure-reason
+    # A non-fallback engine error (137/143=signal, generic non-zero) — propagate
+    # immediately, classifying it as engine-error.
+    #
+    # A per-tier stage timeout (exit 124 from GNU `timeout`) is treated
+    # separately (#1018): it is a first-class, NON-retryable condition, so a
+    # same-budget retry — on this engine or a fallback engine — is deliberately
+    # avoided (we already `return` here rather than `continue`). Classify it as a
+    # distinct `timeout` reason and record the tier/budget/elapsed sidecars the
+    # escalation comment surfaces, so the caller escalates to a human on the
+    # FIRST occurrence instead of burning a same-budget model call.
+    if [ "$rc" -eq 124 ]; then
+      printf 'timeout\n' > /tmp/dev-lead-failure-reason
+      printf 'action\n' > /tmp/dev-lead-timeout-tier
+      printf '%s\n' "${ACTION_TIMEOUT_SEC}" > /tmp/dev-lead-timeout-budget
+      printf '%s\n' "$(( _t_end - _t_start ))" > /tmp/dev-lead-timeout-elapsed
+    else
+      printf 'engine-error\n' > /tmp/dev-lead-failure-reason
+    fi
     return "$rc"
   done
 

tests/dev-lead/unit/test_engine_fallback.bats

@@ -28,6 +28,7 @@ setup() {
 teardown() {
   rm -f "$GITHUB_ENV" "$GITHUB_OUTPUT" "$TEST_PROMPT"
   rm -f /tmp/dev-lead-failure-reason /tmp/dev-lead-session-output.txt /tmp/dev-lead-rate-limit-reset
+  rm -f /tmp/dev-lead-timeout-tier /tmp/dev-lead-timeout-budget /tmp/dev-lead-timeout-elapsed
   rm -rf "$STUB_BIN_DIR"
 }
 
@@ -292,6 +293,57 @@ GHEOF
   [ "$(cat /tmp/dev-lead-failure-reason)" = "engine-error" ]
 }
 
+# ── timeout classification (#1018) ───────────────────────────────────────────
+# A per-tier stage timeout (exit 124 from GNU `timeout`) must be classified as a
+# distinct, NON-retryable reason=timeout — not engine-error — and must NOT drive
+# a same-budget retry on another engine. The tier/budget/elapsed sidecars carry
+# the context the needs-human escalation comment surfaces.
+
+@test "sidecar: stage timeout (exit 124) → reason=timeout + tier/budget/elapsed sidecars" {
+  _make_stub "claude" 124
+  export ACTION_TIMEOUT_SEC=2100
+  _source_engine "claude"
+
+  run run_writer_with_fallback "$TEST_PROMPT"
+
+  # 124 propagates immediately (distinct from engine-error)
+  [ "$status" -eq 124 ]
+  [ "$(cat /tmp/dev-lead-failure-reason)" = "timeout" ]
+  [ "$(cat /tmp/dev-lead-timeout-tier)" = "action" ]
+  [ "$(cat /tmp/dev-lead-timeout-budget)" = "2100" ]
+  [ -f /tmp/dev-lead-timeout-elapsed ]
+}
+
+@test "sidecar: timeout (124) does NOT fall through to another engine (no same-budget retry)" {
+  # claude times out (124); NEITHER copilot nor gemini (the remaining engines in
+  # the claude→copilot→gemini order) may be tried — each would succeed here if
+  # wrongly invoked, so record both and assert neither ran.
+  _make_stub "claude" 124
+  local gemini_record copilot_record
+  gemini_record="$(mktemp)"; copilot_record="$(mktemp)"
+  _make_recording_stub "gemini" 0 "$gemini_record"
+  # Enable copilot (non-ghp token) and record any `gh copilot` invocation.
+  export COPILOT_GITHUB_TOKEN="stub-token"
+  cat > "$STUB_BIN_DIR/gh" <<GHEOF
+#!/usr/bin/env bash
+case "\$*" in
+  *"copilot"*) echo "\$*" >> "$copilot_record"; echo "success output"; exit 0 ;;
+  *) echo "{}" ;;
+esac
+GHEOF
+  chmod +x "$STUB_BIN_DIR/gh"
+  _source_engine "claude"
+
+  run run_writer_with_fallback "$TEST_PROMPT"
+
+  [ "$status" -eq 124 ]
+  [ "$(cat /tmp/dev-lead-failure-reason)" = "timeout" ]
+  # Same-budget retry on ANY fallback engine must NOT happen.
+  [ ! -s "$copilot_record" ]
+  [ ! -s "$gemini_record" ]
+  rm -f "$gemini_record" "$copilot_record"
+}
+
 @test "sidecar: stale reason cleared on success" {
   printf 'engine-error\n' > /tmp/dev-lead-failure-reason
   _make_stub "claude" 0

tests/dev-lead/unit/test_fix_issue.bats

@@ -498,6 +498,98 @@ GHEOF
   rm -f "$COMMENT_FILE" "$LABEL_FILE"
 }
 
+# ── stage-timeout escalation (#1018) ──────────────────────────────────────────
+
+@test "fix-issue: stage timeout (exit 124) → escalates to needs-human, reason=timeout, no retry marker" {
+  _setup_failure_stubs 124 "operation timed out after 2100s"
+  # Skip gemini (no key) + copilot (classic PAT) so claude's 124 is the sole,
+  # immediately-propagated failure (no cross-engine same-budget retry).
+  unset GEMINI_API_KEY GOOGLE_API_KEY
+  export COPILOT_GITHUB_TOKEN="ghp_stub"  # ghp_* → copilot fallback is skipped
+  export ACTION_TIMEOUT_SEC=2100
+
+  run bash "$FIX_ISSUE_SCRIPT"
+
+  [ "$status" -eq 1 ]
+  local posted; posted=$(cat "$COMMENT_FILE")
+  [[ "$posted" == *"needs human attention"* ]]
+  [[ "$posted" == *"reason=timeout"* ]]
+  # Non-retryable → must NOT post a retryable status=failed marker (the retry
+  # cron would otherwise requeue a same-budget attempt).
+  [[ "$posted" != *"status=failed"* ]]
+  # Comment surfaces tier + elapsed + budget + split/raise guidance.
+  [[ "$posted" == *"Tier:"* ]]
+  [[ "$posted" == *"Elapsed:"* ]]
+  [[ "$posted" == *"Budget:"* ]]
+  [[ "$posted" == *"2100"* ]]
+  [[ "$posted" == *"Split this issue"* ]]
+  [[ "$posted" == *"TIMEOUT_SEC"* ]]
+  # needs-human label applied
+  [[ "$(cat "$LABEL_FILE")" == *"dev-lead:needs-human"* ]]
+
+  rm -f "$COMMENT_FILE" "$LABEL_FILE"
+}
+
+@test "fix-issue: non-124 transient (rate-limit) still retries — no timeout regression" {
+  # A genuine transient (rate-limit, exit 1 + rate-limit phrase) must still take
+  # the retry path (exit 2), NOT the timeout escalation. Skip gemini (no key) so
+  # the failure is deterministic: claude + copilot both rate-limit.
+  _setup_failure_stubs 1 "You've hit your limit · resets 11:20pm (UTC)"
+  unset GEMINI_API_KEY GOOGLE_API_KEY
+  export COPILOT_GITHUB_TOKEN="stub-token"  # copilot also rate-limited via gh stub
+
+  run bash "$FIX_ISSUE_SCRIPT"
+
+  # rate-limited path → exit 2, retry marker, not needs-human
+  [ "$status" -eq 2 ]
+  local posted; posted=$(cat "$COMMENT_FILE")
+  [[ "$posted" == *"will retry"* ]]
+  [[ "$posted" != *"reason=timeout"* ]]
+  [[ "$posted" != *"needs human attention"* ]]
+
+  rm -f "$COMMENT_FILE" "$LABEL_FILE"
+}
+
+@test "fix-issue: late-phase timeout with completed work → surfaces branch, not silently discarded" {
+  _setup_failure_stubs 124 "timed out during a late phase"
+  unset GEMINI_API_KEY GOOGLE_API_KEY
+  export COPILOT_GITHUB_TOKEN="ghp_stub"
+  export ACTION_TIMEOUT_SEC=2100
+
+  # Simulate the #1003 mode: the engine advanced HEAD (a commit) before the
+  # timeout, so pre_engine_sha != current HEAD. git status is clean but the SHA
+  # moved → completed-but-unpushed work must be surfaced.
+  # First rev-parse (pre_engine_sha) reads one value; make the failure-handler's
+  # rev-parse report a different SHA by flipping the env after branch creation is
+  # not possible across processes, so use a wrapper that advances on 2nd call.
+  cat > "$STUB_BIN_DIR/git" <<'GITEOF'
+#!/usr/bin/env bash
+STATE="/tmp/devlead-test-revparse-count"
+case "$*" in
+  "config"*)            exit 0 ;;
+  "checkout -b"*)       exit 0 ;;
+  "rev-parse HEAD")
+    n=0; [ -f "$STATE" ] && n=$(cat "$STATE")
+    n=$((n+1)); echo "$n" > "$STATE"
+    if [ "$n" -le 1 ]; then echo "sha_before"; else echo "sha_after_commit"; fi ;;
+  "status --porcelain") exit 0 ;;
+  *)                    exit 0 ;;
+esac
+GITEOF
+  chmod +x "$STUB_BIN_DIR/git"
+  rm -f /tmp/devlead-test-revparse-count
+
+  run bash "$FIX_ISSUE_SCRIPT"
+
+  [ "$status" -eq 1 ]
+  local posted; posted=$(cat "$COMMENT_FILE")
+  [[ "$posted" == *"reason=timeout"* ]]
+  # Completed-but-unpushed work is surfaced (branch mentioned), not discarded.
+  [[ "$posted" == *"not pushed"* ]] || [[ "$posted" == *"not discarded"* ]]
+
+  rm -f "$COMMENT_FILE" "$LABEL_FILE" /tmp/devlead-test-revparse-count
+}
+
 @test "fix-issue: attempt ceiling (prior attempt=2) → escalates to needs-human" {
   export PRIOR_COMMENTS_JSON='[{"body":"<!-- dev-lead-issue 100 status=failed attempt=2 reason=engine-error run=1 -->"}]'
   _setup_failure_stubs 1 "boom again"