feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) - #1021
Conversation
…s reason=timeout and escalate to human (no same-budget retry)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds first-class timeout handling to dev-lead engine failure flow: ChangesTimeout classification and escalation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant EngineSh as engine.sh
participant Sidecar as /tmp/dev-lead-* sidecars
participant FixIssue as dev-lead-fix-issue.sh
participant GitHub as GitHub Issue
EngineSh->>EngineSh: run_writer_with_fallback starts
EngineSh->>Sidecar: clear stale timeout sidecars
EngineSh->>EngineSh: run_writer (rc=124)
EngineSh->>Sidecar: write reason=timeout, tier, budget, elapsed
FixIssue->>Sidecar: read reason and timeout metadata
alt reason=timeout
FixIssue->>FixIssue: check for unpushed local changes
FixIssue->>GitHub: post needs-human comment (tier, elapsed, budget)
FixIssue->>FixIssue: exit 1 (no retry marker)
else other failure
FixIssue->>GitHub: post retry marker (status=failed)
end
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces first-class handling for stage timeouts (exit 124) in the dev-lead workflow. Instead of triggering a same-budget retry, timeouts are treated as non-retryable and immediately escalated to a human by applying the dev-lead:needs-human label and posting a detailed comment. The changes include capturing elapsed time and budget context, clearing stale sidecar files, and preserving/notifying about unpushed completed work. Unit tests were added to verify these behaviors. A review comment points out a redundant git stub block in tests/dev-lead/unit/test_fix_issue.bats that is immediately overwritten by a subsequent stub block and should be removed.
There was a problem hiding this comment.
Pull request overview
Implements issue #1018 by treating GNU timeout exit code 124 as a first-class non-retryable failure in the dev-lead writer pipeline, persisting reason=timeout (plus tier/budget/elapsed sidecars) and escalating immediately to a dev-lead:needs-human outcome without emitting a retry marker.
Changes:
- Classify exit 124 in
run_writer_with_fallback()asreason=timeoutand persist timeout context sidecars (tier,budget,elapsed), while ensuring no fallback engine is attempted. - In
dev-lead-fix-issue.sh, add an earlyreason=timeoutbranch that escalates to needs-human immediately (no same-budget retry marker) and includes tier/elapsed/budget + split/raise guidance. - Add unit tests covering timeout classification, no-fallback behavior, escalation behavior, and “completed work not silently discarded” signaling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/engine.sh |
Writes reason=timeout and timeout context sidecars on exit 124 and avoids fallback-engine retries for timeouts. |
scripts/dev-lead-fix-issue.sh |
Escalates reason=timeout immediately to needs-human with detailed guidance and a “work completed but not pushed” note when applicable. |
tests/dev-lead/unit/test_engine_fallback.bats |
Adds coverage for timeout sidecar classification and ensures no same-budget fallback to another engine on 124. |
tests/dev-lead/unit/test_fix_issue.bats |
Adds coverage for timeout escalation behavior, non-timeout transient retry behavior, and late-phase timeout with completed work signaling. |
…d test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@donpetry-bot review |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/engine.sh (1)
1314-1348: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake exit 124 win before rate-limit normalization.
Line 1314 receives
run_writer’s normalized status. Inrun_writer, a non-zero rc can still be converted to2when captured output matchesis_rate_limited_files, so an actual124can bypass the Line 1341 timeout branch and re-enter fallback/retry handling. Exclude124from that remap.Proposed fix
- if [ "$rc" -ne 0 ] && [ -n "$_tmp" ] && is_rate_limited_files "$_tmp"; then + if [ "$rc" -ne 0 ] && [ "$rc" -ne 124 ] && [ -n "$_tmp" ] && is_rate_limited_files "$_tmp"; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/engine.sh` around lines 1314 - 1348, The retry/timeout handling in engine.sh is letting a real timeout be normalized into a rate-limit exit and then fall through to fallback behavior; update the status mapping around run_writer and the rc handling so exit 124 is preserved and cannot be remapped to 2 by the rate-limit path. In the logic near run_writer, set/read rc so the timeout branch remains authoritative, and keep the special 124 classification in the main fallback loop so it reaches the timeout reason files instead of continuing to the next engine.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/dev-lead-fix-issue.sh`:
- Around line 128-176: The timeout handling in the main reason-dispatch block
duplicates the same human-escalation flow already used by the missing-binary
path. Extract the shared echo/label/comment/exit-1 sequence into a helper such
as escalate_to_human in scripts/dev-lead-fix-issue.sh, and have both the timeout
branch and the existing missing-binary branch call it with only their
reason-specific message/body content.
In `@tests/dev-lead/unit/test_engine_fallback.bats`:
- Around line 317-330: The timeout fallback test currently only verifies Gemini
is not retried, so it can miss an unwanted Copilot retry. Update the `sidecar:
timeout (124) does NOT fall through to another engine` test in
`test_engine_fallback.bats` to also stub or record `copilot_chat`, since the
fallback order goes through Copilot before Gemini. After `run
run_writer_with_fallback`, assert that both `copilot_chat` and `gemini` were not
called, keeping the existing timeout and failure-reason checks intact.
---
Outside diff comments:
In `@scripts/engine.sh`:
- Around line 1314-1348: The retry/timeout handling in engine.sh is letting a
real timeout be normalized into a rate-limit exit and then fall through to
fallback behavior; update the status mapping around run_writer and the rc
handling so exit 124 is preserved and cannot be remapped to 2 by the rate-limit
path. In the logic near run_writer, set/read rc so the timeout branch remains
authoritative, and keep the special 124 classification in the main fallback loop
so it reaches the timeout reason files instead of continuing to the next engine.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56c473b9-433a-4a55-8a6e-d4b61b36553a
📒 Files selected for processing (4)
scripts/dev-lead-fix-issue.shscripts/engine.shtests/dev-lead/unit/test_engine_fallback.batstests/dev-lead/unit/test_fix_issue.bats
Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU
|
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 00c0317f997373e6d504564ee7b799561edb43f4
Review mode: triage-approved (single reviewer)
Summary
Implements issue #1018: classifies per-tier stage timeouts (exit 124) as a first-class, non-retryable 'timeout' failure reason in engine.sh, and adds an immediate needs-human escalation branch in dev-lead-fix-issue.sh (before the retry/attempt-ceiling logic) that surfaces tier/elapsed/budget, split-or-raise-budget guidance, and any completed-but-unpushed work. Five new bats tests cover timeout classification, no cross-engine same-budget retry, escalation content, transient (rate-limit) retry non-regression, and late-phase-timeout work surfacing. Triage assessment confirmed; classified MEDIUM (non-trivial logic in Actions-executed orchestration scripts) which is within auto-approve bounds.
Linked issue analysis
Closes #1018 (timeout Story B/3). All six acceptance criteria are substantively addressed: (1) exit 124 → reason=timeout distinct from engine-error, written to the existing /tmp sidecar mechanism plus new tier/budget/elapsed sidecars (stale copies cleared at run start); (2) no same-budget retry — engine.sh returns immediately on 124 without falling through to fallback engines, and fix-issue escalates on first occurrence with no retry marker (status=needs-human, so the retry cron skips it); (3) the needs-human comment includes tier, elapsed, budget, and split/raise vars.${raise_var} guidance; (4) non-124 transients (rate-limit) still take the retry path — covered by a dedicated regression test; (5) completed-but-unpushed work is surfaced in the comment (the issue's 'at minimum surface it' option), honestly noting the runner-local branch is not recoverable; (6) tests added in tests/dev-lead/unit/ for both scripts.
Findings
No blocking findings. Verified: the new timeout branch in handle_engine_failure references locals (pre_engine_sha, branch, snippet, run_url, attempt) that are dynamically in scope — the handler is invoked from the function that declares them. Sidecar-file pattern in /tmp matches the pre-existing dev-lead-failure-reason mechanism (no new attack surface on ephemeral runners). Minor, non-blocking: engine.sh only ever writes tier=action in this path, while the fix-issue reader also handles deep/writer tiers and defaults to DEEP_TIMEOUT_SEC when the sidecar is absent — consistent with the deep tier being classified elsewhere. All 5 prior bot review threads (gemini, Copilot, CodeRabbit) are resolved; CodeRabbit's changes-requested review was dismissed after the dev-lead review-changes pass concluded no changes were needed. The run_secret_scanning MCP tool was not available in this environment; the gitleaks CI check passed.
CI status
All completed checks green: shellcheck, bats, unit-tests, CodeQL (actions + python), gitleaks secret scan, AgentShield, Agent Security Scan, SonarCloud quality gate, validate-fixtures, holdout-guard, and all stub/permission guards SUCCESS. Cancelled entries (dev-lead dispatch / review relay) are superseded automation runs, not failures; dependency-audit sub-jobs skipped (no matching ecosystems). CodeRabbit status SUCCESS.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Superseded by automated re-review at
|
Superseded by automated re-review at 00c0317.
|
@donpetry-bot review |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
@donpetry-bot I'm on it — starting a fresh review now. Results will appear in a few minutes. |
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #1021 |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 00c0317f997373e6d504564ee7b799561edb43f4
Review mode: triage-approved (single reviewer)
Summary
Confirmation review (triage-approved mode) of the exit-124 timeout handling fix. The PR classifies a per-tier stage timeout (GNU timeout exit 124) as a first-class, non-retryable reason=timeout in scripts/engine.sh (with tier/budget/elapsed sidecars, stale copies cleared at the start of each run), and adds an escalation branch in scripts/dev-lead-fix-issue.sh that applies dev-lead:needs-human and posts a comment with tier, elapsed, budget, and split/raise guidance on the FIRST occurrence — no retry marker, no same-budget fallback-engine retry. Completed-but-unpushed work is surfaced honestly as unrecoverable context. The triage assessment holds: scope is tight, changes match the linked issue, and prior reviewer feedback was addressed in follow-up commits.
Linked issue analysis
Closes #1018 ([dev-lead timeout B/3]). All six acceptance criteria are substantively addressed: (1) exit 124 → distinct reason=timeout sidecar; (2) timeout is never retried at the same budget — the classification happens on the immediate-propagation path and a dedicated test asserts neither copilot nor gemini fallback runs; (3) the needs-human comment includes tier + elapsed + budget + split-or-raise-vars.*TIMEOUT_SEC guidance; (4) a regression test confirms non-124 transients (rate-limit) still take the retry path; (5) late-phase timeouts with prior commits/staged changes surface a note that work existed but was not pushed (wording made honest per review feedback — the ephemeral-runner branch is not recoverable); (6) bats coverage added in both test_engine_fallback.bats and test_fix_issue.bats.
Findings
No blocking findings.
- All 5 inline review threads (gemini-code-assist, Copilot, CodeRabbit) are resolved; CodeRabbit's final review is APPROVED. Feedback was addressed in commits 304b805 (honest unrecoverable-work wording, dead git-stub removal) and 00c0317 (assert no copilot OR gemini fallback on 124).
- Informational: one resolved thread reply mentions extracting a shared
escalate_to_humanhelper, but the head diff keeps the timeout branch inline — consistent with the earlier agreed decision (with CodeRabbit) to keep escalation branches explicit in this behavior-fix PR and consolidate under #901. - Informational: the timeout classification is wired into
run_writer_with_fallback(action tier, hardcodedtier=action); the escalation handler tolerates missing sidecars with safe defaults, so other tiers degrade gracefully. - Secret scan:
run_secret_scanningMCP tool not available in this run; gitleaks CI check passed and the diff contains only obvious test-fixture stub tokens.
CI status
All checks on the latest runs are green: shellcheck, bats, unit-tests, CodeQL, SonarCloud (quality gate passed), gitleaks secret scan, agent-shield, holdout-guard, validate-fixtures, and all structural guards SUCCESS. CANCELLED entries are superseded runs of the same workflows; SKIPPED entries are inapplicable ecosystem audits.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason=timeout and escalate to human (no same-budget retry) (#1021) * feat: implement issue #1018 — [dev-lead timeout B/3] Catch exit-124 as reason=timeout and escalate to human (no same-budget retry) * fix(timeout-escalation): honest unrecoverable-work wording + drop dead test stub Address advisory review on #1021: - Copilot: the completed-but-unpushed work_note overpromised ("not silently lost", snippet "captured the changes"). The runner-local branch is not pushed and the runner is ephemeral, so the work is NOT recoverable from the run. Reword to state that honestly: surface that work was attempted (not a silent black hole), but it is unrecoverable and the snippet is context only; re-run after splitting/raising the budget to regenerate. - Gemini + Copilot: remove the redundant first git stub in the late-phase timeout test (immediately overwritten by the rev-parse-counter stub). shellcheck clean; fix-issue bats green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU * test(timeout): assert no fallback to copilot OR gemini on a 124 timeout Address CodeRabbit on #1021: the 'does NOT fall through' test only recorded gemini, so a regression that retried copilot (next in the claude→copilot→gemini order) and returned before gemini could still pass. Enable copilot (non-ghp token + gh-copilot recording stub) and assert BOTH fallback engines are untouched on a 124 timeout. The escalation-branch dedup (timeout / missing-binary / exhausted) is deferred to a shared-helper follow-up under #901 to keep this behavior fix focused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes #1018
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit