fix(pr-bot): fail fast when the review model's budget is spent

Claude Code answers an exhausted model budget INSIDE the turn ("You've
reached your Fable limit. Run /usage-credits to continue or switch models
with /model.") and then sits there with nothing to write. The reviewer never
produces a report, so `runTurn` waited out its full 40-minute deadline and
reported a bare "timed out after 40 min without a report", which reads as a
hung reviewer rather than an account that needs attention.

Measured on 2026-09-08: #388, #393, #394 and #377 each lost 40 minutes this
way, and because every attempt counted, all four reached MAX_AUTO_RETRIES and
would NOT have been picked up again once the budget returned. One spent
afternoon quietly took the whole queue out of service.

`findModelLimitNotice()` reads the notice off the pane and `runTurn` returns
a new `limit` outcome instead of waiting. It is consulted in exactly two
places, both of which mean "the turn produced nothing": on a stop where
`isDone()` is still false, and on each timed-out wait slice. A review that
merely discusses usage limits in its own findings therefore cannot be
mistaken for one that hit the wall, and the pattern matches neither the model
name nor a straight apostrophe, since the pane renders a typographic one and
every model prints the same sentence.

A spent budget is an account condition, not a bad PR, so it no longer spends
the per-head retry budget: the queue resumes by itself when the budget does.
Telegram now names the cause and the file to change.

Tests use the pane captured verbatim off the run that lost the 40 minutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-09-08 19:27:43 +02:00
parent a164c07f92
commit 5130ca6633
5 changed files with 104 additions and 10 deletions
+19 -5
View File
@@ -19,7 +19,7 @@
import { randomBytes } from 'crypto';
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import { CodemanClient, stripAnsi, type TurnOutcome } from './codeman-client.js';
import { CodemanClient, ModelLimitError, stripAnsi, type TurnOutcome } from './codeman-client.js';
import type { PrBotConfig } from './config.js';
import {
approveWorkflowRun,
@@ -434,7 +434,10 @@ export class PrBot {
if (report && !existsSync(reportMdPath)) writeFileSync(reportMdPath, last);
}
await this.recordClaudeSessionId(sessionId, rec);
if (!report) throw new Error(await this.describeFailure(sessionId, outcome, started, last));
if (!report) {
const why = await this.describeFailure(sessionId, outcome, started, last);
throw outcome.kind === 'limit' ? new ModelLimitError(why) : new Error(why);
}
const durationMin = Math.max(1, Math.round((Date.now() - started) / 60_000));
Object.assign(rec, {
@@ -460,9 +463,15 @@ export class PrBot {
if (rec) {
rec.status = 'failed';
rec.lastError = reason;
rec.failedAttempts = rec.failedSha === rec.headSha ? (rec.failedAttempts ?? 0) + 1 : 1;
rec.failedSha = rec.headSha;
const givingUp = rec.failedAttempts >= MAX_AUTO_RETRIES;
// A spent model budget is an account condition, not a bad PR, so it must not
// spend the per-head retry budget: otherwise one exhausted afternoon marks every
// open PR "not retrying on my own" and none of them come back when credits do.
const accountCondition = err instanceof ModelLimitError;
if (!accountCondition) {
rec.failedAttempts = rec.failedSha === rec.headSha ? (rec.failedAttempts ?? 0) + 1 : 1;
rec.failedSha = rec.headSha;
}
const givingUp = !accountCondition && (rec.failedAttempts ?? 0) >= MAX_AUTO_RETRIES;
await this.telegram
.sendMessage(
formatReviewFailure(rec, reason) +
@@ -523,6 +532,11 @@ export class PrBot {
}
case 'exit':
return 'the session exited before writing a report';
case 'limit':
return (
`the model budget for these review sessions is spent, so the reviewer never started:\n${outcome.message}\n` +
'Point PR_BOT_MODEL in ~/.codeman/pr-bot.env at a model with headroom and restart codeman-pr-bot.'
);
case 'timeout':
return `timed out after ${minutes} min without a report`;
default:
+50 -2
View File
@@ -50,7 +50,42 @@ export interface SessionRecord {
mode: string;
}
export type TurnOutcome = { kind: 'stop' } | { kind: 'blocked' } | { kind: 'exit' } | { kind: 'timeout' };
export type TurnOutcome =
| { kind: 'stop' }
| { kind: 'blocked' }
| { kind: 'exit' }
| { kind: 'timeout' }
| { kind: 'limit'; message: string };
/**
* Claude Code answers a spent model budget INSIDE the turn ("You've reached your Fable
* limit. Run /usage-credits to continue or switch models with /model.") and then simply
* sits there with nothing to write. Measured 2026-09-08: four reviews each burned their
* whole 40-minute deadline and reported a bare "timed out without a report", which reads
* as a hung reviewer rather than an account that needs attention, and the retries spent
* the per-head budget so the PRs would not have been picked up again once credits
* returned. Matching the notice turns 40 silent minutes into a named failure in seconds.
*
* Deliberately model-agnostic: the same sentence is printed for every model, and the
* apostrophe is typographic on the pane, so neither the model name nor `'` is matched.
*/
const MODEL_LIMIT_PATTERN = /reached your [^\n]{0,40}?\blimit\b|\/usage-credits/i;
/**
* Thrown instead of a plain Error when a review died on a spent model budget, so the
* caller can tell an account condition apart from a review that genuinely failed.
*/
export class ModelLimitError extends Error {
override readonly name = 'ModelLimitError';
}
/** The limit notice as one clean line, or undefined if the screen does not carry it. */
export function findModelLimitNotice(screen: string): string | undefined {
const line = stripAnsi(screen)
.split('\n')
.find((l) => MODEL_LIMIT_PATTERN.test(l));
return line?.replace(/^[\s>|]*(?:\u23bf|\u2514|\u256d|\u2570|\u23a2|\u2502|\u23bd)?\s*/u, '').trim() || undefined;
}
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
@@ -241,9 +276,22 @@ export class CodemanClient {
if (!r.delivered) throw new Error('the prompt was not delivered (pane dead?)');
let wait = r.wait;
let nudged = false;
// Only consulted when the turn produced nothing, so a review that merely QUOTES the
// notice in its report cannot be mistaken for one that hit it.
const limitNotice = async (): Promise<string | undefined> =>
findModelLimitNotice(await this.terminalText(id).catch(() => ''));
while (true) {
if (wait && !wait.timedOut) return toOutcome(wait);
if (wait && !wait.timedOut) {
const outcome = toOutcome(wait);
if (outcome.kind === 'stop' && !opts.isDone?.()) {
const limit = await limitNotice();
if (limit) return { kind: 'limit', message: limit };
}
return outcome;
}
if (opts.isDone?.()) return { kind: 'stop' };
const limit = await limitNotice();
if (limit) return { kind: 'limit', message: limit };
const remaining = opts.deadlineMs - (Date.now() - started);
if (remaining <= 0) return { kind: 'timeout' };
if (!nudged) {
+1 -1
View File
@@ -11,7 +11,7 @@
* worktree's project settings through the git common dir, i.e. the MAIN checkout's
* `.claude/settings.local.json`, whose model pin then silently overrides anything
* written into the worktree (measured 2026-09-05: a worktree pinned to
* `claude-fable-5-1` reported `claude-opus-5[1m]`). A shared clone has its own
* `claude-fable-5-1` reported `claude-opus-5[1m]`, the main checkout's pin). A shared clone has its own
* project root, so Codeman's `modelOverride` and hooks land where the CLI reads them,
* while `objects/info/alternates` keeps the object store shared (no duplication).
*