Skip to content

dev-lead retry doesn't cover failed initial issue implementations (only rate-limited PRs) — issues silently stall #781

Closed
combsmith/.github
#494

Description

@don-petry

Summary

A dev-lead initial issue implementation that fails at the Run issue engine step (for any non-rate-limit reason) has no retry path. dev-lead-retry only scans open PRs for a status=rate-limited marker — it never looks at issues, and it only re-dispatches the rate-limit case. So a failed issue implementation silently falls through the retry net and stays stuck until a human intervenes.

Reproduction (live case: petry-projects/.github#478)

  1. Issue petry-projects/.github#478 was labeled dev-lead at ~02:37 UTC on 2026-06-18.
  2. dev-lead run 27733022075 (event=issues) ran the engine for ~10 min, posted a full implementation-plan comment, then the Run issue step failed:
    ##[error]Engine failed to implement issue #478
    ##[error]Process completed with exit code 1.
    
  3. Over the next ~12h, dev-lead-retry (.github-private) ran on schedule 5+ times (04:58, 10:45, … UTC) — all completed/success — and never re-attempted feat: implement issue #404 — PR Review Agent — failures detected 2026-05-28 #478.
  4. feat: implement issue #404 — PR Review Agent — failures detected 2026-05-28 #478 remains OPEN, still labeled dev-lead, no linked PR, and carries no retry/failure marker label. The only post-failure issues event on it was unrelated label activity, correctly classified intent=skip reason=label-not-dev-lead.

Root cause

dev-lead-retry.yml is scoped — per its own header — to:

"Scans open PRs across the org for status=rate-limited markers and re-dispatches the appropriate dev-lead event once the rate limit clears."

So retry coverage is limited on two axes that both exclude this case:

Axis Retry covers #478
Object open PRs an issue (no PR was ever created)
Failure type status=rate-limited marker generic Engine failed to implement (exit 1)

The initial issue→implementation phase (the Run issue step in dev-lead-reusable) has no failure marker written and no scanner that requeues it. A crash after the plan comment but before a PR exists leaves nothing for retry to find.

Impact

  • Any transient engine failure (timeout, overload, tool error) on a first issue implementation permanently strands the issue — no auto-recovery, no alert. It looks "picked up" (plan comment present, dev-lead label on) but never completes.
  • Erodes trust in the "label it dev-lead → it gets fixed" contract.

Recommendation

Give failed initial issue implementations a retry path. Options (in rough order of preference):

  1. Mark + scan. On Run issue failure, label the issue with a recognizable marker (e.g. dev-lead:retry with an attempt counter in a comment/marker), and extend dev-lead-retry.sh to also scan open issues carrying that marker and re-dispatch the issues intent. Bound attempts (e.g. 3) then fall back to a dev-lead:needs-human label + notification.
  2. Self-requeue. On engine failure, emit a bounded repository_dispatch to re-run the issue intent after a backoff, with an attempt ceiling.
  3. At minimum — visibility. If retry stays PR/rate-limit-only by design, then on a failed initial implementation add a dev-lead:failed label and a comment so the stall is visible and a human is cued, rather than silently looking "in progress."

Acceptance

  • A first-attempt Run issue engine failure results in either an automatic, bounded retry or an explicit needs-human/failed signal — never a silent stall.
  • Retry attempt count is bounded and observable.
  • Covered by a test exercising the failed-issue-implementation path.

References

  • Stuck issue: petry-projects/.github#478
  • Failed run: petry-projects/.github actions run 27733022075 (Run issue → failure)
  • Retry workflow/script: petry-projects/.github-private → .github/workflows/dev-lead-retry.yml, scripts/dev-lead-retry.sh
  • Reusable that runs the issue intent: petry-projects/.github-private → .github/workflows/dev-lead-reusable.yml

Found while monitoring the #478 fix (itself a follow-up from the add-to-project rollout, .github#415).

Activity

  1. github-actions commented on Jun 18, 2026

    @github-actions
    Contributor

    Thanks for the thorough write-up — the reproduction trace, root-cause table, and the three concrete options make this very actionable. Confirmed as a bug: the dev-lead-retry contract silently excludes failed issue implementations, which breaks the "label it → it gets fixed" guarantee. Before we start implementation: of the three options you outlined, do you have a strong preference, or is there a constraint (e.g. avoiding changes to the reusable caller interface, or keeping dev-lead-retry.sh as the single scan surface) that should guide the choice?

  2. don-petry commented on Jun 18, 2026

    @don-petry
    CollaboratorAuthor

    Investigation findings (code-level confirmation)

    I traced the full issue→implementation→retry path. The report is accurate; here is exactly where it breaks, with line references.

    1. The silent stall is in dev-lead-fix-issue.sh

    # scripts/dev-lead-fix-issue.sh:88-92
    elif [ "$engine_rc" -ne 0 ]; then
      echo "::error::Engine failed to implement issue #${ISSUE_NUMBER}"
      rm -f "$prompt_file"
      exit 1
    fi

    On a generic engine failure the script writes no comment, no label, no marker — just a workflow annotation and exit 1. The issue keeps its dev-lead label and looks "in progress" (the plan comment from an earlier step is already posted). This is the silent stall.

    The rate-limit branch (engine_rc == 2, lines 77–87) does post a <!-- dev-lead-rate-limited --> comment — but it only asks a human to re-apply the label. There is no automatic retry even for the rate-limited issue case.

    2. The retry cron never looks at issues

    scripts/dev-lead-retry.sh → scan_repo() (lines 241–268) enumerates only open PRs:

    prs_json=$(gh api --paginate "repos/${repo}/pulls?state=open&per_page=100" ...)

    There is no issue enumeration anywhere in the script. So even if a marker existed, nothing scans for it. Both axes in the report's table are confirmed in code: object = PRs only, failure type = status=rate-limited only.

    3. The re-dispatch plumbing has no "issue" path

    The retry mechanism fires repository_dispatch events, and the classifier requires a PR number:

    • dev-lead-intent.sh:381 — if [ -z "$pr_number" ]; then emit_skip "no-pr-number-in-payload". An issue-only payload is dropped immediately.
    • Recognized dispatch types are only dev-lead-ci-failure and dev-lead-reviews-retry (dev-lead-intent.sh:402-438), mirrored in the thin caller stub dev-lead.yml:34-35.
    • The reusable workflow's concurrency router (dev-lead-reusable.yml:53-63) routes client_payload.pr_number → PR lane but has no client_payload.issue_number branch.

    So delivering a failed issue back to the issue intent requires a new dispatch type end-to-end (stub trigger → classifier → concurrency lane), not just a scanner tweak.

    4. There is no test or marker convention for the failed-issue path

    • No tests/**/*retry*.bats exists for dev-lead-retry.sh at all (only the e2e 07-rate-limit-retry.sh dry-run).
    • Existing marker conventions to mirror: review markers like <!-- dev-lead-fix-reviews pr=N sha=SHA intent=X status=rate-limited reset=ISO --> (dev-lead-fix-reviews.sh:733), and the attempt-cap convention MAX_ATTEMPTS=3 from auto-rebase-retry.sh:40.

    Proposed plan

    I recommend Option 1 (mark + scan) — it keeps retry logic centralized where it already lives (dev-lead-retry.sh) and reuses the existing marker/dispatch idiom. To de-risk the consumer-facing change, I'd ship it in two phases so the "never silent" guarantee lands first.

    Re: the bot's open question about constraints — Option 1 does require touching the consumer-facing caller interface: dev-lead.yml is copied into every consuming repo, so adding a repository_dispatch type is a contract change that must go through standards-sync (same as when dev-lead-reviews-retry was added). If avoiding any caller-interface change is a hard requirement, fall back to Option 2 (self-requeue) — but note a step that is killed by timeout may die before it can self-dispatch, which is exactly the transient-failure case we care about. That tradeoff is why I prefer Option 1.

    Phase 1 — Visibility & marker (satisfies "never a silent stall")

    1. In dev-lead-fix-issue.sh, on the generic-failure branch (and unify the rate-limit branch):
      • Compute attempt by counting prior <!-- dev-lead-issue N ... --> markers in the issue's comments, +1.
      • If attempt < MAX_ATTEMPTS (default 3): post a machine-readable marker comment
        <!-- dev-lead-issue ${ISSUE_NUMBER} status=failed attempt=${attempt} --> (rate-limit branch uses status=rate-limited reset=${ISO}) plus a short human-readable note.
      • If attempt >= MAX_ATTEMPTS: apply a dev-lead:needs-human label + a visible "automated retries exhausted" comment, and write no retryable marker.
    2. Result: the stall is always observable (marker comment or needs-human label) even before any auto-retry exists.

    Phase 2 — Auto-retry scanner

    1. dev-lead-retry.sh: add issue scanning that enumerates repos/${repo}/issues?state=open&labels=dev-lead, filters out PRs (.pull_request == null) and any carrying dev-lead:needs-human, then for each looks for a non-superseded dev-lead-issue marker — honoring reset= for the rate-limited case and attempt < MAX_ATTEMPTS.
    2. Add a new dev-lead-issue-retry dispatch carrying client_payload.issue_number (+ attempt).
    3. dev-lead-intent.sh: restructure the repository_dispatch branch so issue-retry reads issue_number (not pr_number), generalize the hands-off label lookup to use the issue number, and route to the existing issue intent.
    4. dev-lead.yml (stub) + dev-lead-reusable.yml: add dev-lead-issue-retry to repository_dispatch.types, and add a client_payload.issue_number → dev-lead-issue-<n> branch to the concurrency router so a retry shares the issue lane (no duplicate concurrent run). Propagate the stub change via standards-sync.
    5. Terminal/dedup: the existing check_existing_pr() in dev-lead-fix-issue.sh already prevents duplicate work once a PR exists; the attempt counter is the hard loop bound.

    Tests (acceptance criterion 3)

    • New tests/dev-lead/unit/test_dev_lead_retry.bats: issue scanning, attempt bounding, reset honoring, needs-human/terminal skip.
    • Extend test_fix_issue.bats: failure path writes the marker, increments attempt, applies dev-lead:needs-human at the ceiling.
    • New fixture repository_dispatch_issue_retry.json + classifier coverage in the test_intent_* suite.
    • Extend e2e 07-rate-limit-retry.sh (or a sibling) to exercise the failed-issue path.

    Acceptance mapping

    • First-attempt failure → bounded auto-retry or explicit needs-human/failed signal (Phase 1 guarantees the signal; Phase 2 adds the retry).
    • Attempt count bounded (MAX_ATTEMPTS=3) and observable (in marker + label).
    • Covered by a test exercising the failed-issue-implementation path.

    I've left the issue open and not started implementation — happy to proceed with Phase 1 first (lowest-risk, ships the guarantee) on claude/issue-781-analysis-ecvnhc once you confirm the approach.


    Generated by Claude Code

  3. don-petry commented on Jun 18, 2026

    @don-petry
    CollaboratorAuthor

    Revised scope (supersedes the plan above)

    Folding in failure-cause visibility. The key realization: the cause is already captured by the engine layer (run_writer redacts engine output to /tmp/dev-lead-session-output.txt and appends a "Dev-Lead session output" block to $GITHUB_STEP_SUMMARY, engine.sh:1105-1142) and already classified inside run_writer_with_fallback (engine.sh:1152-1214: any_rate_limited / any_missing / engine-error) — both are then discarded before dev-lead-fix-issue.sh sees them. So this is mostly plumbing existing signals through to the issue and the retry marker, not new capture. No new workflow — everything rides the existing Run issue step in dev-lead-reusable.yml, the existing retry cron, and the existing step-summary/artifact machinery.

    Note on the live case: the only error that escaped #478 was the generic Engine failed to implement issue #478 / exit 1 — i.e. the actual cause was never surfaced. It may still be in run 27733022075's "Dev-Lead session output" summary, but nothing diagnostic reached the issue. That blind spot is what part A/B below close.

    A. Engine layer — classify & expose the cause (scripts/engine.sh)

    1. run_writer_with_fallback: before each terminal return, write a one-line reason to /tmp/dev-lead-failure-reason (mirrors the existing /tmp/dev-lead-rate-limit-reset sidecar): rate-limited (all engines exit 2) · missing-binary (any exit 127 — deterministic/infra) · engine-error (other). No control-flow / exit-code changes.
    2. run_writer: move the redact + persist + $GITHUB_STEP_SUMMARY write so it also covers the rate-limit early-return path (currently return 2 at 1091-1095 happens before the summary block). Result: session output is visible on the run page for all failure classes, not just generic ones.

    B. Issue handler — surface cause on the issue + marker (scripts/dev-lead-fix-issue.sh)

    Replace the silent exit 1 (88-92) and unify the rate-limit branch (77-87). At failure:

    • Read /tmp/dev-lead-failure-reason (default engine-error) and /tmp/dev-lead-rate-limit-reset.
    • Compute attempt from prior <!-- dev-lead-issue ${ISSUE_NUMBER} ... --> markers + 1.
    • Build a redacted tail (~40 lines) of /tmp/dev-lead-session-output.txt and a run deep-link ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}.
    • Decision matrix:
      Reason attempt < MAX attempt ≥ MAX
      rate-limited retryable marker status=rate-limited reset=… + ack comment dev-lead:needs-human + comment
      engine-error retryable marker status=failed + cause snippet + run link dev-lead:needs-human + comment
      missing-binary no retry → dev-lead:needs-human + comment (deterministic infra; retry won't help) same
    • Marker format: <!-- dev-lead-issue ${N} status=<failed|rate-limited> attempt=${K} reason=<r> run=<id> [reset=ISO] -->. Human-readable body always includes reason + run link (+ snippet for engine-error).
    • Exit codes unchanged (exit 1/exit 2) so the workflow run still reflects failure.

    C. Retry scanner — scan issues (scripts/dev-lead-retry.sh)

    • Add scan_repo_issues: enumerate repos/${repo}/issues?state=open&labels=dev-lead&per_page=100, filter .pull_request == null, skip dev-lead:needs-human.
    • Per issue: newest non-superseded dev-lead-issue marker → honor reset= for rate-limited, enforce attempt < MAX_ATTEMPTS (default 3, per auto-rebase-retry.sh:40), skip if an open dev-lead/issue-N* PR already exists (terminal/dedup).
    • Dispatch dev-lead-issue-retry with client_payload.issue_number (+ attempt). Cause data rides the marker — scanner never re-derives it. Called from scan_repo in the same pass (same DRY_RUN/stagger semantics). Update script header.

    D. Dispatch routing (scripts/dev-lead-intent.sh)

    • Restructure the repository_dispatch branch so the early pr_number requirement (line 381) no longer drops issue-only payloads. Read dispatch_type first; for dev-lead-issue-retry, read issue_number, run the hands-off label lookup against the issue number, and route to the existing issue intent with {issue_number} context. Existing types unchanged.

    E. Trigger + concurrency wiring

    • .github/workflows/dev-lead.yml (thin caller stub): add dev-lead-issue-retry to repository_dispatch.types. Propagate to consumers via standards-sync — this is a caller-interface contract change, same path as when dev-lead-reviews-retry was added.
    • .github/workflows/dev-lead-reusable.yml: add a client_payload.issue_number → dev-lead-issue-<n> branch to the concurrency router (53-63) so a retry shares the issue lane (no duplicate concurrent run vs. a label event).

    F. Tests

    • New tests/dev-lead/unit/test_dev_lead_retry.bats: issue scan, attempt bounding, reset honoring, needs-human/terminal skip, open-PR dedup.
    • Extend test_fix_issue.bats: failure path writes marker + cause snippet + run link; missing-binary → needs-human with no retryable marker; attempt ceiling → needs-human.
    • Extend test_engine_fallback.bats: /tmp/dev-lead-failure-reason value per failure class; session output written on the rate-limit path.
    • New fixture repository_dispatch_issue_retry.json + test_intent_* coverage.
    • Extend e2e 07-rate-limit-retry.sh (or sibling) for the failed-issue path.

    G. Docs/standards

    • Update the dev-lead section of AGENTS.md / ci-standards.md and the consumer manifest for the new dispatch type and the dev-lead:needs-human label semantics.

    Acceptance mapping

    • First-attempt failure → bounded auto-retry or explicit needs-human/failed signal — never silent (A+B guarantee the signal even before C lands).
    • Attempt count bounded (MAX_ATTEMPTS=3) and observable (marker attempt= + label).
    • Failure cause is logged and visible on the issue (reason + run link + redacted snippet) and on the run summary for every failure class — and drives retry-vs-needs-human routing.
    • Covered by tests exercising the failed-issue-implementation path.

    Phasing (unchanged intent, now cause-aware)

    • Phase 1 (visibility, lowest-risk): A + B + tests. Lands the "never silent" guarantee with cause. No caller-interface change.
    • Phase 2 (auto-retry): C + D + E + standards-sync. Adds bounded auto-retry on top.

    Ready to start Phase 1 on claude/issue-781-analysis-ecvnhc on your go.


    Generated by Claude Code

  4. don-petry commented on Jun 19, 2026

    @don-petry
    CollaboratorAuthor

    Step 0 — live root cause of the #478 stall (run 27733022075)

    Pulled the failed Run issue job log (the prior session couldn't reach petry-projects/.github). The generic Engine failed to implement issue #478 / exit code 1 was hiding a 600-second per-tier timeout kill:

    Time (UTC) Event
    02:37:28 Run issue step starts; engine: claude (triage: haiku 4.5 → deep: opus 4.8 …)
    02:37:30.3 Switched to a new branch 'dev-lead/issue-478-20260618-0237' + [headroom] claude usage 0% — ok (not rate-limited)
    (10 min silence — no engine output)
    02:47:30.5 ##[error]Engine failed to implement issue #478 → exit code 1

    That's a clean 600.2s wall (02:37:30.3 → 02:47:30.5). The writer runs through run_writer_with_fallback (dev-lead-fix-issue.sh:76), and the deep tier is bounded by DEEP_TIMEOUT_SEC (default 600, engine.sh:37; the run's env had it blank, so the default applied). The opus 4.8 writer invocation ran past 600s and was SIGTERM-killed by GNU timeout (rc 124).

    Cause class: engine-error (a transient/retryable timeout — engine.sh:43 explicitly lists 124=GNU timeout as transient). Not rate-limit, not missing-binary.

    Why this is the textbook case for #781

    • The failure was transient (a single 600s timeout, no retry on the writer path) — exactly the kind of run that should auto-retry, but there is no issue-retry path, so feat: implement issue #404 — PR Review Agent — failures detected 2026-05-28 #478 stranded.
    • The real cause (timeout/rc=124) never escaped — only the generic exit 1 reached the run; nothing reached the issue. That's the visibility blind spot Phase 1 (A: failure-reason sidecar + B: cause-on-issue marker/comment) closes, and the missing auto-retry is Phase 2 (C/D/E).

    Proceeding with Phase 1 → Phase 2 on claude/issue-781-analysis-ecvnhc.

  5. 50 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsneeds-human-reviewFlagged by automated PR review agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions