diff --git a/src/agent-attempt-retry.test.ts b/src/agent-attempt-retry.test.ts new file mode 100644 index 0000000..816d05e --- /dev/null +++ b/src/agent-attempt-retry.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; + +import { resolveAttemptRetryAction } from './agent-attempt-retry.js'; + +describe('resolveAttemptRetryAction', () => { + it('uses the streamed Codex trigger message as the rotation message', () => { + const errorMessage = + 'unexpected status 401 Unauthorized: Missing bearer or basic authentication in header'; + + const action = resolveAttemptRetryAction({ + provider: 'codex', + canRetryClaudeCredentials: false, + canRetryCodex: true, + attempt: { + sawOutput: false, + streamedTriggerReason: { + reason: 'auth-expired', + message: errorMessage, + } as any, + }, + }); + + expect(action).toEqual({ + kind: 'codex', + trigger: { reason: 'auth-expired' }, + rotationMessage: errorMessage, + }); + }); +}); diff --git a/src/agent-attempt-retry.ts b/src/agent-attempt-retry.ts index 49b5491..bafca9c 100644 --- a/src/agent-attempt-retry.ts +++ b/src/agent-attempt-retry.ts @@ -10,6 +10,7 @@ import { getErrorMessage } from './utils.js'; export interface AttemptStreamedTrigger { reason: AgentTriggerReason; retryAfterMs?: number; + message?: string; } export interface AttemptRetryState { @@ -131,7 +132,10 @@ export function resolveAttemptRetryAction(args: { attempt: Pick; rotationMessage?: string | null; }): AttemptRetryAction { - const normalizedRotationMessage = args.rotationMessage ?? undefined; + const normalizedRotationMessage = + args.rotationMessage ?? + args.attempt.streamedTriggerReason?.message ?? + undefined; const claudeTrigger = resolveClaudeRetryTrigger({ canRetryClaudeCredentials: args.canRetryClaudeCredentials, diff --git a/src/agent-error-detection.test.ts b/src/agent-error-detection.test.ts index cad0f6e..79cbd1b 100644 --- a/src/agent-error-detection.test.ts +++ b/src/agent-error-detection.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { classifyAgentError, classifyClaudeAuthError, + classifyCodexAuthError, detectClaudeProviderFailureMessage, isClaudeOrgAccessDeniedMessage, shouldRotateClaudeToken, @@ -66,6 +67,32 @@ describe('agent-error-detection', () => { }); }); + it('classifies Codex reused refresh-token errors as auth-expired', () => { + expect( + classifyCodexAuthError( + 'Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.', + ), + ).toEqual({ + category: 'auth-expired', + reason: 'auth-expired', + }); + }); + + it('classifies a Codex auth-expired trigger reason as auth-expired', () => { + expect(classifyCodexAuthError('auth-expired')).toEqual({ + category: 'auth-expired', + reason: 'auth-expired', + }); + }); + + it('classifies Codex workspace credit exhaustion as rate-limit', () => { + expect(classifyAgentError('Workspace out of credits')).toEqual({ + category: 'rate-limit', + reason: '429', + retryAfterMs: undefined, + }); + }); + it('marks only Claude quota/auth reasons as Claude rotation reasons', () => { expect(shouldRotateClaudeToken('429')).toBe(true); expect(shouldRotateClaudeToken('usage-exhausted')).toBe(true); diff --git a/src/agent-error-detection.ts b/src/agent-error-detection.ts index 1eff808..9664e1e 100644 --- a/src/agent-error-detection.ts +++ b/src/agent-error-detection.ts @@ -249,6 +249,8 @@ export function classifyAgentError( lower.includes('429') || lower.includes('rate limit') || lower.includes('usage limit') || + lower.includes('out of credits') || + lower.includes('workspace out of credits') || lower.includes('hit your limit') || lower.includes('too many requests') || lower.includes('rate_limit') @@ -335,11 +337,17 @@ export function classifyCodexAuthError( const lower = error.toLowerCase(); if ( + lower.includes('auth-expired') || + lower.includes('auth expired') || lower.includes('401') || lower.includes('authentication_error') || lower.includes('failed to authenticate') || + lower.includes('access token could not be refreshed') || lower.includes('oauth token has expired') || + lower.includes('refresh token was already used') || lower.includes('refresh your existing token') || + lower.includes('log out and sign in again') || + lower.includes('app_session_terminated') || lower.includes('unauthorized') ) { return { category: 'auth-expired', reason: 'auth-expired' }; diff --git a/src/agent-runner-environment.test.ts b/src/agent-runner-environment.test.ts index 1c6b8f8..c944a05 100644 --- a/src/agent-runner-environment.test.ts +++ b/src/agent-runner-environment.test.ts @@ -5,9 +5,18 @@ import path from 'path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { EJCLAW_ENV } from 'ejclaw-runners-shared'; -const { mockReadEnvFile, mockGetActiveCodexAuthPath } = vi.hoisted(() => ({ +const { + mockReadEnvFile, + mockGetActiveCodexAuthPath, + mockGetCodexAccountCount, + mockClaimCodexAuthLease, + mockFindCodexAccountIndexByAuthPath, +} = vi.hoisted(() => ({ mockReadEnvFile: vi.fn<() => Record>(), mockGetActiveCodexAuthPath: vi.fn<() => string | null>(), + mockGetCodexAccountCount: vi.fn<() => number>(), + mockClaimCodexAuthLease: vi.fn<() => null>(), + mockFindCodexAccountIndexByAuthPath: vi.fn<() => number | null>(), })); vi.mock('./config.js', () => ({ @@ -29,6 +38,9 @@ vi.mock('./env.js', () => ({ vi.mock('./codex-token-rotation.js', () => ({ getActiveCodexAuthPath: mockGetActiveCodexAuthPath, + getCodexAccountCount: mockGetCodexAccountCount, + claimCodexAuthLease: mockClaimCodexAuthLease, + findCodexAccountIndexByAuthPath: mockFindCodexAccountIndexByAuthPath, })); vi.mock('./token-rotation.js', () => ({ @@ -159,6 +171,12 @@ describe('prepareGroupEnvironment codex auth handling', () => { mockReadEnvFile.mockReset(); mockGetActiveCodexAuthPath.mockReset(); + mockGetCodexAccountCount.mockReset(); + mockGetCodexAccountCount.mockReturnValue(0); + mockClaimCodexAuthLease.mockReset(); + mockClaimCodexAuthLease.mockReturnValue(null); + mockFindCodexAccountIndexByAuthPath.mockReset(); + mockFindCodexAccountIndexByAuthPath.mockReturnValue(null); }); afterEach(() => { @@ -240,6 +258,36 @@ describe('prepareGroupEnvironment codex auth handling', () => { expect(auth).toEqual(rotatedAuth); }); + it('fails fast instead of launching Codex without OAuth when rotation accounts exist but no lease is available', () => { + const rotatedAuthPath = path.join(tempRoot, 'rotated-auth.json'); + fs.writeFileSync( + rotatedAuthPath, + JSON.stringify({ + auth_mode: 'chatgpt', + tokens: { access_token: 'locked-token' }, + }), + ); + mockGetCodexAccountCount.mockReturnValue(1); + mockClaimCodexAuthLease.mockReturnValue(null); + mockGetActiveCodexAuthPath.mockReturnValue(rotatedAuthPath); + mockReadEnvFile.mockReturnValue({}); + + expect(() => prepareGroupEnvironment(group, false, 'dc:test')).toThrow( + /auth-expired: All Codex rotation accounts unavailable/, + ); + + const authPath = path.join( + tempRoot, + 'sessions', + group.folder, + 'services', + 'codex-main', + '.codex', + 'auth.json', + ); + expect(fs.existsSync(authPath)).toBe(false); + }); + it('uses the failover owner prompt pack for codex-review when it owns an explicit failover lease', () => { vi.mocked(config.isReviewService).mockReturnValue(true); vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); @@ -411,6 +459,12 @@ describe('prepareGroupEnvironment Codex MCP room role env', () => { mockReadEnvFile.mockReset(); mockGetActiveCodexAuthPath.mockReset(); + mockGetCodexAccountCount.mockReset(); + mockGetCodexAccountCount.mockReturnValue(0); + mockClaimCodexAuthLease.mockReset(); + mockClaimCodexAuthLease.mockReturnValue(null); + mockFindCodexAccountIndexByAuthPath.mockReset(); + mockFindCodexAccountIndexByAuthPath.mockReturnValue(null); resetRoutingMocks(); }); @@ -484,6 +538,12 @@ describe('prepareGroupEnvironment room skill overrides', () => { }); mockReadEnvFile.mockReset(); mockGetActiveCodexAuthPath.mockReset(); + mockGetCodexAccountCount.mockReset(); + mockGetCodexAccountCount.mockReturnValue(0); + mockClaimCodexAuthLease.mockReset(); + mockClaimCodexAuthLease.mockReturnValue(null); + mockFindCodexAccountIndexByAuthPath.mockReset(); + mockFindCodexAccountIndexByAuthPath.mockReturnValue(null); }); afterEach(() => { @@ -616,6 +676,12 @@ describe('prepareGroupEnvironment Codex goals handling', () => { mockReadEnvFile.mockReset(); mockGetActiveCodexAuthPath.mockReset(); + mockGetCodexAccountCount.mockReset(); + mockGetCodexAccountCount.mockReturnValue(0); + mockClaimCodexAuthLease.mockReset(); + mockClaimCodexAuthLease.mockReturnValue(null); + mockFindCodexAccountIndexByAuthPath.mockReset(); + mockFindCodexAccountIndexByAuthPath.mockReturnValue(null); vi.mocked(config.isReviewService).mockReturnValue(false); vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(false); vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ @@ -706,6 +772,12 @@ describe('prepareReadonlySessionEnvironment codex compatibility', () => { mockReadEnvFile.mockReset(); mockGetActiveCodexAuthPath.mockReset(); + mockGetCodexAccountCount.mockReset(); + mockGetCodexAccountCount.mockReturnValue(0); + mockClaimCodexAuthLease.mockReset(); + mockClaimCodexAuthLease.mockReturnValue(null); + mockFindCodexAccountIndexByAuthPath.mockReset(); + mockFindCodexAccountIndexByAuthPath.mockReturnValue(null); }); afterEach(() => { diff --git a/src/agent-runner-environment.ts b/src/agent-runner-environment.ts index e857acb..e5bebf9 100644 --- a/src/agent-runner-environment.ts +++ b/src/agent-runner-environment.ts @@ -12,7 +12,13 @@ import { } from './config.js'; import { logger } from './logger.js'; import { readEnvFile } from './env.js'; -import { getActiveCodexAuthPath } from './codex-token-rotation.js'; +import { + claimCodexAuthLease, + findCodexAccountIndexByAuthPath, + getActiveCodexAuthPath, + getCodexAccountCount, + type CodexAuthLease, +} from './codex-token-rotation.js'; import { readCodexFeatureFromFile } from './codex-config-features.js'; import { ensureClaudeSessionSettings } from './claude-session-settings.js'; import { @@ -140,17 +146,32 @@ function ensureClaudeGlobalSettingsFile(sessionDir: string): void { fs.writeFileSync(settingsFile, '{}\n'); } -function syncHostCodexSessionFiles(sessionCodexDir: string): void { +export interface PreparedCodexSessionAuth { + canonicalAuthPath: string; + sessionAuthPath: string; + accountIndex: number | null; + lease?: CodexAuthLease; +} + +function syncHostCodexSessionFiles( + sessionCodexDir: string, +): PreparedCodexSessionAuth | null { const hostCodexDir = path.join(os.homedir(), '.codex'); fs.mkdirSync(sessionCodexDir, { recursive: true }); const authDst = path.join(sessionCodexDir, 'auth.json'); - const rotatedAuthSrc = getActiveCodexAuthPath(); + const hasRotationAccounts = getCodexAccountCount() > 0; + const lease = claimCodexAuthLease(); + const rotatedAuthSrc = + lease?.authPath ?? (!hasRotationAccounts ? getActiveCodexAuthPath() : null); + const fallbackAuthSrc = path.join(hostCodexDir, 'auth.json'); const authSrc = rotatedAuthSrc && fs.existsSync(rotatedAuthSrc) ? rotatedAuthSrc - : path.join(hostCodexDir, 'auth.json'); - if (fs.existsSync(authSrc)) { + : !hasRotationAccounts && fs.existsSync(fallbackAuthSrc) + ? fallbackAuthSrc + : null; + if (authSrc) { fs.copyFileSync(authSrc, authDst); } else if (fs.existsSync(authDst)) { fs.unlinkSync(authDst); @@ -163,6 +184,28 @@ function syncHostCodexSessionFiles(sessionCodexDir: string): void { fs.copyFileSync(src, dst); } } + + if ( + !rotatedAuthSrc || + authSrc !== rotatedAuthSrc || + !fs.existsSync(authDst) + ) { + lease?.release(); + if (hasRotationAccounts) { + throw new Error( + 'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex', + ); + } + return null; + } + + return { + canonicalAuthPath: rotatedAuthSrc, + sessionAuthPath: authDst, + accountIndex: + lease?.accountIndex ?? findCodexAccountIndexByAuthPath(rotatedAuthSrc), + ...(lease ? { lease } : {}), + }; } function upsertEjclawMcpServerSection(args: { @@ -357,7 +400,7 @@ function prepareCodexSessionEnvironment(args: { useFailoverPromptPack: boolean; memoryBriefing?: string; skillOverrides?: StoredRoomSkillOverride[]; -}): void { +}): PreparedCodexSessionAuth | null { // API key auth intentionally removed — Codex uses OAuth only. // Never pass any API key to Codex child process to prevent API billing. delete args.env.OPENAI_API_KEY; @@ -376,7 +419,7 @@ function prepareCodexSessionEnvironment(args: { if (codexEffort) args.env.CODEX_EFFORT = codexEffort; const sessionCodexDir = path.join(args.sessionRootDir, '.codex'); - syncHostCodexSessionFiles(sessionCodexDir); + const codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir); const overlayPath = path.join(args.groupDir, '.codex', 'config.toml'); const sessionConfigPath = path.join(sessionCodexDir, 'config.toml'); @@ -506,16 +549,19 @@ function prepareCodexSessionEnvironment(args: { delete args.env.ANTHROPIC_BASE_URL; delete args.env.CLAUDE_CODE_OAUTH_TOKEN; args.env.CODEX_HOME = sessionCodexDir; + return codexSessionAuth; } export interface PreparedGroupEnvironment { env: Record; groupDir: string; runnerDir: string; + codexSessionAuth?: PreparedCodexSessionAuth | null; } export interface PreparedReadonlySessionEnvironment { codexHomeDir?: string; + codexSessionAuth?: PreparedCodexSessionAuth | null; } export function prepareGroupEnvironment( @@ -657,8 +703,9 @@ export function prepareGroupEnvironment( runtimeTaskId, }); + let codexSessionAuth: PreparedCodexSessionAuth | null = null; if (agentType === 'codex') { - prepareCodexSessionEnvironment({ + codexSessionAuth = prepareCodexSessionEnvironment({ env, envVars, projectRoot, @@ -677,7 +724,7 @@ export function prepareGroupEnvironment( prepareClaudeEnvironment({ env, envVars, group }); } - return { env, groupDir, runnerDir }; + return { env, groupDir, runnerDir, codexSessionAuth }; } /** @@ -716,6 +763,7 @@ export function prepareReadonlySessionEnvironment(args: { skillOverrides, } = args; const projectRoot = process.cwd(); + let codexSessionAuth: PreparedCodexSessionAuth | null = null; fs.mkdirSync(sessionDir, { recursive: true }); ensureClaudeSessionSettings(sessionDir); @@ -785,7 +833,7 @@ export function prepareReadonlySessionEnvironment(args: { if (sessionClaudeMd) { fs.writeFileSync(sessionClaudeMdPath, sessionClaudeMd + '\n'); const sessionCodexDir = path.join(sessionDir, '.codex'); - syncHostCodexSessionFiles(sessionCodexDir); + codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir); fs.writeFileSync( path.join(sessionCodexDir, 'AGENTS.md'), sessionClaudeMd + '\n', @@ -826,5 +874,5 @@ export function prepareReadonlySessionEnvironment(args: { } else if (fs.existsSync(sessionClaudeMdPath)) { fs.unlinkSync(sessionClaudeMdPath); } - return { codexHomeDir }; + return { codexHomeDir, codexSessionAuth }; } diff --git a/src/agent-runner.ts b/src/agent-runner.ts index fae5827..7d7a234 100644 --- a/src/agent-runner.ts +++ b/src/agent-runner.ts @@ -14,7 +14,9 @@ import { import { prepareReadonlySessionEnvironment, prepareGroupEnvironment, + type PreparedCodexSessionAuth, } from './agent-runner-environment.js'; +import { syncCodexSessionAuthBack } from './codex-token-rotation.js'; import { runSpawnedAgentProcess } from './agent-runner-process.js'; import { getStoredRoomSkillOverrides } from './db.js'; export { @@ -76,6 +78,36 @@ function readRoomSkillOverridesForRunner( } } +function releaseCodexAuthSession( + auth: PreparedCodexSessionAuth | null | undefined, +): void { + auth?.lease?.release(); +} + +function finalizeCodexAuthSession( + auth: PreparedCodexSessionAuth | null | undefined, +): void { + if (!auth) return; + try { + syncCodexSessionAuthBack({ + canonicalAuthPath: auth.canonicalAuthPath, + sessionAuthPath: auth.sessionAuthPath, + accountIndex: auth.accountIndex, + }); + } catch (err) { + logger.warn( + { + err, + canonicalAuthPath: auth.canonicalAuthPath, + accountIndex: auth.accountIndex, + }, + 'Failed to sync Codex session auth back to canonical slot', + ); + } finally { + auth.lease?.release(); + } +} + export async function runAgentProcess( group: RegisteredGroup, input: AgentInput, @@ -92,18 +124,15 @@ export async function runAgentProcess( // ── Host process mode (owner) ─────────────────────────────────── const startTime = Date.now(); const skillOverrides = readRoomSkillOverridesForRunner(input.chatJid); - const { env, groupDir, runnerDir } = prepareGroupEnvironment( - group, - input.isMain, - input.chatJid, - { - memoryBriefing: input.memoryBriefing, - runtimeTaskId: input.runtimeTaskId, - useTaskScopedSession: input.useTaskScopedSession, - skillOverrides, - roomRole: input.roomRoleContext?.role, - }, - ); + const prepared = prepareGroupEnvironment(group, input.isMain, input.chatJid, { + memoryBriefing: input.memoryBriefing, + runtimeTaskId: input.runtimeTaskId, + useTaskScopedSession: input.useTaskScopedSession, + skillOverrides, + roomRole: input.roomRoleContext?.role, + }); + const { env, groupDir, runnerDir } = prepared; + let codexSessionAuth = prepared.codexSessionAuth; // Apply env overrides (caller-provided) if (envOverrides) { @@ -131,6 +160,8 @@ export async function runAgentProcess( skillOverrides, }); if ((group.agentType || 'claude-code') === 'codex') { + releaseCodexAuthSession(codexSessionAuth); + codexSessionAuth = readonlySession.codexSessionAuth; env.CODEX_HOME = path.join(envOverrides.CLAUDE_CONFIG_DIR, '.codex'); if (readonlySession.codexHomeDir) { env.HOME = readonlySession.codexHomeDir; @@ -152,6 +183,7 @@ export async function runAgentProcess( { runnerDir, chatJid: input.chatJid, runId: input.runId }, 'Runner not built. Run: cd runners/agent-runner && bun install && bun run build', ); + releaseCodexAuthSession(codexSessionAuth); return { status: 'error', result: null, @@ -199,6 +231,22 @@ export async function runAgentProcess( logsDir, startTime, onOutput, - }).then(resolve); + }) + .then((output) => { + finalizeCodexAuthSession(codexSessionAuth); + resolve(output); + }) + .catch((err: unknown) => { + finalizeCodexAuthSession(codexSessionAuth); + logger.error( + { err, processName, chatJid: input.chatJid, runId: input.runId }, + 'Spawned agent process runner failed', + ); + resolve({ + status: 'error', + result: null, + error: err instanceof Error ? err.message : String(err), + }); + }); }); } diff --git a/src/codex-token-rotation.test.ts b/src/codex-token-rotation.test.ts index 85dc5f6..be4ec7b 100644 --- a/src/codex-token-rotation.test.ts +++ b/src/codex-token-rotation.test.ts @@ -165,6 +165,192 @@ describe('codex-token-rotation d7 ≥ 100% auto-skip', () => { expect(mod.getCodexAccountCount()).toBe(4); expect(mod.getActiveCodexAuthPath()).not.toBe(fallbackAuthPath); }); + + 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({ + category: 'auth-expired', + reason: 'auth-expired', + }); + + const mod = await import('./codex-token-rotation.js'); + mod.initCodexTokenRotation(); + + expect(mod.rotateCodexToken('refresh token was already used')).toBe(true); + + const accounts = mod.getAllCodexAccounts(); + expect(accounts[0]).toEqual( + expect.objectContaining({ + isAuthDead: true, + authStatus: 'dead_auth', + }), + ); + expect(accounts[0].isRateLimited).toBe(false); + expect(accounts[1].isActive).toBe(true); + }); + + it('recovers a dead_auth account when canonical auth.json is refreshed before lease claim', async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z')); + try { + 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'); + const utils = await import('./utils.js'); + mod.initCodexTokenRotation(); + + expect(mod.rotateCodexToken('refresh token was already used')).toBe(true); + expect(mod.getAllCodexAccounts()[0].isAuthDead).toBe(true); + + const refreshedAt = new Date('2026-01-01T00:00:10.000Z'); + const authPath = path.join(tempHome, '.codex-accounts', '0', 'auth.json'); + fs.writeFileSync( + authPath, + JSON.stringify({ + auth_mode: 'chatgpt', + tokens: { account_id: 'acct-0', access_token: 'refreshed-token' }, + }), + ); + fs.utimesSync(authPath, refreshedAt, refreshedAt); + + mod.setCurrentCodexAccountIndex(0); + const lease = mod.claimCodexAuthLease(); + try { + expect(lease).toEqual( + expect.objectContaining({ accountIndex: 0, authPath }), + ); + expect(mod.getAllCodexAccounts()[0]).toEqual( + expect.objectContaining({ + authStatus: 'healthy', + isAuthDead: false, + isActive: true, + }), + ); + expect(vi.mocked(utils.writeJsonFile)).toHaveBeenCalled(); + } finally { + lease?.release(); + } + } finally { + vi.useRealTimers(); + } + }); + + it('leases different accounts for concurrent Codex runs', async () => { + const mod = await import('./codex-token-rotation.js'); + mod.initCodexTokenRotation(); + + const first = mod.claimCodexAuthLease(); + const second = mod.claimCodexAuthLease(); + + try { + expect(first).toEqual( + expect.objectContaining({ + accountIndex: 0, + authPath: expect.any(String), + }), + ); + expect(second).toEqual( + expect.objectContaining({ + accountIndex: 1, + authPath: expect.any(String), + }), + ); + expect(second?.authPath).not.toBe(first?.authPath); + } finally { + second?.release(); + first?.release(); + } + }); + + it('syncs a refreshed session auth.json back to the canonical slot atomically', async () => { + const mod = await import('./codex-token-rotation.js'); + mod.initCodexTokenRotation(); + + const canonicalAuthPath = path.join( + tempHome, + '.codex-accounts', + '0', + 'auth.json', + ); + const sessionAuthPath = path.join( + tempHome, + 'session', + '.codex', + 'auth.json', + ); + fs.mkdirSync(path.dirname(sessionAuthPath), { recursive: true }); + fs.writeFileSync( + sessionAuthPath, + JSON.stringify({ + auth_mode: 'chatgpt', + tokens: { + account_id: 'acct-0', + access_token: 'new-access', + refresh_token: 'new-refresh', + }, + }), + ); + + const synced = mod.syncCodexSessionAuthBack({ + canonicalAuthPath, + sessionAuthPath, + accountIndex: 0, + }); + + expect(synced).toBe(true); + const canonical = JSON.parse(fs.readFileSync(canonicalAuthPath, 'utf-8')); + expect(canonical.tokens).toEqual( + expect.objectContaining({ + account_id: 'acct-0', + access_token: 'new-access', + refresh_token: 'new-refresh', + }), + ); + }); + + it('refuses to sync a session auth.json that belongs to another Codex account', async () => { + const mod = await import('./codex-token-rotation.js'); + mod.initCodexTokenRotation(); + + const canonicalAuthPath = path.join( + tempHome, + '.codex-accounts', + '0', + 'auth.json', + ); + const before = fs.readFileSync(canonicalAuthPath, 'utf-8'); + const sessionAuthPath = path.join( + tempHome, + 'wrong-account', + '.codex', + 'auth.json', + ); + fs.mkdirSync(path.dirname(sessionAuthPath), { recursive: true }); + fs.writeFileSync( + sessionAuthPath, + JSON.stringify({ + auth_mode: 'chatgpt', + tokens: { + account_id: 'acct-other', + access_token: 'other-access', + refresh_token: 'other-refresh', + }, + }), + ); + + expect( + mod.syncCodexSessionAuthBack({ + canonicalAuthPath, + sessionAuthPath, + accountIndex: 0, + }), + ).toBe(false); + expect(fs.readFileSync(canonicalAuthPath, 'utf-8')).toBe(before); + }); }); describe('codex-token-rotation single-account fallback', () => { diff --git a/src/codex-token-rotation.ts b/src/codex-token-rotation.ts index aad2c6a..1ed0f93 100644 --- a/src/codex-token-rotation.ts +++ b/src/codex-token-rotation.ts @@ -23,7 +23,6 @@ import { DATA_DIR } from './config.js'; import { logger } from './logger.js'; import { computeCooldownUntil, - findNextAvailable, parseRetryAfterFromError, } from './token-rotation-base.js'; import { readJsonFile, writeJsonFile } from './utils.js'; @@ -34,15 +33,27 @@ interface CodexAccount { index: number; authPath: string; accountId: string; + authFileMtimeMs: number; planType: string; subscriptionUntil: string | null; rateLimitedUntil: number | null; + authStatus: 'healthy' | 'dead_auth'; + authDeadAt: number | null; + authDeadReason: string | null; + leasedUntil: number | null; + leaseId: string | null; lastUsagePct?: number; lastUsageD7Pct?: number; resetAt?: string; resetD7At?: string; } +export interface CodexAuthLease { + accountIndex: number; + authPath: string; + release: () => void; +} + export type CodexRotationTriggerResult = | { shouldRotate: false; @@ -77,10 +88,20 @@ const accounts: CodexAccount[] = []; let currentIndex = 0; let initialized = false; +const CODEX_AUTH_LEASE_MS = 2 * 60 * 60_000; + const ACCOUNTS_DIR = path.join(os.homedir(), '.codex-accounts'); const DEFAULT_CODEX_DIR = path.join(os.homedir(), '.codex'); const DEFAULT_AUTH_PATH = path.join(DEFAULT_CODEX_DIR, 'auth.json'); +function readAuthFileMtimeMs(authPath: string): number { + try { + return fs.statSync(authPath).mtimeMs; + } catch { + return 0; + } +} + function loadCodexAccount( authPath: string, fallbackAccountId: string, @@ -99,9 +120,15 @@ function loadCodexAccount( index: accounts.length, authPath, accountId, + authFileMtimeMs: readAuthFileMtimeMs(authPath), planType: jwt.planType, subscriptionUntil: jwt.expiresAt, rateLimitedUntil: null, + authStatus: 'healthy', + authDeadAt: null, + authDeadReason: null, + leasedUntil: null, + leaseId: null, }); return true; } @@ -149,6 +176,8 @@ function saveCodexState(): void { const state = { currentIndex, rateLimits: accounts.map((a) => a.rateLimitedUntil), + authDeadAts: accounts.map((a) => a.authDeadAt), + authDeadReasons: accounts.map((a) => a.authDeadReason), usagePcts: accounts.map((a) => a.lastUsagePct ?? null), usageD7Pcts: accounts.map((a) => a.lastUsageD7Pct ?? null), resetAts: accounts.map((a) => a.resetAt ?? null), @@ -167,6 +196,8 @@ function loadCodexState(quiet = false): void { const state = readJsonFile<{ currentIndex?: number; rateLimits?: (number | null)[]; + authDeadAts?: (number | null)[]; + authDeadReasons?: (string | null)[]; usagePcts?: (number | null)[]; usageD7Pcts?: (number | null)[]; resetAts?: (string | null)[]; @@ -175,6 +206,7 @@ function loadCodexState(quiet = false): void { if (!state) return; const now = Date.now(); + let restoredDeadAuth = false; if ( typeof state.currentIndex === 'number' && state.currentIndex < accounts.length @@ -195,6 +227,39 @@ function loadCodexState(quiet = false): void { } } } + if (Array.isArray(state.authDeadAts)) { + for ( + let i = 0; + i < Math.min(state.authDeadAts.length, accounts.length); + i++ + ) { + const deadAt = state.authDeadAts[i]; + const acct = accounts[i]; + if (typeof deadAt === 'number' && deadAt > 0) { + const currentMtime = readAuthFileMtimeMs(acct.authPath); + acct.authFileMtimeMs = currentMtime; + if (currentMtime > deadAt) { + acct.authStatus = 'dead_auth'; + acct.authDeadAt = deadAt; + acct.authDeadReason = state.authDeadReasons?.[i] ?? 'auth-expired'; + restoredDeadAuth = + restoreDeadAuthIfAuthFileChanged(acct) || restoredDeadAuth; + } else { + acct.authStatus = 'dead_auth'; + acct.authDeadAt = deadAt; + acct.authDeadReason = state.authDeadReasons?.[i] ?? 'auth-expired'; + acct.rateLimitedUntil = null; + } + } + } + } + if ( + currentIndex < accounts.length && + accounts[currentIndex]?.authStatus === 'dead_auth' + ) { + const nextIdx = findNextCodexAvailable(currentIndex); + if (nextIdx !== null) currentIndex = nextIdx; + } if (Array.isArray(state.usagePcts)) { for ( let i = 0; @@ -239,6 +304,7 @@ function loadCodexState(quiet = false): void { 'Codex rotation state restored', ); } + if (restoredDeadAuth) saveCodexState(); } /** @@ -309,6 +375,265 @@ export function getCodexAuthPath( return accounts[accountIndex]?.authPath ?? null; } +function codexLockPath(authPath: string): string { + return path.join(path.dirname(authPath), '.ejclaw-auth.lock'); +} + +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function readExistingLease(lockPath: string): { + leaseId?: string; + pid?: number; + expiresAt?: number; +} | null { + const data = readJsonFile<{ + leaseId?: string; + pid?: number; + expiresAt?: number; + }>(lockPath); + return data ?? null; +} + +function tryAcquireDiskLease( + acct: CodexAccount, + leaseId: string, + now: number, +): boolean { + const lockPath = codexLockPath(acct.authPath); + const payload = JSON.stringify( + { + leaseId, + pid: process.pid, + accountIndex: acct.index, + createdAt: now, + expiresAt: now + CODEX_AUTH_LEASE_MS, + }, + null, + 2, + ); + + try { + fs.writeFileSync(lockPath, payload, { flag: 'wx', mode: 0o600 }); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'EEXIST') return false; + } + + const existing = readExistingLease(lockPath); + const expired = Boolean(existing?.expiresAt && existing.expiresAt <= now); + const deadPid = Boolean(existing?.pid && !isPidAlive(existing.pid)); + if (!expired && !deadPid) return false; + + try { + fs.unlinkSync(lockPath); + } catch { + return false; + } + + try { + fs.writeFileSync(lockPath, payload, { flag: 'wx', mode: 0o600 }); + return true; + } catch { + return false; + } +} + +function releaseDiskLease(authPath: string, leaseId: string): void { + const lockPath = codexLockPath(authPath); + const existing = readExistingLease(lockPath); + if (existing?.leaseId && existing.leaseId !== leaseId) return; + try { + fs.unlinkSync(lockPath); + } catch { + // Best effort only. Expired/stale locks are cleared by the next claimant. + } +} + +function isCodexAccountUsable( + acct: CodexAccount, + now = Date.now(), + opts?: { ignoreRateLimits?: boolean; ignoreD7?: boolean }, +): boolean { + if (restoreDeadAuthIfAuthFileChanged(acct)) saveCodexState(); + if (acct.authStatus === 'dead_auth') return false; + if ( + !opts?.ignoreRateLimits && + acct.rateLimitedUntil && + acct.rateLimitedUntil > now + ) { + return false; + } + if ( + !opts?.ignoreD7 && + acct.lastUsageD7Pct != null && + acct.lastUsageD7Pct >= 100 + ) { + return false; + } + if (acct.leasedUntil && acct.leasedUntil > now) return false; + + const existing = readExistingLease(codexLockPath(acct.authPath)); + if (!existing) return true; + if (existing.expiresAt && existing.expiresAt <= now) return true; + if (existing.pid && !isPidAlive(existing.pid)) return true; + return false; +} + +function restoreDeadAuthIfAuthFileChanged(acct: CodexAccount): boolean { + if (acct.authStatus !== 'dead_auth' || !acct.authDeadAt) return false; + const currentMtime = readAuthFileMtimeMs(acct.authPath); + acct.authFileMtimeMs = currentMtime; + if (currentMtime <= acct.authDeadAt) return false; + + const previousDeadAt = acct.authDeadAt; + acct.authStatus = 'healthy'; + acct.authDeadAt = null; + acct.authDeadReason = null; + acct.rateLimitedUntil = null; + logger.info( + { + transition: 'rotation:auth-file-refreshed', + accountIndex: acct.index, + previousDeadAt: new Date(previousDeadAt).toISOString(), + authFileMtime: new Date(currentMtime).toISOString(), + }, + `Codex account #${acct.index + 1}/${accounts.length} marked healthy after auth.json refresh`, + ); + return true; +} + +function markCodexAccountDeadAuth(acct: CodexAccount, reason?: string): void { + acct.authStatus = 'dead_auth'; + acct.authDeadAt = Date.now(); + acct.authDeadReason = reason || 'auth-expired'; + acct.rateLimitedUntil = null; + acct.leasedUntil = null; + acct.leaseId = null; + logger.warn( + { + transition: 'rotation:dead-auth', + accountIndex: acct.index, + accountId: acct.accountId, + reason: acct.authDeadReason, + }, + `Codex account #${acct.index + 1}/${accounts.length} marked dead; re-auth required`, + ); +} + +function updateAccountMetadataFromAuthFile(acct: CodexAccount): void { + const data = readJsonFile<{ + tokens?: { account_id?: string; id_token?: string }; + }>(acct.authPath); + if (data?.tokens?.account_id) acct.accountId = data.tokens.account_id; + const jwt = parseJwtAuth(data?.tokens?.id_token || ''); + acct.planType = jwt.planType; + acct.subscriptionUntil = jwt.expiresAt; + acct.authFileMtimeMs = readAuthFileMtimeMs(acct.authPath); +} + +export function claimCodexAuthLease(): CodexAuthLease | null { + if (accounts.length === 0) return null; + const now = Date.now(); + const attempts = accounts.length; + for (let offset = 0; offset < attempts; offset += 1) { + const idx = (currentIndex + offset) % accounts.length; + const acct = accounts[idx]; + if (!isCodexAccountUsable(acct, now)) continue; + const leaseId = `${process.pid}-${now}-${idx}-${Math.random() + .toString(36) + .slice(2)}`; + if (!tryAcquireDiskLease(acct, leaseId, now)) continue; + currentIndex = idx; + acct.leasedUntil = now + CODEX_AUTH_LEASE_MS; + acct.leaseId = leaseId; + return { + accountIndex: idx, + authPath: acct.authPath, + release: () => { + if (acct.leaseId === leaseId) { + acct.leasedUntil = null; + acct.leaseId = null; + } + releaseDiskLease(acct.authPath, leaseId); + }, + }; + } + return null; +} + +export function syncCodexSessionAuthBack(args: { + canonicalAuthPath: string; + sessionAuthPath: string; + accountIndex?: number | null; +}): boolean { + if (!fs.existsSync(args.sessionAuthPath)) return false; + if (!fs.existsSync(args.canonicalAuthPath)) return false; + + const canonical = readJsonFile<{ + auth_mode?: string; + tokens?: { account_id?: string }; + }>(args.canonicalAuthPath); + const session = readJsonFile<{ + auth_mode?: string; + tokens?: { account_id?: string }; + }>(args.sessionAuthPath); + if (!canonical || !session?.tokens) return false; + + const canonicalAccount = canonical.tokens?.account_id; + const sessionAccount = session.tokens.account_id; + if ( + canonicalAccount && + sessionAccount && + canonicalAccount !== sessionAccount + ) { + logger.warn( + { + canonicalAuthPath: args.canonicalAuthPath, + sessionAuthPath: args.sessionAuthPath, + accountIndex: args.accountIndex ?? null, + }, + 'Refusing to sync Codex session auth back: account mismatch', + ); + return false; + } + + const sessionRaw = fs.readFileSync(args.sessionAuthPath, 'utf-8'); + const canonicalRaw = fs.readFileSync(args.canonicalAuthPath, 'utf-8'); + if (sessionRaw === canonicalRaw) return false; + + const tmpPath = `${args.canonicalAuthPath}.tmp-${process.pid}-${Date.now()}`; + fs.writeFileSync(tmpPath, sessionRaw, { mode: 0o600 }); + fs.renameSync(tmpPath, args.canonicalAuthPath); + + const idx = + args.accountIndex ?? + findCodexAccountIndexByAuthPath(args.canonicalAuthPath); + if (idx != null && accounts[idx]) { + const acct = accounts[idx]; + acct.authStatus = 'healthy'; + acct.authDeadAt = null; + acct.authDeadReason = null; + updateAccountMetadataFromAuthFile(acct); + saveCodexState(); + } + + logger.info( + { + accountIndex: idx, + canonicalAuthPath: args.canonicalAuthPath, + }, + 'Synced refreshed Codex session auth back to canonical account slot', + ); + return true; +} + export function detectCodexRotationTrigger( error?: string | null, ): CodexRotationTriggerResult { @@ -341,16 +666,23 @@ export function rotateCodexToken( const previousIndex = currentIndex; const acct = accounts[currentIndex]; - const cooldownUntil = computeCooldownUntil(errorMessage); - acct.rateLimitedUntil = cooldownUntil; - acct.lastUsagePct = 100; - // Extract reset time string from error for display - const retryAt = parseRetryAfterFromError(errorMessage); - if (retryAt) { - acct.resetAt = new Date(retryAt).toISOString(); + const authFailure = classifyCodexAuthError(errorMessage); + let cooldownUntil: number | null = null; + + if (authFailure.category === 'auth-expired') { + markCodexAccountDeadAuth(acct, errorMessage || authFailure.reason); + } else { + cooldownUntil = computeCooldownUntil(errorMessage); + acct.rateLimitedUntil = cooldownUntil; + acct.lastUsagePct = 100; + // Extract reset time string from error for display + const retryAt = parseRetryAfterFromError(errorMessage); + if (retryAt) { + acct.resetAt = new Date(retryAt).toISOString(); + } } - const nextIdx = findNextAvailable(accounts, currentIndex, opts); + const nextIdx = findNextCodexAvailable(currentIndex, opts); if (nextIdx !== null) { accounts[nextIdx].rateLimitedUntil = null; currentIndex = nextIdx; @@ -364,6 +696,7 @@ export function rotateCodexToken( ignoreRL: opts?.ignoreRateLimits ?? false, cooldownUntil: cooldownUntil != null ? new Date(cooldownUntil).toISOString() : null, + authDead: authFailure.category === 'auth-expired', reason: errorMessage ?? null, }, `Codex rotated to account #${currentIndex + 1}/${accounts.length}`, @@ -380,28 +713,40 @@ export function rotateCodexToken( ignoreRL: opts?.ignoreRateLimits ?? false, cooldownUntil: cooldownUntil != null ? new Date(cooldownUntil).toISOString() : null, + authDead: authFailure.category === 'auth-expired', reason: errorMessage ?? null, }, - 'All Codex accounts are rate-limited', + authFailure.category === 'auth-expired' + ? 'All Codex accounts unavailable after auth failure; re-auth required' + : 'All Codex accounts are rate-limited', ); + saveCodexState(); return false; } /** * Find the next Codex account that is neither rate-limited nor 7d-exhausted. */ -function findNextCodexAvailable(fromIndex?: number): number | null { +function findNextCodexAvailable( + fromIndex?: number, + opts?: { ignoreRateLimits?: boolean }, +): number | null { const now = Date.now(); const start = fromIndex ?? currentIndex; for (let i = 1; i < accounts.length; i++) { const idx = (start + i) % accounts.length; const acct = accounts[idx]; - const rlOk = !acct.rateLimitedUntil || acct.rateLimitedUntil <= now; - const usageOk = acct.lastUsageD7Pct == null || acct.lastUsageD7Pct < 100; - if (rlOk && usageOk) return idx; + if (isCodexAccountUsable(acct, now, opts)) return idx; } - // All exhausted — fall back to rate-limit-only check - return findNextAvailable(accounts, start); + // All d7-exhausted — fall back to rate-limit/dead/lease checks only. + for (let i = 1; i < accounts.length; i++) { + const idx = (start + i) % accounts.length; + const acct = accounts[idx]; + if (isCodexAccountUsable(acct, now, { ...opts, ignoreD7: true })) { + return idx; + } + } + return null; } /** @@ -469,9 +814,17 @@ export function updateCodexAccountUsage( export function markCodexTokenHealthy(): void { if (accounts.length === 0) return; const acct = accounts[currentIndex]; + let changed = false; + if (acct?.authStatus === 'dead_auth') { + acct.authStatus = 'healthy'; + acct.authDeadAt = null; + acct.authDeadReason = null; + changed = true; + } if (acct?.rateLimitedUntil) { const previousCooldownUntil = acct.rateLimitedUntil; acct.rateLimitedUntil = null; + changed = true; logger.info( { transition: 'rotation:clear-rate-limit', @@ -481,8 +834,8 @@ export function markCodexTokenHealthy(): void { }, 'Cleared Codex account rate-limit state after successful response', ); - saveCodexState(); } + if (changed) saveCodexState(); } export function getCodexAccountCount(): number { @@ -495,6 +848,10 @@ export function getAllCodexAccounts(): { planType: string; isActive: boolean; isRateLimited: boolean; + isAuthDead?: boolean; + authStatus?: 'healthy' | 'dead_auth'; + authDeadAt?: string; + isLeased?: boolean; cachedUsagePct?: number; cachedUsageD7Pct?: number; resetAt?: string; @@ -507,6 +864,10 @@ export function getAllCodexAccounts(): { planType: a.planType, isActive: i === currentIndex, isRateLimited: Boolean(a.rateLimitedUntil && a.rateLimitedUntil > now), + isAuthDead: a.authStatus === 'dead_auth', + authStatus: a.authStatus, + authDeadAt: a.authDeadAt ? new Date(a.authDeadAt).toISOString() : undefined, + isLeased: Boolean(a.leasedUntil && a.leasedUntil > now), cachedUsagePct: a.lastUsagePct, cachedUsageD7Pct: a.lastUsageD7Pct, resetAt: a.resetAt, diff --git a/src/message-agent-executor-attempt-runner.test.ts b/src/message-agent-executor-attempt-runner.test.ts new file mode 100644 index 0000000..7e3b7cd --- /dev/null +++ b/src/message-agent-executor-attempt-runner.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it, vi } from 'vitest'; + +const mockRunAgentProcess = vi.hoisted(() => vi.fn()); + +vi.mock('./agent-runner.js', () => ({ + runAgentProcess: mockRunAgentProcess, +})); + +vi.mock('./codex-token-rotation.js', () => ({ + getCodexAccountCount: vi.fn(() => 2), +})); + +import { runMessageAgentAttempt } from './message-agent-executor-attempt-runner.js'; +import type { AgentOutput } from './agent-runner.js'; + +describe('runMessageAgentAttempt', () => { + it('stores pre-stream Codex launch errors in paired summary', async () => { + const error = new Error( + 'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex', + ); + mockRunAgentProcess.mockRejectedValueOnce(error); + const updateSummary = vi.fn(); + + const attempt = await runMessageAgentAttempt({ + provider: 'codex', + currentSessionId: undefined, + isClaudeCodeAgent: false, + canRetryClaudeCredentials: false, + shouldPersistSession: false, + effectiveGroup: { + name: 'Test', + folder: 'test', + trigger: '@test', + added_at: new Date().toISOString(), + agentType: 'codex', + }, + agentInput: { + prompt: 'arbiter prompt', + groupFolder: 'test', + chatJid: 'dc:test', + runId: 'run-test', + isMain: false, + assistantName: 'Andy', + }, + activeRole: 'arbiter', + effectiveServiceId: 'codex-review', + effectiveAgentType: 'codex', + sessionFolder: 'test:arbiter', + onPersistSession: vi.fn(), + registerProcess: vi.fn(), + onOutput: vi.fn(async (_output: AgentOutput) => undefined), + pairedExecutionLifecycle: { + updateSummary, + recordFinalOutputBeforeDelivery: vi.fn(() => false), + }, + log: { + child: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + } as any, + }); + + expect(attempt.error).toBe(error); + expect(attempt.sawOutput).toBe(false); + expect(updateSummary).toHaveBeenCalledWith({ + errorText: + 'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex', + }); + }); +}); diff --git a/src/message-agent-executor-attempt-runner.ts b/src/message-agent-executor-attempt-runner.ts index 344ce63..64a9126 100644 --- a/src/message-agent-executor-attempt-runner.ts +++ b/src/message-agent-executor-attempt-runner.ts @@ -14,6 +14,7 @@ import { shouldResetCodexSessionOnAgentFailure, shouldResetSessionOnAgentFailure, } from './session-recovery.js'; +import { getErrorMessage } from './utils.js'; import type { AgentType, OutboundAttachment, @@ -162,6 +163,9 @@ class MessageAgentAttemptRunner { ); return this.buildAttempt({ output }); } catch (error) { + this.args.pairedExecutionLifecycle.updateSummary({ + errorText: getErrorMessage(error), + }); return this.buildAttempt({ error }); } } diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index d91d08f..d4fe420 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -505,8 +505,11 @@ class PairedExecutionLifecycleController implements PairedExecutionLifecycle { if (!missingVisibleVerdict) { return false; } - this.pairedExecutionSummary = + const noVerdictSummary = 'Execution completed without a visible terminal verdict.'; + this.pairedExecutionSummary = this.pairedExecutionSummary + ? `${this.pairedExecutionSummary}\n${noVerdictSummary}` + : noVerdictSummary; log.warn( { pairedTaskId: pairedExecutionContext?.task.id ?? null, diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts index b9ded78..1d20c34 100644 --- a/src/message-agent-executor-rules.test.ts +++ b/src/message-agent-executor-rules.test.ts @@ -172,6 +172,19 @@ describe('message-agent-executor-rules', () => { ).toBe('pending'); }); + it('does not requeue a silent arbiter failure caused by Codex account auth expiry', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'arbiter', + executionStatus: 'failed', + sawOutput: false, + taskStatus: 'arbiter_requested', + outputSummary: + 'Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.\nExecution completed without a visible terminal verdict.', + }), + ).toBe('none'); + }); + it('resolves pending arbiter follow-up requeue after repeated owner execution failures', () => { expect( resolvePairedFollowUpQueueAction({ diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts index 2ec47f8..e405d02 100644 --- a/src/message-agent-executor-rules.ts +++ b/src/message-agent-executor-rules.ts @@ -3,6 +3,7 @@ import { resolveNextTurnAction, shouldRetrySilentOwnerExecution, } from './message-runtime-rules.js'; +import { classifyCodexAuthError } from './agent-error-detection.js'; import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedRoomRole, PairedTaskStatus } from './types.js'; export { @@ -17,6 +18,17 @@ 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) + ); +} + export function resolvePairedFollowUpQueueAction(args: { completedRole: PairedRoomRole; executionStatus: 'succeeded' | 'failed'; @@ -24,6 +36,15 @@ export function resolvePairedFollowUpQueueAction(args: { taskStatus: PairedTaskStatus | null; outputSummary?: string | null; }): PairedFollowUpQueueAction { + if ( + args.executionStatus === 'failed' && + args.sawOutput === false && + (args.completedRole === 'reviewer' || args.completedRole === 'arbiter') && + isSilentCodexAccountFailure(args.outputSummary) + ) { + return 'none'; + } + if ( shouldRetrySilentOwnerExecution({ completedRole: args.completedRole, diff --git a/src/paired-execution-context-arbiter.ts b/src/paired-execution-context-arbiter.ts index c204816..83214c0 100644 --- a/src/paired-execution-context-arbiter.ts +++ b/src/paired-execution-context-arbiter.ts @@ -1,15 +1,52 @@ import { logger } from './logger.js'; +import { + classifyAgentError, + classifyCodexAuthError, +} 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; + summary?: string | null; }): void { - const { task, taskId } = args; + const { task, taskId, summary } = args; + if (isTerminalCodexAccountFailure(summary)) { + const now = new Date().toISOString(); + transitionPairedTaskStatus({ + taskId, + currentStatus: task.status, + nextStatus: 'completed', + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + arbiter_verdict: 'escalate', + arbiter_requested_at: null, + completion_reason: 'arbiter_codex_unavailable', + }, + }); + logger.warn( + { + taskId, + role: 'arbiter', + status: task.status, + summary: summary?.slice(0, 200), + }, + 'Completed arbiter task after terminal Codex account failure instead of re-arming arbitration loop', + ); + return; + } const now = new Date().toISOString(); const fallbackStatus = task.status === 'in_arbitration' || task.status === 'arbiter_requested' diff --git a/src/paired-execution-context-routing.test.ts b/src/paired-execution-context-routing.test.ts index a058868..023c0c6 100644 --- a/src/paired-execution-context-routing.test.ts +++ b/src/paired-execution-context-routing.test.ts @@ -197,6 +197,33 @@ describe('paired execution routing loop guards', () => { ); }); + it('does not re-arm arbiter loop after terminal Codex account failure', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'in_arbitration', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'arbiter', + 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: 'arbiter_codex_unavailable', + }), + ); + }); + it('keeps arbiter REVISE on owner flow while clearing stale loop counters', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 216f56e..fc1dced 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -666,7 +666,11 @@ export function completePairedExecutionContext(args: { return; } if (role === 'arbiter') { - handleFailedArbiterExecution({ task, taskId }); + handleFailedArbiterExecution({ + task, + taskId, + summary: args.summary, + }); return; } handleFailedOwnerExecution({ task, taskId, summary: args.summary }); diff --git a/src/provider-retry.test.ts b/src/provider-retry.test.ts index 7835af8..fc06628 100644 --- a/src/provider-retry.test.ts +++ b/src/provider-retry.test.ts @@ -224,6 +224,7 @@ describe('runCodexRotationLoop', () => { expect(outcome).toEqual({ type: 'success' }); expect(rotateCodexToken).toHaveBeenCalledTimes(1); + expect(rotateCodexToken).toHaveBeenCalledWith('auth-expired'); expect(markCodexTokenHealthy).toHaveBeenCalledTimes(1); }); diff --git a/src/provider-retry.ts b/src/provider-retry.ts index a5ac85c..d384cdc 100644 --- a/src/provider-retry.ts +++ b/src/provider-retry.ts @@ -36,6 +36,7 @@ import { clearGlobalFailover } from './service-routing.js'; export interface TriggerInfo { reason: AgentTriggerReason; retryAfterMs?: number; + message?: string; } export interface RotationAttemptResult { @@ -206,6 +207,34 @@ function evaluateCodexAttempt( return { type: 'error' }; } + if ( + !attempt.sawOutput && + typeof output.error === 'string' && + output.error.length > 0 + ) { + const retryTrigger = + attempt.streamedTriggerReason && + isCodexRotationReason(attempt.streamedTriggerReason.reason) + ? { + shouldRotate: true as const, + reason: attempt.streamedTriggerReason.reason, + } + : detectCodexRotationTrigger(output.error); + if (retryTrigger.shouldRotate) { + return { + type: 'continue', + trigger: { reason: retryTrigger.reason }, + rotationMessage: output.error, + }; + } + + logger.error( + { ...logContext, provider: 'codex', error: output.error }, + 'Rotated Codex account returned an error field without visible output', + ); + return { type: 'error' }; + } + if ( !attempt.sawOutput && attempt.streamedTriggerReason && @@ -350,7 +379,7 @@ export async function runClaudeRotationLoop( } export async function runCodexRotationLoop( - initialTrigger: { reason: CodexRotationReason }, + initialTrigger: { reason: CodexRotationReason; message?: string }, runAttempt: () => Promise, logContext: Record, rotationMessage?: string, @@ -358,7 +387,13 @@ export async function runCodexRotationLoop( let trigger = initialTrigger; let lastRotationMessage = rotationMessage; - while (getCodexAccountCount() > 1 && rotateCodexToken(lastRotationMessage)) { + while (getCodexAccountCount() > 1) { + const rotationReasonMessage = + lastRotationMessage ?? trigger.message ?? trigger.reason; + if (!rotateCodexToken(rotationReasonMessage)) { + break; + } + logger.info( { ...logContext, reason: trigger.reason }, 'Codex account unhealthy, retrying with rotated account', diff --git a/src/streamed-output-evaluator.test.ts b/src/streamed-output-evaluator.test.ts index 953afca..56f1e41 100644 --- a/src/streamed-output-evaluator.test.ts +++ b/src/streamed-output-evaluator.test.ts @@ -513,18 +513,26 @@ describe('evaluateStreamedOutput', () => { describe('error → Codex rotation trigger', () => { it('returns rotation trigger for Codex primary', () => { + const errorMessage = + 'unexpected status 401 Unauthorized: Missing bearer or basic authentication in header'; vi.mocked(detectCodexRotationTrigger).mockReturnValue({ shouldRotate: true, reason: '429', }); const result = evaluateStreamedOutput( - errorOutput('429 rate limit'), + errorOutput(errorMessage), freshState(), codexOpts, ); - expect(result.newTrigger).toEqual({ reason: '429' }); - expect(result.state.streamedTriggerReason).toEqual({ reason: '429' }); + expect(result.newTrigger).toEqual({ + reason: '429', + message: errorMessage, + }); + expect(result.state.streamedTriggerReason).toEqual({ + reason: '429', + message: errorMessage, + }); }); it('does not check Codex rotation for Claude agent type', () => { diff --git a/src/streamed-output-evaluator.ts b/src/streamed-output-evaluator.ts index ffad9f0..a84f557 100644 --- a/src/streamed-output-evaluator.ts +++ b/src/streamed-output-evaluator.ts @@ -19,6 +19,7 @@ import { export interface StreamedTriggerReason { reason: AgentTriggerReason; retryAfterMs?: number; + message?: string; } export interface StreamedOutputState { @@ -139,6 +140,28 @@ export function evaluateStreamedOutput( nextState.sawSuccessNullResultWithoutOutput = true; } + if ( + isPrimaryCodex && + typeof output.error === 'string' && + output.error.length > 0 && + !nextState.sawOutput && + !nextState.streamedTriggerReason + ) { + const trigger = detectCodexRotationTrigger(output.error); + if (trigger.shouldRotate) { + const newTrigger: StreamedTriggerReason = { + reason: trigger.reason, + message: output.error, + }; + nextState.streamedTriggerReason = newTrigger; + return { + state: nextState, + shouldForwardOutput: !options.shortCircuitTriggeredErrors, + newTrigger, + }; + } + } + if ( output.status === 'error' && !nextState.sawOutput && @@ -157,7 +180,7 @@ export function evaluateStreamedOutput( } else if (isPrimaryCodex) { const trigger = detectCodexRotationTrigger(output.error); if (trigger.shouldRotate) { - newTrigger = { reason: trigger.reason }; + newTrigger = { reason: trigger.reason, message: output.error }; } }