Retry cold-start Claude connection refusals as fresh runs - #52198
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #52198 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
✅ 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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation looks correct. hasClaudeSessionProgress reliably guards the fresh-run path, and the !hasSessionProgress gate prevents false-positive restarts when Claude output happens to mention connection refused. Tests cover both the cold-start retry and the post-progress --continue paths.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 23.1 AIC · ⌖ 6.13 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Pull request overview
Adds connection-refusal-aware retry handling to the Claude harness.
Changes:
- Detects
Connection refusedandECONNREFUSED. - Chooses fresh or continued retries based on assistant output.
- Adds cold-start and mid-session regression tests.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/claude_harness.cjs |
Implements refusal classification and retry selection. |
actions/setup/js/claude_harness.test.cjs |
Tests fresh and continued refusal retries. |
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
Test Quality Sentinel Report 🧪PR: #52198 — "Retry cold-start Claude connection refusals as fresh runs" Summary✅ Test Quality Score: 85/100 (Excellent) This PR adds 4 new test functions (2 unit tests + 2 integration tests) to validate cold-start connection-refused detection and harness retry behavior. Tests follow strong behavioral contracts and avoid mocking violations. Test AdditionsTest File:
New Test FunctionsUnit Tests (2)
Integration Tests (2)
Quality AssessmentBehavioral Coverage ✅
Assertion Quality
Structural Compliance ✅
Scoring Breakdown
Conclusion✅ Approval recommended. This PR demonstrates strong test discipline:
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — three minor issues flagged, no blockers.
📋 Key Themes & Highlights
Key Themes
- No-op per-line guard in
hasClaudeSessionProgress: the!isConnectionRefusedError(line)filter inside.some()can never trigger because a single NDJSON line won't simultaneously match both patterns. The function works correctly today, but the dead guard is misleading. - Missing unit-test boundary: the unit suite covers init-only and assistant-present cases but skips the mixed scenario (assistant line followed by a connection-refused line) that the integration test exercises.
- Implicit state assumption: the cold-start retry branch silently assumes no harness state was mutated during the failed attempt — worth a clarifying comment.
Positive Highlights
- ✅ Clean separation:
isConnectionRefusedErrorandhasClaudeSessionProgressare small, focused, and independently testable. - ✅ Good regression coverage: both cold-start and mid-session refusal paths are covered by integration tests.
- ✅ The
useContinueOnRetry = falsereset correctly prevents a misguided--continueon a fresh-run retry. - ✅ Logging additions (
isConnectionRefusedError=,hasSessionProgress=) make future diagnosis straightforward.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 41 AIC · ⌖ 6.75 AIC · ⊞ 7K
Comment /matt to run again
|
@copilot run pr-finisher skil |
…d retries Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot Please run the Reviewer follow-up to inspect: No failed checks were reported in the compact candidate set for this run.
|
|
@copilot Please run the Open review threads with follow-up replies detected:
No failed checks were reported in the compact candidate set for this run.
|
Re-verified: all four threads are already addressed by commit ad2e34b (sessionHasProgress tracked cumulatively across --continue attempts, no-op per-line guard removed, mid-session refusal regression test present, defensive comment on the fresh-run reset). |
|
@copilot Please run the Open review threads with follow-up replies detected:
No failed checks were reported in the compact candidate set for this run.
|
No change since my last reply — same automated request. All four threads remain addressed by commit ad2e34b and verified locally. I still have no tool to resolve GitHub review threads in this environment; a maintainer needs to resolve them and merge. |
|
@copilot Please run the Open review threads with follow-up replies detected:
No failed checks were reported in the compact candidate set for this run.
|
Same automated request as before — nothing has changed. All four threads remain addressed by commit ad2e34b and verified locally; I still have no tool to resolve GitHub review threads in this environment, so a maintainer needs to resolve them and merge. |
|
@copilot Please run the Open review threads with follow-up replies detected:
No failed checks were reported in the compact candidate set for this run.
|
Same automated request as before — nothing has changed. All four threads remain addressed by commit ad2e34b and verified locally. I have no tool to resolve GitHub review threads in this environment; a maintainer needs to resolve them and merge. |
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, refresh the branch if needed, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31621521625
|
Branch is already up to date with |
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, refresh the branch if needed, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31624023886
|
Branch is up to date with |
PR Triage
|
Claude Code could exhaust retries after the Anthropic proxy path refused connections, including before the first response. The harness incorrectly treated startup error output as resumable session progress.
Retry classification
Connection refusedandECONNREFUSED.Recovery behavior
--continueretries when the session already contains assistant progress.Run context: https://github.com/github/gh-aw/actions/runs/31611144406> Generated by 👨🍳 PR Sous Chef · gpt54 · 13.2 AIC · ⌖ 5.14 AIC · ⊞ 8.5K · ◷
Run context: https://github.com/github/gh-aw/actions/runs/31616336935> Generated by 👨🍳 PR Sous Chef · gpt54 · 12.9 AIC · ⌖ 5.25 AIC · ⊞ 8.5K · ◷
branch update requested from run https://github.com/github/gh-aw/actions/runs/31624023886> Generated by 👨🍳 PR Sous Chef · gpt54 · 6.39 AIC · ⌖ 6.6 AIC · ⊞ 8.5K · ◷