Skip to content

Commit 2990c70

Browse files
OAGrclaude
andcommitted
fix(pr-patrol): reduce compute waste with issue-type budgets and timeout abandonment
PR Patrol was burning 90%+ of compute on failed attempts: - Timeouts (30 min each) didn't count toward abandonment, causing infinite retries - All issue types got the same 40-turn / 30-min budget regardless of complexity - No early-exit guidance in prompts, so Claude kept trying unfixable issues - Reflection used expensive Sonnet model for simple log analysis Changes: - Unify failure tracking: timeouts now count toward abandonment (2 failures = abandoned) - Add per-issue-type budgets: missing-issue-ref gets 5 turns/3 min, ci-failure gets 25/15, etc. - Add "when to stop early" section to prompts with clear unfixable-scenario detection - CI failure prompt now explicitly lists human-required checks to skip immediately - Reflection uses haiku model with 5-min timeout instead of sonnet/30-min - Include pr-patrol tests in vitest config - Add computeBudget tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dacc446 commit 2990c70

File tree

3 files changed

+153
-27
lines changed

3 files changed

+153
-27
lines changed

crux/pr-patrol/index.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
checkMergeEligibility,
44
findMergeCandidates,
55
detectIssues,
6+
computeBudget,
67
type GqlPrNode,
78
} from './index.ts';
89

@@ -383,6 +384,46 @@ describe('findMergeCandidates', () => {
383384
});
384385
});
385386

387+
// ── computeBudget ────────────────────────────────────────────────────────────
388+
389+
describe('computeBudget', () => {
390+
it('gives small budget for missing-issue-ref only', () => {
391+
const budget = computeBudget(['missing-issue-ref']);
392+
expect(budget.maxTurns).toBe(5);
393+
expect(budget.timeoutMinutes).toBe(3);
394+
});
395+
396+
it('gives small budget for missing-testplan only', () => {
397+
const budget = computeBudget(['missing-testplan']);
398+
expect(budget.maxTurns).toBe(8);
399+
expect(budget.timeoutMinutes).toBe(5);
400+
});
401+
402+
it('gives medium budget for ci-failure', () => {
403+
const budget = computeBudget(['ci-failure']);
404+
expect(budget.maxTurns).toBe(25);
405+
expect(budget.timeoutMinutes).toBe(15);
406+
});
407+
408+
it('gives full budget for conflict', () => {
409+
const budget = computeBudget(['conflict']);
410+
expect(budget.maxTurns).toBe(40);
411+
expect(budget.timeoutMinutes).toBe(30);
412+
});
413+
414+
it('uses highest budget when multiple issues present', () => {
415+
const budget = computeBudget(['missing-issue-ref', 'ci-failure']);
416+
expect(budget.maxTurns).toBe(25);
417+
expect(budget.timeoutMinutes).toBe(15);
418+
});
419+
420+
it('conflict dominates when mixed with smaller issues', () => {
421+
const budget = computeBudget(['missing-testplan', 'conflict', 'missing-issue-ref']);
422+
expect(budget.maxTurns).toBe(40);
423+
expect(budget.timeoutMinutes).toBe(30);
424+
});
425+
});
426+
386427
// ── detectIssues (regression test after refactor) ────────────────────────────
387428

388429
describe('detectIssues', () => {

crux/pr-patrol/index.ts

Lines changed: 111 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -172,20 +172,27 @@ function markProcessed(key: number | string): void {
172172
);
173173
}
174174

175-
function getMaxTurnsFailCount(key: number | string): number {
176-
const file = join(STATE_DIR, `max-turns-${key}`);
177-
if (!existsSync(file)) return 0;
178-
return parseInt(readFileSync(file, 'utf-8').trim(), 10) || 0;
175+
function getFailCount(key: number | string): number {
176+
// Check both new and legacy file names for backwards compat
177+
const newFile = join(STATE_DIR, `failures-${key}`);
178+
const legacyFile = join(STATE_DIR, `max-turns-${key}`);
179+
if (existsSync(newFile)) {
180+
return parseInt(readFileSync(newFile, 'utf-8').trim(), 10) || 0;
181+
}
182+
if (existsSync(legacyFile)) {
183+
return parseInt(readFileSync(legacyFile, 'utf-8').trim(), 10) || 0;
184+
}
185+
return 0;
179186
}
180187

181-
function recordMaxTurnsFailure(key: number | string): number {
182-
const count = getMaxTurnsFailCount(key) + 1;
183-
writeFileSync(join(STATE_DIR, `max-turns-${key}`), String(count));
188+
function recordFailure(key: number | string): number {
189+
const count = getFailCount(key) + 1;
190+
writeFileSync(join(STATE_DIR, `failures-${key}`), String(count));
184191
return count;
185192
}
186193

187194
function isAbandoned(key: number | string): boolean {
188-
return getMaxTurnsFailCount(key) >= 2;
195+
return getFailCount(key) >= 2;
189196
}
190197

191198
// ── Main Branch CI Check ────────────────────────────────────────────────────
@@ -321,21 +328,27 @@ async function fixMainBranch(status: MainBranchStatus, config: PatrolConfig): Pr
321328
const elapsedS = Math.floor((Date.now() - startTime) / 1000);
322329

323330
if (result.timedOut) {
331+
const failCount = recordFailure(MAIN_BRANCH_KEY);
324332
outcome = 'timeout';
325-
reason = `Killed after ${config.timeoutMinutes}m timeout`;
326-
log(`✗ Main branch fix timed out after ${config.timeoutMinutes}m`);
333+
reason = `Killed after ${config.timeoutMinutes}m timeout — attempt ${failCount}`;
334+
log(`✗ Main branch fix timed out after ${config.timeoutMinutes}m (attempt ${failCount})`);
335+
336+
if (failCount >= 2) {
337+
reason = `Abandoned after ${failCount} failures (timeout)`;
338+
log(`✗ Main branch fix abandoned after ${failCount} failures`);
339+
}
327340
} else if (result.exitCode === 0 && !result.hitMaxTurns) {
328341
outcome = 'fixed';
329342
log(`✓ Main branch CI fix processed (${elapsedS}s)`);
330343
} else if (result.hitMaxTurns) {
331-
const failCount = recordMaxTurnsFailure(MAIN_BRANCH_KEY);
344+
const failCount = recordFailure(MAIN_BRANCH_KEY);
332345
outcome = 'max-turns';
333346
reason = `Hit max turns (${config.maxTurns}) — attempt ${failCount}`;
334347
log(`⚠ Main branch fix hit max turns after ${elapsedS}s`);
335348

336349
if (failCount >= 2) {
337-
reason = `Abandoned after ${failCount} max-turns failures`;
338-
log(`✗ Main branch fix abandoned after ${failCount} max-turns failures`);
350+
reason = `Abandoned after ${failCount} failures`;
351+
log(`✗ Main branch fix abandoned after ${failCount} failures`);
339352
}
340353
} else {
341354
outcome = 'error';
@@ -703,6 +716,37 @@ const ISSUE_SCORES: Record<PrIssueType, number> = {
703716
'bot-review-nitpick': 15,
704717
};
705718

719+
// ── Issue-type-specific resource limits ──────────────────────────────────────
720+
// Scale max-turns and timeout based on the hardest issue in a PR.
721+
// This prevents trivial issues from consuming the full 40-turn / 30-min budget.
722+
723+
interface IssueBudget {
724+
maxTurns: number;
725+
timeoutMinutes: number;
726+
}
727+
728+
const ISSUE_BUDGETS: Record<PrIssueType, IssueBudget> = {
729+
conflict: { maxTurns: 40, timeoutMinutes: 30 },
730+
'ci-failure': { maxTurns: 25, timeoutMinutes: 15 },
731+
'bot-review-major': { maxTurns: 25, timeoutMinutes: 15 },
732+
'missing-issue-ref': { maxTurns: 5, timeoutMinutes: 3 },
733+
stale: { maxTurns: 10, timeoutMinutes: 5 },
734+
'missing-testplan': { maxTurns: 8, timeoutMinutes: 5 },
735+
'bot-review-nitpick':{ maxTurns: 8, timeoutMinutes: 5 },
736+
};
737+
738+
/** Compute the budget for a PR based on its hardest issue. */
739+
export function computeBudget(issues: PrIssueType[]): IssueBudget {
740+
let maxTurns = 5;
741+
let timeoutMinutes = 3;
742+
for (const issue of issues) {
743+
const budget = ISSUE_BUDGETS[issue];
744+
if (budget.maxTurns > maxTurns) maxTurns = budget.maxTurns;
745+
if (budget.timeoutMinutes > timeoutMinutes) timeoutMinutes = budget.timeoutMinutes;
746+
}
747+
return { maxTurns, timeoutMinutes };
748+
}
749+
706750
/** Pure function — computes priority score for a detected PR. */
707751
export function computeScore(pr: DetectedPr): number {
708752
let score = 0;
@@ -927,9 +971,12 @@ ${issues.join(', ')}
927971
### CI Failure
928972
- Check CI status: gh pr checks ${num} --repo ${repo}
929973
- Read the failing check logs to understand the failure
930-
- Fix the issue (build error, test failure, lint error)
931-
- Run locally to verify: pnpm build and/or pnpm test
932-
- Commit and push the fix`);
974+
- **STOP IMMEDIATELY and report** if ANY of these apply:
975+
- The check requires a human action (adding a label like \`rules-change-reviewed\`, manual approval, etc.)
976+
- The failure is in a Vercel deployment or external service (not a code issue)
977+
- The same check is also failing on the \`main\` branch (pre-existing, not caused by this PR)
978+
- The failure is a permissions or authentication issue
979+
- If the failure IS a code issue you can fix: fix it, run locally to verify (pnpm build / pnpm test), commit and push`);
933980
}
934981

935982
if (issues.includes('missing-testplan')) {
@@ -979,7 +1026,14 @@ ${issues.join(', ')}
9791026
- Use git push --force-with-lease (never --force) when pushing rebased branches
9801027
- Do not modify files unrelated to the fix
9811028
- Do NOT run /agent-session-start or /agent-session-ready-PR — this is a targeted fix, not a full session
982-
- Do NOT create new branches — work on the existing PR branch`);
1029+
- Do NOT create new branches — work on the existing PR branch
1030+
1031+
## When to stop early
1032+
- **If the issue requires human intervention** (adding labels, approvals, external service fixes): output a clear summary of why and stop immediately. Do not attempt workarounds.
1033+
- **If the issue is pre-existing** (also failing on main, not introduced by this PR): state that and stop.
1034+
- **If you've tried 2+ approaches and none worked**: stop and summarize what you tried. Do not keep cycling through the same strategies.
1035+
- **If the fix is "no action needed"** (e.g., no matching issue exists for missing-issue-ref): say so and stop. Not every detected issue requires a code change.
1036+
- Stopping early with a clear explanation is BETTER than burning through all turns without progress.`);
9831037

9841038
return sections.join('\n');
9851039
}
@@ -1108,20 +1162,48 @@ async function fixPr(pr: ScoredPr, config: PatrolConfig): Promise<void> {
11081162

11091163
await claimPr(pr.number, config.repo);
11101164

1165+
// Compute issue-specific budget (capped by global config)
1166+
const budget = computeBudget(pr.issues);
1167+
const effectiveMaxTurns = Math.min(budget.maxTurns, config.maxTurns);
1168+
const effectiveTimeout = Math.min(budget.timeoutMinutes, config.timeoutMinutes);
1169+
1170+
log(` Budget: ${effectiveMaxTurns} max-turns, ${effectiveTimeout}m timeout (based on: ${pr.issues.join(', ')})`);
1171+
11111172
const prompt = buildPrompt(pr, config.repo);
11121173
const startTime = Date.now();
11131174

11141175
let outcome: FixOutcome = 'fixed';
11151176
let reason = '';
11161177

11171178
try {
1118-
const result = await spawnClaude(prompt, config);
1179+
const result = await spawnClaude(prompt, {
1180+
...config,
1181+
maxTurns: effectiveMaxTurns,
1182+
timeoutMinutes: effectiveTimeout,
1183+
});
11191184
const elapsedS = Math.floor((Date.now() - startTime) / 1000);
11201185

11211186
if (result.timedOut) {
1187+
// Timeouts count toward abandonment — a PR that times out repeatedly
1188+
// is likely unfixable and should not keep burning compute.
1189+
const failCount = recordFailure(pr.number);
11221190
outcome = 'timeout';
1123-
reason = `Killed after ${config.timeoutMinutes}m timeout`;
1124-
log(`✗ PR #${pr.number} timed out after ${config.timeoutMinutes}m`);
1191+
reason = `Killed after ${effectiveTimeout}m timeout — attempt ${failCount}`;
1192+
log(`✗ PR #${pr.number} timed out after ${effectiveTimeout}m (attempt ${failCount})`);
1193+
1194+
if (failCount >= 2) {
1195+
reason = `Abandoned after ${failCount} failures (timeout)`;
1196+
log(`✗ PR #${pr.number} abandoned after ${failCount} consecutive failures`);
1197+
await githubApi(
1198+
`/repos/${config.repo}/issues/${pr.number}/comments`,
1199+
{
1200+
method: 'POST',
1201+
body: {
1202+
body: `🤖 **PR Patrol**: Abandoning automatic fix after ${failCount} failed attempts (timed out each time).\n\n**Issues detected**: ${pr.issues.join(', ')}\n**Last attempt**: ${elapsedS}s, ${effectiveTimeout}m timeout\n\nThis PR likely needs human intervention to resolve.`,
1203+
},
1204+
},
1205+
).catch(() => log(' Warning: could not post abandonment comment'));
1206+
}
11251207
} else if (result.exitCode === 0 && !result.hitMaxTurns) {
11261208
log(`✓ PR #${pr.number} processed successfully (${elapsedS}s)`);
11271209
outcome = 'fixed';
@@ -1132,27 +1214,27 @@ async function fixPr(pr: ScoredPr, config: PatrolConfig): Promise<void> {
11321214
await githubApi(`/repos/${config.repo}/issues/${pr.number}/comments`, {
11331215
method: 'POST',
11341216
body: {
1135-
body: `🤖 **PR Patrol** ran for ${elapsedS}s (${config.maxTurns} max turns, model: ${config.model}).\n\n**Issues detected**: ${pr.issues.join(', ')}\n\n**Result**:\n${summary}`,
1217+
body: `🤖 **PR Patrol** ran for ${elapsedS}s (${effectiveMaxTurns} max turns, model: ${config.model}).\n\n**Issues detected**: ${pr.issues.join(', ')}\n\n**Result**:\n${summary}`,
11361218
},
11371219
}).catch(() => log(' Warning: could not post summary comment'));
11381220
}
11391221
} else if (result.hitMaxTurns) {
1140-
const failCount = recordMaxTurnsFailure(pr.number);
1222+
const failCount = recordFailure(pr.number);
11411223
outcome = 'max-turns';
1142-
reason = `Hit max turns (${config.maxTurns}) — attempt ${failCount}`;
1143-
log(`⚠ PR #${pr.number} hit max turns after ${elapsedS}s`);
1224+
reason = `Hit max turns (${effectiveMaxTurns}) — attempt ${failCount}`;
1225+
log(`⚠ PR #${pr.number} hit max turns after ${elapsedS}s (attempt ${failCount})`);
11441226

11451227
if (failCount >= 2) {
1146-
reason = `Abandoned after ${failCount} max-turns failures`;
1228+
reason = `Abandoned after ${failCount} failures`;
11471229
log(
1148-
`✗ PR #${pr.number} abandoned after ${failCount} max-turns failures`,
1230+
`✗ PR #${pr.number} abandoned after ${failCount} consecutive failures`,
11491231
);
11501232
await githubApi(
11511233
`/repos/${config.repo}/issues/${pr.number}/comments`,
11521234
{
11531235
method: 'POST',
11541236
body: {
1155-
body: `🤖 **PR Patrol**: Abandoning automatic fix after ${failCount} failed attempts (hit max turns each time).\n\n**Issues detected**: ${pr.issues.join(', ')}\n**Last attempt**: ${elapsedS}s, ${config.maxTurns} turns\n\nThis PR likely needs human intervention to resolve.`,
1237+
body: `🤖 **PR Patrol**: Abandoning automatic fix after ${failCount} failed attempts.\n\n**Issues detected**: ${pr.issues.join(', ')}\n**Last attempt**: ${elapsedS}s, ${effectiveMaxTurns} turns\n\nThis PR likely needs human intervention to resolve.`,
11561238
},
11571239
},
11581240
).catch(() => log(' Warning: could not post abandonment comment'));
@@ -1246,6 +1328,8 @@ ${recentEntries}
12461328
const result = await spawnClaude(prompt, {
12471329
...config,
12481330
maxTurns: 10, // Reflection needs fewer turns
1331+
model: 'haiku', // Reflection is log analysis — doesn't need sonnet
1332+
timeoutMinutes: 5, // Should complete quickly
12491333
});
12501334
const elapsedS = Math.floor((Date.now() - startTime) / 1000);
12511335
const filedIssue = /Created issue #|created.*#\d/.test(result.output);

crux/vitest.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export default defineConfig({
2020
'wiki-server/**/*.test.ts',
2121
'evals/**/*.test.ts',
2222
'health/**/*.test.ts',
23+
'pr-patrol/**/*.test.ts',
2324
],
2425
exclude: [
2526
'claims/archive/**',

0 commit comments

Comments
 (0)