Retry Claude runs after AWF/MCP startup failures#55864
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the implementation label and has 0 new lines of code in business logic directories (threshold: 100).
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
Non-blocking from a code-quality perspective.
Reviewed areas
- Fresh-run retry path for AWF/MCP startup failures before Claude session progress
- Exit-code extraction from unstructured AWF startup logs
- Regression coverage for both behaviors
I did not find a changed-line correctness, reliability, or maintainability issue strong enough to block this PR.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 4.17 AIC · ⌖ 6.85 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
One small simplification opportunity in the AWF retry path. net: -8 lines possible.
Generated by ✂️ Ponytail Reviewer for #55864 · codex · mai10 · 6.43 AIC · ⌖ 3.3 AIC · ⊞ 16.7K
Comment /ponytail to run again
🧪 Test Quality Sentinel Report - APPROVED✅ Score: 100/100 — Excellent Analysis Summary:
Test 1: Fresh retry detection (claude_harness.test.cjs:704) Test 2: Exit code extraction (log_parser_bootstrap.test.cjs:82) Verdict: ✅ Passed. High-value design contracts with complete edge-case coverage. No violations.
|
There was a problem hiding this comment.
Pull request overview
Adds recovery and diagnostics for Claude startup failures.
Changes:
- Detects AWF/MCP startup failures and attempts fresh retries.
- Extracts AWF exit codes when structured logs are absent.
- Adds regression tests for both behaviors.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/claude_harness.cjs |
Adds startup-failure retry classification. |
actions/setup/js/claude_harness.test.cjs |
Tests fresh startup retries. |
actions/setup/js/log_parser_bootstrap.cjs |
Extracts AWF process exit codes. |
actions/setup/js/log_parser_bootstrap.test.cjs |
Tests exit-code diagnostics. |
Review details
Suppressed comments (1)
actions/setup/js/claude_harness.cjs:634
- When this startup failure repeats after the default one fresh retry, the startup budget condition becomes false and execution falls through to the generic
result.hasOutputbranch below.shouldRetryWithContinue()then returns true, so the next invocation incorrectly uses--continueeven thoughsessionHasProgressis still false. Handle every matched no-progress startup failure in this branch and stop once its startup budget is exhausted.
if (awfStartupFailure && !sessionHasProgress && attempt < maxRetries && startupRetriesUsed < startupRetryLimit) {
startupRetriesUsed++;
useContinueOnRetry = false;
log(`attempt ${attempt + 1}: no Claude session progress — retrying startup as fresh run ` + `(startup retry ${startupRetriesUsed}/${startupRetryLimit}, next attempt ${attempt + 2}/${maxRetries + 1})`);
return { action: "retry", nextDelayMs: initialDelayMs };
}
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
Review: Retry Claude runs after AWF/MCP startup failures
Overall: The approach is sound — detecting AWF/MCP startup failures early and retrying as a fresh run (not --continue) is the right strategy. Two non-blocking issues worth addressing before merging:
1. AWF_STARTUP_FAILURE_PATTERN is over-broad (line 101, claude_harness.cjs)
The .*? between the prefix (awf, mcp gateway) and the failure keyword allows any intervening text. A post-startup error line that mentions awf as context (e.g. awf session 5: step failed) would match and trigger an unnecessary fresh-run retry. Tightening the pattern (bounding the gap or matching specific known phrases) would reduce false positives.
2. Unguarded case: awfStartupFailure && sessionHasProgress (line ~632, claude_harness.cjs)
When both flags are true the code silently falls into the normal --continue retry path, which the comment says is "not viable" for startup failures. Adding an explicit log entry (or a guard that prevents --continue when awfStartupFailure) for this edge case would make the invariant auditable and prevent silent misbehavior.
Both are COMMENT-level suggestions; no blocking correctness or security issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 25.5 AIC · ⌖ 9.38 AIC · ⊞ 6.2K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes on correctness and test coverage gaps.
📋 Key Themes & Highlights
Key Themes
- Regex over-breadth (
claude_harness.cjs:101):AWF_STARTUP_FAILURE_PATTERNcan match mid-run MCP error lines, causing false-positive fresh retries. - Pattern as sole gate (
claude_harness.cjs:629): If AWF emits output that doesn't match the pattern, the code falls through to--continue— the bug this PR is meant to fix. - Missing unit tests for the predicate (
claude_harness.test.cjs):isAWFStartupFailurehas no isolated true/false unit tests; only the integration stub exercises it. - Possible test-discovery gap (
log_parser_bootstrap.test.cjs:82): The new test is inserted as a comma expression, which can silently prevent registration in some Vitest setups.
Positive Highlights
- ✅ Clean separation of
isAWFStartupFailureas an exported predicate — matches the project's existingisXfunction pattern. - ✅
useContinueOnRetry = falseis set beforereturn { action: "retry" }— correct and safe ordering. - ✅ Good regression test for the happy path (retry succeeds on second attempt without
--continue). - ✅ AWF exit code extraction in
log_parser_bootstrap.cjsis a nice targeted fix with clear fallback semantics.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 46.9 AIC · ⌖ 10.4 AIC · ⊞ 7.6K
Comment /matt to run again
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32907942669
|
|
@gh-aw-bot Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Fixed in 46c4311. I moved AWF startup retry handling to the generated outer AWF invocation, removed the in-harness AWF startup regex/helper, added outer-wrapper regression coverage, verified the log parser test is discovered, and ran targeted Go/JS validation. |
Completed. The branch is refreshed against |
|
Triage update: Category: bug | Risk: high | Score: 70/100 (impact 38 + urgency 22 + quality 10) | Action: fast_track | Batch: workflow-runtime
|
|
Triage update: Category: bug | Risk: high | Score: 70/100 (impact 38 + urgency 22 + quality 10) | Action: fast_track | Batch: workflow-runtime
|
1 similar comment
|
Triage update: Category: bug | Risk: high | Score: 70/100 (impact 38 + urgency 22 + quality 10) | Action: fast_track | Batch: workflow-runtime
|
|
$ Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32972030991
|
…ix-cli-startup-error Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Completed in 4a982ce. I refreshed the branch against |
|
@copilot apply to all agentic engine harnesses |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32979958387
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 0c032f6. The shared AWF wrapper now applies startup-failure fresh retries to built-in harnessed engines (Claude, Codex, Copilot, Gemini, and Pi) using engine-specific harness markers, with updated regression coverage, docs, wasm goldens, and compiled workflow locks. |
Completed in 0c032f6. The shared AWF wrapper applies startup-failure fresh retries to the remaining built-in agent harnesses: Claude, Codex, Copilot, Gemini, and Pi. I verified the production harness inventory, confirmed all existing review threads are resolved, checked Sous Chef run 32979958387 (no failed jobs), and reran |
|
@copilot refactor the generated code into a shell script |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in ceceeba. The generated AWF retry loop now delegates to |
|
/smoke-copilot |
|
📰 BREAKING: Smoke Copilot is now investigating this issue comment. Sources say the story is developing... |
Avenger intermittently fails before Claude emits structured logs when AWF or MCP gateway startup fails. These failures were treated as resumable partial runs, causing invalid
--continueretries and opaqueexitCode=unknowndiagnostics.Fresh startup retries
--continue.Actionable failure diagnostics
Process exiting with code: Noutput when the harness did not emit its own completion status.Regression coverage