Skip to content

fix(scheduler): idle-fill must not out-race a repeated infra_error (AGT-4305) #623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
unohee merged 1 commit into from
Sep 10, 2026
Merged

fix(scheduler): idle-fill must not out-race a repeated infra_error (AGT-4305) #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions src/automation/autonomousRunner.infraError.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,113 @@ describe('AutonomousRunner infra_error handling (INT-2010)', () => {
expect(history[0]).toMatchObject({ failureCause: 'timeout', finalStatus: 'infra_error' });
});
});

// AGT-4305: AGT-4257's idle_fill exists so free slots do not sit empty, but
// bypassing the 1h infra_error backoff instantly and forever, on the SAME
// issue, turns a transient provider hiccup into a worker+tester+reviewer
// spend loop when that issue is the only candidate around to fill slots with
// (observed on vela: AX-1272's reviewer call died to the same 360s OpenRouter
// timeout 9 of ~13 attempts over 4h+, retried within ~90s every time).
describe('idle-fill must not out-race a repeated infra_error on the same issue (AGT-4305)', () => {
type InternalRunner = {
scheduler: TaskScheduler;
filterAlreadyProcessed(tasks: TaskItem[]): TaskItem[];
consecutiveInfraErrorCounts: Map<string, number>;
failedTaskRetryTimes: Map<string, number>;
};

const backoffTask = (): TaskItem => ({
...task(),
linearState: 'Todo',
linearProject: { id: 'project', name: 'Repo' },
});

it('still idle-fills through the real 1h backoff below the threshold (control, unchanged AGT-4257 behavior)', async () => {
const source = mockTaskSource();
runnerExecution.setTaskSource(source);
const runner = new AutonomousRunner(cfg()) as unknown as InternalRunner;

await runN(runner.scheduler, 'infra_error', 2); // below MAX_CONSECUTIVE_INFRA_IDLE_FILL (3)
expect(runner.failedTaskRetryTimes.get('ISSUE-1')).toBeGreaterThan(Date.now());

const filtered = runner.filterAlreadyProcessed([backoffTask()]);
expect(filtered.map((t) => t.issueId)).toContain('ISSUE-1');
});

it('stops bypassing the 1h backoff once the same issue has died to infra_error 3 times running', async () => {
const source = mockTaskSource();
runnerExecution.setTaskSource(source);
const runner = new AutonomousRunner(cfg()) as unknown as InternalRunner;

await runN(runner.scheduler, 'infra_error', 3);
expect(runner.consecutiveInfraErrorCounts.get('ISSUE-1')).toBe(3);
expect(runner.failedTaskRetryTimes.get('ISSUE-1')).toBeGreaterThan(Date.now());

const filtered = runner.filterAlreadyProcessed([backoffTask()]);
expect(filtered.map((t) => t.issueId)).not.toContain('ISSUE-1');
});

it('resets the streak once the same issue reaches a real (non-infra) outcome', async () => {
const source = mockTaskSource();
runnerExecution.setTaskSource(source);
const runner = new AutonomousRunner(cfg()) as unknown as InternalRunner;

await runN(runner.scheduler, 'infra_error', 3);
expect(runner.consecutiveInfraErrorCounts.get('ISSUE-1')).toBe(3);

const approved: PipelineResult = { ...result('approved'), success: true };
runner.scheduler.startTask(task(), '/repo', async () => approved);
await new Promise((resolve) => setTimeout(resolve, 25));

expect(runner.consecutiveInfraErrorCounts.has('ISSUE-1')).toBe(false);
});

// The gate above only fires through `legacyIsAuthority` — false whenever a
// durable run already exists and the ledger is primary. Any issue that has
// ever failed once already has a durable row, so the durable-ledger
// idle-fill branch (`idleLiftable`, separate code above) needed its own copy
// of this gate. Without it, this whole describe block would be green while
// the actual production incident (AX-1272, durable/primary ledger) kept
// looping — caught in independent review before this test existed.
it('the durable-ledger RETRY_AT idle-fill branch also stops bypassing backoff after 3 consecutive infra_errors', async () => {
const LEDGER_TASK: TaskItem = {
id: 'ISSUE-1', issueId: 'ISSUE-1', issueIdentifier: 'ISSUE-1',
source: 'linear', title: 'reviewer keeps timing out', priority: 2, createdAt: 0,
linearState: 'Todo', linearProject: { id: 'project', name: 'Repo' },
};
const dbPath = join(tempDir, 'automation.db');

const source = mockTaskSource();
runnerExecution.setTaskSource(source);
const runner = new AutonomousRunner(cfg({ automationLedgerMode: 'primary', automationDbPath: dbPath })) as unknown as InternalRunner & {
durableRuns: {
observeTask(task: TaskItem, repo: string): void;
getRun(id: string): { state: string } | null;
close(): void;
};
};
runner.durableRuns.observeTask(LEDGER_TASK, '/repo');

// Build the legacy-tracked streak the same way production does (the
// increment itself is unconditional — see the 'failed' handler).
await runN(runner.scheduler, 'infra_error', 3);
expect(runner.consecutiveInfraErrorCounts.get('ISSUE-1')).toBe(3);

// Seed the durable row into RETRY_AT with a future retryAt, the way the
// real infra_error ledger transition leaves it (durableRunCoordinator.ts).
const { RunLedger } = await import('./runLedger.js');
const ledger = new RunLedger(dbPath);
const claim = ledger.claimRun('ISSUE-1', { ownerInstanceId: 'seed', leaseMs: 60_000, maxActiveForProject: 1 });
expect(claim).not.toBeNull();
expect(ledger.transition(claim!, 'RETRY_AT', {
retryAt: Date.now() + 3_600_000, errorCode: 'infra_error',
})).toBe(true);
ledger.close();

const filtered = runner.filterAlreadyProcessed([LEDGER_TASK]);
expect(filtered.map((t) => t.issueId)).not.toContain('ISSUE-1');
expect(runner.durableRuns.getRun('ISSUE-1')?.state).toBe('RETRY_AT'); // still parked, not lifted

runner.durableRuns.close(); // this test opens its own primary-mode ledger handle
});
});
67 changes: 63 additions & 4 deletions src/automation/autonomousRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,17 @@ export class AutonomousRunner {
private completedTaskIds = new Set<string>();
private failedTaskCounts = new Map<string, number>();
private failedTaskRetryTimes = new Map<string, number>(); // issueId → next retry timestamp (ms)
// issueId → consecutive infra_error count since its last cleared/terminal
// outcome (success, permanent block, operator park, or manual recovery — the
// same events that already clear failedTaskRetryTimes for this issueId).
// A non-infra failure that is retried again (rejection under the limit,
// 'superseded', etc.) leaves this untouched by design: it only needs to be
// conservative in one direction, never resetting is safe, silently resetting
// on the wrong event is not. Not persisted across restarts — a restart is
// itself a reasonable "give it a fresh run" signal, and losing the streak on
// restart only ever makes the gate below MORE permissive, never less safe.
// (AGT-4305)
private consecutiveInfraErrorCounts = new Map<string, number>();

/**
* Bring a durably backed-off run forward because its answer landed.
Expand All @@ -438,6 +449,7 @@ export class AutonomousRunner {
if (!this.answerArrivedFor(issueId)) return false;
if (!this.durableRuns.readmitParkedRun(issueId, OPERATOR_PARK_REASON)) return false;
clearRetryTime(issueId, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(issueId);
return true;
}

Expand Down Expand Up @@ -486,6 +498,10 @@ export class AutonomousRunner {
// the reviewer already called out (INT-2474). Persisted; cleared on success.
private lastFailureDetails = new Map<string, LastFailureEntry>();
private static readonly MAX_RETRY_COUNT = 4; // Increased from 2 to allow more retries with backoff
// Consecutive infra_error attempts (same issue, no intervening non-infra
// outcome) allowed to bypass backoff via idle-fill before the real 1h
// backoff is enforced instead. (AGT-4305)
private static readonly MAX_CONSECUTIVE_INFRA_IDLE_FILL = 3;

// Rate-limit hold: epoch ms until which all task execution is paused.
// Set when any adapter returns a 429 / usage_limit_reached response (INT-1906).
Expand Down Expand Up @@ -607,6 +623,7 @@ export class AutonomousRunner {
this.completedTaskIds.add(task.issueId);
clearRejection(task.issueId); // Clear rejection count on success
clearRetryTime(task.issueId, this.failedTaskRetryTimes); // Clear retry backoff time
this.consecutiveInfraErrorCounts.delete(task.issueId);
this.lastFailureDetails.delete(task.issueId); // Stale feedback must not haunt future work
this.saveTaskState();
// Track project-level pace (5h rolling window)
Expand All @@ -623,6 +640,7 @@ export class AutonomousRunner {
}

if (result.success && task.issueId && this.durableRuns.isPrimary) {
this.consecutiveInfraErrorCounts.delete(task.issueId);
await this.drainDurableOutbox().catch((error) =>
console.error('[Outbox] Completion delivery pass failed:', error));
const durableState = this.durableRuns.getRun(task.issueId)?.state;
Expand Down Expand Up @@ -854,8 +872,10 @@ export class AutonomousRunner {
// Fixed mid-range backoff — we intentionally don't bump failure counts,
// so there's no attempt number to scale by.
const nextRetryTime = setRetryTime(task.issueId, 3, this.failedTaskRetryTimes);
const infraStreak = (this.consecutiveInfraErrorCounts.get(task.issueId) ?? 0) + 1;
this.consecutiveInfraErrorCounts.set(task.issueId, infraStreak);
this.saveTaskState();
console.warn(`[Scheduler] Infra error for ${taskCtx} (NOT counted toward STUCK) — backoff retry ${formatRetryTime(nextRetryTime)}: ${detail}`);
console.warn(`[Scheduler] Infra error for ${taskCtx} (NOT counted toward STUCK, consecutive: ${infraStreak}) — backoff retry ${formatRetryTime(nextRetryTime)}: ${detail}`);
} else {
console.warn(`[Scheduler] Infra error for ${taskCtx} (NOT counted toward STUCK): ${detail}`);
}
Expand Down Expand Up @@ -892,6 +912,7 @@ export class AutonomousRunner {
const { code, reason } = result.operatorPark;
this.completedTaskIds.add(task.issueId); // no retry changes what the fence saw
clearRetryTime(task.issueId, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(task.issueId);
recordLastFailureDetail(this.taskStateRef, task.issueId, reason);
this.saveTaskState();
console.warn(`[Scheduler] ${taskCtx} parked for the operator (${code}): ${reason}`);
Expand Down Expand Up @@ -934,6 +955,7 @@ export class AutonomousRunner {
const attempts = getRejectionCount(task.issueId) + (this.failedTaskCounts.get(task.issueId) ?? 0) + 1;
this.completedTaskIds.add(task.issueId); // no retry can move an environmental wall
clearRetryTime(task.issueId, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(task.issueId);
clearRejection(task.issueId);
recordLastFailureDetail(this.taskStateRef, task.issueId, infeasDetail);
const ownsRun = parkRunForHuman(
Expand Down Expand Up @@ -990,6 +1012,7 @@ export class AutonomousRunner {
// Max rejections reached - permanently block
this.completedTaskIds.add(task.issueId); // Prevent re-selection
clearRetryTime(task.issueId, this.failedTaskRetryTimes); // Clear retry time
this.consecutiveInfraErrorCounts.delete(task.issueId);
const ownsRun = parkRunForHuman(
this.durableRuns, task.issueId,
`Reviewer rejected ${rejectionCount} attempts: ${feedback}`,
Expand Down Expand Up @@ -1057,6 +1080,7 @@ export class AutonomousRunner {
// Max retries exceeded - permanently block
this.completedTaskIds.add(task.issueId); // Prevent re-selection
clearRetryTime(task.issueId, this.failedTaskRetryTimes); // Clear retry time
this.consecutiveInfraErrorCounts.delete(task.issueId);
const ownsRun = parkRunForHuman(
this.durableRuns, task.issueId,
`Autonomous execution failed ${count} times: ${failureDetail}`,
Expand Down Expand Up @@ -1227,7 +1251,29 @@ export class AutonomousRunner {
// without spending budget. WAITING_EXTERNAL is a run whose published
// effect is still pending, not a park: lifting it re-runs the task on
// top of its own in-flight publish.
const idleLiftable = (durableRun.state === 'RETRY_AT' && (durableRun.retryAt ?? 0) > Date.now())
//
// A RETRY_AT row can be parked there for infra_error same as any other
// reason, and this is the durable-ledger counterpart of the legacy
// idle-fill bypass gated below by `consecutiveInfraErrorCounts` — without
// it here too, an issue whose durable row already exists (true for
// anything that has ever failed once) never reaches that legacy gate at
// all, since `legacyIsAuthority` is false whenever this block ran and
// left a durable run in place. (AGT-4305 — this is the branch AX-1272
// was actually looping through in production.)
// Only throttle a RETRY_AT that is CURRENTLY backed off for infra_error —
// `consecutiveInfraErrorCounts` does not reset on a later, unrelated
// rejection/failure retry for the same issue (by design; see the field
// comment), so without the lastErrorCode check a stale infra streak
// would keep throttling idle-fill for a RETRY_AT caused by an ordinary
// task-level rejection long after the infra episode ended.
const infraStreak = durableRun.lastErrorCode === 'infra_error'
? this.consecutiveInfraErrorCounts.get(id) ?? 0
: 0;
const idleLiftable = (
durableRun.state === 'RETRY_AT'
&& (durableRun.retryAt ?? 0) > Date.now()
&& infraStreak < AutonomousRunner.MAX_CONSECUTIVE_INFRA_IDLE_FILL
)
|| durableRun.state === 'NEEDS_SPEC'
|| durableRun.state === 'NEEDS_ENV';
if (idleLiftable && idleFillBudget > 0 && this.durableRuns.markReady(id)) {
Expand Down Expand Up @@ -1298,6 +1344,7 @@ export class AutonomousRunner {
this.failedTaskCounts.delete(id);
clearRejection(id); // Clear rejection count on recovery
clearRetryTime(id, this.failedTaskRetryTimes); // Clear retry backoff time
this.consecutiveInfraErrorCounts.delete(id);
if (isStuck) toUnstick.push(id); // strip the stuck label so it is not re-skipped
recovered++;
return true;
Expand All @@ -1317,6 +1364,7 @@ export class AutonomousRunner {
this.failedTaskCounts.delete(id);
clearRejection(id);
clearRetryTime(id, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(id);
if (isStuck) toUnstick.push(id);
recovered++;
return true;
Expand Down Expand Up @@ -1369,11 +1417,21 @@ export class AutonomousRunner {
// an `ask_human` park, so left alone it makes the operator's reply land
// up to two hours after they sent it.
if (!canRetryNow(id, this.failedTaskRetryTimes)) {
const infraStreak = this.consecutiveInfraErrorCounts.get(id) ?? 0;
if (this.answerArrivedFor(id)) {
clearRetryTime(id, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(id);
answered++;
} else if (idleFillBudget > 0) {
// AGT-4257: free slots chew the backoff instead of sitting idle.
} else if (idleFillBudget > 0 && infraStreak < AutonomousRunner.MAX_CONSECUTIVE_INFRA_IDLE_FILL) {
// AGT-4257: free slots chew the backoff instead of sitting idle — but
// not when the same issue has died to infra_error (timeout/CLI
// failure, not a task failure) several times running with an idle
// fill each time. Retrying instantly with an unchanged payload just
// re-hits the same wall (AX-1272, 2026-09-10: same reviewer 360s
// timeout, 9 of ~13 attempts over 4h+, ~90s apart every time because
// this was the only candidate to fill idle slots with). Past the
// threshold, honor the real 1h backoff so a transient provider issue
// gets time to actually clear instead of being hammered. (AGT-4305)
idleFillBudget--;
clearRetryTime(id, this.failedTaskRetryTimes);
recovered++;
Expand Down Expand Up @@ -1772,6 +1830,7 @@ export class AutonomousRunner {
this.completedTaskIds.add(issueId);
clearRejection(issueId);
clearRetryTime(issueId, this.failedTaskRetryTimes);
this.consecutiveInfraErrorCounts.delete(issueId);
this.lastFailureDetails.delete(issueId);
}
if (finalized.size > 0) this.saveTaskState();
Expand Down
Loading