ci: raise check-fixer max_retries to 10

[jwbron]

Summary

• Bump defaults.max_retries and every per-job override in shared/check-fixers.yml from 2/3 to 10.
• The previous cap escalated to a human after just a couple of attempts, which was too aggressive for PRs with multiple small fixable failures (lint, formatting, YAML trailing whitespace, etc.).

Test plan

• Land on a PR with a fixable failure and confirm the check-fixer makes more than 3 attempts before escalating.
• Confirm the escalation comment still fires when the new cap of 10 is exhausted.

[egg-reviewer]

Comment review — PR is already merged, posting observations for the record.

Summary

The change is a pure configuration bump: max_retries: 2/3 → 10 across defaults and every per-job override. No code-path changes. The retry/escalation logic in action/build-check-fixer-prompt.sh:111-119 (if attempts >= job_max: → escalation) and the state-increment steps in .github/workflows/reusable-check-fixer.yml:378-424 continue to work as before, just escalating later.

No correctness, security, or robustness issues introduced. No tests are required for a value change of this shape.

Non-blocking observations

1. Flat 10 ignores workflow cost asymmetry

shared/check-fixers.yml mixes very different jobs under the same cap:

• Lint / Python, Shell, YAML — sonnet, 15 min, often deterministically fixable, with non_llm_fix shortcuts on the first attempt. A cap of 10 here is cheap and matches the stated motivation ("PRs with multiple small fixable failures").
• Test / Unit Tests — opus, 15 min.
• Test / Integration Tests / Integration Tests — opus, 30 min.

Each fixer attempt also triggers a full CI re-run (~30+ min) before the next cycle, so 10 attempts on a stuck integration-test PR represents on the order of ~10 hours of compute (fixer cycles + CI re-runs) before a human is paged. The motivating case (multiple small lint fixes that need more than 3 passes to clear) doesn't really apply to opus-driven integration tests, which are typically either deterministically fixable in 1–2 passes or not LLM-fixable at all.

Consider differentiating: e.g. keep Lint at 10, but cap Test workflows lower (4–5). This preserves the headroom for the actual motivating case without quintupling worst-case spend on workflows where the marginal attempts rarely succeed.

2. No cycle / progress detection

The state counter (reusable-check-fixer.yml:378-424) increments unconditionally per attempt — there's no check for "did this attempt actually reduce the failure set" or "are we oscillating between two fixes." With cap=3, an oscillating LLM burned ~1.5 cycles before a human took over. With cap=10, it burns ~5. That's not introduced by this PR, but raising the cap makes the worst case meaningfully worse. Worth keeping in mind if you start seeing the autofixer thrashing on a PR — escalating sooner is cheap.

3. Test plan was not executed

The PR description's test plan has both items unchecked:

• Land on a PR with a fixable failure and confirm the check-fixer makes more than 3 attempts before escalating.
• Confirm the escalation comment still fires when the new cap of 10 is exhausted.

The second item could have been validated pre-merge by temporarily lowering the cap or by exercising the prompt builder directly with a synthesized AUTOFIX_STATE JSON where attempts >= max. Since the change landed without verification, worth watching the next stuck PR to confirm the escalation path still fires at the new cap.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs