fix: stop Codex pool retry loops

This commit is contained in:
ejclaw
2026-06-02 05:29:32 +09:00
parent 03d4c81192
commit 5648b61075
16 changed files with 369 additions and 24 deletions

View File

@@ -85,6 +85,19 @@ describe('agent-error-detection', () => {
});
});
it('does not classify the internal Codex pool-unavailable sentinel as an auth failure', () => {
expect(
classifyCodexAuthError(
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex',
),
).toEqual({ category: 'none', reason: '' });
expect(
classifyCodexAuthError(
'Codex rotation pool unavailable: all rotation accounts are currently dead, rate-limited, or locked',
),
).toEqual({ category: 'none', reason: '' });
});
it('classifies Codex workspace credit exhaustion as rate-limit', () => {
expect(classifyAgentError('Workspace out of credits')).toEqual({
category: 'rate-limit',

View File

@@ -231,6 +231,35 @@ const NONE: AgentErrorClassification = {
reason: '',
};
export function isCodexPoolUnavailableError(
error: string | null | undefined,
): boolean {
if (!error) return false;
return (
/all\s+codex(?:\s+rotation)?\s+accounts(?:\s+are)?\s+unavailable/i.test(
error,
) || /codex\s+rotation\s+pool\s+unavailable/i.test(error)
);
}
export function isTerminalCodexAccountFailure(
error: string | null | undefined,
): boolean {
if (!error) return false;
if (isCodexPoolUnavailableError(error)) return true;
if (classifyCodexAuthError(error).category !== 'none') return true;
const lower = error.toLowerCase();
if (
classifyAgentError(error).category === 'rate-limit' &&
(lower.includes('workspace out of credits') ||
lower.includes('out of credits') ||
lower.includes('codex'))
) {
return true;
}
return false;
}
/**
* Classify an agent error string into a category.
* Handles patterns common to both Claude and Codex: 429, 503, network.
@@ -334,6 +363,7 @@ export function classifyCodexAuthError(
error: string | null | undefined,
): AgentErrorClassification {
if (!error) return NONE;
if (isCodexPoolUnavailableError(error)) return NONE;
const lower = error.toLowerCase();
if (

View File

@@ -273,7 +273,7 @@ describe('prepareGroupEnvironment codex auth handling', () => {
mockReadEnvFile.mockReturnValue({});
expect(() => prepareGroupEnvironment(group, false, 'dc:test')).toThrow(
/auth-expired: All Codex rotation accounts unavailable/,
/Codex rotation pool unavailable/,
);
const authPath = path.join(

View File

@@ -193,7 +193,7 @@ function syncHostCodexSessionFiles(
lease?.release();
if (hasRotationAccounts) {
throw new Error(
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex',
'Codex rotation pool unavailable: all rotation accounts are currently dead, rate-limited, or locked; re-auth or clear stale state before launching Codex',
);
}
return null;

View File

@@ -6,6 +6,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock('./agent-error-detection.js', () => ({
classifyAgentError: vi.fn(() => ({ category: 'none', reason: '' })),
classifyCodexAuthError: vi.fn(() => ({ category: 'none', reason: '' })),
isCodexPoolUnavailableError: vi.fn(
(error: string | null | undefined) =>
/all\s+codex(?:\s+rotation)?\s+accounts(?:\s+are)?\s+unavailable/i.test(
error ?? '',
) || /codex\s+rotation\s+pool\s+unavailable/i.test(error ?? ''),
),
}));
vi.mock('./config.js', () => ({
@@ -166,6 +172,34 @@ describe('codex-token-rotation d7 ≥ 100% auto-skip', () => {
expect(mod.getActiveCodexAuthPath()).not.toBe(fallbackAuthPath);
});
it('does not mutate account health for the internal all-accounts-unavailable sentinel', async () => {
const agentErrors = await import('./agent-error-detection.js');
vi.mocked(agentErrors.classifyCodexAuthError).mockReturnValueOnce({
category: 'auth-expired',
reason: 'auth-expired',
});
const mod = await import('./codex-token-rotation.js');
mod.initCodexTokenRotation();
expect(
mod.rotateCodexToken(
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex',
),
).toBe(false);
const accounts = mod.getAllCodexAccounts();
expect(accounts[0]).toEqual(
expect.objectContaining({
isActive: true,
isAuthDead: false,
authStatus: 'healthy',
isRateLimited: false,
}),
);
expect(accounts[1].isActive).toBe(false);
});
it('marks refresh-token reuse as dead auth instead of a recoverable cooldown', async () => {
const agentErrors = await import('./agent-error-detection.js');
vi.mocked(agentErrors.classifyCodexAuthError).mockReturnValueOnce({

View File

@@ -17,6 +17,7 @@ import path from 'path';
import {
classifyAgentError,
classifyCodexAuthError,
isCodexPoolUnavailableError,
type CodexRotationReason,
} from './agent-error-detection.js';
import { DATA_DIR } from './config.js';
@@ -638,6 +639,9 @@ export function detectCodexRotationTrigger(
error?: string | null,
): CodexRotationTriggerResult {
if (!error) return { shouldRotate: false, reason: '' };
if (isCodexPoolUnavailableError(error)) {
return { shouldRotate: false, reason: '' };
}
// Common patterns (429, 503, network) — delegated to SSOT
const common = classifyAgentError(error);
@@ -664,6 +668,19 @@ export function rotateCodexToken(
): boolean {
if (accounts.length <= 1) return false;
if (isCodexPoolUnavailableError(errorMessage)) {
logger.warn(
{
transition: 'rotation:skip-pool-unavailable-sentinel',
currentIndex,
totalAccounts: accounts.length,
reason: errorMessage ?? null,
},
'Refusing to mark Codex accounts unhealthy from internal pool-unavailable sentinel',
);
return false;
}
const previousIndex = currentIndex;
const acct = accounts[currentIndex];
const authFailure = classifyCodexAuthError(errorMessage);

View File

@@ -143,6 +143,43 @@ describe('paired turn attempt restart recovery', () => {
]);
});
it('clears stale running attempts for already completed tasks without replaying them', () => {
const task = makeTask({
id: 'completed-stale-running-task',
status: 'completed',
completion_reason: 'done',
});
createPairedTask(task);
const turnIdentity = buildPairedTurnIdentity({
taskId: task.id,
taskUpdatedAt: task.updated_at,
intentKind: 'owner-turn',
role: 'owner',
});
markPairedTurnRunning({
turnIdentity,
executorServiceId: CODEX_MAIN_SERVICE_ID,
executorAgentType: 'codex',
runId: 'completed-run-before-restart',
});
expect(
recoverInterruptedPairedTurnAttemptsForService({
serviceIds: [CODEX_MAIN_SERVICE_ID],
now: '2026-05-27T00:11:00.000Z',
}),
).toEqual([]);
expect(getPairedTurnAttempts(turnIdentity.turnId)).toEqual([
expect.objectContaining({
state: 'failed',
active_run_id: null,
completed_at: '2026-05-27T00:11:00.000Z',
last_error: 'Interrupted by service restart before completion.',
}),
]);
});
it('recovers codex executor attempts even when the orchestration lease used the claude service id', () => {
const task = makeTask({ id: 'cross-service-lease-task' });
createPairedTask(task);

View File

@@ -462,6 +462,24 @@ export function recoverInterruptedPairedTurnAttemptsForServiceInDatabase(
const error =
args.error ?? 'Interrupted by service restart before completion.';
return database.transaction(() => {
database
.prepare(
`
UPDATE paired_turn_attempts
SET state = 'failed',
active_run_id = NULL,
updated_at = ?,
completed_at = ?,
last_error = COALESCE(last_error, ?)
WHERE state = 'running'
AND executor_service_id IN (${servicePlaceholders})
AND task_id IN (
SELECT id FROM paired_tasks WHERE status = 'completed'
)
`,
)
.run(now, now, error, ...serviceIds);
const rows = database
.prepare(
`

View File

@@ -207,6 +207,19 @@ describe('message-agent-executor-rules', () => {
).toBe('pending');
});
it('does not requeue a silent owner failure caused by Codex pool unavailability', () => {
expect(
resolvePairedFollowUpQueueAction({
completedRole: 'owner',
executionStatus: 'failed',
sawOutput: false,
taskStatus: 'active',
outputSummary:
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.',
}),
).toBe('none');
});
it('does not request an executor-side follow-up after successful owner output moved the task to review_ready', () => {
expect(
resolvePairedFollowUpQueueAction({

View File

@@ -3,7 +3,7 @@ import {
resolveNextTurnAction,
shouldRetrySilentOwnerExecution,
} from './message-runtime-rules.js';
import { classifyCodexAuthError } from './agent-error-detection.js';
import { isTerminalCodexAccountFailure } from './agent-error-detection.js';
import { parseVisibleVerdict } from './paired-verdict.js';
import type { PairedRoomRole, PairedTaskStatus } from './types.js';
export {
@@ -19,14 +19,7 @@ export {
export type PairedFollowUpQueueAction = 'pending' | 'none';
function isSilentCodexAccountFailure(summary?: string | null): boolean {
if (!summary) return false;
if (classifyCodexAuthError(summary).category !== 'none') return true;
const lower = summary.toLowerCase();
return (
lower.includes('workspace out of credits') ||
lower.includes('out of credits') ||
/all\s+codex(?:\s+rotation)?\s+accounts/i.test(summary)
);
return isTerminalCodexAccountFailure(summary);
}
export function resolvePairedFollowUpQueueAction(args: {
@@ -39,7 +32,6 @@ export function resolvePairedFollowUpQueueAction(args: {
if (
args.executionStatus === 'failed' &&
args.sawOutput === false &&
(args.completedRole === 'reviewer' || args.completedRole === 'arbiter') &&
isSilentCodexAccountFailure(args.outputSummary)
) {
return 'none';

View File

@@ -1,21 +1,11 @@
import { logger } from './logger.js';
import {
classifyAgentError,
classifyCodexAuthError,
} from './agent-error-detection.js';
import { isTerminalCodexAccountFailure } from './agent-error-detection.js';
import { transitionPairedTaskStatus } from './paired-task-status.js';
import { classifyArbiterVerdict } from './paired-verdict.js';
import type { PairedTask } from './types.js';
const ARBITER_RESOLUTION_ROUND_TRIP_COUNT = 0;
function isTerminalCodexAccountFailure(summary?: string | null): boolean {
if (!summary) return false;
if (classifyCodexAuthError(summary).category !== 'none') return true;
if (classifyAgentError(summary).category === 'rate-limit') return true;
return /all\s+codex(?:\s+rotation)?\s+accounts/i.test(summary);
}
export function handleFailedArbiterExecution(args: {
task: PairedTask;
taskId: string;

View File

@@ -7,6 +7,7 @@ import {
getPairedWorkspace,
hasActiveCiWatcherForChat,
} from './db.js';
import { isTerminalCodexAccountFailure } from './agent-error-detection.js';
import { logger } from './logger.js';
import { markPairedTaskReviewReady } from './paired-workspace-manager.js';
import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
@@ -30,6 +31,31 @@ export function handleFailedOwnerExecution(args: {
}): void {
const { task, taskId, summary } = args;
const now = new Date().toISOString();
if (isTerminalCodexAccountFailure(summary)) {
transitionPairedTaskStatus({
taskId,
currentStatus: task.status,
nextStatus: 'completed',
expectedUpdatedAt: task.updated_at,
updatedAt: now,
patch: {
arbiter_verdict: 'escalate',
arbiter_requested_at: null,
completion_reason: 'owner_codex_unavailable',
},
});
logger.warn(
{
taskId,
role: 'owner',
status: task.status,
summary: summary?.slice(0, 200),
},
'Completed owner task after terminal Codex account failure instead of retrying owner loop',
);
return;
}
const nextFailureCount = (task.owner_failure_count ?? 0) + 1;
if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) {

View File

@@ -1,5 +1,6 @@
import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
import { getPairedWorkspace } from './db.js';
import { isTerminalCodexAccountFailure } from './agent-error-detection.js';
import { logger } from './logger.js';
import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
import { transitionPairedTaskStatus } from './paired-task-status.js';
@@ -19,6 +20,31 @@ export function handleFailedReviewerExecution(args: {
const { task, taskId, summary } = args;
const now = new Date().toISOString();
if (isTerminalCodexAccountFailure(summary)) {
transitionPairedTaskStatus({
taskId,
currentStatus: task.status,
nextStatus: 'completed',
expectedUpdatedAt: task.updated_at,
updatedAt: now,
patch: {
arbiter_verdict: 'escalate',
arbiter_requested_at: null,
completion_reason: 'reviewer_codex_unavailable',
},
});
logger.warn(
{
taskId,
role: 'reviewer',
status: task.status,
summary: summary?.slice(0, 200),
},
'Completed reviewer task after terminal Codex account failure instead of preserving review loop',
);
return;
}
if (summary) {
const verdict = parseVisibleVerdict(summary);
const signal = resolveReviewerFailureSignal({

View File

@@ -224,6 +224,60 @@ describe('paired execution routing loop guards', () => {
);
});
it('completes reviewer task after terminal Codex account failure instead of preserving review_ready loop', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_review',
updated_at: '2026-03-28T00:00:05.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'reviewer',
status: 'failed',
summary:
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'completed',
arbiter_verdict: 'escalate',
arbiter_requested_at: null,
completion_reason: 'reviewer_codex_unavailable',
}),
);
});
it('completes owner task after terminal Codex account failure instead of retrying owner forever', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'active',
updated_at: '2026-03-28T00:00:05.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'owner',
status: 'failed',
summary:
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'completed',
arbiter_verdict: 'escalate',
arbiter_requested_at: null,
completion_reason: 'owner_codex_unavailable',
}),
);
});
it('keeps arbiter REVISE on owner flow while clearing stale loop counters', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({

View File

@@ -143,7 +143,12 @@ vi.mock('./service-routing.js', () => ({
hasReviewerLease: vi.fn(() => false),
}));
import { _initTestDatabase, createTask, getTaskById } from './db.js';
import {
_initTestDatabase,
createPairedTask,
createTask,
getTaskById,
} from './db.js';
import * as codexTokenRotation from './codex-token-rotation.js';
import { TIMEZONE } from './config.js';
import * as serviceRouting from './service-routing.js';
@@ -304,6 +309,52 @@ describe('task scheduler', () => {
);
});
it('requeues an orphaned review_ready paired task once its CI watcher is gone', async () => {
createPairedTask({
id: 'paired-review-ready-orphan',
chat_jid: 'review@g.us',
group_folder: 'review-group',
owner_service_id: 'codex-main',
reviewer_service_id: 'claude',
owner_agent_type: 'codex',
reviewer_agent_type: 'claude-code',
arbiter_agent_type: null,
title: null,
source_ref: 'HEAD',
plan_notes: null,
review_requested_at: null,
round_trip_count: 1,
status: 'review_ready',
arbiter_verdict: null,
arbiter_requested_at: null,
completion_reason: null,
owner_failure_count: 0,
created_at: '2026-02-22T00:00:00.000Z',
updated_at: '2026-02-22T00:00:01.000Z',
});
const enqueueMessageCheck = vi.fn();
const deps = {
serviceAgentType: 'codex' as const,
roomBindings: () => ({}),
getSessions: () => ({}),
queue: { enqueueTask: vi.fn(), enqueueMessageCheck } as any,
onProcess: () => {},
sendMessage: async () => {},
};
await runSchedulerTickOnce(deps);
expect(enqueueMessageCheck).toHaveBeenCalledTimes(1);
expect(enqueueMessageCheck).toHaveBeenCalledWith(
'review@g.us',
expect.stringContaining('review-group'),
);
await runSchedulerTickOnce(deps);
expect(enqueueMessageCheck).toHaveBeenCalledTimes(1);
});
it('can execute one scheduler tick without starting the timer loop', async () => {
const dueAt = new Date(Date.now() - 60_000).toISOString();
createTask({

View File

@@ -12,8 +12,10 @@ import { ASSISTANT_NAME, SCHEDULER_POLL_INTERVAL, TIMEZONE } from './config.js';
import { AgentOutput, runAgentProcess } from './agent-runner.js';
import {
getAllTasks,
getAllOpenPairedTasks,
deleteTask,
getDueTasks,
hasActiveCiWatcherForChat,
getTaskById,
logTaskRun,
updateTask,
@@ -44,6 +46,8 @@ import {
} from './task-suspension.js';
import { getTaskQueueJid, isGitHubCiTask } from './task-watch-status.js';
import { ScheduledTask } from './types.js';
import { resolveGroupIpcPath } from './group-folder.js';
import { schedulePairedFollowUpOnce } from './paired-follow-up-scheduler.js';
import {
hasTaskExceededMaxDuration,
resolveTaskExecutionContext,
@@ -506,6 +510,7 @@ export async function runSchedulerTickOnce(
): Promise<void> {
// Unified service: process all agent types, not just the service default.
const nowMs = Date.now();
reconcileOrphanedPairedReviewReadyTasks(deps);
const activeTasks = getAllTasks().filter((task) => task.status === 'active');
for (const task of activeTasks) {
@@ -550,6 +555,45 @@ export async function runSchedulerTickOnce(
}
}
function reconcileOrphanedPairedReviewReadyTasks(
deps: SchedulerDependencies,
): void {
for (const task of getAllOpenPairedTasks()) {
if (task.status !== 'review_ready') {
continue;
}
if (hasActiveCiWatcherForChat(task.chat_jid)) {
continue;
}
const scheduled = schedulePairedFollowUpOnce({
chatJid: task.chat_jid,
runId: `scheduler-review-ready-${Date.now().toString(36)}`,
task,
intentKind: 'reviewer-turn',
enqueue: () =>
deps.queue.enqueueMessageCheck(
task.chat_jid,
resolveGroupIpcPath(task.group_folder),
),
});
if (!scheduled) {
continue;
}
logger.info(
{
taskId: task.id,
chatJid: task.chat_jid,
groupFolder: task.group_folder,
status: task.status,
},
'Re-queued orphaned review_ready paired task without active CI watcher',
);
}
}
let schedulerRunning = false;
let schedulerTimer: ReturnType<typeof setTimeout> | null = null;
let schedulerLoopFn: (() => Promise<void>) | null = null;