From 2ab3bddc0e4578d05fdc9002aa56ac709b636a65 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 15 Apr 2026 03:11:14 +0900 Subject: [PATCH] Recover Codex compaction failures in paired owner flow --- src/db/base-schema.ts | 1 + src/db/bootstrap.test.ts | 1 + src/db/migrations/011_owner-failure-count.ts | 23 ++++++ src/db/migrations/index.ts | 2 + src/db/paired-state.ts | 13 +++- src/message-agent-executor-attempt-runner.ts | 16 +++- src/message-agent-executor-lifecycle.ts | 77 +++++++++++++++++++- src/message-agent-executor-rules.test.ts | 11 +++ src/message-agent-executor.test.ts | 66 +++++++++++++++++ src/message-runtime-rules.test.ts | 26 ++++++- src/message-runtime-rules.ts | 4 +- src/paired-execution-context-owner.ts | 69 ++++++++++++++++-- src/paired-execution-context-shared.ts | 15 ++++ src/paired-execution-context.test.ts | 30 +++++++- src/paired-execution-context.ts | 7 +- src/session-recovery.test.ts | 35 +++++++++ src/session-recovery.ts | 23 ++++++ src/streamed-output-evaluator.test.ts | 29 +++++++- src/streamed-output-evaluator.ts | 18 ++++- src/types.ts | 1 + 20 files changed, 449 insertions(+), 18 deletions(-) create mode 100644 src/db/migrations/011_owner-failure-count.ts diff --git a/src/db/base-schema.ts b/src/db/base-schema.ts index 96c0b3d..1fd66ba 100644 --- a/src/db/base-schema.ts +++ b/src/db/base-schema.ts @@ -118,6 +118,7 @@ export function applyBaseSchema(database: Database): void { plan_notes TEXT, review_requested_at TEXT, round_trip_count INTEGER NOT NULL DEFAULT 0, + owner_failure_count INTEGER NOT NULL DEFAULT 0, status TEXT NOT NULL DEFAULT 'active', arbiter_verdict TEXT, arbiter_requested_at TEXT, diff --git a/src/db/bootstrap.test.ts b/src/db/bootstrap.test.ts index ee4eaf9..c375201 100644 --- a/src/db/bootstrap.test.ts +++ b/src/db/bootstrap.test.ts @@ -35,6 +35,7 @@ function getExpectedSchemaMigrations(): Array<{ { version: 8, name: 'paired_task_schema_cleanup' }, { version: 9, name: 'paired_workspace_project_schema_cleanup' }, { version: 10, name: 'paired_turn_provenance_upgrade' }, + { version: 11, name: 'owner_failure_count' }, ]; } diff --git a/src/db/migrations/011_owner-failure-count.ts b/src/db/migrations/011_owner-failure-count.ts new file mode 100644 index 0000000..f05efc4 --- /dev/null +++ b/src/db/migrations/011_owner-failure-count.ts @@ -0,0 +1,23 @@ +import type { Database } from 'bun:sqlite'; + +import { tableHasColumn } from './helpers.js'; +import type { SchemaMigrationDefinition } from './types.js'; + +export const OWNER_FAILURE_COUNT_MIGRATION: SchemaMigrationDefinition = { + version: 11, + name: 'owner_failure_count', + apply(database: Database) { + if (!tableHasColumn(database, 'paired_tasks', 'owner_failure_count')) { + database.exec(` + ALTER TABLE paired_tasks + ADD COLUMN owner_failure_count INTEGER NOT NULL DEFAULT 0 + `); + } + + database.exec(` + UPDATE paired_tasks + SET owner_failure_count = 0 + WHERE owner_failure_count IS NULL + `); + }, +}; diff --git a/src/db/migrations/index.ts b/src/db/migrations/index.ts index df4133f..85eaf42 100644 --- a/src/db/migrations/index.ts +++ b/src/db/migrations/index.ts @@ -10,6 +10,7 @@ import { RUNTIME_SERVICE_METADATA_MIGRATION } from './007_runtime-service-metada import { PAIRED_TASK_SCHEMA_CLEANUP_MIGRATION } from './008_paired-task-schema-cleanup.js'; import { PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION } from './009_paired-workspace-project-schema-cleanup.js'; import { PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION } from './010_paired-turn-provenance-upgrade.js'; +import { OWNER_FAILURE_COUNT_MIGRATION } from './011_owner-failure-count.js'; import type { SchemaMigrationArgs, SchemaMigrationDefinition, @@ -28,6 +29,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [ PAIRED_TASK_SCHEMA_CLEANUP_MIGRATION, PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION, PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION, + OWNER_FAILURE_COUNT_MIGRATION, ]; function ensureSchemaMigrationsTable(database: Database): void { diff --git a/src/db/paired-state.ts b/src/db/paired-state.ts index 0a08504..44eaa90 100644 --- a/src/db/paired-state.ts +++ b/src/db/paired-state.ts @@ -50,6 +50,7 @@ export type PairedTaskUpdates = Partial< | 'plan_notes' | 'review_requested_at' | 'round_trip_count' + | 'owner_failure_count' | 'status' | 'arbiter_verdict' | 'arbiter_requested_at' @@ -171,6 +172,7 @@ export function createPairedTaskInDatabase( plan_notes, review_requested_at, round_trip_count, + owner_failure_count, status, arbiter_verdict, arbiter_requested_at, @@ -178,7 +180,7 @@ export function createPairedTaskInDatabase( created_at, updated_at ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, ) .run( @@ -195,6 +197,7 @@ export function createPairedTaskInDatabase( task.plan_notes, task.review_requested_at, task.round_trip_count, + task.owner_failure_count ?? 0, task.status, task.arbiter_verdict, task.arbiter_requested_at, @@ -279,6 +282,10 @@ export function updatePairedTaskInDatabase( fields.push('round_trip_count = ?'); values.push(updates.round_trip_count); } + if (updates.owner_failure_count !== undefined) { + fields.push('owner_failure_count = ?'); + values.push(updates.owner_failure_count); + } if (updates.status !== undefined) { fields.push('status = ?'); values.push(updates.status); @@ -338,6 +345,10 @@ export function updatePairedTaskIfUnchangedInDatabase( fields.push('round_trip_count = ?'); values.push(updates.round_trip_count); } + if (updates.owner_failure_count !== undefined) { + fields.push('owner_failure_count = ?'); + values.push(updates.owner_failure_count); + } if (updates.status !== undefined) { fields.push('status = ?'); values.push(updates.status); diff --git a/src/message-agent-executor-attempt-runner.ts b/src/message-agent-executor-attempt-runner.ts index 7dd62a4..30d015d 100644 --- a/src/message-agent-executor-attempt-runner.ts +++ b/src/message-agent-executor-attempt-runner.ts @@ -5,7 +5,10 @@ import type { AttemptStreamedTrigger } from './agent-attempt-retry.js'; import { runAgentProcess, type AgentOutput } from './agent-runner.js'; import { getCodexAccountCount } from './codex-token-rotation.js'; import type { PreparedPairedExecutionContext } from './paired-execution-context.js'; -import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; +import { + shouldResetCodexSessionOnAgentFailure, + shouldResetSessionOnAgentFailure, +} from './session-recovery.js'; import type { AgentType, RegisteredGroup, RoomRoleContext } from './types.js'; export interface MessageAgentAttempt { @@ -148,6 +151,13 @@ export async function runMessageAgentAttempt(args: { ) { resetSessionRequested = true; } + if ( + !isClaudeCodeAgent && + provider === 'codex' && + shouldResetCodexSessionOnAgentFailure(output) + ) { + resetSessionRequested = true; + } if ( output.newSessionId && !resetSessionRequested && @@ -197,7 +207,9 @@ export async function runMessageAgentAttempt(args: { ? outputText.slice(0, 160) : output.error?.slice(0, 160), }, - 'Suppressed retryable Claude session failure from chat output', + provider === 'claude' + ? 'Suppressed retryable Claude session failure from chat output' + : 'Suppressed retryable Codex session failure from chat output', ); return; } diff --git a/src/message-agent-executor-lifecycle.ts b/src/message-agent-executor-lifecycle.ts index 0206e7f..95c63b5 100644 --- a/src/message-agent-executor-lifecycle.ts +++ b/src/message-agent-executor-lifecycle.ts @@ -12,7 +12,11 @@ import type { } from './agent-error-detection.js'; import type { PairedExecutionLifecycle } from './message-agent-executor-paired.js'; import type { MessageAgentAttempt } from './message-agent-executor-attempt-runner.js'; -import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; +import { + shouldResetCodexSessionOnAgentFailure, + shouldResetSessionOnAgentFailure, + shouldRetryFreshCodexSessionOnAgentFailure, +} from './session-recovery.js'; import { getErrorMessage } from './utils.js'; type AttemptResult = 'success' | 'error'; @@ -208,6 +212,57 @@ export async function executeMessageAgentAttemptLifecycle(args: { }; }; + const isRetryableCodexSessionFailure = ( + attempt: MessageAgentAttempt, + ): boolean => { + if (provider !== 'codex' || attempt.sawOutput) { + return false; + } + + if (attempt.retryableSessionFailureDetected === true) { + return true; + } + + if (attempt.error == null) { + return false; + } + + return shouldRetryFreshCodexSessionOnAgentFailure({ + result: null, + error: getErrorMessage(attempt.error), + }); + }; + + const recoverRetryableCodexSessionFailure = async ( + attempt: MessageAgentAttempt, + ): Promise<{ + attempt: MessageAgentAttempt; + resolved: AttemptResult | null; + }> => { + if (!isRetryableCodexSessionFailure(attempt)) { + return { attempt, resolved: null }; + } + + clearStoredSession(); + clearRoleSdkSessions(); + log.warn( + 'Cleared poisoned Codex session before visible output, retrying fresh session', + ); + + const freshAttempt = await runTrackedAttempt('codex'); + if (!isRetryableCodexSessionFailure(freshAttempt)) { + return { attempt: freshAttempt, resolved: null }; + } + + clearStoredSession(); + log.warn('Fresh Codex retry also hit a retryable session failure'); + log.error('Retryable Codex session failure persisted after fresh retry'); + return { + attempt: freshAttempt, + resolved: 'error', + }; + }; + const handlePrimaryAttemptFailure = async ( attempt: MessageAgentAttempt, rotationMessage: string, @@ -294,6 +349,19 @@ export async function executeMessageAgentAttemptLifecycle(args: { ); } + if ( + !isClaudeCodeAgent && + provider === 'codex' && + (resetSessionRequested || shouldResetCodexSessionOnAgentFailure(output)) + ) { + clearStoredSession(); + clearRoleSdkSessions(); + log.warn( + { sessionFolder }, + 'Cleared poisoned Codex session after unrecoverable error', + ); + } + if (output.status === 'error') { return handlePrimaryAttemptFailure( attempt, @@ -354,6 +422,13 @@ export async function executeMessageAgentAttemptLifecycle(args: { } primaryAttempt = recoveredSessionAttempt.attempt; + const recoveredCodexSessionAttempt = + await recoverRetryableCodexSessionFailure(primaryAttempt); + if (recoveredCodexSessionAttempt.resolved) { + return recoveredCodexSessionAttempt.resolved; + } + primaryAttempt = recoveredCodexSessionAttempt.attempt; + if (primaryAttempt.error) { return handlePrimaryAttemptFailure( primaryAttempt, diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts index 0662955..74583ad 100644 --- a/src/message-agent-executor-rules.test.ts +++ b/src/message-agent-executor-rules.test.ts @@ -172,6 +172,17 @@ describe('message-agent-executor-rules', () => { ).toBe('pending'); }); + it('resolves pending arbiter follow-up requeue after repeated owner execution failures', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'owner', + executionStatus: 'failed', + sawOutput: false, + taskStatus: 'arbiter_requested', + }), + ).toBe('pending'); + }); + it('does not request an executor-side follow-up after successful owner output moved the task to review_ready', () => { expect( resolvePairedFollowUpQueueAction({ diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 824cef9..2ff6275 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -196,7 +196,9 @@ vi.mock('./agent-error-detection.js', async (importOriginal) => { }); vi.mock('./session-recovery.js', () => ({ + shouldResetCodexSessionOnAgentFailure: vi.fn(() => false), shouldResetSessionOnAgentFailure: vi.fn(() => false), + shouldRetryFreshCodexSessionOnAgentFailure: vi.fn(() => false), shouldRetryFreshSessionOnAgentFailure: vi.fn(() => false), })); @@ -3330,6 +3332,70 @@ describe('runAgentForGroup Claude rotation', () => { expect(deps.clearSession).toHaveBeenCalledWith('test-claude'); }); + it('drops a poisoned Codex session id before retrying a fresh session after remote compaction failure', async () => { + const group = { + ...makeGroup(), + folder: 'test-codex', + agentType: 'codex' as const, + }; + const deps = { + ...makeDeps(), + getSessions: () => ({ 'test-codex': 'stale-codex-session-id' }), + }; + + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_agent_type: 'codex', + reviewer_agent_type: null, + arbiter_agent_type: null, + owner_service_id: 'codex-main', + reviewer_service_id: null, + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked(sessionRecovery.shouldRetryFreshCodexSessionOnAgentFailure) + .mockReturnValueOnce(true) + .mockReturnValueOnce(false); + vi.mocked(sessionRecovery.shouldResetCodexSessionOnAgentFailure) + .mockReturnValueOnce(true) + .mockReturnValueOnce(false); + + vi.mocked(agentRunner.runAgentProcess) + .mockImplementationOnce(async (_group, input) => { + expect(input.sessionId).toBe('stale-codex-session-id'); + throw new Error( + "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'", + ); + }) + .mockImplementationOnce(async (_group, input, _onProcess, onOutput) => { + expect(input.sessionId).toBeUndefined(); + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'fresh Codex retry success', + }); + return { + status: 'success', + result: 'fresh Codex retry success', + newSessionId: 'fresh-codex-session-id', + }; + }); + + const result = await runAgentForGroup(deps, { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-stale-codex-session-id-retry', + onOutput: async () => {}, + }); + + expect(result).toBe('success'); + expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(2); + expect(deps.clearSession).toHaveBeenCalledWith('test-codex'); + }); + it('suppresses a usage-exhausted banner even when Claude already emitted progress text', async () => { const outputs: string[] = []; diff --git a/src/message-runtime-rules.test.ts b/src/message-runtime-rules.test.ts index f31c406..b0692de 100644 --- a/src/message-runtime-rules.test.ts +++ b/src/message-runtime-rules.test.ts @@ -133,7 +133,7 @@ describe('message-runtime-rules', () => { ).toEqual({ kind: 'none' }); }); - it('dispatches owner delivery success through a paired follow-up enqueue only for reviewer turns', () => { + it('dispatches owner delivery success through a paired follow-up enqueue for reviewer or arbiter turns', () => { expect( resolveFollowUpDispatch({ source: 'owner-delivery-success', @@ -143,6 +143,15 @@ describe('message-runtime-rules', () => { kind: 'enqueue', queueKind: 'paired-follow-up', }); + expect( + resolveFollowUpDispatch({ + source: 'owner-delivery-success', + nextTurnAction: { kind: 'arbiter-turn' }, + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); expect( resolveFollowUpDispatch({ source: 'owner-delivery-success', @@ -151,6 +160,21 @@ describe('message-runtime-rules', () => { ).toEqual({ kind: 'none' }); }); + it('dispatches failed owner recovery through a paired follow-up when execution escalated to arbiter', () => { + expect( + resolveFollowUpDispatch({ + source: 'executor-recovery', + nextTurnAction: { kind: 'arbiter-turn' }, + completedRole: 'owner', + executionStatus: 'failed', + sawOutput: false, + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); + }); + it('dispatches reviewer and arbiter delivery success through paired follow-up enqueue when a handoff is pending', () => { expect( resolveFollowUpDispatch({ diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index 3e152da..7a284f9 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -183,7 +183,8 @@ export function resolveFollowUpDispatch(args: { args.source === 'owner-delivery-success' || args.completedRole === 'owner' ) { - return args.nextTurnAction.kind === 'reviewer-turn' + return args.nextTurnAction.kind === 'reviewer-turn' || + args.nextTurnAction.kind === 'arbiter-turn' ? { kind: 'enqueue', queueKind: 'paired-follow-up' } : { kind: 'none' }; } @@ -211,6 +212,7 @@ export function resolveFollowUpDispatch(args: { return { kind: 'none' }; } if ( + args.completedRole !== 'owner' && args.completedRole !== 'reviewer' && args.completedRole !== 'arbiter' ) { diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 1c2f9dd..9d2b7c4 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -21,26 +21,72 @@ import { import type { PairedTask } from './types.js'; type OwnerFinalizeOutcome = 'stop' | 're_review'; +const OWNER_FAILURE_ESCALATION_THRESHOLD = 2; export function handleFailedOwnerExecution(args: { task: PairedTask; taskId: string; + summary?: string | null; }): void { - const { task, taskId } = args; + const { task, taskId, summary } = args; + const now = new Date().toISOString(); + const nextFailureCount = (task.owner_failure_count ?? 0) + 1; + + if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) { + requestArbiterOrEscalate({ + taskId, + currentStatus: task.status, + expectedUpdatedAt: task.updated_at, + now, + arbiterLogMessage: + 'Owner failed repeatedly without a visible verdict — requesting arbiter', + escalateLogMessage: + 'Owner failed repeatedly without a visible verdict — escalating to user', + logContext: { + taskId, + role: 'owner', + previousStatus: task.status, + ownerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + patch: { + owner_failure_count: nextFailureCount, + }, + }); + return; + } + if (task.status !== 'active') { - const now = new Date().toISOString(); transitionPairedTaskStatus({ taskId, currentStatus: task.status, nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, + patch: { + owner_failure_count: nextFailureCount, + }, + }); + } else { + applyPairedTaskPatch({ + taskId, + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + owner_failure_count: nextFailureCount, + }, }); - logger.info( - { taskId, role: 'owner', previousStatus: task.status }, - 'Reset task to active after failed execution', - ); } + logger.info( + { + taskId, + role: 'owner', + previousStatus: task.status, + ownerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + 'Reset task to active after failed owner execution', + ); } function handleOwnerFinalizeCompletion(args: { @@ -90,6 +136,9 @@ function handleOwnerFinalizeCompletion(args: { hasNewChanges, summary: summary?.slice(0, 100), }, + patch: { + owner_failure_count: 0, + }, }); return 'stop'; } @@ -102,6 +151,9 @@ function handleOwnerFinalizeCompletion(args: { nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, + patch: { + owner_failure_count: 0, + }, }); } logger.info( @@ -126,6 +178,7 @@ function handleOwnerFinalizeCompletion(args: { updatedAt: now, patch: { completion_reason: 'done', + owner_failure_count: 0, }, }); logger.info( @@ -166,6 +219,7 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: { updatedAt: now, patch: { round_trip_count: task.round_trip_count + 1, + owner_failure_count: 0, }, }); if (hasActiveCiWatcherForChat(task.chat_jid)) { @@ -229,6 +283,9 @@ export function handleOwnerCompletion(args: { ownerVerdict, summary: summary?.slice(0, 100), }, + patch: { + owner_failure_count: 0, + }, }); return; } diff --git a/src/paired-execution-context-shared.ts b/src/paired-execution-context-shared.ts index 58d2f95..18d3faa 100644 --- a/src/paired-execution-context-shared.ts +++ b/src/paired-execution-context-shared.ts @@ -247,6 +247,7 @@ export function transitionPairedTaskStatus(args: { plan_notes?: string | null; review_requested_at?: string | null; round_trip_count?: number; + owner_failure_count?: number; arbiter_verdict?: string | null; arbiter_requested_at?: string | null; completion_reason?: string | null; @@ -290,6 +291,7 @@ export function applyPairedTaskPatch(args: { plan_notes?: string | null; review_requested_at?: string | null; round_trip_count?: number; + owner_failure_count?: number; status?: PairedTaskStatus; arbiter_verdict?: string | null; arbiter_requested_at?: string | null; @@ -324,6 +326,17 @@ export function requestArbiterOrEscalate(args: { arbiterLogMessage: string; escalateLogMessage: string; logContext?: Record; + patch?: { + title?: string | null; + source_ref?: string | null; + plan_notes?: string | null; + review_requested_at?: string | null; + round_trip_count?: number; + owner_failure_count?: number; + arbiter_verdict?: string | null; + arbiter_requested_at?: string | null; + completion_reason?: string | null; + }; }): boolean { const { taskId, @@ -342,6 +355,7 @@ export function requestArbiterOrEscalate(args: { expectedUpdatedAt, updatedAt: now, patch: { + ...args.patch, arbiter_requested_at: now, }, }); @@ -358,6 +372,7 @@ export function requestArbiterOrEscalate(args: { expectedUpdatedAt, updatedAt: now, patch: { + ...args.patch, completion_reason: 'escalated', }, }); diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index f737260..47ca04a 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -139,6 +139,7 @@ function buildPairedTask(overrides: Partial = {}): PairedTask { plan_notes: null, review_requested_at: null, round_trip_count: 0, + owner_failure_count: 0, status: 'active', arbiter_verdict: null, arbiter_requested_at: null, @@ -980,7 +981,7 @@ describe('paired execution context', () => { ); }); - it('still resets owner tasks to active after failed execution', () => { + it('increments owner failure count and resets owner tasks to active after failed execution', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ status: 'merge_ready', @@ -998,11 +999,38 @@ describe('paired execution context', () => { 'task-1', expect.objectContaining({ status: 'active', + owner_failure_count: 1, updated_at: expect.any(String), }), ); }); + it('requests arbiter after repeated owner execution failures without a visible verdict', () => { + vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true); + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'active', + owner_failure_count: 1, + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'failed', + summary: "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'", + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'arbiter_requested', + owner_failure_count: 2, + arbiter_requested_at: expect.any(String), + }), + ); + }); + it('releases the execution lease even when a completion handler throws', () => { const transitionError = new Error('transition failed'); vi.mocked(db.getPairedTaskById).mockReturnValue( diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index c464d22..bb58c88 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -137,6 +137,7 @@ function createActiveTaskForRoom(args: { plan_notes: null, review_requested_at: null, round_trip_count: 0, + owner_failure_count: 0, status: 'active', arbiter_verdict: null, arbiter_requested_at: null, @@ -369,7 +370,7 @@ export function preparePairedExecutionContext(args: { expectedUpdatedAt: latestTask.updated_at, updatedAt: now, patch: { - ...(hasHuman ? { round_trip_count: 0 } : {}), + ...(hasHuman ? { round_trip_count: 0, owner_failure_count: 0 } : {}), }, }); } else { @@ -378,7 +379,7 @@ export function preparePairedExecutionContext(args: { expectedUpdatedAt: latestTask.updated_at, updatedAt: now, patch: { - ...(hasHuman ? { round_trip_count: 0 } : {}), + ...(hasHuman ? { round_trip_count: 0, owner_failure_count: 0 } : {}), }, }); } @@ -551,7 +552,7 @@ export function completePairedExecutionContext(args: { handleFailedArbiterExecution({ task, taskId }); return; } - handleFailedOwnerExecution({ task, taskId }); + handleFailedOwnerExecution({ task, taskId, summary: args.summary }); return; } diff --git a/src/session-recovery.test.ts b/src/session-recovery.test.ts index 349d1f9..c4ba31d 100644 --- a/src/session-recovery.test.ts +++ b/src/session-recovery.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from 'vitest'; import { + shouldResetCodexSessionOnAgentFailure, shouldResetSessionOnAgentFailure, + shouldRetryFreshCodexSessionOnAgentFailure, shouldRetryFreshSessionOnAgentFailure, } from './session-recovery.js'; @@ -99,3 +101,36 @@ describe('shouldRetryFreshSessionOnAgentFailure', () => { ).toBe(false); }); }); + +describe('shouldResetCodexSessionOnAgentFailure', () => { + it('matches remote compact task failures that mention prompt_cache_retention', () => { + expect( + shouldResetCodexSessionOnAgentFailure({ + result: null, + error: + "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'", + }), + ).toBe(true); + }); + + it('does not match unrelated Codex failures', () => { + expect( + shouldResetCodexSessionOnAgentFailure({ + result: null, + error: 'Codex process exited with code 1', + }), + ).toBe(false); + }); +}); + +describe('shouldRetryFreshCodexSessionOnAgentFailure', () => { + it('retries the same remote compact task failure with a fresh session', () => { + expect( + shouldRetryFreshCodexSessionOnAgentFailure({ + result: null, + error: + "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'", + }), + ).toBe(true); + }); +}); diff --git a/src/session-recovery.ts b/src/session-recovery.ts index a098f3a..6ce675a 100644 --- a/src/session-recovery.ts +++ b/src/session-recovery.ts @@ -17,6 +17,11 @@ const SESSION_RETRY_PATTERNS = [ /invalid[^\n]*signature[^\n]*thinking/i, ]; +const CODEX_SESSION_RESET_PATTERNS = [ + /Error running remote compact task/i, + /prompt_cache_retention/i, +]; + function toText(value: string | object | null | undefined): string[] { if (!value) return []; if (typeof value === 'string') return [value]; @@ -51,3 +56,21 @@ export function shouldRetryFreshSessionOnAgentFailure( SESSION_RETRY_PATTERNS.some((pattern) => pattern.test(text)), ); } + +export function shouldResetCodexSessionOnAgentFailure( + output: Pick, +): boolean { + const texts = [ + ...toText(getAgentOutputText(output)), + ...toText(output.error), + ]; + return texts.some((text) => + CODEX_SESSION_RESET_PATTERNS.some((pattern) => pattern.test(text)), + ); +} + +export function shouldRetryFreshCodexSessionOnAgentFailure( + output: Pick, +): boolean { + return shouldResetCodexSessionOnAgentFailure(output); +} diff --git a/src/streamed-output-evaluator.test.ts b/src/streamed-output-evaluator.test.ts index faf065b..dad210e 100644 --- a/src/streamed-output-evaluator.test.ts +++ b/src/streamed-output-evaluator.test.ts @@ -21,6 +21,7 @@ vi.mock('./codex-token-rotation.js', () => ({ })); vi.mock('./session-recovery.js', () => ({ + shouldRetryFreshCodexSessionOnAgentFailure: vi.fn(() => false), shouldRetryFreshSessionOnAgentFailure: vi.fn(() => false), })); @@ -34,7 +35,10 @@ import { isClaudeAuthError, } from './agent-error-detection.js'; import { detectCodexRotationTrigger } from './codex-token-rotation.js'; -import { shouldRetryFreshSessionOnAgentFailure } from './session-recovery.js'; +import { + shouldRetryFreshCodexSessionOnAgentFailure, + shouldRetryFreshSessionOnAgentFailure, +} from './session-recovery.js'; import { evaluateStreamedOutput, @@ -91,6 +95,7 @@ beforeEach(() => { shouldRotate: false, reason: '', }); + vi.mocked(shouldRetryFreshCodexSessionOnAgentFailure).mockReturnValue(false); vi.mocked(shouldRetryFreshSessionOnAgentFailure).mockReturnValue(false); }); @@ -252,6 +257,28 @@ describe('evaluateStreamedOutput', () => { }); }); + describe('Codex retryable session failure suppression', () => { + it('suppresses remote compact failures before any visible output', () => { + vi.mocked(shouldRetryFreshCodexSessionOnAgentFailure).mockReturnValue( + true, + ); + + const result = evaluateStreamedOutput( + successOutput( + "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'", + 'intermediate', + ), + freshState(), + codexOpts, + ); + + expect(result.shouldForwardOutput).toBe(false); + expect(result.suppressedRetryableSessionFailure).toBe(true); + expect(result.state.retryableSessionFailureDetected).toBe(true); + expect(result.state.sawOutput).toBe(false); + }); + }); + describe('Claude auth-expired banner', () => { it('suppresses output and returns newTrigger', () => { vi.mocked(isClaudeAuthExpiredMessage).mockReturnValue(true); diff --git a/src/streamed-output-evaluator.ts b/src/streamed-output-evaluator.ts index b78fc1d..a853328 100644 --- a/src/streamed-output-evaluator.ts +++ b/src/streamed-output-evaluator.ts @@ -15,7 +15,10 @@ import { } from './agent-error-detection.js'; import type { AgentOutput } from './agent-runner.js'; import { detectCodexRotationTrigger } from './codex-token-rotation.js'; -import { shouldRetryFreshSessionOnAgentFailure } from './session-recovery.js'; +import { + shouldRetryFreshCodexSessionOnAgentFailure, + shouldRetryFreshSessionOnAgentFailure, +} from './session-recovery.js'; export interface StreamedTriggerReason { reason: AgentTriggerReason; @@ -74,6 +77,19 @@ export function evaluateStreamedOutput( }; } + if ( + isPrimaryCodex && + !state.sawOutput && + shouldRetryFreshCodexSessionOnAgentFailure(output) + ) { + nextState.retryableSessionFailureDetected = true; + return { + state: nextState, + shouldForwardOutput: false, + suppressedRetryableSessionFailure: true, + }; + } + if ( isPrimaryClaude && output.status === 'success' && diff --git a/src/types.ts b/src/types.ts index ff64efa..b06efe6 100644 --- a/src/types.ts +++ b/src/types.ts @@ -82,6 +82,7 @@ export interface PairedTask { plan_notes: string | null; review_requested_at: string | null; round_trip_count: number; + owner_failure_count?: number | null; status: PairedTaskStatus; arbiter_verdict: string | null; arbiter_requested_at: string | null;