From 83c45c3ce394a0f0e7d066b9e8871e769f672707 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Mon, 6 Apr 2026 20:38:10 +0900 Subject: [PATCH] Extract executor retry decision rules --- src/message-agent-executor-rules.test.ts | 115 +++++++ src/message-agent-executor-rules.ts | 139 ++++++++ src/message-agent-executor.ts | 416 +++++++++-------------- src/paired-turn-fallback.test.ts | 28 ++ src/provider-retry.test.ts | 95 ++++++ src/provider-retry.ts | 132 +++++++ 6 files changed, 662 insertions(+), 263 deletions(-) create mode 100644 src/message-agent-executor-rules.test.ts create mode 100644 src/message-agent-executor-rules.ts diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts new file mode 100644 index 0000000..57471f3 --- /dev/null +++ b/src/message-agent-executor-rules.test.ts @@ -0,0 +1,115 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('./agent-error-detection.js', () => ({ + classifyRotationTrigger: vi.fn(() => ({ + shouldRetry: false, + reason: '', + })), +})); + +vi.mock('./codex-token-rotation.js', () => ({ + detectCodexRotationTrigger: vi.fn(() => ({ + shouldRotate: false, + reason: '', + })), +})); + +import { classifyRotationTrigger } from './agent-error-detection.js'; +import { detectCodexRotationTrigger } from './codex-token-rotation.js'; +import { + isRetryableClaudeSessionFailureAttempt, + resolveClaudeRetryTrigger, + resolveCodexRetryTrigger, + resolvePairedFollowUpQueueAction, +} from './message-agent-executor-rules.js'; + +describe('message-agent-executor-rules', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('resolves a Claude retry trigger from streamed trigger state', () => { + expect( + resolveClaudeRetryTrigger({ + canRetryClaudeCredentials: true, + provider: 'claude', + attempt: { + sawOutput: false, + streamedTriggerReason: { reason: 'auth-expired', retryAfterMs: 5000 }, + }, + }), + ).toEqual({ + reason: 'auth-expired', + retryAfterMs: 5000, + }); + }); + + it('returns null for Claude retry when output was already visible', () => { + expect( + resolveClaudeRetryTrigger({ + canRetryClaudeCredentials: true, + provider: 'claude', + attempt: { sawOutput: true }, + fallbackMessage: '429 rate limit', + }), + ).toBeNull(); + expect(classifyRotationTrigger).not.toHaveBeenCalled(); + }); + + it('resolves a Codex retry trigger from the codex detector', () => { + vi.mocked(detectCodexRotationTrigger).mockReturnValue({ + shouldRotate: true, + reason: 'auth-expired', + }); + + expect( + resolveCodexRetryTrigger({ + canRetryCodex: true, + attempt: {}, + rotationMessage: '401 oauth token has expired', + }), + ).toEqual({ reason: 'auth-expired' }); + }); + + it('detects retryable Claude session failures from either flag or classifier', () => { + expect( + isRetryableClaudeSessionFailureAttempt({ + attempt: { + sawOutput: false, + retryableSessionFailureDetected: true, + }, + isClaudeCodeAgent: true, + provider: 'claude', + shouldRetryFreshSessionOnAgentFailure: vi.fn(() => false), + }), + ).toBe(true); + + const shouldRetryFreshSessionOnAgentFailure = vi.fn(() => true); + expect( + isRetryableClaudeSessionFailureAttempt({ + attempt: { + sawOutput: false, + error: new Error('stale session'), + }, + isClaudeCodeAgent: true, + provider: 'claude', + shouldRetryFreshSessionOnAgentFailure, + }), + ).toBe(true); + expect(shouldRetryFreshSessionOnAgentFailure).toHaveBeenCalledWith({ + result: null, + error: 'stale session', + }); + }); + + it('resolves pending reviewer follow-up requeue from task state', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'reviewer', + executionStatus: 'failed', + sawOutput: false, + taskStatus: 'review_ready', + }), + ).toBe('pending'); + }); +}); diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts new file mode 100644 index 0000000..9215300 --- /dev/null +++ b/src/message-agent-executor-rules.ts @@ -0,0 +1,139 @@ +import { + classifyRotationTrigger, + type AgentTriggerReason, + type CodexRotationReason, +} from './agent-error-detection.js'; +import { detectCodexRotationTrigger } from './codex-token-rotation.js'; +import type { PairedRoomRole, PairedTaskStatus } from './types.js'; +import { getErrorMessage } from './utils.js'; + +export interface ExecutorStreamedTrigger { + reason: AgentTriggerReason; + retryAfterMs?: number; +} + +export interface ExecutorAttemptState { + sawOutput: boolean; + retryableSessionFailureDetected?: boolean; + streamedTriggerReason?: ExecutorStreamedTrigger; + error?: unknown; +} + +export function isRetryableClaudeSessionFailureAttempt(args: { + attempt: ExecutorAttemptState; + isClaudeCodeAgent: boolean; + provider: 'claude' | 'codex'; + shouldRetryFreshSessionOnAgentFailure: (args: { + result: null; + error: string; + }) => boolean; +}): boolean { + if ( + !args.isClaudeCodeAgent || + args.provider !== 'claude' || + args.attempt.sawOutput + ) { + return false; + } + + if (args.attempt.retryableSessionFailureDetected === true) { + return true; + } + + if (args.attempt.error == null) { + return false; + } + + return args.shouldRetryFreshSessionOnAgentFailure({ + result: null, + error: getErrorMessage(args.attempt.error), + }); +} + +export function resolveClaudeRetryTrigger(args: { + canRetryClaudeCredentials: boolean; + provider: 'claude' | 'codex'; + attempt: Pick; + fallbackMessage?: string | null; +}): { reason: AgentTriggerReason; retryAfterMs?: number } | null { + if ( + !( + args.canRetryClaudeCredentials && + args.provider === 'claude' && + !args.attempt.sawOutput + ) + ) { + return null; + } + + if (args.attempt.streamedTriggerReason) { + return { + reason: args.attempt.streamedTriggerReason.reason, + retryAfterMs: args.attempt.streamedTriggerReason.retryAfterMs, + }; + } + + const trigger = classifyRotationTrigger(args.fallbackMessage); + if (!trigger.shouldRetry) { + return null; + } + + return { + reason: trigger.reason, + retryAfterMs: trigger.retryAfterMs, + }; +} + +export function resolveCodexRetryTrigger(args: { + canRetryCodex: boolean; + attempt: Pick; + rotationMessage?: string | null; +}): { reason: CodexRotationReason } | null { + if (!args.canRetryCodex) { + return null; + } + + if (args.attempt.streamedTriggerReason) { + return { + reason: args.attempt.streamedTriggerReason.reason as CodexRotationReason, + }; + } + + const trigger = detectCodexRotationTrigger(args.rotationMessage); + if (!trigger.shouldRotate) { + return null; + } + + return { reason: trigger.reason }; +} + +export type PairedFollowUpQueueAction = + | 'generic' + | 'pending' + | 'skip-inline-finalize' + | 'none'; + +export function resolvePairedFollowUpQueueAction(args: { + completedRole: PairedRoomRole; + executionStatus: 'succeeded' | 'failed'; + sawOutput: boolean; + taskStatus: PairedTaskStatus | null; +}): PairedFollowUpQueueAction { + if (args.executionStatus === 'succeeded' && args.sawOutput) { + return args.completedRole === 'reviewer' && + args.taskStatus === 'merge_ready' + ? 'skip-inline-finalize' + : args.taskStatus !== 'completed' + ? 'generic' + : 'none'; + } + + const shouldRequeuePendingPairedTurn = + (args.completedRole === 'reviewer' || args.completedRole === 'arbiter') && + (args.taskStatus === 'review_ready' || + args.taskStatus === 'in_review' || + args.taskStatus === 'arbiter_requested' || + args.taskStatus === 'in_arbitration' || + args.taskStatus === 'merge_ready'); + return shouldRequeuePendingPairedTurn ? 'pending' : 'none'; +} diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index ecdcf9f..48ab6d5 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -37,12 +37,20 @@ import { resolveEffectiveAgentType, resolveSessionFolder, } from './message-runtime-rules.js'; +import { + isRetryableClaudeSessionFailureAttempt, + resolveClaudeRetryTrigger, + resolveCodexRetryTrigger, + resolvePairedFollowUpQueueAction, +} from './message-agent-executor-rules.js'; import { buildRoomRoleContext } from './room-role-context.js'; import { - classifyRotationTrigger, type AgentTriggerReason, } from './agent-error-detection.js'; -import { runClaudeRotationLoop } from './provider-retry.js'; +import { + runClaudeRotationLoop, + runCodexRotationLoop, +} from './provider-retry.js'; import { shouldResetSessionOnAgentFailure, shouldRetryFreshSessionOnAgentFailure, @@ -69,9 +77,7 @@ import { } from './streamed-output-evaluator.js'; import { detectCodexRotationTrigger, - rotateCodexToken, getCodexAccountCount, - markCodexTokenHealthy, } from './codex-token-rotation.js'; import type { CodexRotationReason } from './agent-error-detection.js'; import { getTokenCount } from './token-rotation.js'; @@ -169,6 +175,7 @@ export async function runAgentForGroup( forceFreshClaudeReviewerSession ? undefined : storedSessionId; + const provider = isClaudeCodeAgent ? 'claude' : 'codex'; const memoryBriefing = currentSessionId ? undefined : await buildRoomMemoryBriefing({ @@ -626,91 +633,27 @@ export async function runAgentForGroup( initialTrigger: { reason: CodexRotationReason }, rotationMessage?: string, ): Promise<'success' | 'error'> => { - let trigger = initialTrigger; - let lastRotationMessage = rotationMessage; - - while ( - getCodexAccountCount() > 1 && - rotateCodexToken(lastRotationMessage) - ) { - log.info( - { reason: trigger.reason }, - 'Codex account unhealthy, retrying with rotated account', - ); - - const retryAttempt = await runAttempt('codex'); - - if (retryAttempt.error) { - const errMsg = getErrorMessage(retryAttempt.error); - const retryTrigger = detectCodexRotationTrigger(errMsg); - if (retryTrigger.shouldRotate) { - trigger = { reason: retryTrigger.reason }; - lastRotationMessage = errMsg; - continue; - } - - log.error( - { provider: 'codex', err: retryAttempt.error }, - 'Rotated Codex account also threw', - ); - return 'error'; - } - - const retryOutput = retryAttempt.output; - if (!retryOutput) { - log.error( - { provider: 'codex' }, - 'Rotated Codex account produced no output object', - ); - return 'error'; - } - - if ( - !retryAttempt.sawOutput && - retryAttempt.streamedTriggerReason && - retryOutput.status !== 'error' - ) { - trigger = { - reason: retryAttempt.streamedTriggerReason - .reason as CodexRotationReason, + const outcome = await runCodexRotationLoop( + initialTrigger, + async () => { + const attempt = await runAttempt('codex'); + return { + output: attempt.output, + thrownError: attempt.error, + sawOutput: attempt.sawOutput, + streamedTriggerReason: attempt.streamedTriggerReason, }; - lastRotationMessage = - typeof retryOutput.result === 'string' - ? retryOutput.result - : undefined; - continue; - } + }, + { + chatJid, + group: group.name, + groupFolder: group.folder, + runId, + }, + rotationMessage, + ); - if (retryOutput.status === 'error') { - const retryTrigger = retryAttempt.streamedTriggerReason - ? { - shouldRotate: true, - reason: retryAttempt.streamedTriggerReason - .reason as CodexRotationReason, - } - : detectCodexRotationTrigger(retryOutput.error); - - if (retryTrigger.shouldRotate) { - trigger = { reason: retryTrigger.reason }; - lastRotationMessage = retryOutput.error ?? undefined; - continue; - } - - log.error( - { - provider: 'codex', - error: retryOutput.error, - }, - 'Rotated Codex account failed', - ); - return 'error'; - } - - markCodexTokenHealthy(); - return 'success'; - } - - return 'error'; + return outcome.type === 'success' ? 'success' : 'error'; }; type AgentAttempt = Awaited>; @@ -754,43 +697,16 @@ export async function runAgentForGroup( } }; - const resolveClaudeRetryTrigger = ( - attempt: AgentAttempt, - fallbackMessage?: string | null, - ): { reason: AgentTriggerReason; retryAfterMs?: number } | null => { - if ( - !( - canRetryClaudeCredentials && - provider === 'claude' && - !attempt.sawOutput - ) - ) { - return null; - } - - if (attempt.streamedTriggerReason) { - return { - reason: attempt.streamedTriggerReason.reason, - retryAfterMs: attempt.streamedTriggerReason.retryAfterMs, - }; - } - - const trigger = classifyRotationTrigger(fallbackMessage); - if (!trigger.shouldRetry) { - return null; - } - - return { - reason: trigger.reason, - retryAfterMs: trigger.retryAfterMs, - }; - }; - const retryClaudeAttemptIfNeeded = async ( attempt: AgentAttempt, rotationMessage?: string | null, ): Promise<'success' | 'error' | null> => { - const trigger = resolveClaudeRetryTrigger(attempt, rotationMessage); + const trigger = resolveClaudeRetryTrigger({ + canRetryClaudeCredentials, + provider, + attempt, + fallbackMessage: rotationMessage, + }); if (!trigger) { return null; } @@ -811,17 +727,12 @@ export async function runAgentForGroup( attempt: AgentAttempt, rotationMessage?: string | null, ): Promise<'success' | 'error' | null> => { - if (isClaudeCodeAgent || getCodexAccountCount() <= 1) { - return null; - } - - const trigger = attempt.streamedTriggerReason - ? { - shouldRotate: true, - reason: attempt.streamedTriggerReason.reason as CodexRotationReason, - } - : detectCodexRotationTrigger(rotationMessage); - if (!trigger.shouldRotate) { + const trigger = resolveCodexRetryTrigger({ + canRetryCodex: !isClaudeCodeAgent && getCodexAccountCount() > 1, + attempt, + rotationMessage, + }); + if (!trigger) { return null; } @@ -845,108 +756,93 @@ export async function runAgentForGroup( return 'error'; }; - type PairedFollowUpQueueAction = - | 'generic' - | 'pending' - | 'skip-inline-finalize' - | 'none'; - type PairedTaskStatus = NonNullable< - ReturnType - >['status']; + const getPairedExecutionStatus = (): 'succeeded' | 'failed' => + pairedExecutionStatus; - const resolvePairedFollowUpQueueAction = ( - completedRole: PairedRoomRole, - executionStatus: 'succeeded' | 'failed', - sawOutput: boolean, - taskStatus: PairedTaskStatus | null, - ): PairedFollowUpQueueAction => { - if (executionStatus === 'succeeded' && sawOutput) { - return completedRole === 'reviewer' && taskStatus === 'merge_ready' - ? 'skip-inline-finalize' - : taskStatus !== 'completed' - ? 'generic' - : 'none'; + const isRetryableClaudeSessionFailure = ( + attempt: AgentAttempt, + ): boolean => + isRetryableClaudeSessionFailureAttempt({ + attempt, + isClaudeCodeAgent, + provider, + shouldRetryFreshSessionOnAgentFailure, + }); + + const recoverRetryableClaudeSessionFailure = async ( + attempt: AgentAttempt, + ): Promise<{ attempt: AgentAttempt; resolved: 'success' | 'error' | null }> => { + if (!isRetryableClaudeSessionFailure(attempt)) { + return { attempt, resolved: null }; } - const shouldRequeuePendingPairedTurn = - (completedRole === 'reviewer' || completedRole === 'arbiter') && - (taskStatus === 'review_ready' || - taskStatus === 'in_review' || - taskStatus === 'arbiter_requested' || - taskStatus === 'in_arbitration' || - taskStatus === 'merge_ready'); - return shouldRequeuePendingPairedTurn ? 'pending' : 'none'; + currentSessionId = undefined; + deps.clearSession(sessionFolder); + clearRoleSdkSessions(); + log.warn( + 'Cleared poisoned Claude session before visible output, retrying fresh session', + ); + + const freshAttempt = await runAttempt('claude'); + if (!isRetryableClaudeSessionFailure(freshAttempt)) { + return { attempt: freshAttempt, resolved: null }; + } + + currentSessionId = undefined; + deps.clearSession(sessionFolder); + log.warn('Fresh Claude retry also hit a retryable session failure'); + log.error('Retryable Claude session failure persisted after fresh retry'); + return { + attempt: freshAttempt, + resolved: maybeHandoffAfterError('session-failure', freshAttempt), + }; }; - const provider = isClaudeCodeAgent ? 'claude' : 'codex'; - - try { - let primaryAttempt = await runAttempt(provider); - - const isRetryableClaudeSessionFailure = ( - attempt: Awaited>, - ): boolean => - isClaudeCodeAgent && - provider === 'claude' && - !attempt.sawOutput && - (attempt.retryableSessionFailureDetected === true || - (attempt.error != null && - shouldRetryFreshSessionOnAgentFailure({ - result: null, - error: getErrorMessage(attempt.error), - }))); - - if (isRetryableClaudeSessionFailure(primaryAttempt)) { - currentSessionId = undefined; - deps.clearSession(sessionFolder); - clearRoleSdkSessions(); - log.warn( - 'Cleared poisoned Claude session before visible output, retrying fresh session', - ); - - primaryAttempt = await runAttempt('claude'); - - if (isRetryableClaudeSessionFailure(primaryAttempt)) { - currentSessionId = undefined; - deps.clearSession(sessionFolder); - log.warn('Fresh Claude retry also hit a retryable session failure'); - - log.error( - 'Retryable Claude session failure persisted after fresh retry', - ); - return maybeHandoffAfterError('session-failure', primaryAttempt); - } + const handlePrimaryAttemptFailure = async ( + attempt: AgentAttempt, + rotationMessage: string, + ): Promise<'success' | 'error'> => { + const claudeRetryResult = await retryClaudeAttemptIfNeeded( + attempt, + rotationMessage, + ); + if (claudeRetryResult) { + return claudeRetryResult; } - if (primaryAttempt.error) { - const errMsg = getErrorMessage(primaryAttempt.error); - const claudeRetryResult = await retryClaudeAttemptIfNeeded( - primaryAttempt, - errMsg, - ); - if (claudeRetryResult) { - return claudeRetryResult; - } - - const codexRetryResult = await retryCodexAttemptIfNeeded( - primaryAttempt, - errMsg, - ); - if (codexRetryResult) { - return codexRetryResult; - } + const codexRetryResult = await retryCodexAttemptIfNeeded( + attempt, + rotationMessage, + ); + if (codexRetryResult) { + return codexRetryResult; + } + if (attempt.error) { log.error( { provider, - err: primaryAttempt.error, + err: attempt.error, }, 'Agent error', ); return 'error'; } - const output = primaryAttempt.output; + log.error( + { + provider, + error: attempt.output?.error, + }, + 'Agent process error', + ); + return 'error'; + }; + + const finalizePrimaryAttempt = async ( + attempt: AgentAttempt, + ): Promise<'success' | 'error'> => { + const output = attempt.output; if (!output) { log.error({ provider }, 'Agent produced no output object'); return 'error'; @@ -963,9 +859,8 @@ export async function runAgentForGroup( : null); } - if (!primaryAttempt.sawOutput && output.status !== 'error') { - const claudeRetryResult = - await retryClaudeAttemptIfNeeded(primaryAttempt); + if (!attempt.sawOutput && output.status !== 'error') { + const claudeRetryResult = await retryClaudeAttemptIfNeeded(attempt); if (claudeRetryResult) { return claudeRetryResult; } @@ -983,34 +878,14 @@ export async function runAgentForGroup( } if (output.status === 'error') { - const claudeRetryResult = await retryClaudeAttemptIfNeeded( - primaryAttempt, - output.error, + return handlePrimaryAttemptFailure( + attempt, + output.error ?? 'Agent process error', ); - if (claudeRetryResult) { - return claudeRetryResult; - } - - const codexRetryResult = await retryCodexAttemptIfNeeded( - primaryAttempt, - output.error, - ); - if (codexRetryResult) { - return codexRetryResult; - } - - log.error( - { - provider, - error: output.error, - }, - 'Agent process error', - ); - return 'error'; } const codexRetryResult = await retryCodexAttemptIfNeeded( - primaryAttempt, + attempt, output.error ?? output.result, ); if (codexRetryResult) { @@ -1018,20 +893,20 @@ export async function runAgentForGroup( } // Unresolved streamed trigger — rotation was unavailable or output was - // already forwarded. Surfaces as an error since there is no alternative provider. - if (primaryAttempt.streamedTriggerReason) { + // already forwarded. Surfaces as an error since there is no alternative provider. + if (attempt.streamedTriggerReason) { if ( isClaudeCodeAgent && maybeHandoffToCodex( - primaryAttempt.streamedTriggerReason.reason, - primaryAttempt.sawVisibleOutput, + attempt.streamedTriggerReason.reason, + attempt.sawVisibleOutput, ) ) { return 'success'; } log.error( { - reason: primaryAttempt.streamedTriggerReason.reason, + reason: attempt.streamedTriggerReason.reason, }, 'Agent trigger detected but could not be resolved', ); @@ -1040,30 +915,45 @@ export async function runAgentForGroup( // success-null-result with no visible output — agent returned nothing useful. // But if output was already delivered to Discord (sawOutput), treat as success. - if ( - primaryAttempt.sawSuccessNullResultWithoutOutput && - !primaryAttempt.sawOutput - ) { - log.error( - 'Agent returned success with null result and no visible output', - ); + if (attempt.sawSuccessNullResultWithoutOutput && !attempt.sawOutput) { + log.error('Agent returned success with null result and no visible output'); return 'error'; } pairedExecutionStatus = 'succeeded'; - pairedSawOutput = primaryAttempt.sawOutput; + pairedSawOutput = attempt.sawOutput; return 'success'; + }; + + try { + let primaryAttempt = await runAttempt(provider); + const recoveredSessionAttempt = + await recoverRetryableClaudeSessionFailure(primaryAttempt); + if (recoveredSessionAttempt.resolved) { + return recoveredSessionAttempt.resolved; + } + primaryAttempt = recoveredSessionAttempt.attempt; + + if (primaryAttempt.error) { + return await handlePrimaryAttemptFailure( + primaryAttempt, + getErrorMessage(primaryAttempt.error), + ); + } + + return await finalizePrimaryAttempt(primaryAttempt); } finally { if (pairedExecutionContext && !pairedExecutionCompleted) { const completedRole = roomRoleContext?.role ?? 'owner'; + const completionStatus = getPairedExecutionStatus(); // Owner was interrupted without producing output (e.g. /stop) — // treat as failed so reviewer is not auto-triggered. const effectiveStatus = completedRole === 'owner' && - pairedExecutionStatus === 'succeeded' && + completionStatus === 'succeeded' && !pairedSawOutput ? 'failed' - : pairedExecutionStatus; + : completionStatus; completePairedExecutionContext({ taskId: pairedExecutionContext.task.id, role: completedRole, @@ -1123,12 +1013,12 @@ export async function runAgentForGroup( if (pairedExecutionContext) { const completedRole = roomRoleContext?.role ?? 'owner'; const finishedCheck = getPairedTaskById(pairedExecutionContext.task.id); - const queueAction = resolvePairedFollowUpQueueAction( + const queueAction = resolvePairedFollowUpQueueAction({ completedRole, - pairedExecutionStatus, - pairedSawOutput, - finishedCheck?.status ?? null, - ); + executionStatus: pairedExecutionStatus, + sawOutput: pairedSawOutput, + taskStatus: finishedCheck?.status ?? null, + }); if (queueAction === 'generic') { deps.queue.enqueueMessageCheck(chatJid); } else if (queueAction === 'skip-inline-finalize') { diff --git a/src/paired-turn-fallback.test.ts b/src/paired-turn-fallback.test.ts index 4917b86..7a9143a 100644 --- a/src/paired-turn-fallback.test.ts +++ b/src/paired-turn-fallback.test.ts @@ -115,4 +115,32 @@ describe('resolveCodexFallbackHandoff', () => { logMessage: 'Fallback disabled for role, skipping handoff', }); }); + + it('returns none when visible output was already emitted', () => { + const result = resolveCodexFallbackHandoff({ + activeRole: 'reviewer', + effectiveAgentType: 'claude-code', + hasReviewer: true, + fallbackEnabled: true, + reason: 'auth-expired', + sawVisibleOutput: true, + prompt: 'please review', + }); + + expect(result).toEqual({ type: 'none' }); + }); + + it('returns none when no reviewer is configured for the room', () => { + const result = resolveCodexFallbackHandoff({ + activeRole: 'owner', + effectiveAgentType: 'claude-code', + hasReviewer: false, + fallbackEnabled: true, + reason: '429', + sawVisibleOutput: false, + prompt: 'please continue', + }); + + expect(result).toEqual({ type: 'none' }); + }); }); diff --git a/src/provider-retry.test.ts b/src/provider-retry.test.ts index dc32d51..4809b16 100644 --- a/src/provider-retry.test.ts +++ b/src/provider-retry.test.ts @@ -9,6 +9,16 @@ vi.mock('./logger.js', () => ({ }, })); +vi.mock('./codex-token-rotation.js', () => ({ + detectCodexRotationTrigger: vi.fn(() => ({ + shouldRotate: false, + reason: '', + })), + getCodexAccountCount: vi.fn(() => 1), + markCodexTokenHealthy: vi.fn(), + rotateCodexToken: vi.fn(() => false), +})); + vi.mock('./token-rotation.js', () => ({ rotateToken: vi.fn(() => false), getTokenCount: vi.fn(() => 1), @@ -21,6 +31,13 @@ vi.mock('./token-refresh.js', () => ({ })); import { runClaudeRotationLoop } from './provider-retry.js'; +import { runCodexRotationLoop } from './provider-retry.js'; +import { + detectCodexRotationTrigger, + getCodexAccountCount, + markCodexTokenHealthy, + rotateCodexToken, +} from './codex-token-rotation.js'; import { getCurrentTokenIndex, getTokenCount, @@ -184,3 +201,81 @@ describe('runClaudeRotationLoop', () => { expect(rotateToken).toHaveBeenNthCalledWith(2, 'retry with next token'); }); }); + +describe('runCodexRotationLoop', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getCodexAccountCount).mockReturnValue(1); + vi.mocked(rotateCodexToken).mockReturnValue(false); + }); + + it('rotates and succeeds after a Codex auth trigger', async () => { + vi.mocked(getCodexAccountCount).mockReturnValue(2); + vi.mocked(rotateCodexToken).mockReturnValueOnce(true); + + const outcome = await runCodexRotationLoop( + { reason: 'auth-expired' }, + async () => ({ + output: { status: 'success', result: 'ok' }, + sawOutput: true, + }), + { runId: 'rotate-codex-auth' }, + ); + + expect(outcome).toEqual({ type: 'success' }); + expect(rotateCodexToken).toHaveBeenCalledTimes(1); + expect(markCodexTokenHealthy).toHaveBeenCalledTimes(1); + }); + + it('continues rotation when streamed trigger arrives before visible output', async () => { + vi.mocked(getCodexAccountCount).mockReturnValue(2); + vi.mocked(rotateCodexToken).mockReturnValueOnce(true).mockReturnValueOnce(false); + + let attempts = 0; + const outcome = await runCodexRotationLoop( + { reason: 'auth-expired' }, + async () => { + attempts += 1; + if (attempts === 1) { + return { + output: { status: 'success', result: 'rotate next' }, + sawOutput: false, + streamedTriggerReason: { reason: 'auth-expired' }, + }; + } + + return { + output: { status: 'success', result: 'ok' }, + sawOutput: true, + }; + }, + { runId: 'rotate-codex-streamed' }, + ); + + expect(outcome).toEqual({ type: 'error' }); + expect(rotateCodexToken).toHaveBeenNthCalledWith(2, 'rotate next'); + }); + + it('uses the Codex detector for thrown errors', async () => { + vi.mocked(getCodexAccountCount).mockReturnValue(2); + vi.mocked(rotateCodexToken).mockReturnValueOnce(true); + vi.mocked(detectCodexRotationTrigger).mockReturnValue({ + shouldRotate: true, + reason: 'auth-expired', + }); + + const outcome = await runCodexRotationLoop( + { reason: 'auth-expired' }, + async () => ({ + thrownError: new Error('OAuth token expired'), + sawOutput: false, + }), + { runId: 'rotate-codex-error' }, + ); + + expect(outcome).toEqual({ type: 'error' }); + expect(detectCodexRotationTrigger).toHaveBeenCalledWith( + 'OAuth token expired', + ); + }); +}); diff --git a/src/provider-retry.ts b/src/provider-retry.ts index 149b5d1..f19081b 100644 --- a/src/provider-retry.ts +++ b/src/provider-retry.ts @@ -9,9 +9,16 @@ import type { AgentOutput } from './agent-runner.js'; import { getAgentOutputText } from './agent-output.js'; import { classifyRotationTrigger, + type CodexRotationReason, shouldRotateClaudeToken, type AgentTriggerReason, } from './agent-error-detection.js'; +import { + detectCodexRotationTrigger, + getCodexAccountCount, + markCodexTokenHealthy, + rotateCodexToken, +} from './codex-token-rotation.js'; import { logger } from './logger.js'; import { getErrorMessage } from './utils.js'; import { @@ -42,6 +49,15 @@ export type RotationOutcome = | { type: 'success'; sawOutput: boolean } | { type: 'error'; trigger?: TriggerInfo }; +export interface CodexRotationAttemptResult { + output?: Pick; + thrownError?: unknown; + sawOutput: boolean; + streamedTriggerReason?: TriggerInfo; +} + +export type CodexRotationOutcome = { type: 'success' } | { type: 'error' }; + type AttemptOutcome = | { type: 'success'; sawOutput: boolean } | { type: 'error'; trigger?: TriggerInfo } @@ -51,6 +67,15 @@ type AttemptOutcome = rotationMessage?: string; }; +type CodexAttemptOutcome = + | { type: 'success' } + | { type: 'error' } + | { + type: 'continue'; + trigger: { reason: CodexRotationReason }; + rotationMessage?: string; + }; + function evaluateClaudeAttempt( attempt: RotationAttemptResult, logContext: Record, @@ -149,6 +174,77 @@ function evaluateClaudeAttempt( return { type: 'success', sawOutput: attempt.sawOutput }; } +function evaluateCodexAttempt( + attempt: CodexRotationAttemptResult, + logContext: Record, +): CodexAttemptOutcome { + if (attempt.thrownError) { + const errMsg = getErrorMessage(attempt.thrownError); + const retryTrigger = detectCodexRotationTrigger(errMsg); + if (retryTrigger.shouldRotate) { + return { + type: 'continue', + trigger: { reason: retryTrigger.reason }, + rotationMessage: errMsg, + }; + } + + logger.error( + { ...logContext, provider: 'codex', err: attempt.thrownError }, + 'Rotated Codex account also threw', + ); + return { type: 'error' }; + } + + const output = attempt.output; + if (!output) { + logger.error( + { ...logContext, provider: 'codex' }, + 'Rotated Codex account produced no output object', + ); + return { type: 'error' }; + } + + if ( + !attempt.sawOutput && + attempt.streamedTriggerReason && + output.status !== 'error' + ) { + return { + type: 'continue', + trigger: { + reason: attempt.streamedTriggerReason.reason as CodexRotationReason, + }, + rotationMessage: getAgentOutputText(output) ?? undefined, + }; + } + + if (output.status === 'error') { + const retryTrigger = attempt.streamedTriggerReason + ? { + shouldRotate: true, + reason: attempt.streamedTriggerReason.reason as CodexRotationReason, + } + : detectCodexRotationTrigger(output.error); + if (retryTrigger.shouldRotate) { + return { + type: 'continue', + trigger: { reason: retryTrigger.reason }, + rotationMessage: output.error ?? undefined, + }; + } + + logger.error( + { ...logContext, provider: 'codex', error: output.error }, + 'Rotated Codex account failed', + ); + return { type: 'error' }; + } + + markCodexTokenHealthy(); + return { type: 'success' }; +} + // ── Shared rotation loop ───────────────────────────────────────── /** @@ -238,3 +334,39 @@ export async function runClaudeRotationLoop( ); return { type: 'error', trigger }; } + +export async function runCodexRotationLoop( + initialTrigger: { reason: CodexRotationReason }, + runAttempt: () => Promise, + logContext: Record, + rotationMessage?: string, +): Promise { + let trigger = initialTrigger; + let lastRotationMessage = rotationMessage; + + while ( + getCodexAccountCount() > 1 && + rotateCodexToken(lastRotationMessage) + ) { + logger.info( + { ...logContext, reason: trigger.reason }, + 'Codex account unhealthy, retrying with rotated account', + ); + + const rotatedAttemptOutcome = evaluateCodexAttempt( + await runAttempt(), + logContext, + ); + if (rotatedAttemptOutcome.type === 'success') { + return rotatedAttemptOutcome; + } + if (rotatedAttemptOutcome.type === 'error') { + return rotatedAttemptOutcome; + } + + trigger = rotatedAttemptOutcome.trigger; + lastRotationMessage = rotatedAttemptOutcome.rotationMessage; + } + + return { type: 'error' }; +}