Fix Claude harness retrying invalid-JSON body error with --continue instead of a fresh run - #51793
Conversation
…resh run Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a Claude harness mitigation for invalid-JSON API failures caused by corrupted resumable sessions.
Changes:
- Detects invalid-JSON request-body errors.
- Forces fresh retries and permanently disables
--continue. - Adds classifier-focused tests.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/claude_harness.cjs |
Implements detection, diagnostics, and fresh-run retry behavior. |
actions/setup/js/claude_harness.test.cjs |
Tests invalid-JSON error classification. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| if (isInvalidJsonBody && attempt < maxRetries && result.hasOutput) { | ||
| useContinueOnRetry = false; | ||
| continueDisabledPermanently = true; | ||
| log(`attempt ${attempt + 1}: invalid JSON request body (transport-level serialization bug, likely following a permission_denied) — retrying as fresh run (--continue disabled permanently, attempt ${attempt + 2}/${maxRetries + 1})`); | ||
| continue; |
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ Test Quality Sentinel completed test quality analysis.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #51793 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix is correct and well-structured. The isInvalidJsonBody check correctly precedes the generic --continue retry, continueDisabledPermanently prevents future regressions, and the result.hasOutput guard is consistent with other branches. Tests cover the classifier thoroughly. The existing inline comment about adding integration/retry-policy tests is the only outstanding suggestion.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 22.1 AIC · ⌖ 7.07 AIC · ⊞ 5.4K
Test Quality Sentinel 🧪 — PR #51793Test Quality Score: 100/100 ✅ Excellent SummaryThis PR adds comprehensive test coverage for the new Coverage Quality:
Test Breakdown
Quality Signals✅ Design invariant protection: All 8 tests verify that the function correctly distinguishes ✅ Real-world coverage: Test case 3 directly mirrors the failing scenario from issue #51790 (permission_denied → corrupted JSON → 400 error). ✅ False-positive prevention: 5 negative assertions ensure the classifier doesn't misfire on other error types — critical for a branching control-flow classifier. ✅ Edge case handling: Case-insensitivity check ensures robustness across API output variations. ✅ No test inflation: 36 test lines for 43 lines of production code is a healthy 0.84:1 ratio. ✅ Pattern classifier correctness: Simple, clear assertions on regex behavior — no hidden implementation details or implicit mocking. Status: ✅ APPROVED — Test Quality Sentinel analysis complete.
|
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 36.4 AIC · ⌖ 7.42 AIC · ⊞ 7.1K
Comment /matt to run again
| // command. Retrying with --continue would resend the same corrupted on-disk | ||
| // session state and reproduce the identical error, so force a fresh run and | ||
| // permanently disable --continue for the remainder of this driver invocation. | ||
| if (isInvalidJsonBody && attempt < maxRetries && result.hasOutput) { |
There was a problem hiding this comment.
[/diagnosing-bugs] When isInvalidJsonBody is true but attempt >= maxRetries or !result.hasOutput, the guard is skipped and execution falls through to the generic partial-execution branch — which may still attempt a --continue retry, exactly the bug this PR intends to prevent.
💡 Suggested fix — mirror the isNoDeferredMarker pattern
if (isInvalidJsonBody) {
if (attempt < maxRetries && result.hasOutput) {
useContinueOnRetry = false;
continueDisabledPermanently = true;
log(`attempt ${attempt + 1}: invalid JSON request body ... retrying as fresh run`);
continue;
}
// exhausted retries or no output — stop; do NOT fall through to generic branch
break;
}isNoDeferredMarker already does this: it has an explicit break for the non-retriable sub-case (line ~582). The new block lacks that safeguard.
@copilot please address this.
There was a problem hiding this comment.
Fixed in fe68e68: the isInvalidJsonBody block now mirrors isNoDeferredMarker — it has an explicit break when retries are exhausted or there's no output, so it no longer falls through to the generic partial-execution --continue retry. Also added a stubbed harness-level regression test covering this retry-policy branch.
There was a problem hiding this comment.
Generated by ✂️ Ponytail Reviewer for #51793 · auto · 19.9 AIC · ⌖ 3.28 AIC · ⊞ 6.8K
Comment /ponytail to run again
| continueDisabledPermanently = true; | ||
| log(`attempt ${attempt + 1}: invalid JSON request body (transport-level serialization bug, likely following a permission_denied) — retrying as fresh run (--continue disabled permanently, attempt ${attempt + 2}/${maxRetries + 1})`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
L574-595: shrink: isNoDeferredMarker and isInvalidJsonBody blocks are identical (same reset + log + continue). if ((isNoDeferredMarker || isInvalidJsonBody) && attempt < maxRetries && result.hasOutput), ~10 fewer lines.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
…e, close IP-2864 AC gaps - Re-pin v0.85.4 -> v0.86.2 and record the pin + its reason in the workflow header (IP-2864 AC). v0.86.2 carries the Claude-harness retry fix (github/gh-aw#51793) whose trigger scenario is exactly this workflow's engine + strict bash allow-list, plus enforced secret redaction in step summaries and patch artifacts (#50777/#50778). Verified behaviour-neutral for the DOCS-2 write strategy: safe-output schemas and the close_older_pull_requests handler are unchanged between the two versions. - Release notes now travel hub-side, not in the dispatch payload: a pre-agent step mints a token from the read-only source App (Releases API is covered by contents: read - no permission change) and writes the body to ./source/RELEASE_NOTES.md. The payload lands in a public repo's workflow run, and source release bodies now carry real content naming private infrastructure (desci-infra 2.0.0, 2026-08-12). Contract change lands in desci-infra#675 in the same breath. - Add the claude-code-action fallback note (IP-2864 AC), flag it as unvalidated for repository_dispatch. - Relevance gate: add docs/service-auth.md (mapped for authentication and roles pages, previously unreachable); document that the gate is a deliberate triggering subset of the map, with the ride-along rows named in the knowledge base. - Knowledge base: account for the previously unmapped pages (technical-deep-dive/onchain-lab.md, module-registry/**, data/README.md, and the orphan api-reference/IPNFT-api.md) in the out-of-scope list. - stop-after refreshed at compile: expiry is now 2026-08-28 10:16:52. If this PR has not merged by then, run `gh aw compile --refresh-stop-time` again before merging or the pilot arrives expired. Recompiled with gh-aw v0.86.2; `gh aw validate --strict` and `gh aw lint` both clean; agent job verified still contents: read only, bots allow-list and max-ai-credits unchanged in the regenerated lock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A
permission_deniedon a compound Bash command is sometimes followed, on the very next turn, by the Anthropic API rejecting the request with400 The request body is not valid JSON. The harness's failure classifier had no bucket for this signature, so it fell through to the generic "partial execution" path and retried with--continue— resending the same corrupted on-disk session state and reproducing the identical error, burning the run's last retry.Changes
INVALID_JSON_BODY_ERROR_PATTERN/isInvalidJsonBodyError()inactions/setup/js/claude_harness.cjsto recognize"request body is not valid JSON"in the CLI output.--continue, and permanently disables--continuefor the rest of the driver invocation — same treatment as the existingisNoDeferredMarkerErrorpath, since resuming would just resend the same broken session state.isOverloadedError,isRateLimitError, etc.) for easier triage of future occurrences.isInvalidJsonBodyErrorinclaude_harness.test.cjs, including the true-to-log-format scenario from the failing run.Note: the underlying serialization bug lives inside the third-party Claude Code CLI binary, outside this repo, so it can't be patched directly — this change is the harness-side mitigation so a single serialization glitch doesn't consume the whole retry budget.